diff options
-rw-r--r-- | lib/spack/spack/fetch_strategy.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/solver/asp.py | 8 | ||||
-rw-r--r-- | lib/spack/spack/spec.py | 4 | ||||
-rw-r--r-- | lib/spack/spack/test/cmd/install.py | 1 | ||||
-rw-r--r-- | lib/spack/spack/test/conftest.py | 31 | ||||
-rw-r--r-- | lib/spack/spack/test/versions.py | 30 | ||||
-rw-r--r-- | lib/spack/spack/version.py | 83 | ||||
-rw-r--r-- | var/spack/repos/builtin.mock/packages/git-test-commit/package.py | 2 |
8 files changed, 129 insertions, 32 deletions
diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 834a408443..1a7976bf64 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -1595,7 +1595,7 @@ def for_package_version(pkg, version): # if it's a commit, we must use a GitFetchStrategy if version.is_commit and hasattr(pkg, "git"): # Populate the version with comparisons to other commits - version.generate_commit_lookup(pkg) + version.generate_commit_lookup(pkg.name) fetcher = GitFetchStrategy(git=pkg.git, commit=str(version)) return fetcher diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index bbce46026d..6a79f37acd 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -2050,6 +2050,14 @@ class SpecBuilder(object): for s in self._specs.values(): spack.spec.Spec.ensure_no_deprecated(s) + # Add git version lookup info to concrete Specs (this is generated for + # abstract specs as well but the Versions may be replaced during the + # 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) + return self._specs diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index b4c778ee76..e933984a06 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -5047,9 +5047,7 @@ class SpecParser(spack.parse.Parser): spec.name and spec.versions.concrete and isinstance(spec.version, vn.Version) and spec.version.is_commit ): - pkg = spec.package - if hasattr(pkg, 'git'): - spec.version.generate_commit_lookup(pkg) + spec.version.generate_commit_lookup(spec.fullname) return specs diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index 3fdfe24aa4..402b500c3e 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -263,6 +263,7 @@ def test_install_commit( 'git', 'file://%s' % repo_path, raising=False) + # Use the earliest commit in the respository commit = commits[-1] spec = spack.spec.Spec('git-test-commit@%s' % commit) spec.concretize() diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 32c73af18c..d4d2c64aaa 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -75,7 +75,17 @@ commit_counter = 0 @pytest.fixture -def mock_git_version_info(tmpdir, scope="function"): +def override_git_repos_cache_path(tmpdir): + saved = spack.paths.user_repos_cache_path + tmp_path = tmpdir.mkdir('git-repo-cache-path-for-tests') + spack.paths.user_repos_cache_path = str(tmp_path) + yield + spack.paths.user_repos_cache_path = saved + + +@pytest.fixture +def mock_git_version_info(tmpdir, override_git_repos_cache_path, + scope="function"): """Create a mock git repo with known structure The structure of commits in this repo is as follows:: @@ -116,10 +126,16 @@ def mock_git_version_info(tmpdir, scope="function"): git('config', 'user.name', 'Spack') git('config', 'user.email', 'spack@spack.io') + commits = [] + + def latest_commit(): + return git('rev-list', '-n1', 'HEAD', output=str, error=str).strip() + # Add two commits on main branch write_file(filename, '[]') git('add', filename) commit('first commit') + commits.append(latest_commit()) # Get name of default branch (differs by git version) main = git('rev-parse', '--abbrev-ref', 'HEAD', output=str, error=str).strip() @@ -127,37 +143,42 @@ def mock_git_version_info(tmpdir, scope="function"): # Tag second commit as v1.0 write_file(filename, "[1, 0]") commit('second commit') + commits.append(latest_commit()) git('tag', 'v1.0') # Add two commits and a tag on 1.x branch git('checkout', '-b', '1.x') write_file(filename, "[1, 0, '', 1]") commit('first 1.x commit') + commits.append(latest_commit()) write_file(filename, "[1, 1]") commit('second 1.x commit') + commits.append(latest_commit()) git('tag', 'v1.1') # Add two commits and a tag on main branch git('checkout', main) write_file(filename, "[1, 0, '', 1]") commit('third main commit') + commits.append(latest_commit()) write_file(filename, "[2, 0]") commit('fourth main commit') + commits.append(latest_commit()) git('tag', 'v2.0') # Add two more commits on 1.x branch to ensure we aren't cheating by using time git('checkout', '1.x') write_file(filename, "[1, 1, '', 1]") commit('third 1.x commit') + commits.append(latest_commit()) write_file(filename, "[1, 2]") commit('fourth 1.x commit') + commits.append(latest_commit()) git('tag', '1.2') # test robust parsing to different syntax, no v - # Get the commits in topo order - log = git('log', '--all', '--pretty=format:%H', '--topo-order', - output=str, error=str) - commits = [c for c in log.split('\n') if c] + # The commits are ordered with the last commit first in the list + commits = list(reversed(commits)) # Return the git directory to install, the filename used, and the commits yield repo_path, filename, commits diff --git a/lib/spack/spack/test/versions.py b/lib/spack/spack/test/versions.py index 9b89222162..dcffcf379f 100644 --- a/lib/spack/spack/test/versions.py +++ b/lib/spack/spack/test/versions.py @@ -607,6 +607,36 @@ def test_versions_from_git(mock_git_version_info, monkeypatch, mock_packages): assert str(comparator) == expected +@pytest.mark.skipif(sys.platform == 'win32', + reason="Not supported on Windows (yet)") +def test_git_hash_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.PackageBase, + 'git', 'file://%s' % repo_path, + raising=False) + + # Spec based on earliest commit + spec0 = spack.spec.Spec('git-test-commit@%s' % commits[-1]) + spec0.concretize() + assert spec0.satisfies('@:0') + assert not spec0.satisfies('@1.0') + + # Spec based on second commit (same as version 1.0) + spec1 = spack.spec.Spec('git-test-commit@%s' % commits[-2]) + spec1.concretize() + assert spec1.satisfies('@1.0') + assert not spec1.satisfies('@1.1:') + + # Spec based on 4th commit (in timestamp order) + spec4 = spack.spec.Spec('git-test-commit@%s' % commits[-4]) + spec4.concretize() + assert spec4.satisfies('@1.1') + assert spec4.satisfies('@1.0:1.2') + + 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') diff --git a/lib/spack/spack/version.py b/lib/spack/spack/version.py index d3dd01c3a5..495cfee6e3 100644 --- a/lib/spack/spack/version.py +++ b/lib/spack/spack/version.py @@ -171,7 +171,7 @@ class Version(object): "version", "separators", "string", - "commit_lookup", + "_commit_lookup", "is_commit", "commit_version", ] @@ -188,7 +188,7 @@ class Version(object): raise ValueError("Bad characters in version string: %s" % string) # An object that can lookup git commits to compare them to versions - self.commit_lookup = None + self._commit_lookup = None self.commit_version = None segments = SEGMENT_REGEX.findall(string) self.version = tuple( @@ -467,7 +467,13 @@ class Version(object): else: return VersionList() - def generate_commit_lookup(self, pkg): + @property + def commit_lookup(self): + if self._commit_lookup: + self._commit_lookup.get(self.string) + return self._commit_lookup + + def generate_commit_lookup(self, pkg_name): """ Use the git fetcher to look up a version for a commit. @@ -483,17 +489,13 @@ class Version(object): fetcher: the fetcher to use. versions: the known versions of the package """ - if self.commit_lookup: - return # Sanity check we have a commit if not self.is_commit: tty.die("%s is not a commit." % self) # Generate a commit looker-upper - self.commit_lookup = CommitLookup(pkg) - self.commit_lookup.get(self.string) - self.commit_lookup.save() + self._commit_lookup = CommitLookup(pkg_name) class VersionRange(object): @@ -991,24 +993,55 @@ class CommitLookup(object): Version.is_commit returns True to allow for comparisons between git commits and versions as represented by tags in the git repository. """ - def __init__(self, pkg): - self.pkg = pkg - - # We require the full git repository history - import spack.fetch_strategy # break cycle - fetcher = spack.fetch_strategy.GitFetchStrategy(git=pkg.git) - fetcher.get_full_repo = True - self.fetcher = fetcher + def __init__(self, pkg_name): + self.pkg_name = pkg_name self.data = {} - # Cache data in misc_cache - key_base = 'git_metadata' - if not self.repository_uri.startswith('/'): - key_base += '/' - self.cache_key = key_base + self.repository_uri - spack.caches.misc_cache.init_entry(self.cache_key) - self.cache_path = spack.caches.misc_cache.cache_path(self.cache_key) + self._pkg = None + self._fetcher = None + self._cache_key = None + self._cache_path = None + + # The following properties are used as part of a lazy reference scheme + # to avoid querying the package repository until it is necessary (and + # in particular to wait until after the configuration has been + # assembled) + @property + def cache_key(self): + if not self._cache_key: + key_base = 'git_metadata' + if not self.repository_uri.startswith('/'): + key_base += '/' + self._cache_key = key_base + self.repository_uri + + # Cache data in misc_cache + # If this is the first lazy access, initialize the cache as well + spack.caches.misc_cache.init_entry(self.cache_key) + return self._cache_key + + @property + def cache_path(self): + if not self._cache_path: + self._cache_path = spack.caches.misc_cache.cache_path( + self.cache_key) + return self._cache_path + + @property + def pkg(self): + if not self._pkg: + self._pkg = spack.repo.get(self.pkg_name) + return self._pkg + + @property + def fetcher(self): + if not self._fetcher: + # We require the full git repository history + import spack.fetch_strategy # break cycle + fetcher = spack.fetch_strategy.GitFetchStrategy(git=self.pkg.git) + fetcher.get_full_repo = True + self._fetcher = fetcher + return self._fetcher @property def repository_uri(self): @@ -1073,6 +1106,10 @@ class CommitLookup(object): # Lookup commit info with working_dir(dest): + # TODO: we need to update the local tags if they changed on the + # remote instance, simply adding '-f' may not be sufficient + # (if commits are deleted on the remote, this command alone + # won't properly update the local rev-list) self.fetcher.git("fetch", '--tags') # Ensure commit is an object known to git diff --git a/var/spack/repos/builtin.mock/packages/git-test-commit/package.py b/var/spack/repos/builtin.mock/packages/git-test-commit/package.py index 7f33bca4f0..0eec7c8b62 100644 --- a/var/spack/repos/builtin.mock/packages/git-test-commit/package.py +++ b/var/spack/repos/builtin.mock/packages/git-test-commit/package.py @@ -17,6 +17,8 @@ class GitTestCommit(Package): version('2.0', tag='v2.0') def install(self, spec, prefix): + # It is assumed for the test which installs this package, that it will + # be using the earliest commit, which is contained in the range @:0 assert spec.satisfies('@:0') mkdir(prefix.bin) |