Merge lp:~sergiusens/snapcraft/cleanup into lp:~snappy-dev/snapcraft/core

[Sergio Schvezov]

Document and cleanup the plugin base class

This cleans up the base class to only expose things plugins will need, since the basic schema includes 'source' options it makes sense for pull to implement that as well automatically if supported.

This also moves everything from the base plugin the plugin.py (which should be renamed to lifecycle.py in a future MP, this one is already too big) and any other thing to its own supporting class.

This branch also includes all the plugin cleanup required for things to keep on working.

This all started with trying to get the catkin plugin out of using the private implementation (setup_stage_packages)

[John Lenton (chipaca)]

Looks good. A couple of comments on code that I know is just moved, so not a blocker to land.

review: Approve

[Leo Arias (elopio)]

This is a nice step forward, thanks.
I'm marking it needs fixing because of a couple of typos. Also some wording suggestions in the diff.

review: Needs Fixing

[lp:~sergiusens/snapcraft/cleanup]

210. By Sergio Schvezov on 2015-10-19

Use a dictionary for source type handlers

211. By Sergio Schvezov on 2015-10-19

Compile the tar regex only once

[Sergio Schvezov (sergiusens)]

Thanks for the reviews, I've updated da things

[lp:~sergiusens/snapcraft/cleanup]

212. By Sergio Schvezov on 2015-10-19

Improve docstrings

[Leo Arias (elopio)]

+1

review: Approve

=== modified file 'snapcraft/__init__.py'
--- snapcraft/__init__.py	2015-10-14 20:00:25 +0000
+++ snapcraft/__init__.py	2015-10-19 19:01:20 +0000
@@ -17,7 +17,6 @@
 import contextlib
 import logging
 import os
-import re
 
 import snapcraft.common
 import snapcraft.sources
@@ -31,15 +30,14 @@
 
     @classmethod
     def schema(cls):
