Repository: ARMmbed/yotta
Pull request #723 (2016-02-26 14:54:55)

support yotta install name@version
Fixes longstanding bug #687 / #279.

Needs tests before merging.



Patch:
diff --git a/yotta/install.py b/yotta/install.py
index f1a5448..28e2816 100644
--- a/yotta/install.py
+++ b/yotta/install.py
@@ -7,7 +7,6 @@
 import argparse
 import logging
 import os
-import re
 
 # Component, , represents an installed component, internal
 from yotta.lib import component
@@ -21,9 +20,6 @@ from yotta.lib import folders
 # --config option, , , internal
 from yotta import options
 
-GitHub_Ref_RE = re.compile('[a-zA-Z0-9-]*/([a-zA-Z0-9-]*)')
-
-
 def addOptions(parser):
     options.config.addTo(parser)
     parser.add_argument('component', default=None, nargs='?',
@@ -145,20 +141,16 @@ def installComponentAsDependency(args, current_component):
             logging.error(error)
         return 1
     modules_dir = current_component.modulesPath()
-    #!!! FIXME: non-registry component spec support (see also installComponent
-    # below), for these the original source should be included in the version
-    # spec, too
-    github_ref_match = GitHub_Ref_RE.match(args.component)
+
+    from yotta.lib import sourceparse
+    # check if we have both a name and specification
+    component_name, component_spec = sourceparse.parseModuleNameAndSpec(args.component)
+    logging.info('%s, %s', component_name, component_spec)
+
+    if component_name == current_component.getName():
+        logging.error('will not install module %s as a dependency of itself', component_name)
+        return -1
     try:
-        if github_ref_match:
-            component_name = github_ref_match.group(1)
-            component_spec = args.component
-        else:
-            component_name = args.component
-            component_spec = '*'
-        if component_name == current_component.getName():
-            logging.error('will not install module %s as a dependency of itself', component_name)
-            return -1
         installed = access.satisfyVersion(
                 component_name,
                 component_spec,
@@ -166,7 +158,7 @@ def installComponentAsDependency(args, current_component):
                   search_paths = [modules_dir],
              working_directory = modules_dir
         )
-    except access_common.Unavailable as e:
+    except access_common.AccessException as e:
         logging.error(e)
         return 1
 
@@ -197,29 +189,19 @@ def installComponent(args):
     path = folders.globalInstallDirectory() if args.act_globally else os.getcwd()
     logging.debug('install component %s to %s' % (args.component, path))
 
-    # !!! FIXME: should support other URL specs, spec matching should be in
-    # access module
-    github_ref_match = GitHub_Ref_RE.match(args.component)
+    from yotta.lib import sourceparse
+    # check if we have both a name and specification
+    component_name, component_spec = sourceparse.parseModuleNameAndSpec(args.component)
+
     try:
-        if github_ref_match:
-            component_name = github_ref_match.group(1)
-            access.satisfyVersion(
-                      component_name,
-                      args.component,
-                           available = dict(),
-                        search_paths = [path],
-                   working_directory = path
-            )
-        else:
-            component_name = args.component
-            access.satisfyVersion(
-                      component_name,
-                                 '*',
-                           available = dict(),
-                        search_paths = [path],
-                   working_directory = path
-            )
-    except access_common.Unavailable as e:
+        access.satisfyVersion(
+                  component_name,
+                  component_spec,
+                       available = dict(),
+                    search_paths = [path],
+               working_directory = path
+        )
+    except access_common.AccessException as e:
         logging.error('%s', e)
         return 1
     os.chdir(component_name)
diff --git a/yotta/lib/access.py b/yotta/lib/access.py
index ba5e5b4..30d7517 100644
--- a/yotta/lib/access.py
+++ b/yotta/lib/access.py
@@ -51,7 +51,12 @@ def remoteComponentFor(name, version_required, registry='modules'):
         Raises an exception if any arguments are invalid.
     '''
 
-    vs = sourceparse.parseSourceURL(version_required)
+    try:
+        vs = sourceparse.parseSourceURL(version_required)
+    except ValueError as e:
+        raise access_common.Unavailable(
+            '%s' % (e)
+        )
 
     if vs.source_type == 'registry':
         if registry not in ('modules', 'targets'):
@@ -251,12 +256,17 @@ def satisfyVersionFromSearchPaths(name, version_required, search_paths, update=F
     # Pack, , base class for targets and components, internal
     from yotta.lib import pack
 
-    v    = None
+    v = None
+    try:
+        sv = sourceparse.parseSourceURL(version_required)
+    except ValueError as e:
+        logging.error(e)
+        return None
 
     try:
         local_version = searchPathsFor(
             name,
-            sourceparse.parseSourceURL(version_required).semanticSpec(),
+            sv.semanticSpec(),
             search_paths,
             type,
             inherit_shrinkwrap = inherit_shrinkwrap
diff --git a/yotta/lib/sourceparse.py b/yotta/lib/sourceparse.py
index 00f4491..ce9ef39 100644
--- a/yotta/lib/sourceparse.py
+++ b/yotta/lib/sourceparse.py
@@ -11,7 +11,6 @@ try:
     from urlparse import urlsplit
 except ImportError:
     from urllib.parse import urlsplit #pylint: disable=no-name-in-module,import-error
-import re
 
 # version, , represent versions and specifications, internal
 from yotta.lib import version
@@ -48,10 +47,33 @@ class VersionSource(object):
             return self.semantic_spec.match(v)
 
 
+def _splitFragment(url):
+    parsed = urlsplit(url)
+    if '#' in url:
+        return url[:url.index('#')], parsed.fragment
+    else:
+        return url, None
+
+def _getGithubRef(source_url):
+    import re
+    # something/something#spec = github
+    defragmented, fragment = _splitFragment(source_url)
+    github_match = re.match('^[a-z0-9_-]+/([a-z0-9_-]+)$', defragmented, re.I)
+    if github_match:
+        return github_match.group(1), VersionSource('github', defragmented, fragment)
+
+    # something/something@spec = github
+    alternate_github_match = re.match('([a-z0-9_-]+/([a-z0-9_-]+)) *@?([~^><=.0-9a-z\*-]*)$', source_url, re.I)
+    if alternate_github_match:
+        return alternate_github_match.group(2), VersionSource('github', alternate_github_match.group(1), alternate_github_match.group(3))
+
+    return None, None
+
 def parseSourceURL(source_url):
     ''' Parse the specified version source URL (or version spec), and return an
         instance of VersionSource
     '''
+    import re
     parsed = urlsplit(source_url)
 
     if '#' in source_url:
@@ -78,14 +100,45 @@ def parseSourceURL(source_url):
     elif parsed.scheme.startswith('hg+') or parsed.path.endswith('.hg'):
         # hg+anything://anything or anything.hg is a hg repo:
         return VersionSource('hg', without_fragment, parsed.fragment)
-    elif re.match('^[a-z0-9_-]+/[a-z0-9_-]+$', without_fragment, re.I):
-        # something/something#spec = github
-        return VersionSource('github', without_fragment, parsed.fragment)
 
     # something/something@spec = github
-    alternate_github_match = re.match('([a-z0-9_-]+/[a-z0-9_-]+) *@?([~^><=.0-9a-z\*-]*)', source_url, re.I)
-    if alternate_github_match:
-        return VersionSource('github', alternate_github_match.group(0), alternate_github_match.group(1))
+    # something/something#spec = github
+    module_name, github_match = _getGithubRef(source_url)
+    if github_match:
+        return github_match
+
+    raise ValueError("Invalid version specification: \"%s\"" % (source_url))
+
+def parseModuleNameAndSpec(module_name_and_spec):
+    ''' Parse modulename[@versionspec] and return a tuple
+        (module_name_string, version_spec_string).
+
+        Also accepts raw github version specs (Owner/reponame#whatever), as the
+        name can be deduced from these.
+
+        Note that the specification split from the name is not validated. If
+        there is no specification (just a module name) passed in, then '*' will
+        be returned as the specification.
+    '''
+    import re
+    # fist check if this is a raw github specification that we can get the
+    # module name from:
+    name, spec = _getGithubRef(module_name_and_spec)
+    if name and spec:
+        return name, module_name_and_spec
+
+    # next split at the first @, if any
+    name = module_name_and_spec.split('@')[0]
+    spec = module_name_and_spec[len(name)+1:]
+
+    name = name.strip()
+
+    # if there's no specification, return the explicit any-version
+    # specification:
+    if not spec:
+        spec = '*'
 
-    raise ValueError("Invalid version source url: \"%s\"" % (source_url))
+    import sys
+    sys.stdout.flush()
 
+    return name, spec
diff --git a/yotta/main.py b/yotta/main.py
index 012df7d..5bae4f8 100644
--- a/yotta/main.py
+++ b/yotta/main.py
@@ -94,7 +94,8 @@ def main():
     addParser('init', 'init', 'Create a new module.')
     addParser('install', 'install',
         'Add a specific module as a dependency, and download it, or install all '+
-        'dependencies for the current module.'
+        'dependencies for the current module. Use yotta install '+
+        'modulename@version to install a specific version.'
     )
     addParser('build', 'build',
         'Build the current module. Options can be passed to the underlying '+


Test patch:
diff --git a/yotta/test/test_sourceparse.py b/yotta/test/test_sourceparse.py
index ef68e5c..412f4ad 100644
--- a/yotta/test/test_sourceparse.py
+++ b/yotta/test/test_sourceparse.py
@@ -31,7 +31,6 @@ Github_URLs = [
     'username/reponame@^1.2.3',
     'username/reponame@-1.2.3',
     'username/reponame@branch-or-tag-name',
-
 ]
 
 Git_URLs = [
@@ -64,13 +63,7 @@ test_invalid_urls = [
 ]
 
 
-class TestGitAccess(unittest.TestCase):
-    def setUp(self):
-        pass
-
-    def tearDown(self):
-        pass
-
+class TestParseSourceURL(unittest.TestCase):
     def test_registryURLs(self):
         for url in Registry_URLs:
             sv = sourceparse.parseSourceURL(url)
@@ -91,6 +84,54 @@ class TestGitAccess(unittest.TestCase):
             sv = sourceparse.parseSourceURL(url)
             self.assertEqual(sv.source_type, 'hg')
 
+    def test_invalid(self):
+        for url in test_invalid_urls:
+            self.assertRaises(ValueError, sourceparse.parseSourceURL, url)
+
+Valid_Names = [
+    'validname',
+    'another-validname',
+    'another-valid2name123'
+]
+
+class TestParseModuleNameAndSpec(unittest.TestCase):
+    def test_validNames(self):
+        for name in Valid_Names:
+            n, s = sourceparse.parseModuleNameAndSpec(name)
+            self.assertEqual(n, name)
+            self.assertEqual(s, '*')
+
+    def test_GithubRefs(self):
+        for url in Github_URLs:
+            n, s = sourceparse.parseModuleNameAndSpec(url)
+            self.assertEqual(n, 'reponame')
+
+    def test_atVersion(self):
+        for name in Valid_Names:
+            for v in Registry_URLs:
+                if len(v):
+                    nv = name + '@' + v
+                    n, s = sourceparse.parseModuleNameAndSpec(nv)
+                    self.assertEqual(n, name)
+                    self.assertEqual(s, v)
+
+    def test_atGitURL(self):
+        for name in Valid_Names:
+            for v in Git_URLs:
+                nv = name + '@' + v
+                n, s = sourceparse.parseModuleNameAndSpec(nv)
+                self.assertEqual(n, name)
+                self.assertEqual(s, v)
+
+    def test_atHGURL(self):
+        for name in Valid_Names:
+            for v in HG_URLs:
+                nv = name + '@' + v
+                n, s = sourceparse.parseModuleNameAndSpec(nv)
+                self.assertEqual(n, name)
+                self.assertEqual(s, v)
+
+
 if __name__ == '__main__':
     unittest.main()