Feature: use git branches/tags as versions (spack#31200)

Building on spack#24639, this allows versions to be prefixed by `git.`. If a version begins `git.`, it is treated as a git ref, and handled as git commits are starting in the referenced PR.

An exception is made for versions that are `git.develop`, `git.main`, `git.master`, `git.head`, or `git.trunk`. Those are assumed to be greater than all other versions, as those prefixed strings are in other contexts.

lib/spack/spack/cmd/checksum.py

@@ -16,7 +16,7 @@
 import spack.util.crypto
 from spack.package_base import preferred_version
 from spack.util.naming import valid_fully_qualified_module_name
-from spack.version import Version, ver
+from spack.version import VersionBase, ver
 
 description = "checksum available versions of a package"
 section = "packaging"
@@ -65,7 +65,7 @@ def checksum(parser, args):
         remote_versions = None
         for version in versions:
             version = ver(version)
-            if not isinstance(version, Version):
+            if not isinstance(version, VersionBase):
                 tty.die("Cannot generate checksums for version lists or "
                         "version ranges. Use unambiguous versions.")
             url = pkg.find_valid_url_for_version(version)

lib/spack/spack/directives.py

@@ -46,7 +46,7 @@ class OpenMpi(Package):
 from spack.dependency import Dependency, canonical_deptype, default_deptype
 from spack.fetch_strategy import from_kwargs
 from spack.resource import Resource
-from spack.version import Version, VersionChecksumError
+from spack.version import GitVersion, Version, VersionChecksumError, VersionLookupError
 
 __all__ = ['DirectiveError', 'DirectiveMeta', 'version', 'conflicts', 'depends_on',
            'extends', 'provides', 'patch', 'variant', 'resource']
@@ -330,7 +330,17 @@ def _execute_version(pkg):
             kwargs['checksum'] = checksum
 
         # Store kwargs for the package to later with a fetch_strategy.
-        pkg.versions[Version(ver)] = kwargs
+        version = Version(ver)
+        if isinstance(version, GitVersion):
+            if not hasattr(pkg, 'git') and 'git' not in kwargs:
+                msg = "Spack version directives cannot include git hashes fetched from"
+                msg += " URLs. Error in package '%s'\n" % pkg.name
+                msg += "    version('%s', " % version.string
+                msg += ', '.join("%s='%s'" % (argname, value)
+                                 for argname, value in kwargs.items())
+                msg += ")"
+                raise VersionLookupError(msg)
+        pkg.versions[version] = kwargs
     return _execute_version
 
 

lib/spack/spack/fetch_strategy.py

@@ -1575,16 +1575,30 @@ def for_package_version(pkg, version):
 
     check_pkg_attributes(pkg)
 
-    if not isinstance(version, spack.version.Version):
+    if not isinstance(version, spack.version.VersionBase):
         version = spack.version.Version(version)
 
     # if it's a commit, we must use a GitFetchStrategy
-    if version.is_commit and hasattr(pkg, "git"):
+    if isinstance(version, spack.version.GitVersion):
+        if not hasattr(pkg, "git"):
+            raise FetchError(
+                "Cannot fetch git version for %s. Package has no 'git' attribute" %
+                pkg.name
+            )
         # Populate the version with comparisons to other commits
-        version.generate_commit_lookup(pkg.name)
+        version.generate_git_lookup(pkg.name)
+
+        # For GitVersion, we have no way to determine whether a ref is a branch or tag
+        # Fortunately, we handle branches and tags identically, except tags are
+        # handled slightly more conservatively for older versions of git.
+        # We call all non-commit refs tags in this context, at the cost of a slight
+        # performance hit for branches on older versions of git.
+        # Branches cannot be cached, so we tell the fetcher not to cache tags/branches
+        ref_type = 'commit' if version.is_commit else 'tag'
         kwargs = {
             'git': pkg.git,
-            'commit': str(version)
+            ref_type: version.ref,
+            'no_cache': True,
         }
         kwargs['submodules'] = getattr(pkg, 'submodules', False)
         fetcher = GitFetchStrategy(**kwargs)

lib/spack/spack/package_base.py

@@ -62,7 +62,7 @@
 from spack.util.executable import ProcessError, which
 from spack.util.package_hash import package_hash
 from spack.util.prefix import Prefix
-from spack.version import Version
+from spack.version import GitVersion, Version, VersionBase
 
 if sys.version_info[0] >= 3:
     FLAG_HANDLER_RETURN_TYPE = Tuple[
@@ -1041,7 +1041,7 @@ def all_urls_for_version(self, version, custom_url_for_version=None):
         return self._implement_all_urls_for_version(version, uf)
 
     def _implement_all_urls_for_version(self, version, custom_url_for_version=None):
-        if not isinstance(version, Version):
+        if not isinstance(version, VersionBase):
             version = Version(version)
 
         urls = []
@@ -1505,7 +1505,7 @@ def do_fetch(self, mirror_only=False):
         checksum = spack.config.get('config:checksum')
         fetch = self.stage.managed_by_spack
         if checksum and fetch and (self.version not in self.versions) \
-                and (not self.version.is_commit):
+                and (not isinstance(self.version, GitVersion)):
             tty.warn("There is no checksum on file to fetch %s safely." %
                      self.spec.cformat('{name}{@version}'))
 

lib/spack/spack/solver/asp.py

@@ -1429,11 +1429,16 @@ def key_fn(item):
                     continue
 
                 known_versions = self.possible_versions[dep.name]
-                if (not dep.version.is_commit and
+                if (not isinstance(dep.version, spack.version.GitVersion) and
                     any(v.satisfies(dep.version) for v in known_versions)):
                     # some version we know about satisfies this constraint, so we
                     # should use that one. e.g, if the user asks for qt@5 and we
-                    # know about qt@5.5.
+                    # know about qt@5.5. This ensures we don't add under-specified
+                    # versions to the solver
+                    #
+                    # For git versions, we know the version is already fully specified
+                    # so we don't have to worry about whether it's an under-specified
+                    # version
                     continue
 
                 # if there is a concrete version on the CLI *that we know nothing
@@ -1678,7 +1683,7 @@ def define_virtual_constraints(self):
 
         # extract all the real versions mentioned in version ranges
         def versions_for(v):
-            if isinstance(v, spack.version.Version):
+            if isinstance(v, spack.version.VersionBase):
                 return [v]
             elif isinstance(v, spack.version.VersionRange):
                 result = [v.start] if v.start else []
@@ -2187,8 +2192,8 @@ def build_specs(self, function_tuples):
         # concretization process)
         for root in self._specs.values():
             for spec in root.traverse():
-                if spec.version.is_commit:
-                    spec.version.generate_commit_lookup(spec.fullname)
+                if isinstance(spec.version, spack.version.GitVersion):
+                    spec.version.generate_git_lookup(spec.fullname)
 
         return self._specs
 

lib/spack/spack/spec.py

@@ -5154,9 +5154,9 @@ def do_parse(self):
             # Note: VersionRange(x, x) is currently concrete, hence isinstance(...).
             if (
                 spec.name and spec.versions.concrete and
-                isinstance(spec.version, vn.Version) and spec.version.is_commit
+                isinstance(spec.version, vn.GitVersion)
             ):
-                spec.version.generate_commit_lookup(spec.fullname)
+                spec.version.generate_git_lookup(spec.fullname)
 
         return specs
 

lib/spack/spack/test/versions.py

@@ -17,7 +17,14 @@
 import spack.package_base
 import spack.spec
 from spack.util.executable import which
-from spack.version import Version, VersionList, VersionRange, ver
+from spack.version import (
+    GitVersion,
+    Version,
+    VersionBase,
+    VersionList,
+    VersionRange,
+    ver,
+)
 
 
 def assert_ver_lt(a, b):
@@ -520,7 +527,7 @@ def test_repr_and_str():
 
     def check_repr_and_str(vrs):
         a = Version(vrs)
-        assert repr(a) == "Version('" + vrs + "')"
+        assert repr(a) == "VersionBase('" + vrs + "')"
         b = eval(repr(a))
         assert a == b
         assert str(a) == vrs
@@ -544,19 +551,19 @@ def test_get_item():
     assert isinstance(a[1], int)
     # Test slicing
     b = a[0:2]
-    assert isinstance(b, Version)
+    assert isinstance(b, VersionBase)
     assert b == Version('0.1')
-    assert repr(b) == "Version('0.1')"
+    assert repr(b) == "VersionBase('0.1')"
     assert str(b) == '0.1'
     b = a[0:3]
-    assert isinstance(b, Version)
+    assert isinstance(b, VersionBase)
     assert b == Version('0.1_2')
-    assert repr(b) == "Version('0.1_2')"
+    assert repr(b) == "VersionBase('0.1_2')"
     assert str(b) == '0.1_2'
     b = a[1:]
-    assert isinstance(b, Version)
+    assert isinstance(b, VersionBase)
     assert b == Version('1_2-3')
-    assert repr(b) == "Version('1_2-3')"
+    assert repr(b) == "VersionBase('1_2-3')"
     assert str(b) == '1_2-3'
     # Raise TypeError on tuples
     with pytest.raises(TypeError):
@@ -597,7 +604,7 @@ def test_versions_from_git(mock_git_version_info, monkeypatch, mock_packages):
         spec = spack.spec.Spec('git-test-commit@%s' % commit)
         version = spec.version
         comparator = [str(v) if not isinstance(v, int) else v
-                      for v in version._cmp(version.commit_lookup)]
+                      for v in version._cmp(version.ref_lookup)]
 
         with working_dir(repo_path):
             which('git')('checkout', commit)
@@ -637,6 +644,43 @@ def test_git_hash_comparisons(
     assert spec4.satisfies('@1.0:1.2')
 
 
+@pytest.mark.skipif(sys.platform == 'win32',
+                    reason="Not supported on Windows (yet)")
+def test_git_ref_comparisons(
+        mock_git_version_info, install_mockery, mock_packages, monkeypatch):
+    """Check that hashes compare properly to versions
+    """
+    repo_path, filename, commits = mock_git_version_info
+    monkeypatch.setattr(spack.package_base.PackageBase,
+                        'git', 'file://%s' % repo_path,
+                        raising=False)
+
+    # Spec based on tag v1.0
+    spec_tag = spack.spec.Spec('git-test-commit@git.v1.0')
+    spec_tag.concretize()
+    assert spec_tag.satisfies('@1.0')
+    assert not spec_tag.satisfies('@1.1:')
+    assert str(spec_tag.version) == 'git.v1.0'
+
+    # Spec based on branch 1.x
+    spec_branch = spack.spec.Spec('git-test-commit@git.1.x')
+    spec_branch.concretize()
+    assert spec_branch.satisfies('@1.2')
+    assert spec_branch.satisfies('@1.1:1.3')
+    assert str(spec_branch.version) == 'git.1.x'
+
+
+@pytest.mark.parametrize('string,git', [
+    ('1.2.9', False),
+    ('gitmain', False),
+    ('git.foo', True),
+    ('git.abcdabcdabcdabcdabcdabcdabcdabcdabcdabcd', True),
+    ('abcdabcdabcdabcdabcdabcdabcdabcdabcdabcd', True),
+])
+def test_version_git_vs_base(string, git):
+    assert isinstance(Version(string), GitVersion) == git
+
+
 def test_version_range_nonempty():
     assert Version('1.2.9') in VersionRange('1.2.0', '1.2')
     assert Version('1.1.1') in ver('1.0:1')