-        '''
-        Returns a json-schema for the plugin's properties as a dictionary.
+        """Return a json-schema for the plugin's properties as a dictionary.
         Of importance to plugin authors is the 'properties' keyword and
         optionally the 'requires' keyword with a list of required
         'properties'.
 
         By default the the properties will be that of a standard VCS, override
         in custom implementations if required.
-        '''
+        """
         return {
             '$schema': 'http://json-schema.org/draft-04/schema#',
             'type': 'object',
@@ -67,6 +65,7 @@
 
     @property
     def PLUGIN_STAGE_SOURCES(self):
+        """Define additional sources.list."""
         return getattr(self, '_PLUGIN_STAGE_SOURCES', [])
 
     def __init__(self, name, options):
@@ -92,19 +91,74 @@
 
     # The API
     def pull(self):
-        return True
+        """Pull the source code and/or internal prereqs to build the part.
+
+        By default, the base implementation for pull will use the following
+        part properties to retrieve source code:
+
+        - source
+        - source-branch
+        - source-tag
+        - source-type
+
+        If source is empty or does not exist, the phase will be skipped.
+
+        Override or inherit from this method if you need to implement or
+        enhance with custom pull logic.
+        """
+        if not getattr(self.options, 'source', None):
+            return True
+        try:
+            return snapcraft.sources.get(
+                self.sourcedir, self.builddir, self.options)
+        except ValueError as e:
+            logger.error('Unrecognized source %r for part %r: %s.',
+                         self.options.source, self.name, e)
+            snapcraft.common.fatal()
+        except snapcraft.sources.IncompatibleOptionsError as e:
+            logger.error(
+                'Issues while setting up sources for part \'%s\': %s.',
+                self.name,
+                e.message)
+            snapcraft.common.fatal()
 
     def build(self):
+        """Build the source code retrieved from the pull phase.
+
+        The base implementation does nothing by default. Override this method
+        if you need to process the source code to make it runnable.
+        """
         return True
 
     def snap_fileset(self):
-        """Returns one iteratables of globs specific to the plugin:
+        """Return a list of files to include or exclude in the resulting snap
+
+        The staging phase of a plugin's lifecycle may populate many things
+        into the staging directory in order to succeed in building a project.
+        During the stripping phase and in order to have a clean snap, the
+        plugin can provide additional logic for stripping build components
+        from the final snap and alleviate the part author from doing so for
+        repetetive filesets.
+
+        These are the rules to honor when creating such list:
+
             - includes can be just listed
             - excludes must be preceded by -
-           For example: (['bin', 'lib', '-include'])"""
+
+        For example::
+            (['bin', 'lib', '-include'])
+        """
         return ([])
 
     def env(self, root):
+        """Return a list with the execution environment for building.
+
+        Plugins often need special environment variables exported to the
+        system for some builds to take place. This is a list of strings
+        of the form key=value. The parameter root is the path to this part.
+
+        :param str root: The root for the part
+        """
         return []
 
     # Helpers
@@ -113,7 +167,7 @@
             cwd = self.builddir
         if True:
             print(' '.join(cmd))
-        self.makedirs(cwd)
+        os.makedirs(cwd, exist_ok=True)
         return snapcraft.common.run(cmd, cwd=cwd, **kwargs)
 
     def run_output(self, cmd, cwd=None, **kwargs):
@@ -121,96 +175,5 @@
             cwd = self.builddir
         if True:
             print(' '.join(cmd))
-        self.makedirs(cwd)
+        os.makedirs(cwd, exist_ok=True)
         return snapcraft.common.run_output(cmd, cwd=cwd, **kwargs)
-
-    def isurl(self, url):
-        return snapcraft.common.isurl(url)
-
-    def get_source(self, source, source_type=None, source_tag=None,
-                   source_branch=None):
-        try:
-            handler_class = _get_source_handler(source_type, source)
-        except ValueError as e:
-            logger.error('Unrecognized source %r for part %r: %s.', source,
-                         self.name, e)
-            snapcraft.common.fatal()
-
-        try:
-            handler = handler_class(source, self.sourcedir, source_tag,
-                                    source_branch)
-        except snapcraft.sources.IncompatibleOptionsError as e:
-            logger.error(
-                'Issues while setting up sources for part \'%s\': %s.',
-                self.name,
-                e.message)
-            snapcraft.common.fatal()
-        if not handler.pull():
-            return False
-        return handler.provision(self.builddir)
-
-    def handle_source_options(self):
-        stype = getattr(self.options, 'source_type', None)
-        stag = getattr(self.options, 'source_tag', None)
-        sbranch = getattr(self.options, 'source_branch', None)
-        return self.get_source(self.options.source,
-                               source_type=stype,
-                               source_tag=stag,
-                               source_branch=sbranch)
-
-    def makedirs(self, d):
-        os.makedirs(d, exist_ok=True)
-
-    def setup_stage_packages(self):
-        if self.stage_packages:
-            ubuntu = snapcraft.repo.Ubuntu(self.ubuntudir,
-                                           sources=self.PLUGIN_STAGE_SOURCES)
-            ubuntu.get(self.stage_packages)
-            ubuntu.unpack(self.installdir)
-            self._fixup(self.installdir)
-
-    def _fixup(self, root):
-        if os.path.isfile(os.path.join(root, 'usr', 'bin', 'xml2-config')):
-            self.run(
-                ['sed', '-i', '-e', 's|prefix=/usr|prefix={}/usr|'.
-                    format(root),
-                 os.path.join(root, 'usr', 'bin', 'xml2-config')])
-        if os.path.isfile(os.path.join(root, 'usr', 'bin', 'xslt-config')):
-            self.run(
-                ['sed', '-i', '-e', 's|prefix=/usr|prefix={}/usr|'.
-                    format(root),
-                 os.path.join(root, 'usr', 'bin', 'xslt-config')])
-
-
-def _get_source_handler(source_type, source):
-    if not source_type:
-        source_type = _get_source_type_from_uri(source)
-
-    if source_type == 'bzr':
-        handler = snapcraft.sources.Bazaar
-    elif source_type == 'git':
-        handler = snapcraft.sources.Git
-    elif source_type == 'mercurial' or source_type == 'hg':
-        handler = snapcraft.sources.Mercurial
-    elif source_type == 'tar':
-        handler = snapcraft.sources.Tar
-    else:
-        handler = snapcraft.sources.Local
-
-    return handler
-
-
-def _get_source_type_from_uri(source):
-    source_type = ''
-    if source.startswith("bzr:") or source.startswith("lp:"):
-        source_type = 'bzr'
-    elif source.startswith("git:"):
-        source_type = 'git'
-    elif re.compile(r'.*\.((tar\.(xz|gz|bz2))|tgz)$').match(source):
-        source_type = 'tar'
-    elif snapcraft.common.isurl(source):
-        raise ValueError('No handler to manage source')
-    elif not os.path.isdir(source):
-        raise ValueError('Local source is not a directory')
-
-    return source_type
 
=== modified file 'snapcraft/plugin.py'
--- snapcraft/plugin.py	2015-10-15 17:01:43 +0000
+++ snapcraft/plugin.py	2015-10-19 19:01:20 +0000
@@ -137,6 +137,13 @@
         with open(self.statefile, 'w+') as f:
             f.write(stage)
 
+    def _setup_stage_packages(self):
+        if self.code.stage_packages:
+            ubuntu = snapcraft.repo.Ubuntu(
+                self.code.ubuntudir, sources=self.code.PLUGIN_STAGE_SOURCES)
+            ubuntu.get(self.code.stage_packages)
+            ubuntu.unpack(self.code.installdir)
+
     def pull(self, force=False):
         if not self.should_stage_run('pull', force):
             return True
@@ -145,7 +152,7 @@
         self.notify_stage("Pulling")
 
         try:
-            self.code.setup_stage_packages()
+            self._setup_stage_packages()
         except repo.PackageNotFoundError as e:
             logger.error(e.message)
             return False
 
=== modified file 'snapcraft/plugins/ant.py'
--- snapcraft/plugins/ant.py	2015-10-09 17:43:20 +0000
+++ snapcraft/plugins/ant.py	2015-10-19 19:01:20 +0000
@@ -33,10 +33,6 @@
         super().__init__(name, options)
         self.build_packages.append('ant')
 
-    def pull(self):
-        super().pull()
-        return self.handle_source_options()
-
     def build(self):
         super().build()
         if not self.run(['ant']):
 
=== modified file 'snapcraft/plugins/catkin.py'
--- snapcraft/plugins/catkin.py	2015-10-16 19:05:42 +0000
+++ snapcraft/plugins/catkin.py	2015-10-19 19:01:20 +0000
@@ -20,6 +20,7 @@
 import logging
 
 import snapcraft
+import snapcraft.repo
 
 logger = logging.getLogger(__name__)
 
@@ -61,6 +62,19 @@
         self.dependencies = ['ros-core']
         self.package_deps_found = False
         self.package_local_deps = {}
+        self._deb_packages = []
+
+    def pull(self):
+        if not super().pull():
+            return False
+
+        try:
+            self._setup_deb_packages()
+        except snapcraft.repo.PackageNotFoundError as e:
+            logger.error(e.message)
+            return False
+
+        return True
 
     def env(self, root):
         return [
@@ -124,19 +138,18 @@
                     self.package_local_deps[pkg].add(dep)
                     continue
 
-                # If we're already getting this through a stage package,
+                # If we're already getting this through a deb package,
                 # we don't need it
-                if self.options.stage_packages and (
-                        dep in self.options.stage_packages or
-                        dep.replace('_', '-') in self.options.stage_packages):
+                if (dep in self._deb_packages or
+                        dep.replace('_', '-') in self._deb_packages):
                     continue
 
                 # Get the ROS package for it
-                self.stage_packages.append(
+                self._deb_packages.append(
                     'ros-'+self.options.rosversion+'-'+dep.replace('_', '-'))
 
                 if dep == 'roscpp':
-                    self.stage_packages.append('g++')
+                    self._deb_packages.append('g++')
 
     def _find_package_deps(self):
         if self.package_deps_found:
@@ -158,13 +171,14 @@
 
         self.package_deps_found = True
 
-    def setup_stage_packages(self):
-        if not self.handle_source_options():
-            return False
-
+    def _setup_deb_packages(self):
         self._find_package_deps()
 
-        return super().setup_stage_packages()
+        if self._deb_packages:
+            ubuntu = snapcraft.repo.Ubuntu(
+                self.ubuntudir, sources=self.PLUGIN_STAGE_SOURCES)
+            ubuntu.get(self._deb_packages)
+            ubuntu.unpack(self.installdir)
 
     def _rosrun(self, commandlist, cwd=None):
         with tempfile.NamedTemporaryFile(mode='w') as f:
 
=== modified file 'snapcraft/plugins/jdk.py'
--- snapcraft/plugins/jdk.py	2015-10-09 02:18:59 +0000
+++ snapcraft/plugins/jdk.py	2015-10-19 19:01:20 +0000
@@ -23,6 +23,9 @@
         super().__init__(name, options)
         self.stage_packages.append('default-jdk')
 
+    def pull(self):
+        return True
+
     def env(self, root):
         return ['JAVA_HOME=%s/usr/lib/jvm/default-java' % root,
                 'PATH=%s/usr/lib/jvm/default-java/bin:'
 
=== modified file 'snapcraft/plugins/make.py'
--- snapcraft/plugins/make.py	2015-10-09 17:43:20 +0000
+++ snapcraft/plugins/make.py	2015-10-19 19:01:20 +0000
@@ -23,9 +23,6 @@
         super().__init__(name, options)
         self.build_packages.append('make')
 
-    def pull(self):
-        return self.handle_source_options()
-
     def build(self):
         return self.run(['make']) and \
             self.run(['make', 'install', 'DESTDIR=' + self.installdir])
 
=== modified file 'snapcraft/plugins/maven.py'
--- snapcraft/plugins/maven.py	2015-10-09 17:43:20 +0000
+++ snapcraft/plugins/maven.py	2015-10-19 19:01:20 +0000
@@ -32,10 +32,6 @@
         super().__init__(name, options)
         self.build_packages.append('maven')
 
-    def pull(self):
-        super().pull()
-        return self.handle_source_options()
-
     def build(self):
         super().build()
 
 
=== modified file 'snapcraft/plugins/python2.py'
--- snapcraft/plugins/python2.py	2015-10-09 17:43:20 +0000
+++ snapcraft/plugins/python2.py	2015-10-19 19:01:20 +0000
@@ -45,7 +45,7 @@
             root, 'usr', 'lib', self.python_version, 'dist-packages')]
 
     def pull(self):
-        if self.options.source and not self.handle_source_options():
+        if not super().pull():
             return False
 
         return self._pip()
 
=== modified file 'snapcraft/plugins/python3.py'
--- snapcraft/plugins/python3.py	2015-10-09 17:43:20 +0000
+++ snapcraft/plugins/python3.py	2015-10-19 19:01:20 +0000
@@ -45,7 +45,7 @@
             root, 'usr', 'lib', self.python_version, 'dist-packages')]
 
     def pull(self):
-        if self.options.source and not self.handle_source_options():
+        if not super().pull():
             return False
 
         return self._pip()
 
=== modified file 'snapcraft/plugins/scons.py'
--- snapcraft/plugins/scons.py	2015-10-09 17:43:20 +0000
+++ snapcraft/plugins/scons.py	2015-10-19 19:01:20 +0000
@@ -40,9 +40,6 @@
         super().__init__(name, options)
         self.build_packages.append('scons')
 
-    def pull(self):
-        return self.handle_source_options()
-
     def build(self):
         env = os.environ.copy()
         env['DESTDIR'] = self.installdir
 
=== modified file 'snapcraft/repo.py'
--- snapcraft/repo.py	2015-10-16 20:38:56 +0000
+++ snapcraft/repo.py	2015-10-19 19:01:20 +0000
@@ -135,6 +135,7 @@
                 raise UnpackError(pkg)
 
         _fix_contents(rootdir)
+        _fix_xml_tools(rootdir)
 
     def _manifest_dep_names(self):
         manifest_dep_names = set()
@@ -252,6 +253,20 @@
                 _fix_filemode(path)
 
 
+def _fix_xml_tools(root):
+    xml2_config_path = os.path.join(root, 'usr', 'bin', 'xml2-config')
+    if os.path.isfile(xml2_config_path):
+        snapcraft.common.run(
+            ['sed', '-i', '-e', 's|prefix=/usr|prefix={}/usr|'.
+                format(root), xml2_config_path])
+
+    xslt_config_path = os.path.join(root, 'usr', 'bin', 'xslt-config')
+    if os.path.isfile(xslt_config_path):
+        snapcraft.common.run(
+            ['sed', '-i', '-e', 's|prefix=/usr|prefix={}/usr|'.
+                format(root), xslt_config_path])
+
+
 def _fix_filemode(path):
     mode = stat.S_IMODE(os.stat(path).st_mode)
     if mode & 0o4000 or mode & 0o2000:
 
=== modified file 'snapcraft/sources.py'
--- snapcraft/sources.py	2015-10-14 18:14:47 +0000
+++ snapcraft/sources.py	2015-10-19 19:01:20 +0000
@@ -227,3 +227,51 @@
         os.symlink(path, dst)
 
         return True
+
+
+def get(sourcedir, builddir, options):
+    source_type = getattr(options, 'source_type', None)
+    source_tag = getattr(options, 'source_tag', None)
+    source_branch = getattr(options, 'source_branch', None)
+
+    handler_class = _get_source_handler(source_type, options.source)
+    handler = handler_class(options.source, sourcedir, source_tag,
+                            source_branch)
+    if not handler.pull():
+        return False
+    return handler.provision(builddir)
+
+
+_source_handler = {
+    'bzr': Bazaar,
+    'git': Git,
+    'hg': Mercurial,
+    'mercurial': Mercurial,
+    'tar': Tar,
+}
+
+
+def _get_source_handler(source_type, source):
+    if not source_type:
+        source_type = _get_source_type_from_uri(source)
+
+    return _source_handler.get(source_type, Local)
+
+
+_tar_type_regex = re.compile(r'.*\.((tar\.(xz|gz|bz2))|tgz)$')
+
+
+def _get_source_type_from_uri(source):
+    source_type = ''
+    if source.startswith("bzr:") or source.startswith("lp:"):
+        source_type = 'bzr'
+    elif source.startswith("git:"):
+        source_type = 'git'
+    elif _tar_type_regex.match(source):
+        source_type = 'tar'
+    elif snapcraft.common.isurl(source):
+        raise ValueError('No handler to manage source')
+    elif not os.path.isdir(source):
+        raise ValueError('Local source is not a directory')
+
+    return source_type
 
=== modified file 'snapcraft/tests/test_base_plugin.py'
--- snapcraft/tests/test_base_plugin.py	2015-10-14 20:00:25 +0000
+++ snapcraft/tests/test_base_plugin.py	2015-10-19 19:01:20 +0000
@@ -16,30 +16,32 @@
 
 import fixtures
 import logging
-import os
 import unittest.mock
 
 import snapcraft
 from snapcraft import tests
 
 
+class MockOptions:
+
+    def __init__(self, source, source_type=None, source_branch=None,
+                 source_tag=None):
+        self.source = source
+        self.source_type = source_type
+        self.source_branch = source_branch
+        self.source_tag = source_tag
+
+
 class TestBasePlugin(tests.TestCase):
 
-    def test_isurl(self):
-        plugin = snapcraft.BasePlugin('mock', {})
-        self.assertTrue(plugin.isurl('git://'))
-        self.assertTrue(plugin.isurl('bzr://'))
-        self.assertFalse(plugin.isurl('./'))
-        self.assertFalse(plugin.isurl('/foo'))
-        self.assertFalse(plugin.isurl('/fo:o'))
-
     def test_get_source_with_unrecognized_source_must_raise_error(self):
         fake_logger = fixtures.FakeLogger(level=logging.ERROR)
         self.useFixture(fake_logger)
 
-        plugin = snapcraft.BasePlugin('test_plugin', 'dummy_options')
+        options = MockOptions('unrecognized://test_source')
+        plugin = snapcraft.BasePlugin('test_plugin', options)
         with self.assertRaises(SystemExit) as raised:
-            plugin.get_source('unrecognized://test_source')
+            plugin.pull()
 
         self.assertEqual(raised.exception.code, 1, 'Wrong exit code returned.')
         expected = (
@@ -52,10 +54,11 @@
         fake_logger = fixtures.FakeLogger(level=logging.ERROR)
         self.useFixture(fake_logger)
 
+        options = MockOptions('file')
         mock_isdir.return_value = False
-        plugin = snapcraft.BasePlugin('test_plugin', 'dummy_options')
+        plugin = snapcraft.BasePlugin('test_plugin', options)
         with self.assertRaises(SystemExit) as raised:
-            plugin.get_source('file')
+            plugin.pull()
 
         mock_isdir.assert_called_once_with('file')
         self.assertEqual(raised.exception.code, 1, 'Wrong exit code returned.')
@@ -64,30 +67,6 @@
             "Local source is not a directory.\n")
         self.assertEqual(expected, fake_logger.output)
 
-    def test_makedirs_with_existing_dir(self):
-        plugin = snapcraft.BasePlugin('dummy_plugin', 'dummy_options')
-        plugin.makedirs(self.path)
-        self.assertTrue(os.path.exists(self.path))
-
-    def test_makedirs_with_unexisting_dir(self):
-        path = os.path.join(self.path, 'unexisting')
-        plugin = snapcraft.BasePlugin('dummy_plugin', 'dummy_options')
-        plugin.makedirs(path)
-        self.assertTrue(os.path.exists(path))
-
-    def test_get_tar_source_from_uri(self):
-        sources = [
-            'https://golang.tar.gz',
-            'https://golang.tar.xz',
-            'https://golang.tar.bz2',
-            'https://golang.tar.tgz',
-        ]
-
-        for source in sources:
-            with self.subTest(key=source):
-                self.assertEqual(
-                    snapcraft._get_source_type_from_uri(source), 'tar')
-
 
 class GetSourceWithBranches(tests.TestCase):
 
@@ -108,11 +87,11 @@
         fake_logger = fixtures.FakeLogger(level=logging.ERROR)
         self.useFixture(fake_logger)
 
-        plugin = snapcraft.BasePlugin('test_plugin', 'dummy_options')
+        options = MockOptions('lp:source', self.source_type,
+                              self.source_branch, self.source_tag)
+        plugin = snapcraft.BasePlugin('test_plugin', options)
         with self.assertRaises(SystemExit) as raised:
-            plugin.get_source(
-                'dummy_source', source_type=self.source_type,
-                source_branch=self.source_branch, source_tag=self.source_tag)
+            plugin.pull()
 
         self.assertEqual(raised.exception.code, 1, 'Wrong exit code returned.')
         expected = (
@@ -146,11 +125,12 @@
         fake_logger = fixtures.FakeLogger(level=logging.ERROR)
         self.useFixture(fake_logger)
 
-        plugin = snapcraft.BasePlugin('test_plugin', 'dummy_options')
+        options = MockOptions('lp:this', self.source_type, self.source_branch,
+                              self.source_tag)
+        plugin = snapcraft.BasePlugin('test_plugin', options)
+
         with self.assertRaises(SystemExit) as raised:
-            plugin.get_source(
-                'dummy_source', source_type=self.source_type,
-                source_branch=self.source_branch, source_tag=self.source_tag)
+            plugin.pull()
 
         self.assertEqual(raised.exception.code, 1, 'Wrong exit code returned.')
         expected = (
 
=== modified file 'snapcraft/tests/test_common.py'
--- snapcraft/tests/test_common.py	2015-07-23 17:26:18 +0000
+++ snapcraft/tests/test_common.py	2015-10-19 19:01:20 +0000
@@ -40,3 +40,10 @@
         plugindir = os.path.join(self.path, 'testplugin')
         common.set_plugindir(plugindir)
         self.assertEqual(plugindir, common.get_plugindir())
+
+    def test_isurl(self):
+        self.assertTrue(common.isurl('git://'))
+        self.assertTrue(common.isurl('bzr://'))
+        self.assertFalse(common.isurl('./'))
+        self.assertFalse(common.isurl('/foo'))
+        self.assertFalse(common.isurl('/fo:o'))
 
=== modified file 'snapcraft/tests/test_plugin.py'
--- snapcraft/tests/test_plugin.py	2015-10-09 17:43:20 +0000
+++ snapcraft/tests/test_plugin.py	2015-10-19 19:01:20 +0000
@@ -52,6 +52,7 @@
         p.statefile = tempfile.NamedTemporaryFile().name
         self.addCleanup(os.remove, p.statefile)
         p.code = Mock()
+        p._setup_stage_packages = Mock()
         # pull once
         p.pull()
         p.code.pull.assert_called()
 
=== modified file 'snapcraft/tests/test_sources.py'
--- snapcraft/tests/test_sources.py	2015-10-14 18:14:47 +0000
+++ snapcraft/tests/test_sources.py	2015-10-19 19:01:20 +0000
@@ -19,7 +19,8 @@
 import threading
 import unittest.mock
 
-from snapcraft import sources
+import snapcraft.sources
+
 from snapcraft import tests
 
 
@@ -56,7 +57,7 @@
         tar_file_name = 'test.tar'
         source = 'http://{}:{}/{file_name}'.format(
             *server.server_address, file_name=tar_file_name)
-        tar_source = sources.Tar(source, dest_dir)
+        tar_source = snapcraft.sources.Tar(source, dest_dir)
 
         self.assertTrue(tar_source.pull())
 
@@ -87,7 +88,7 @@
 class TestBazaar(SourceTestCase):
 
     def test_pull(self):
-        bzr = sources.Bazaar('lp:my-source', 'source_dir')
+        bzr = snapcraft.sources.Bazaar('lp:my-source', 'source_dir')
 
         bzr.pull()
 
@@ -96,7 +97,8 @@
             ['bzr', 'branch', 'lp:my-source', 'source_dir'], cwd=os.getcwd())
 
     def test_pull_tag(self):
-        bzr = sources.Bazaar('lp:my-source', 'source_dir', source_tag='tag')
+        bzr = snapcraft.sources.Bazaar(
+            'lp:my-source', 'source_dir', source_tag='tag')
         bzr.pull()
 
         self.mock_run.assert_called_once_with(
@@ -106,7 +108,8 @@
     def test_pull_existing_with_tag(self):
         self.mock_path_exists.return_value = True
 
-        bzr = sources.Bazaar('lp:my-source', 'source_dir', source_tag='tag')
+        bzr = snapcraft.sources.Bazaar(
+            'lp:my-source', 'source_dir', source_tag='tag')
         bzr.pull()
 
         self.mock_run.assert_called_once_with(
@@ -114,15 +117,17 @@
              'source_dir'], cwd=os.getcwd())
 
     def test_provision(self):
-        bzr = sources.Bazaar('lp:my-source', 'source_dir')
+        bzr = snapcraft.sources.Bazaar('lp:my-source', 'source_dir')
         bzr.provision('dst')
 
         self.mock_run.assert_called_once_with(
             ['cp', '-Trfa', 'source_dir', 'dst'], cwd=os.getcwd())
 
     def test_init_with_source_branch_raises_exception(self):
-        with self.assertRaises(sources.IncompatibleOptionsError) as raised:
-            sources.Bazaar('lp:mysource', 'source_dir', source_branch='branch')
+        with self.assertRaises(
+                snapcraft.sources.IncompatibleOptionsError) as raised:
+            snapcraft.sources.Bazaar('lp:mysource', 'source_dir',
+                                     source_branch='branch')
 
         expected_message = 'can\'t specify a source-branch for a bzr source'
         self.assertEqual(raised.exception.message, expected_message)
@@ -131,7 +136,7 @@
 class TestGit(SourceTestCase):
 
     def test_pull(self):
-        git = sources.Git('git://my-source', 'source_dir')
+        git = snapcraft.sources.Git('git://my-source', 'source_dir')
 
         git.pull()
 
@@ -139,8 +144,8 @@
             ['git', 'clone', 'git://my-source', 'source_dir'], cwd=os.getcwd())
 
     def test_pull_branch(self):
-        git = sources.Git('git://my-source', 'source_dir',
-                          source_branch='my-branch')
+        git = snapcraft.sources.Git('git://my-source', 'source_dir',
+                                    source_branch='my-branch')
         git.pull()
 
         self.mock_run.assert_called_once_with(
@@ -148,7 +153,8 @@
              'source_dir'], cwd=os.getcwd())
 
     def test_pull_tag(self):
-        git = sources.Git('git://my-source', 'source_dir', source_tag='tag')
+        git = snapcraft.sources.Git('git://my-source', 'source_dir',
+                                    source_tag='tag')
         git.pull()
 
         self.mock_run.assert_called_once_with(
@@ -158,7 +164,7 @@
     def test_pull_existing(self):
         self.mock_path_exists.return_value = True
 
-        git = sources.Git('git://my-source', 'source_dir')
+        git = snapcraft.sources.Git('git://my-source', 'source_dir')
         git.pull()
 
         self.mock_run.assert_called_once_with(
@@ -168,7 +174,8 @@
     def test_pull_existing_with_tag(self):
         self.mock_path_exists.return_value = True
 
-        git = sources.Git('git://my-source', 'source_dir', source_tag='tag')
+        git = snapcraft.sources.Git('git://my-source', 'source_dir',
+                                    source_tag='tag')
         git.pull()
 
         self.mock_run.assert_called_once_with(
@@ -178,8 +185,8 @@
     def test_pull_existing_with_branch(self):
         self.mock_path_exists.return_value = True
 
-        git = sources.Git('git://my-source', 'source_dir',
-                          source_branch='my-branch')
+        git = snapcraft.sources.Git('git://my-source', 'source_dir',
+                                    source_branch='my-branch')
         git.pull()
 
         self.mock_run.assert_called_once_with(
@@ -187,16 +194,17 @@
              'refs/heads/my-branch'], cwd=os.getcwd())
 
     def test_provision(self):
-        bzr = sources.Git('git://my-source', 'source_dir')
+        bzr = snapcraft.sources.Git('git://my-source', 'source_dir')
         bzr.provision('dst')
 
         self.mock_run.assert_called_once_with(
             ['cp', '-Trfa', 'source_dir', 'dst'], cwd=os.getcwd())
 
     def test_init_with_source_branch_and_tag_raises_exception(self):
-        with self.assertRaises(sources.IncompatibleOptionsError) as raised:
-            sources.Git('git://mysource', 'source_dir', source_tag='tag',
-                        source_branch='branch')
+        with self.assertRaises(
+                snapcraft.sources.IncompatibleOptionsError) as raised:
+            snapcraft.sources.Git('git://mysource', 'source_dir',
+                                  source_tag='tag', source_branch='branch')
 
         expected_message = \
             'can\'t specify both source-tag and source-branch for a git source'
@@ -206,15 +214,15 @@
 class TestMercurial(SourceTestCase):
 
     def test_pull(self):
-        hg = sources.Mercurial('hg://my-source', 'source_dir')
+        hg = snapcraft.sources.Mercurial('hg://my-source', 'source_dir')
         hg.pull()
 
         self.mock_run.assert_called_once_with(
             ['hg', 'clone', 'hg://my-source', 'source_dir'], cwd=os.getcwd())
 
     def test_pull_branch(self):
-        hg = sources.Mercurial('hg://my-source', 'source_dir',
-                               source_branch='my-branch')
+        hg = snapcraft.sources.Mercurial('hg://my-source', 'source_dir',
+                                         source_branch='my-branch')
         hg.pull()
 
         self.mock_run.assert_called_once_with(
@@ -222,8 +230,8 @@
              'source_dir'], cwd=os.getcwd())
 
     def test_pull_tag(self):
-        hg = sources.Mercurial('hg://my-source', 'source_dir',
-                               source_tag='tag')
+        hg = snapcraft.sources.Mercurial('hg://my-source', 'source_dir',
+                                         source_tag='tag')
         hg.pull()
 
         self.mock_run.assert_called_once_with(
@@ -233,7 +241,7 @@
     def test_pull_existing(self):
         self.mock_path_exists.return_value = True
 
-        hg = sources.Mercurial('hg://my-source', 'source_dir')
+        hg = snapcraft.sources.Mercurial('hg://my-source', 'source_dir')
         hg.pull()
 
         self.mock_run.assert_called_once_with(
@@ -242,8 +250,8 @@
     def test_pull_existing_with_tag(self):
         self.mock_path_exists.return_value = True
 
-        hg = sources.Mercurial('hg://my-source', 'source_dir',
-                               source_tag='tag')
+        hg = snapcraft.sources.Mercurial('hg://my-source', 'source_dir',
+                                         source_tag='tag')
         hg.pull()
 
         self.mock_run.assert_called_once_with(
@@ -252,8 +260,8 @@
     def test_pull_existing_with_branch(self):
         self.mock_path_exists.return_value = True
 
-        hg = sources.Mercurial('hg://my-source', 'source_dir',
-                               source_branch='my-branch')
+        hg = snapcraft.sources.Mercurial('hg://my-source', 'source_dir',
+                                         source_branch='my-branch')
         hg.pull()
 
         self.mock_run.assert_called_once_with(
@@ -261,16 +269,18 @@
             cwd=os.getcwd())
 
     def test_provision(self):
-        bzr = sources.Mercurial('hg://my-source', 'source_dir')
+        bzr = snapcraft.sources.Mercurial('hg://my-source', 'source_dir')
         bzr.provision('dst')
 
         self.mock_run.assert_called_once_with(
             ['cp', '-Trfa', 'source_dir', 'dst'], cwd=os.getcwd())
 
     def test_init_with_source_branch_and_tag_raises_exception(self):
-        with self.assertRaises(sources.IncompatibleOptionsError) as raised:
-            sources.Mercurial('hg://mysource', 'source_dir', source_tag='tag',
-                              source_branch='branch')
+        with self.assertRaises(
+                snapcraft.sources.IncompatibleOptionsError) as raised:
+            snapcraft.sources.Mercurial(
+                'hg://mysource', 'source_dir', source_tag='tag',
+                source_branch='branch')
 
         expected_message = (
             'can\'t specify both source-tag and source-branch for a mercurial '
@@ -303,11 +313,11 @@
         self.addCleanup(patcher.stop)
 
     def test_pull(self):
-        local = sources.Local('.', 'source_dir')
+        local = snapcraft.sources.Local('.', 'source_dir')
         self.assertTrue(local.pull())
 
     def test_provision(self):
-        local = sources.Local('.', 'source_dir')
+        local = snapcraft.sources.Local('.', 'source_dir')
         local.provision('dst')
 
         self.mock_rmdir.assert_called_once_with('dst')
@@ -317,9 +327,25 @@
     def test_provision_when_target_is_file(self):
         self.mock_isdir.return_value = False
 
-        local = sources.Local('.', 'source_dir')
+        local = snapcraft.sources.Local('.', 'source_dir')
         local.provision('dst')
 
         self.mock_remove.assert_called_once_with('dst')
         self.mock_symlink.assert_called_once_with(
             '/home/ubuntu/sources/snap/source', 'dst')
+
+
+class TestUri(tests.TestCase):
+
+    def test_get_tar_source_from_uri(self):
+        sources = [
+            'https://golang.tar.gz',
+            'https://golang.tar.xz',
+            'https://golang.tar.bz2',
+            'https://golang.tar.tgz',
+        ]
+
+        for source in sources:
+            with self.subTest(key=source):
+                self.assertEqual(
+                    snapcraft.sources._get_source_type_from_uri(source), 'tar')