From 20b53b8320b8c9dcb7d32821c8ed99b58c30eda2 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 17 Jan 2023 10:22:09 -0800 Subject: [PATCH 01/39] updating changes before troubleshooting style --- lib/spack/spack/parser.py | 3 +++ lib/spack/spack/test/spec_syntax.py | 18 +++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index b4748b259f774c..a9af2e7c26be46 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -406,6 +406,9 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: matches = spack.environment.active_environment().get_by_hash(dag_hash) if not matches: matches = spack.store.db.get_by_hash(dag_hash) + if not matches: + query = spack.binary_distribution.BinaryCacheQuery(True) + matches = query(self.ctx.current_token.value) if not matches: raise spack.spec.NoSuchHashError(dag_hash) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 87d1c7b8e04098..4c866b2aaf7a10 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -629,10 +629,12 @@ def test_spec_by_hash_tokens(text, tokens): parser = SpecParser(text) assert parser.tokens() == tokens - @pytest.mark.db -def test_spec_by_hash(database): +def test_spec_by_hash(database, config): mpileaks = database.query_one("mpileaks ^zmpi") + a = spack.spec.Spec("a").concretized() + monkeypatch.setattr(spack.binary_distribution, + "update_cache_and_get_specs", lambda: a) hash_str = f"/{mpileaks.dag_hash()}" assert str(SpecParser(hash_str).next_spec()) == str(mpileaks) @@ -643,9 +645,11 @@ def test_spec_by_hash(database): name_version_and_hash = f"{mpileaks.name}@{mpileaks.version} /{mpileaks.dag_hash()[:5]}" assert str(SpecParser(name_version_and_hash).next_spec()) == str(mpileaks) + a_hash = f"/{a.dag_hash()}" + assert SpecParser(a_hash).next_spec() == a @pytest.mark.db -def test_dep_spec_by_hash(database): +def test_dep_spec_by_hash(database, config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") zmpi = database.query_one("zmpi") fake = database.query_one("fake") @@ -675,7 +679,7 @@ def test_dep_spec_by_hash(database): @pytest.mark.db -def test_multiple_specs_with_hash(database): +def test_multiple_specs_with_hash(database, config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") callpath_mpich2 = database.query_one("callpath ^mpich2") @@ -707,7 +711,7 @@ def test_multiple_specs_with_hash(database): @pytest.mark.db -def test_ambiguous_hash(mutable_database, default_mock_concretization): +def test_ambiguous_hash(mutable_database, default_mock_concretization, config): x1 = default_mock_concretization("a") x2 = x1.copy() x1._hash = "xyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy" @@ -725,7 +729,7 @@ def test_ambiguous_hash(mutable_database, default_mock_concretization): @pytest.mark.db -def test_invalid_hash(database): +def test_invalid_hash(database, config): zmpi = database.query_one("zmpi") mpich = database.query_one("mpich") @@ -741,7 +745,7 @@ def test_invalid_hash(database): @pytest.mark.db -def test_nonexistent_hash(database): +def test_nonexistent_hash(database, config): """Ensure we get errors for non existent hashes.""" specs = database.query() From 9d02385d9c8ff166333410122121dcf07c623a8e Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Wed, 18 Jan 2023 15:23:01 -0800 Subject: [PATCH 02/39] debugged tests --- lib/spack/spack/binary_distribution.py | 2 +- lib/spack/spack/test/spec_syntax.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index a406101d2cb120..4074fbcf168e74 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -420,7 +420,7 @@ def update(self, with_cooldown=False): self._write_local_index_cache() - if all_methods_failed: + if configured_mirror_urls and all_methods_failed: raise FetchCacheError(fetch_errors) if fetch_errors: tty.warn( diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 4c866b2aaf7a10..971255d3c7da5d 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -630,11 +630,10 @@ def test_spec_by_hash_tokens(text, tokens): assert parser.tokens() == tokens @pytest.mark.db -def test_spec_by_hash(database, config): +def test_spec_by_hash(database, monkeypatch, mutable_empty_config): mpileaks = database.query_one("mpileaks ^zmpi") a = spack.spec.Spec("a").concretized() - monkeypatch.setattr(spack.binary_distribution, - "update_cache_and_get_specs", lambda: a) + monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [a]) hash_str = f"/{mpileaks.dag_hash()}" assert str(SpecParser(hash_str).next_spec()) == str(mpileaks) @@ -648,8 +647,9 @@ def test_spec_by_hash(database, config): a_hash = f"/{a.dag_hash()}" assert SpecParser(a_hash).next_spec() == a + @pytest.mark.db -def test_dep_spec_by_hash(database, config): +def test_dep_spec_by_hash(database, mutable_empty_config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") zmpi = database.query_one("zmpi") fake = database.query_one("fake") @@ -679,7 +679,7 @@ def test_dep_spec_by_hash(database, config): @pytest.mark.db -def test_multiple_specs_with_hash(database, config): +def test_multiple_specs_with_hash(database, mutable_empty_config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") callpath_mpich2 = database.query_one("callpath ^mpich2") @@ -711,7 +711,7 @@ def test_multiple_specs_with_hash(database, config): @pytest.mark.db -def test_ambiguous_hash(mutable_database, default_mock_concretization, config): +def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_empty_config): x1 = default_mock_concretization("a") x2 = x1.copy() x1._hash = "xyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy" @@ -745,7 +745,7 @@ def test_invalid_hash(database, config): @pytest.mark.db -def test_nonexistent_hash(database, config): +def test_nonexistent_hash(database, mutable_empty_config): """Ensure we get errors for non existent hashes.""" specs = database.query() From ab39de32abed767514d838e926538cf0497ae6ec Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Thu, 19 Jan 2023 12:06:17 -0800 Subject: [PATCH 03/39] style --- lib/spack/spack/test/spec_syntax.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 971255d3c7da5d..62e296cabe2291 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -629,6 +629,7 @@ def test_spec_by_hash_tokens(text, tokens): parser = SpecParser(text) assert parser.tokens() == tokens + @pytest.mark.db def test_spec_by_hash(database, monkeypatch, mutable_empty_config): mpileaks = database.query_one("mpileaks ^zmpi") From 4dd74b42585a15779a6691048fdff66eee8414ce Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Thu, 19 Jan 2023 13:10:33 -0800 Subject: [PATCH 04/39] added doc for BinaryCacheQuery --- lib/spack/spack/binary_distribution.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 4074fbcf168e74..5a45c4e55a3055 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -2426,6 +2426,10 @@ def __init__(self, all_architectures): self.possible_specs = specs def __call__(self, spec, **kwargs): + """ + Args: + spec (str): The spec being searched for in its string representation or hash. + """ matches = [] if spec.startswith("/"): # Matching a DAG hash From 84004d6e6d06c56ea0f5c68102a9ea85c6e84702 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Thu, 19 Jan 2023 13:14:06 -0800 Subject: [PATCH 05/39] forgot one mutable_empty_config --- lib/spack/spack/test/spec_syntax.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 62e296cabe2291..40689d1e2bee8d 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -730,7 +730,7 @@ def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_e @pytest.mark.db -def test_invalid_hash(database, config): +def test_invalid_hash(database, mutable_empty_config): zmpi = database.query_one("zmpi") mpich = database.query_one("mpich") From b6046668b8d3fd3b8ba9d2c66c5ec70456b74634 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 21 Feb 2023 10:32:22 -0800 Subject: [PATCH 06/39] folding in changes to satisfies --- lib/spack/spack/parser.py | 28 +++--------- lib/spack/spack/spec.py | 66 ++++++++++++++++++++++++++++- lib/spack/spack/test/spec_syntax.py | 10 ++++- 3 files changed, 78 insertions(+), 26 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index a9af2e7c26be46..be6e9b5054f3ef 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -315,7 +315,6 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: Return The object passed as argument """ - import spack.environment # Needed to retrieve by hash # If we start with a package name we have a named spec, we cannot # accept another package name afterwards in a node @@ -400,30 +399,15 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: value = value.strip("'\" ") initial_spec._add_flag(name, value, propagate=True) elif not self.has_hash and self.ctx.accept(TokenType.DAG_HASH): - dag_hash = self.ctx.current_token.value[1:] - matches = [] - if spack.environment.active_environment(): - matches = spack.environment.active_environment().get_by_hash(dag_hash) - if not matches: - matches = spack.store.db.get_by_hash(dag_hash) - if not matches: - query = spack.binary_distribution.BinaryCacheQuery(True) - matches = query(self.ctx.current_token.value) - if not matches: - raise spack.spec.NoSuchHashError(dag_hash) - - if len(matches) != 1: - raise spack.spec.AmbiguousHashError( - f"Multiple packages specify hash beginning '{dag_hash}'.", *matches - ) - spec_by_hash = matches[0] - if not spec_by_hash.satisfies(initial_spec): - raise spack.spec.InvalidHashError(initial_spec, spec_by_hash.dag_hash()) - initial_spec._dup(spec_by_hash) - + initial_spec.abstract_hash = self.ctx.current_token.value[1:] + print("found abstract hash: setting {}".format(self.ctx.current_token.value[1:])) self.has_hash = True else: break + if self.has_hash: + print("final parsed abstract hash is {}".format(initial_spec.abstract_hash)) + print("final parsed spec is {}".format(initial_spec)) + print("type of parsed spec is {}".format(type(initial_spec))) return initial_spec diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index ced35d5c7d0609..04313382fb44b4 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1258,6 +1258,7 @@ def copy(self, *args, **kwargs): class Spec(object): #: Cache for spec's prefix, computed lazily in the corresponding property _prefix = None + abstract_hash = None @staticmethod def default_arch(): @@ -1272,7 +1273,7 @@ def __init__( normal=False, concrete=False, external_path=None, - external_modules=None, + external_modules=None ): """Create a new Spec. @@ -1832,6 +1833,44 @@ def process_hash_bit_prefix(self, bits): """Get the first bits of the DAG hash as an integer type.""" return spack.util.hash.base32_prefix_bits(self.process_hash(), bits) + def lookup_hash(self): + """Given a spec with nothing but a DAG hash, attempt to populate the values by looking up + the hash in the environment, store, or finally, binary caches.""" + import spack.environment + + if not hasattr(self, 'abstract_hash'): + print("spec {} doesn't even know what an abstract hash is!".format(self)) + return + + if not getattr(self, 'abstract_hash', None): + return + print("lookup is trying abstract hash {}".format(self.abstract_hash)) + matches = [] + if spack.environment.active_environment(): + matches = spack.environment.active_environment().get_by_hash(self.abstract_hash) + if not matches: + matches = spack.store.db.get_by_hash(self.abstract_hash) + if not matches: + query = spack.binary_distribution.BinaryCacheQuery(True) + matches = query(self.ctx.current_token.value) + if not matches: + raise spack.spec.NoSuchHashError(self.abstract_hash) + + if len(matches) != 1: + raise spack.spec.AmbiguousHashError( + f"Multiple packages specify hash beginning '{self.abstract_hash}'.", *matches + ) + return matches[0] + + def replace_hash(self): + if not self.abstract_hash: + return + spec_by_hash = self.lookup_hash() + if not spec_by_hash._satisfies(self): + raise spack.spec.InvalidHashError(self, spec_by_hash.dag_hash()) + self._dup(spec_by_hash) + print("I found {0} with dag hash {1}".format(self, self.dag_hash())) + def to_node_dict(self, hash=ht.dag_hash): """Create a dictionary representing the state of this Spec. @@ -2588,6 +2627,8 @@ def _old_concretize(self, tests=False, deprecation_warning=True): ) warnings.warn(msg) + self.replace_hash() + if not self.name: raise spack.error.SpecError("Attempting to concretize anonymous spec") @@ -2786,6 +2827,8 @@ def ensure_no_deprecated(root): def _new_concretize(self, tests=False): import spack.solver.asp + self.replace_hash() + if not self.name: raise spack.error.SpecError("Spec has no name; cannot concretize an anonymous spec") @@ -3351,6 +3394,8 @@ def constrain(self, other, deps=True): other = self._autospec(other) + other.lookup_hash() + if not (self.name == other.name or (not self.name) or (not other.name)): raise UnsatisfiableSpecNameError(self.name, other.name) @@ -3573,7 +3618,19 @@ def intersects(self, other: "Spec", deps: bool = True) -> bool: else: return True - def _intersects_dependencies(self, other): + def satisfies(self, other, deps=True, strict=False): + + other = self._autospec(other) + + lhs = self.lookup_hash() or self + rhs = other.lookup_hash() or other + + return lhs._satisfies(rhs, deps=deps, strict=strict) + + def satisfies_dependencies(self, other, strict=False): + """ + This checks constraints on common dependencies against each other. + """ other = self._autospec(other) if not other._dependencies or not self._dependencies: @@ -3755,6 +3812,7 @@ def _dup(self, other, deps=True, cleardeps=True): and self.external_path != other.external_path and self.external_modules != other.external_modules and self.compiler_flags != other.compiler_flags + and self.abstract_hash != other.abstract_hash ) self._package = None @@ -3797,6 +3855,10 @@ def _dup(self, other, deps=True, cleardeps=True): self._concrete = other._concrete + if self.abstract_hash: + print("Replacing dup abstract hash: {0} for spec {1}".format(other.abstract_hash, other.name)) + self.abstract_hash = other.abstract_hash + if self._concrete: self._dunder_hash = other._dunder_hash self._normal = other._normal diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 40689d1e2bee8d..0e975d738dd484 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -637,10 +637,16 @@ def test_spec_by_hash(database, monkeypatch, mutable_empty_config): monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [a]) hash_str = f"/{mpileaks.dag_hash()}" - assert str(SpecParser(hash_str).next_spec()) == str(mpileaks) + b = SpecParser(hash_str).next_spec() + b.replace_hash() + # b here is NOT a Spec--it is still a query_spec! + # THEREFORE, it does not have an abstract_hash attribute, which would be useless anyway. + print("parser found {}".format(b)) + assert b == mpileaks short_hash_str = f"/{mpileaks.dag_hash()[:5]}" - assert str(SpecParser(short_hash_str).next_spec()) == str(mpileaks) + b = SpecParser(short_hash_str).next_spec().concretize() + assert str(b) == str(mpileaks) name_version_and_hash = f"{mpileaks.name}@{mpileaks.version} /{mpileaks.dag_hash()[:5]}" assert str(SpecParser(name_version_and_hash).next_spec()) == str(mpileaks) From fe451ea829ac4fb34cb08778d47d4236fc3fac37 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 17 Jan 2023 10:22:09 -0800 Subject: [PATCH 07/39] checkpointing before cleanup and updating lookup --- lib/spack/spack/test/spec_syntax.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 0e975d738dd484..e88f9b15aaa076 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -629,12 +629,12 @@ def test_spec_by_hash_tokens(text, tokens): parser = SpecParser(text) assert parser.tokens() == tokens - @pytest.mark.db -def test_spec_by_hash(database, monkeypatch, mutable_empty_config): +def test_spec_by_hash(database, config): mpileaks = database.query_one("mpileaks ^zmpi") a = spack.spec.Spec("a").concretized() - monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [a]) + monkeypatch.setattr(spack.binary_distribution, + "update_cache_and_get_specs", lambda: a) hash_str = f"/{mpileaks.dag_hash()}" b = SpecParser(hash_str).next_spec() @@ -654,9 +654,8 @@ def test_spec_by_hash(database, monkeypatch, mutable_empty_config): a_hash = f"/{a.dag_hash()}" assert SpecParser(a_hash).next_spec() == a - @pytest.mark.db -def test_dep_spec_by_hash(database, mutable_empty_config): +def test_dep_spec_by_hash(database, config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") zmpi = database.query_one("zmpi") fake = database.query_one("fake") @@ -686,7 +685,7 @@ def test_dep_spec_by_hash(database, mutable_empty_config): @pytest.mark.db -def test_multiple_specs_with_hash(database, mutable_empty_config): +def test_multiple_specs_with_hash(database, config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") callpath_mpich2 = database.query_one("callpath ^mpich2") @@ -718,7 +717,7 @@ def test_multiple_specs_with_hash(database, mutable_empty_config): @pytest.mark.db -def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_empty_config): +def test_ambiguous_hash(mutable_database, default_mock_concretization, config): x1 = default_mock_concretization("a") x2 = x1.copy() x1._hash = "xyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy" @@ -736,7 +735,7 @@ def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_e @pytest.mark.db -def test_invalid_hash(database, mutable_empty_config): +def test_invalid_hash(database, config): zmpi = database.query_one("zmpi") mpich = database.query_one("mpich") @@ -752,7 +751,7 @@ def test_invalid_hash(database, mutable_empty_config): @pytest.mark.db -def test_nonexistent_hash(database, mutable_empty_config): +def test_nonexistent_hash(database, config): """Ensure we get errors for non existent hashes.""" specs = database.query() From b50f3a5fd16439dea6b4ac147ca9d6b4b3300218 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Wed, 18 Jan 2023 15:23:01 -0800 Subject: [PATCH 08/39] debugged tests --- lib/spack/spack/test/spec_syntax.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index e88f9b15aaa076..9d61ca897907e6 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -630,11 +630,10 @@ def test_spec_by_hash_tokens(text, tokens): assert parser.tokens() == tokens @pytest.mark.db -def test_spec_by_hash(database, config): +def test_spec_by_hash(database, monkeypatch, mutable_empty_config): mpileaks = database.query_one("mpileaks ^zmpi") a = spack.spec.Spec("a").concretized() - monkeypatch.setattr(spack.binary_distribution, - "update_cache_and_get_specs", lambda: a) + monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [a]) hash_str = f"/{mpileaks.dag_hash()}" b = SpecParser(hash_str).next_spec() @@ -654,8 +653,9 @@ def test_spec_by_hash(database, config): a_hash = f"/{a.dag_hash()}" assert SpecParser(a_hash).next_spec() == a + @pytest.mark.db -def test_dep_spec_by_hash(database, config): +def test_dep_spec_by_hash(database, mutable_empty_config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") zmpi = database.query_one("zmpi") fake = database.query_one("fake") @@ -685,7 +685,7 @@ def test_dep_spec_by_hash(database, config): @pytest.mark.db -def test_multiple_specs_with_hash(database, config): +def test_multiple_specs_with_hash(database, mutable_empty_config): mpileaks_zmpi = database.query_one("mpileaks ^zmpi") callpath_mpich2 = database.query_one("callpath ^mpich2") @@ -717,7 +717,7 @@ def test_multiple_specs_with_hash(database, config): @pytest.mark.db -def test_ambiguous_hash(mutable_database, default_mock_concretization, config): +def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_empty_config): x1 = default_mock_concretization("a") x2 = x1.copy() x1._hash = "xyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy" @@ -751,7 +751,7 @@ def test_invalid_hash(database, config): @pytest.mark.db -def test_nonexistent_hash(database, config): +def test_nonexistent_hash(database, mutable_empty_config): """Ensure we get errors for non existent hashes.""" specs = database.query() From 95e41256f3a3dd3070661a3892f9fe50e9a2a08e Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Thu, 19 Jan 2023 12:06:17 -0800 Subject: [PATCH 09/39] style --- lib/spack/spack/test/spec_syntax.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 9d61ca897907e6..ab1126c6fb6cd9 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -629,6 +629,7 @@ def test_spec_by_hash_tokens(text, tokens): parser = SpecParser(text) assert parser.tokens() == tokens + @pytest.mark.db def test_spec_by_hash(database, monkeypatch, mutable_empty_config): mpileaks = database.query_one("mpileaks ^zmpi") From e94b975968f2d0219db6df9e36df36a706395999 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Thu, 19 Jan 2023 13:14:06 -0800 Subject: [PATCH 10/39] forgot one mutable_empty_config --- lib/spack/spack/test/spec_syntax.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index ab1126c6fb6cd9..0e975d738dd484 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -736,7 +736,7 @@ def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_e @pytest.mark.db -def test_invalid_hash(database, config): +def test_invalid_hash(database, mutable_empty_config): zmpi = database.query_one("zmpi") mpich = database.query_one("mpich") From 7ec00f4fc06dacc73cb27ada716846d02341a285 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Thu, 23 Feb 2023 17:08:33 -0800 Subject: [PATCH 11/39] checkpoint for debugging deps --- lib/spack/spack/parser.py | 3 -- lib/spack/spack/spec.py | 55 ++++++++++++++++++++--------- lib/spack/spack/test/spec_syntax.py | 47 +++++++++++++++--------- 3 files changed, 68 insertions(+), 37 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index be6e9b5054f3ef..bb0cc06b506b25 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -400,14 +400,11 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: initial_spec._add_flag(name, value, propagate=True) elif not self.has_hash and self.ctx.accept(TokenType.DAG_HASH): initial_spec.abstract_hash = self.ctx.current_token.value[1:] - print("found abstract hash: setting {}".format(self.ctx.current_token.value[1:])) self.has_hash = True else: break if self.has_hash: print("final parsed abstract hash is {}".format(initial_spec.abstract_hash)) - print("final parsed spec is {}".format(initial_spec)) - print("type of parsed spec is {}".format(type(initial_spec))) return initial_spec diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 04313382fb44b4..bbe677e2f45cad 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1833,18 +1833,11 @@ def process_hash_bit_prefix(self, bits): """Get the first bits of the DAG hash as an integer type.""" return spack.util.hash.base32_prefix_bits(self.process_hash(), bits) - def lookup_hash(self): - """Given a spec with nothing but a DAG hash, attempt to populate the values by looking up - the hash in the environment, store, or finally, binary caches.""" + def _lookup_hash(self): import spack.environment - if not hasattr(self, 'abstract_hash'): - print("spec {} doesn't even know what an abstract hash is!".format(self)) - return - - if not getattr(self, 'abstract_hash', None): - return print("lookup is trying abstract hash {}".format(self.abstract_hash)) + matches = [] if spack.environment.active_environment(): matches = spack.environment.active_environment().get_by_hash(self.abstract_hash) @@ -1852,7 +1845,7 @@ def lookup_hash(self): matches = spack.store.db.get_by_hash(self.abstract_hash) if not matches: query = spack.binary_distribution.BinaryCacheQuery(True) - matches = query(self.ctx.current_token.value) + matches = query("/"+self.abstract_hash) if not matches: raise spack.spec.NoSuchHashError(self.abstract_hash) @@ -1860,16 +1853,41 @@ def lookup_hash(self): raise spack.spec.AmbiguousHashError( f"Multiple packages specify hash beginning '{self.abstract_hash}'.", *matches ) + return matches[0] + def lookup_hash(self): + + spec = self.copy() + + nodes = [node for node in spec.traverse(order="post") if node.abstract_hash] + if self.name == 'mpileaks': + print(nodes) + for node in nodes: + spec[node.name]._dup(node._lookup_hash()) + + return spec + def replace_hash(self): - if not self.abstract_hash: + """Given a spec with nothing but an abstract hash, attempt to populate the values by + looking up the hash in the environment, store, or finally, binary caches.""" + + if not any(node for node in self.traverse(order="post") if node.abstract_hash): return + + # print([spec.abstract_hash for spec in lookup_specs]) + + # for lookup_spec in lookup_specs: + # spec_by_hash = lookup_spec.lookup_hash() + # if not spec_by_hash._satisfies(lookup_spec): + # raise spack.spec.InvalidHashError(lookup_spec, spec_by_hash.dag_hash()) # abstract? + # lookup_spec._dup(spec_by_hash) + # print("I found {0} with dag hash {1}".format(self, self.dag_hash())) + spec_by_hash = self.lookup_hash() if not spec_by_hash._satisfies(self): - raise spack.spec.InvalidHashError(self, spec_by_hash.dag_hash()) + raise spack.spec.InvalidHashError(self, spec_by_hash.dag_hash()) # abstract? self._dup(spec_by_hash) - print("I found {0} with dag hash {1}".format(self, self.dag_hash())) def to_node_dict(self, hash=ht.dag_hash): """Create a dictionary representing the state of this Spec. @@ -3856,7 +3874,7 @@ def _dup(self, other, deps=True, cleardeps=True): self._concrete = other._concrete if self.abstract_hash: - print("Replacing dup abstract hash: {0} for spec {1}".format(other.abstract_hash, other.name)) + print("Replacing dup abstract hash: {0} for spec {1}".format(self.abstract_hash, other.name)) self.abstract_hash = other.abstract_hash if self._concrete: @@ -4058,7 +4076,10 @@ def eq_node(self, other): def _cmp_iter(self): """Lazily yield components of self for comparison.""" - for item in self._cmp_node(): + + cmp_spec = self.lookup_hash() or self + + for item in cmp_spec._cmp_node(): yield item # This needs to be in _cmp_iter so that no specs with different process hashes @@ -4069,10 +4090,10 @@ def _cmp_iter(self): # TODO: they exist for speed. We should benchmark whether it's really worth # TODO: having two types of hashing now that we use `json` instead of `yaml` for # TODO: spec hashing. - yield self.process_hash() if self.concrete else None + yield cmp_spec.process_hash() if cmp_spec.concrete else None def deps(): - for dep in sorted(itertools.chain.from_iterable(self._dependencies.values())): + for dep in sorted(itertools.chain.from_iterable(cmp_spec._dependencies.values())): yield dep.spec.name yield tuple(sorted(dep.deptypes)) yield hash(dep.spec) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 0e975d738dd484..d902e3761f4c02 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -639,20 +639,24 @@ def test_spec_by_hash(database, monkeypatch, mutable_empty_config): hash_str = f"/{mpileaks.dag_hash()}" b = SpecParser(hash_str).next_spec() b.replace_hash() - # b here is NOT a Spec--it is still a query_spec! - # THEREFORE, it does not have an abstract_hash attribute, which would be useless anyway. - print("parser found {}".format(b)) assert b == mpileaks short_hash_str = f"/{mpileaks.dag_hash()[:5]}" - b = SpecParser(short_hash_str).next_spec().concretize() - assert str(b) == str(mpileaks) + b = SpecParser(short_hash_str).next_spec() + b.replace_hash() + assert b == mpileaks name_version_and_hash = f"{mpileaks.name}@{mpileaks.version} /{mpileaks.dag_hash()[:5]}" - assert str(SpecParser(name_version_and_hash).next_spec()) == str(mpileaks) + b = SpecParser(name_version_and_hash).next_spec() + print("The hashes are {0} and {1}".format(name_version_and_hash, b.abstract_hash)) + b.replace_hash() + assert b == mpileaks a_hash = f"/{a.dag_hash()}" - assert SpecParser(a_hash).next_spec() == a + b = SpecParser(a_hash).next_spec() + print("The hashes are {0} and {1}".format(a_hash, b.abstract_hash)) + b.replace_hash() + assert b == a @pytest.mark.db @@ -664,25 +668,34 @@ def test_dep_spec_by_hash(database, mutable_empty_config): assert "fake" in mpileaks_zmpi assert "zmpi" in mpileaks_zmpi - mpileaks_hash_fake = SpecParser(f"mpileaks ^/{fake.dag_hash()}").next_spec() - assert "fake" in mpileaks_hash_fake - assert mpileaks_hash_fake["fake"] == fake + # mpileaks_hash_fake = SpecParser(f"mpileaks ^/{fake.dag_hash()}").next_spec() + # mpileaks_hash_fake.replace_hash() + # print('The specs are {0} and {1}'.format(mpileaks_hash_fake, fake)) + # assert "fake" in mpileaks_hash_fake + # assert mpileaks_hash_fake["fake"] == fake - mpileaks_hash_zmpi = SpecParser( - f"mpileaks %{mpileaks_zmpi.compiler} ^ /{zmpi.dag_hash()}" - ).next_spec() - assert "zmpi" in mpileaks_hash_zmpi - assert mpileaks_hash_zmpi["zmpi"] == zmpi - assert mpileaks_hash_zmpi.compiler == mpileaks_zmpi.compiler + # mpileaks_hash_zmpi = SpecParser( + # f"mpileaks %{mpileaks_zmpi.compiler} ^ /{zmpi.dag_hash()}" + # ).next_spec() + # mpileaks_hash_zmpi.replace_hash() + # assert "zmpi" in mpileaks_hash_zmpi + # assert mpileaks_hash_zmpi["zmpi"] == zmpi + # assert mpileaks_hash_zmpi.compiler == mpileaks_zmpi.compiler mpileaks_hash_fake_and_zmpi = SpecParser( - f"mpileaks ^/{fake.dag_hash()[:4]} ^ /{zmpi.dag_hash()[:5]}" + f"mpileaks ^ fake /{fake.dag_hash()[:4]} ^ zmpi /{zmpi.dag_hash()[:5]}" ).next_spec() + nodes = mpileaks_hash_fake_and_zmpi.traverse() + for node in nodes: + print("Node name: {0}\tAbstract Hash: {1}".format(node, node.abstract_hash)) + mpileaks_hash_fake_and_zmpi.replace_hash() + print('The specs are {0} and {1}'.format(mpileaks_hash_fake_and_zmpi, zmpi)) assert "zmpi" in mpileaks_hash_fake_and_zmpi assert mpileaks_hash_fake_and_zmpi["zmpi"] == zmpi assert "fake" in mpileaks_hash_fake_and_zmpi assert mpileaks_hash_fake_and_zmpi["fake"] == fake + assert False @pytest.mark.db From 8782935ed1776da1e51d1702193085b24ee9d8fb Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Thu, 23 Feb 2023 22:33:56 -0800 Subject: [PATCH 12/39] allow parsing for multiple anonymous deps because abstract hashes appear on anonymous specs, we now need to differentiate between dependencies with the same name (at least when that name is `None`) while constructing the spec from the parser. Doing so uncovered a couple of instances in which specs are assumed to have names, which may be False if they are specified by hash. --- lib/spack/spack/spec.py | 4 ++-- lib/spack/spack/test/spec_syntax.py | 2 +- lib/spack/spack/traverse.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index bbe677e2f45cad..cf72a060a1a3a9 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1566,7 +1566,7 @@ def _set_compiler(self, compiler): def _add_dependency(self, spec: "Spec", *, deptypes: dp.DependencyArgument): """Called by the parser to add another spec as a dependency.""" - if spec.name not in self._dependencies: + if spec.name not in self._dependencies or not spec.name: self.add_dependency_edge(spec, deptypes=deptypes) return @@ -4585,7 +4585,7 @@ def cformat(self, *args, **kwargs): return self.format(*args, **kwargs) def __str__(self): - sorted_nodes = [self] + sorted(self.traverse(root=False), key=lambda x: x.name) + sorted_nodes = [self] + sorted(self.traverse(root=False), key=lambda x: x.name or x.abstract_hash) spec_str = " ^".join(d.format() for d in sorted_nodes) return spec_str.strip() diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index d902e3761f4c02..04d74cd2a56290 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -683,7 +683,7 @@ def test_dep_spec_by_hash(database, mutable_empty_config): # assert mpileaks_hash_zmpi.compiler == mpileaks_zmpi.compiler mpileaks_hash_fake_and_zmpi = SpecParser( - f"mpileaks ^ fake /{fake.dag_hash()[:4]} ^ zmpi /{zmpi.dag_hash()[:5]}" + f"mpileaks ^/{fake.dag_hash()[:4]} ^ /{zmpi.dag_hash()[:5]}" ).next_spec() nodes = mpileaks_hash_fake_and_zmpi.traverse() for node in nodes: diff --git a/lib/spack/spack/traverse.py b/lib/spack/spack/traverse.py index 848643847229b4..9b56b57f741858 100644 --- a/lib/spack/spack/traverse.py +++ b/lib/spack/spack/traverse.py @@ -18,7 +18,7 @@ def sort_edges(edges): - edges.sort(key=lambda edge: edge.spec.name) + edges.sort(key=lambda edge: edge.spec.name or edge.spec.abstract_hash) return edges From 5c5a981e570abe452932002f20c0d8b209bb0bd7 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Thu, 23 Feb 2023 23:03:45 -0800 Subject: [PATCH 13/39] fixed lookup_hash for dependencies with abstract hashes --- lib/spack/spack/spec.py | 29 +++++++++++++++++++++++------ lib/spack/spack/test/spec_syntax.py | 29 ++++++++++++----------------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index cf72a060a1a3a9..911c6fe02bff48 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1857,14 +1857,31 @@ def _lookup_hash(self): return matches[0] def lookup_hash(self): + if self.concrete: + return self - spec = self.copy() + spec = self.copy(deps=False) - nodes = [node for node in spec.traverse(order="post") if node.abstract_hash] - if self.name == 'mpileaks': - print(nodes) - for node in nodes: - spec[node.name]._dup(node._lookup_hash()) + # root spec is replaced + if spec.abstract_hash: + new = spec._lookup_hash() + if not new._satisfies(self): + raise BaseException + spec._dup(new) + return spec + + # Get dependencies that need to be replaced + for node in self.traverse(root=False): + if node.abstract_hash: + new = node._lookup_hash() # do we need a defensive copy here? + if not new._satisfies(node): + raise BaseException + spec._add_dependency(new, deptypes=()) + + # reattach nodes that were not otherwise satisfied by new dependencies + for node in self.traverse(root=False): + if not any(n._satisfies(node) for n in spec.traverse()): + spec._add_dependency(node, deptypes=()) return spec diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 04d74cd2a56290..ca2cd1102b5eeb 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -668,34 +668,29 @@ def test_dep_spec_by_hash(database, mutable_empty_config): assert "fake" in mpileaks_zmpi assert "zmpi" in mpileaks_zmpi - # mpileaks_hash_fake = SpecParser(f"mpileaks ^/{fake.dag_hash()}").next_spec() - # mpileaks_hash_fake.replace_hash() - # print('The specs are {0} and {1}'.format(mpileaks_hash_fake, fake)) - # assert "fake" in mpileaks_hash_fake - # assert mpileaks_hash_fake["fake"] == fake - - # mpileaks_hash_zmpi = SpecParser( - # f"mpileaks %{mpileaks_zmpi.compiler} ^ /{zmpi.dag_hash()}" - # ).next_spec() - # mpileaks_hash_zmpi.replace_hash() - # assert "zmpi" in mpileaks_hash_zmpi - # assert mpileaks_hash_zmpi["zmpi"] == zmpi - # assert mpileaks_hash_zmpi.compiler == mpileaks_zmpi.compiler + mpileaks_hash_fake = SpecParser(f"mpileaks ^/{fake.dag_hash()}").next_spec() + mpileaks_hash_fake.replace_hash() + assert "fake" in mpileaks_hash_fake + assert mpileaks_hash_fake["fake"] == fake + + mpileaks_hash_zmpi = SpecParser( + f"mpileaks %{mpileaks_zmpi.compiler} ^ /{zmpi.dag_hash()}" + ).next_spec() + mpileaks_hash_zmpi.replace_hash() + assert "zmpi" in mpileaks_hash_zmpi + assert mpileaks_hash_zmpi["zmpi"] == zmpi + assert mpileaks_hash_zmpi.compiler == mpileaks_zmpi.compiler mpileaks_hash_fake_and_zmpi = SpecParser( f"mpileaks ^/{fake.dag_hash()[:4]} ^ /{zmpi.dag_hash()[:5]}" ).next_spec() nodes = mpileaks_hash_fake_and_zmpi.traverse() - for node in nodes: - print("Node name: {0}\tAbstract Hash: {1}".format(node, node.abstract_hash)) mpileaks_hash_fake_and_zmpi.replace_hash() - print('The specs are {0} and {1}'.format(mpileaks_hash_fake_and_zmpi, zmpi)) assert "zmpi" in mpileaks_hash_fake_and_zmpi assert mpileaks_hash_fake_and_zmpi["zmpi"] == zmpi assert "fake" in mpileaks_hash_fake_and_zmpi assert mpileaks_hash_fake_and_zmpi["fake"] == fake - assert False @pytest.mark.db From 1d58bf1de53f1317a32d24ecebfc6afc9cdbd97a Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Fri, 24 Feb 2023 09:58:53 -0800 Subject: [PATCH 14/39] updated more test cases --- lib/spack/spack/spec.py | 6 +++--- lib/spack/spack/test/spec_syntax.py | 22 ++++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 911c6fe02bff48..1679ee38f83fa5 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1866,7 +1866,7 @@ def lookup_hash(self): if spec.abstract_hash: new = spec._lookup_hash() if not new._satisfies(self): - raise BaseException + raise InvalidHashError(spec, spec.abstract_hash) spec._dup(new) return spec @@ -1875,7 +1875,7 @@ def lookup_hash(self): if node.abstract_hash: new = node._lookup_hash() # do we need a defensive copy here? if not new._satisfies(node): - raise BaseException + raise InvalidHashError(node, node.abstract_hash) spec._add_dependency(new, deptypes=()) # reattach nodes that were not otherwise satisfied by new dependencies @@ -1903,7 +1903,7 @@ def replace_hash(self): spec_by_hash = self.lookup_hash() if not spec_by_hash._satisfies(self): - raise spack.spec.InvalidHashError(self, spec_by_hash.dag_hash()) # abstract? + raise spack.spec.InvalidHashError(self, spec_by_hash.abstract_hash) self._dup(spec_by_hash) def to_node_dict(self, hash=ht.dag_hash): diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index ca2cd1102b5eeb..499123ebef3967 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -684,7 +684,6 @@ def test_dep_spec_by_hash(database, mutable_empty_config): mpileaks_hash_fake_and_zmpi = SpecParser( f"mpileaks ^/{fake.dag_hash()[:4]} ^ /{zmpi.dag_hash()[:5]}" ).next_spec() - nodes = mpileaks_hash_fake_and_zmpi.traverse() mpileaks_hash_fake_and_zmpi.replace_hash() assert "zmpi" in mpileaks_hash_fake_and_zmpi assert mpileaks_hash_fake_and_zmpi["zmpi"] == zmpi @@ -736,11 +735,13 @@ def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_e # ambiguity in first hash character with pytest.raises(spack.spec.AmbiguousHashError): - SpecParser("/x").next_spec() + b = SpecParser("/x").next_spec() + b.replace_hash() # ambiguity in first hash character AND spec name with pytest.raises(spack.spec.AmbiguousHashError): - SpecParser("a/x").next_spec() + b = SpecParser("a/x").next_spec() + b.replace_hash() @pytest.mark.db @@ -750,13 +751,16 @@ def test_invalid_hash(database, mutable_empty_config): # name + incompatible hash with pytest.raises(spack.spec.InvalidHashError): - SpecParser(f"zmpi /{mpich.dag_hash()}").next_spec() + b = SpecParser(f"zmpi /{mpich.dag_hash()}").next_spec() + b.replace_hash() with pytest.raises(spack.spec.InvalidHashError): - SpecParser(f"mpich /{zmpi.dag_hash()}").next_spec() + b = SpecParser(f"mpich /{zmpi.dag_hash()}").next_spec() + b.replace_hash() # name + dep + incompatible hash with pytest.raises(spack.spec.InvalidHashError): - SpecParser(f"mpileaks ^zmpi /{mpich.dag_hash()}").next_spec() + b = SpecParser(f"mpileaks ^zmpi /{mpich.dag_hash()}").next_spec() + b.replace_hash() @pytest.mark.db @@ -770,7 +774,8 @@ def test_nonexistent_hash(database, mutable_empty_config): assert no_such_hash not in [h[: len(no_such_hash)] for h in hashes] with pytest.raises(spack.spec.NoSuchHashError): - SpecParser(f"/{no_such_hash}").next_spec() + b = SpecParser(f"/{no_such_hash}").next_spec() + b.replace_hash() @pytest.mark.db @@ -788,7 +793,8 @@ def test_redundant_spec(query_str, text_fmt, database): spec = database.query_one(query_str) text = text_fmt.format(spec, hash=spec.dag_hash()) with pytest.raises(spack.spec.RedundantSpecError): - SpecParser(text).next_spec() + b = SpecParser(text).next_spec() + b.replace_hash() @pytest.mark.parametrize( From 920fdb0a14a284769471cb994f9214354f70afd5 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Mon, 27 Feb 2023 16:31:51 -0800 Subject: [PATCH 15/39] updated logic --- lib/spack/spack/spec.py | 38 ++++++++++---------- lib/spack/spack/test/spec_syntax.py | 56 ++++++++++++++--------------- 2 files changed, 46 insertions(+), 48 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 1679ee38f83fa5..ce5e1b0f9ada2f 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1273,7 +1273,7 @@ def __init__( normal=False, concrete=False, external_path=None, - external_modules=None + external_modules=None, ): """Create a new Spec. @@ -1834,10 +1834,10 @@ def process_hash_bit_prefix(self, bits): return spack.util.hash.base32_prefix_bits(self.process_hash(), bits) def _lookup_hash(self): + """Lookup just one spec with an abstract hash, returning a spec from the the environment, + store, or finally, binary caches.""" import spack.environment - print("lookup is trying abstract hash {}".format(self.abstract_hash)) - matches = [] if spack.environment.active_environment(): matches = spack.environment.active_environment().get_by_hash(self.abstract_hash) @@ -1845,7 +1845,7 @@ def _lookup_hash(self): matches = spack.store.db.get_by_hash(self.abstract_hash) if not matches: query = spack.binary_distribution.BinaryCacheQuery(True) - matches = query("/"+self.abstract_hash) + matches = query("/" + self.abstract_hash) if not matches: raise spack.spec.NoSuchHashError(self.abstract_hash) @@ -1857,6 +1857,9 @@ def _lookup_hash(self): return matches[0] def lookup_hash(self): + """Given a spec with an abstract hash, return a copy of the spec with all properties and + dependencies by looking up the hash in the environment, store, or finally, binary caches. + This is non-destructive.""" if self.concrete: return self @@ -1868,6 +1871,9 @@ def lookup_hash(self): if not new._satisfies(self): raise InvalidHashError(spec, spec.abstract_hash) spec._dup(new) + for dep in self.traverse(): + if not any(node._satisfies(dep) for node in spec.traverse()): + raise RedundantSpecError(self, self.abstract_hash) return spec # Get dependencies that need to be replaced @@ -1886,24 +1892,18 @@ def lookup_hash(self): return spec def replace_hash(self): - """Given a spec with nothing but an abstract hash, attempt to populate the values by - looking up the hash in the environment, store, or finally, binary caches.""" + """Given a spec with an abstract hash, attempt to populate all properties and dependencies + by looking up the hash in the environment, store, or finally, binary caches. + This is destructive.""" if not any(node for node in self.traverse(order="post") if node.abstract_hash): return - # print([spec.abstract_hash for spec in lookup_specs]) - - # for lookup_spec in lookup_specs: - # spec_by_hash = lookup_spec.lookup_hash() - # if not spec_by_hash._satisfies(lookup_spec): - # raise spack.spec.InvalidHashError(lookup_spec, spec_by_hash.dag_hash()) # abstract? - # lookup_spec._dup(spec_by_hash) - # print("I found {0} with dag hash {1}".format(self, self.dag_hash())) - spec_by_hash = self.lookup_hash() + if not spec_by_hash._satisfies(self): - raise spack.spec.InvalidHashError(self, spec_by_hash.abstract_hash) + raise InvalidHashError(self, spec_by_hash.abstract_hash) + self._dup(spec_by_hash) def to_node_dict(self, hash=ht.dag_hash): @@ -3890,8 +3890,6 @@ def _dup(self, other, deps=True, cleardeps=True): self._concrete = other._concrete - if self.abstract_hash: - print("Replacing dup abstract hash: {0} for spec {1}".format(self.abstract_hash, other.name)) self.abstract_hash = other.abstract_hash if self._concrete: @@ -4602,7 +4600,9 @@ def cformat(self, *args, **kwargs): return self.format(*args, **kwargs) def __str__(self): - sorted_nodes = [self] + sorted(self.traverse(root=False), key=lambda x: x.name or x.abstract_hash) + sorted_nodes = [self] + sorted( + self.traverse(root=False), key=lambda x: x.name or x.abstract_hash + ) spec_str = " ^".join(d.format() for d in sorted_nodes) return spec_str.strip() diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 499123ebef3967..a43716584c058b 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -637,26 +637,24 @@ def test_spec_by_hash(database, monkeypatch, mutable_empty_config): monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [a]) hash_str = f"/{mpileaks.dag_hash()}" - b = SpecParser(hash_str).next_spec() - b.replace_hash() - assert b == mpileaks + parsed_spec = SpecParser(hash_str).next_spec() + parsed_spec.replace_hash() + assert parsed_spec == mpileaks short_hash_str = f"/{mpileaks.dag_hash()[:5]}" - b = SpecParser(short_hash_str).next_spec() - b.replace_hash() - assert b == mpileaks + parsed_spec = SpecParser(short_hash_str).next_spec() + parsed_spec.replace_hash() + assert parsed_spec == mpileaks name_version_and_hash = f"{mpileaks.name}@{mpileaks.version} /{mpileaks.dag_hash()[:5]}" - b = SpecParser(name_version_and_hash).next_spec() - print("The hashes are {0} and {1}".format(name_version_and_hash, b.abstract_hash)) - b.replace_hash() - assert b == mpileaks + parsed_spec = SpecParser(name_version_and_hash).next_spec() + parsed_spec.replace_hash() + assert parsed_spec == mpileaks a_hash = f"/{a.dag_hash()}" - b = SpecParser(a_hash).next_spec() - print("The hashes are {0} and {1}".format(a_hash, b.abstract_hash)) - b.replace_hash() - assert b == a + parsed_spec = SpecParser(a_hash).next_spec() + parsed_spec.replace_hash() + assert parsed_spec == a @pytest.mark.db @@ -735,13 +733,13 @@ def test_ambiguous_hash(mutable_database, default_mock_concretization, mutable_e # ambiguity in first hash character with pytest.raises(spack.spec.AmbiguousHashError): - b = SpecParser("/x").next_spec() - b.replace_hash() + parsed_spec = SpecParser("/x").next_spec() + parsed_spec.replace_hash() # ambiguity in first hash character AND spec name with pytest.raises(spack.spec.AmbiguousHashError): - b = SpecParser("a/x").next_spec() - b.replace_hash() + parsed_spec = SpecParser("a/x").next_spec() + parsed_spec.replace_hash() @pytest.mark.db @@ -751,16 +749,16 @@ def test_invalid_hash(database, mutable_empty_config): # name + incompatible hash with pytest.raises(spack.spec.InvalidHashError): - b = SpecParser(f"zmpi /{mpich.dag_hash()}").next_spec() - b.replace_hash() + parsed_spec = SpecParser(f"zmpi /{mpich.dag_hash()}").next_spec() + parsed_spec.replace_hash() with pytest.raises(spack.spec.InvalidHashError): - b = SpecParser(f"mpich /{zmpi.dag_hash()}").next_spec() - b.replace_hash() + parsed_spec = SpecParser(f"mpich /{zmpi.dag_hash()}").next_spec() + parsed_spec.replace_hash() # name + dep + incompatible hash with pytest.raises(spack.spec.InvalidHashError): - b = SpecParser(f"mpileaks ^zmpi /{mpich.dag_hash()}").next_spec() - b.replace_hash() + parsed_spec = SpecParser(f"mpileaks ^zmpi /{mpich.dag_hash()}").next_spec() + parsed_spec.replace_hash() @pytest.mark.db @@ -774,8 +772,8 @@ def test_nonexistent_hash(database, mutable_empty_config): assert no_such_hash not in [h[: len(no_such_hash)] for h in hashes] with pytest.raises(spack.spec.NoSuchHashError): - b = SpecParser(f"/{no_such_hash}").next_spec() - b.replace_hash() + parsed_spec = SpecParser(f"/{no_such_hash}").next_spec() + parsed_spec.replace_hash() @pytest.mark.db @@ -783,7 +781,7 @@ def test_nonexistent_hash(database, mutable_empty_config): "query_str,text_fmt", [ ("mpileaks ^zmpi", r"/{hash}%{0.compiler}"), - ("callpath ^zmpi", r"callpath /{hash} ^libelf"), + ("callpath ^zmpi", r"callpath /{hash} ^a"), ("dyninst", r'/{hash} cflags="-O3 -fPIC"'), ("mpileaks ^mpich2", r"mpileaks/{hash} @{0.version}"), ], @@ -793,8 +791,8 @@ def test_redundant_spec(query_str, text_fmt, database): spec = database.query_one(query_str) text = text_fmt.format(spec, hash=spec.dag_hash()) with pytest.raises(spack.spec.RedundantSpecError): - b = SpecParser(text).next_spec() - b.replace_hash() + parsed_spec = SpecParser(text).next_spec() + parsed_spec.replace_hash() @pytest.mark.parametrize( From 5ca2640b1f08c11a8a2714fec086710fc233966a Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Mon, 27 Feb 2023 16:45:17 -0800 Subject: [PATCH 16/39] remaining print statement --- lib/spack/spack/parser.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index bb0cc06b506b25..1c79047f822541 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -403,8 +403,6 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: self.has_hash = True else: break - if self.has_hash: - print("final parsed abstract hash is {}".format(initial_spec.abstract_hash)) return initial_spec From ddafa0b0a89a6d90d93d9c3aa92e097855e4d595 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 28 Feb 2023 09:32:43 -0800 Subject: [PATCH 17/39] style --- lib/spack/spack/spec.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index ce5e1b0f9ada2f..d5609250268c2a 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3654,7 +3654,6 @@ def intersects(self, other: "Spec", deps: bool = True) -> bool: return True def satisfies(self, other, deps=True, strict=False): - other = self._autospec(other) lhs = self.lookup_hash() or self From c14fc75f8426535d07087dfe0a1fc6c455c32681 Mon Sep 17 00:00:00 2001 From: Gregory Becker Date: Thu, 2 Mar 2023 12:00:01 -0800 Subject: [PATCH 18/39] bugfix for cmp_iter --- lib/spack/spack/spec.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index d5609250268c2a..cfcdd794871fb4 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1860,7 +1860,7 @@ def lookup_hash(self): """Given a spec with an abstract hash, return a copy of the spec with all properties and dependencies by looking up the hash in the environment, store, or finally, binary caches. This is non-destructive.""" - if self.concrete: + if self.concrete or not any(node.abstract_hash for node in self.traverse()): return self spec = self.copy(deps=False) @@ -1887,7 +1887,7 @@ def lookup_hash(self): # reattach nodes that were not otherwise satisfied by new dependencies for node in self.traverse(root=False): if not any(n._satisfies(node) for n in spec.traverse()): - spec._add_dependency(node, deptypes=()) + spec._add_dependency(node.copy(), deptypes=()) return spec From ccd7aaf8f1e23b5dd09587d31df84086bf8455ca Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Thu, 2 Mar 2023 12:05:15 -0800 Subject: [PATCH 19/39] cleanup merge --- lib/spack/spack/spec.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index cfcdd794871fb4..9fde81d7dd2c08 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1904,7 +1904,8 @@ def replace_hash(self): if not spec_by_hash._satisfies(self): raise InvalidHashError(self, spec_by_hash.abstract_hash) - self._dup(spec_by_hash) + if spec_by_hash != self: + self._dup(spec_by_hash) def to_node_dict(self, hash=ht.dag_hash): """Create a dictionary representing the state of this Spec. @@ -3428,8 +3429,12 @@ def constrain(self, other, deps=True): raise spack.error.UnsatisfiableSpecError(self, other, "constrain a concrete spec") other = self._autospec(other) - - other.lookup_hash() + if other.abstract_hash: + if not self.abstract_hash or other.abstract_hash.startswith(self.abstract_hash): + self.abstract_hash = other.abstract_hash + elif not self.abstract_hash.startswith(other.abstract_hash): + raise InvalidHashError(self, other.abstract_hash) + # other.lookup_hash() if not (self.name == other.name or (not self.name) or (not other.name)): raise UnsatisfiableSpecNameError(self.name, other.name) From 6b91e54b84d0a7a111ea357e2bf14feefcffb0bb Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 7 Mar 2023 13:20:29 -0800 Subject: [PATCH 20/39] handle more repl hash cases --- lib/spack/spack/spec.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 9fde81d7dd2c08..22d38c4ead507c 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1904,7 +1904,7 @@ def replace_hash(self): if not spec_by_hash._satisfies(self): raise InvalidHashError(self, spec_by_hash.abstract_hash) - if spec_by_hash != self: + if spec_by_hash != self or not self.eq_dag(spec_by_hash): self._dup(spec_by_hash) def to_node_dict(self, hash=ht.dag_hash): @@ -2865,8 +2865,11 @@ def _new_concretize(self, tests=False): self.replace_hash() - if not self.name: - raise spack.error.SpecError("Spec has no name; cannot concretize an anonymous spec") + for node in self.traverse(): + if not node.name: + raise spack.error.SpecError( + f"Spec {node} has no name; cannot concretize an anonymous spec" + ) if self._concrete: return From e08b00556b769d2b3fd49f0969b8e13a1c87e409 Mon Sep 17 00:00:00 2001 From: Hanford Date: Tue, 14 Mar 2023 16:04:44 -0700 Subject: [PATCH 21/39] updated file error handling windows --- lib/spack/spack/parser.py | 7 ++++++- lib/spack/spack/test/spec_syntax.py | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index 1c79047f822541..88758d881e93f6 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -398,7 +398,12 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: name = name.strip("'\" ") value = value.strip("'\" ") initial_spec._add_flag(name, value, propagate=True) - elif not self.has_hash and self.ctx.accept(TokenType.DAG_HASH): + elif self.ctx.accept(TokenType.DAG_HASH): + if initial_spec.abstract_hash: + msg = (f"Parsed multiple hashes /{initial_spec.abstract_hash}and " + "/{self.current_token}. If you were attempting to specify a file path," + " check that it was formatted properly") + raise spack.spec.RedundantSpecError(msg, self.ctx.current_token.value) initial_spec.abstract_hash = self.ctx.current_token.value[1:] self.has_hash = True else: diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index a43716584c058b..88b42d1cfc9042 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -859,13 +859,13 @@ def test_error_conditions(text, exc_cls): [ # Specfile related errors pytest.param( - "/bogus/path/libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS + "/bogus/path/libdwarf.yaml", spack.spec.RedundantSpecError, marks=FAIL_ON_WINDOWS ), pytest.param("../../libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS), pytest.param("./libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS), pytest.param( "libfoo ^/bogus/path/libdwarf.yaml", - spack.spec.NoSuchSpecFileError, + spack.spec.RedundantSpecError, marks=FAIL_ON_WINDOWS, ), pytest.param( @@ -875,11 +875,11 @@ def test_error_conditions(text, exc_cls): "libfoo ^./libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS ), pytest.param( - "/bogus/path/libdwarf.yamlfoobar", spack.spec.SpecFilenameError, marks=FAIL_ON_WINDOWS + "/bogus/path/libdwarf.yamlfoobar", spack.spec.RedundantSpecError, marks=FAIL_ON_WINDOWS ), pytest.param( "libdwarf^/bogus/path/libelf.yamlfoobar ^/path/to/bogus.yaml", - spack.spec.SpecFilenameError, + spack.spec.RedundantSpecError, marks=FAIL_ON_WINDOWS, ), pytest.param( From 655667e739c6c2297ba35be9c04504a112a48cbe Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 14 Mar 2023 16:16:08 -0700 Subject: [PATCH 22/39] style on mac --- lib/spack/spack/parser.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index 88758d881e93f6..e40a5cebbd8641 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -400,9 +400,11 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: initial_spec._add_flag(name, value, propagate=True) elif self.ctx.accept(TokenType.DAG_HASH): if initial_spec.abstract_hash: - msg = (f"Parsed multiple hashes /{initial_spec.abstract_hash}and " - "/{self.current_token}. If you were attempting to specify a file path," - " check that it was formatted properly") + msg = ( + f"Parsed multiple hashes /{initial_spec.abstract_hash}and " + "/{self.current_token}. If you were attempting to specify a file path," + " check that it was formatted properly" + ) raise spack.spec.RedundantSpecError(msg, self.ctx.current_token.value) initial_spec.abstract_hash = self.ctx.current_token.value[1:] self.has_hash = True From 68956f50ad8bfa914d2c6a2bd46a0683ad966fcd Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 14 Mar 2023 17:23:20 -0700 Subject: [PATCH 23/39] cleanup after rebase --- lib/spack/spack/spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 22d38c4ead507c..5bee3780e4a625 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3669,7 +3669,7 @@ def satisfies(self, other, deps=True, strict=False): return lhs._satisfies(rhs, deps=deps, strict=strict) - def satisfies_dependencies(self, other, strict=False): + def _intersects_dependencies(self, other): """ This checks constraints on common dependencies against each other. """ From ac859ed82a56ac7a2121419cad1815d7c967b33b Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 14 Mar 2023 17:58:58 -0700 Subject: [PATCH 24/39] typos and split intersect --- lib/spack/spack/parser.py | 2 +- lib/spack/spack/spec.py | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index e40a5cebbd8641..e6a4f34c982668 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -401,7 +401,7 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: elif self.ctx.accept(TokenType.DAG_HASH): if initial_spec.abstract_hash: msg = ( - f"Parsed multiple hashes /{initial_spec.abstract_hash}and " + f"Parsed multiple hashes /{initial_spec.abstract_hash} and " "/{self.current_token}. If you were attempting to specify a file path," " check that it was formatted properly" ) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 5bee3780e4a625..beee3d039fb633 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3584,6 +3584,15 @@ def _autospec(self, spec_like): return Spec(spec_like) def intersects(self, other: "Spec", deps: bool = True) -> bool: + + other = self._autospec(other) + + lhs = self.lookup_hash() or self + rhs = other.lookup_hash() or other + + return lhs._intersects(rhs, deps) + + def _intersects(self, other: "Spec", deps: bool = True) -> bool: """Return True if there exists at least one concrete spec that matches both self and other, otherwise False. @@ -3661,13 +3670,13 @@ def intersects(self, other: "Spec", deps: bool = True) -> bool: else: return True - def satisfies(self, other, deps=True, strict=False): + def satisfies(self, other, deps=True): other = self._autospec(other) lhs = self.lookup_hash() or self rhs = other.lookup_hash() or other - return lhs._satisfies(rhs, deps=deps, strict=strict) + return lhs._satisfies(rhs, deps=deps) def _intersects_dependencies(self, other): """ @@ -3709,7 +3718,7 @@ def _intersects_dependencies(self, other): return True - def satisfies(self, other: "Spec", deps: bool = True) -> bool: + def _satisfies(self, other: "Spec", deps: bool = True) -> bool: """Return True if all concrete specs matching self also match other, otherwise False. Args: From b467e6eae6c57326b6ee7ab4631c5acd802863f8 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Wed, 15 Mar 2023 14:43:51 -0700 Subject: [PATCH 25/39] restore parsing logic and errors --- lib/spack/spack/parser.py | 26 ++++++++++++++++++-------- lib/spack/spack/test/spec_syntax.py | 10 ++++++---- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index e6a4f34c982668..a45036b20f6efc 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -271,6 +271,8 @@ def next_spec(self, initial_spec: Optional[spack.spec.Spec] = None) -> spack.spe root_spec = SpecNodeParser(self.ctx).parse(initial_spec) while True: if self.ctx.accept(TokenType.DEPENDENCY): + if root_spec.abstract_hash: + raise spack.spec.RedundantSpecError(root_spec, self.ctx.current_token) dependency = SpecNodeParser(self.ctx).parse(spack.spec.Spec()) if dependency == spack.spec.Spec(): @@ -399,15 +401,23 @@ def parse(self, initial_spec: spack.spec.Spec) -> spack.spec.Spec: value = value.strip("'\" ") initial_spec._add_flag(name, value, propagate=True) elif self.ctx.accept(TokenType.DAG_HASH): - if initial_spec.abstract_hash: - msg = ( - f"Parsed multiple hashes /{initial_spec.abstract_hash} and " - "/{self.current_token}. If you were attempting to specify a file path," - " check that it was formatted properly" - ) - raise spack.spec.RedundantSpecError(msg, self.ctx.current_token.value) + # if initial_spec.abstract_hash: + # msg = ( + # f"Parsed multiple hashes /{initial_spec.abstract_hash} and " + # "/{self.current_token}. If you were attempting to specify a file path," + # " check that it was formatted properly" + # ) + # raise spack.spec.RedundantSpecError(msg, self.ctx.current_token.value) initial_spec.abstract_hash = self.ctx.current_token.value[1:] - self.has_hash = True + if self.ctx.next_token and self.ctx.next_token.kind not in [ + TokenType.UNQUALIFIED_PACKAGE_NAME, + TokenType.FULLY_QUALIFIED_PACKAGE_NAME, + TokenType.DAG_HASH, + TokenType.FILENAME, + TokenType.DEPENDENCY, + ]: + raise spack.spec.RedundantSpecError(initial_spec, self.ctx.next_token) + break else: break diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 88b42d1cfc9042..220818cc0e5f41 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -859,13 +859,13 @@ def test_error_conditions(text, exc_cls): [ # Specfile related errors pytest.param( - "/bogus/path/libdwarf.yaml", spack.spec.RedundantSpecError, marks=FAIL_ON_WINDOWS + "/bogus/path/libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS ), pytest.param("../../libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS), pytest.param("./libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS), pytest.param( "libfoo ^/bogus/path/libdwarf.yaml", - spack.spec.RedundantSpecError, + spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS, ), pytest.param( @@ -875,11 +875,13 @@ def test_error_conditions(text, exc_cls): "libfoo ^./libdwarf.yaml", spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS ), pytest.param( - "/bogus/path/libdwarf.yamlfoobar", spack.spec.RedundantSpecError, marks=FAIL_ON_WINDOWS + "/bogus/path/libdwarf.yamlfoobar", + spack.spec.NoSuchSpecFileError, + marks=FAIL_ON_WINDOWS, ), pytest.param( "libdwarf^/bogus/path/libelf.yamlfoobar ^/path/to/bogus.yaml", - spack.spec.RedundantSpecError, + spack.spec.NoSuchSpecFileError, marks=FAIL_ON_WINDOWS, ), pytest.param( From 15f304bc43ffec81e7df51a97449574fba1eef49 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Wed, 15 Mar 2023 15:18:25 -0700 Subject: [PATCH 26/39] style --- lib/spack/spack/spec.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index beee3d039fb633..e96aba5ff8a357 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3584,15 +3584,6 @@ def _autospec(self, spec_like): return Spec(spec_like) def intersects(self, other: "Spec", deps: bool = True) -> bool: - - other = self._autospec(other) - - lhs = self.lookup_hash() or self - rhs = other.lookup_hash() or other - - return lhs._intersects(rhs, deps) - - def _intersects(self, other: "Spec", deps: bool = True) -> bool: """Return True if there exists at least one concrete spec that matches both self and other, otherwise False. @@ -3605,6 +3596,12 @@ def _intersects(self, other: "Spec", deps: bool = True) -> bool: """ other = self._autospec(other) + lhs = self.lookup_hash() or self + rhs = other.lookup_hash() or other + + return lhs._intersects(rhs, deps) + + def _intersects(self, other: "Spec", deps: bool = True) -> bool: if other.concrete and self.concrete: return self.dag_hash() == other.dag_hash() @@ -3671,6 +3668,9 @@ def _intersects(self, other: "Spec", deps: bool = True) -> bool: return True def satisfies(self, other, deps=True): + """ + This checks constraints on common dependencies against each other. + """ other = self._autospec(other) lhs = self.lookup_hash() or self @@ -3679,10 +3679,6 @@ def satisfies(self, other, deps=True): return lhs._satisfies(rhs, deps=deps) def _intersects_dependencies(self, other): - """ - This checks constraints on common dependencies against each other. - """ - other = self._autospec(other) if not other._dependencies or not self._dependencies: # one spec *could* eventually satisfy the other From 1378f77500d9b418550653f7220fafac7538316e Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Wed, 15 Mar 2023 15:29:40 -0700 Subject: [PATCH 27/39] style --- lib/spack/spack/spec.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index e96aba5ff8a357..7aa9a28faccc00 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3679,7 +3679,6 @@ def satisfies(self, other, deps=True): return lhs._satisfies(rhs, deps=deps) def _intersects_dependencies(self, other): - if not other._dependencies or not self._dependencies: # one spec *could* eventually satisfy the other return True From 7fdde64859132ebd2e45b51255512db2cc6c88b6 Mon Sep 17 00:00:00 2001 From: Hanford Date: Thu, 16 Mar 2023 14:41:38 -0700 Subject: [PATCH 28/39] updated windows specfile test realism --- lib/spack/spack/test/spec_syntax.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 220818cc0e5f41..e693ed9a21825b 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -914,7 +914,7 @@ def test_error_conditions(text, exc_cls): ) def test_specfile_error_conditions_windows(text, exc_cls): with pytest.raises(exc_cls): - SpecParser(text).next_spec() + SpecParser(text).all_specs() @pytest.mark.parametrize( From b7374f8431568e0a1842b9ed610143f6b094db17 Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Tue, 28 Mar 2023 20:45:41 -0700 Subject: [PATCH 29/39] coverage, pruning --- lib/spack/spack/parser.py | 2 +- lib/spack/spack/spec.py | 7 ------- lib/spack/spack/test/spec_semantics.py | 13 +++++++++++++ lib/spack/spack/test/spec_syntax.py | 10 +++++++++- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/spack/spack/parser.py b/lib/spack/spack/parser.py index a45036b20f6efc..1775d566aed2a8 100644 --- a/lib/spack/spack/parser.py +++ b/lib/spack/spack/parser.py @@ -275,7 +275,7 @@ def next_spec(self, initial_spec: Optional[spack.spec.Spec] = None) -> spack.spe raise spack.spec.RedundantSpecError(root_spec, self.ctx.current_token) dependency = SpecNodeParser(self.ctx).parse(spack.spec.Spec()) - if dependency == spack.spec.Spec(): + if not dependency.abstract_hash and dependency == spack.spec.Spec(): msg = ( "this dependency sigil needs to be followed by a package name " "or a node attribute (version, variant, etc.)" diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 7aa9a28faccc00..8c43d26b7cea56 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1864,16 +1864,12 @@ def lookup_hash(self): return self spec = self.copy(deps=False) - # root spec is replaced if spec.abstract_hash: new = spec._lookup_hash() if not new._satisfies(self): raise InvalidHashError(spec, spec.abstract_hash) spec._dup(new) - for dep in self.traverse(): - if not any(node._satisfies(dep) for node in spec.traverse()): - raise RedundantSpecError(self, self.abstract_hash) return spec # Get dependencies that need to be replaced @@ -1901,9 +1897,6 @@ def replace_hash(self): spec_by_hash = self.lookup_hash() - if not spec_by_hash._satisfies(self): - raise InvalidHashError(self, spec_by_hash.abstract_hash) - if spec_by_hash != self or not self.eq_dag(spec_by_hash): self._dup(spec_by_hash) diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 63e2253b6886ed..e7408dc862d78f 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -239,6 +239,19 @@ def test_abstract_specs_can_constrain_each_other(self, lhs, rhs, expected): assert c1 == c2 assert c1 == expected + def test_constrain_specs_by_hash(self, default_mock_concretization, database): + """Test that Specs specified only by their hashes can constrain eachother.""" + mpich_dag_hash = "/" + database.query_one("mpich").dag_hash() + assert Spec(mpich_dag_hash[:7]).constrain(Spec(mpich_dag_hash)) is False + + def test_mismatched_constrain_spec_by_hash(self, default_mock_concretization, database): + """Test that Specs specified only by their incompatible hashes fail appropriately.""" + lhs = "/" + database.query_one("callpath ^mpich").dag_hash() + rhs = "/" + database.query_one("callpath ^mpich2").dag_hash() + with pytest.raises(spack.spec.InvalidHashError): + Spec(lhs).constrain(Spec(rhs)) + Spec(lhs[:7]).constrain(Spec(rhs)) + @pytest.mark.parametrize( "lhs,rhs", [("libelf", Spec()), ("libelf", "@0:1"), ("libelf", "@0:1 %gcc")] ) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index e693ed9a21825b..913cab5fd8d502 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -666,10 +666,12 @@ def test_dep_spec_by_hash(database, mutable_empty_config): assert "fake" in mpileaks_zmpi assert "zmpi" in mpileaks_zmpi - mpileaks_hash_fake = SpecParser(f"mpileaks ^/{fake.dag_hash()}").next_spec() + mpileaks_hash_fake = SpecParser(f"mpileaks ^/{fake.dag_hash()} ^zmpi").next_spec() mpileaks_hash_fake.replace_hash() assert "fake" in mpileaks_hash_fake assert mpileaks_hash_fake["fake"] == fake + assert "zmpi" in mpileaks_hash_fake + assert mpileaks_hash_fake["zmpi"] == spack.spec.Spec("zmpi") mpileaks_hash_zmpi = SpecParser( f"mpileaks %{mpileaks_zmpi.compiler} ^ /{zmpi.dag_hash()}" @@ -761,6 +763,12 @@ def test_invalid_hash(database, mutable_empty_config): parsed_spec.replace_hash() +def test_invalid_hash_dep(database, mutable_empty_config): + mpich = database.query_one("mpich") + hash = mpich.dag_hash() + spack.spec.Spec(f"callpath ^zlib/{hash}").replace_hash() + + @pytest.mark.db def test_nonexistent_hash(database, mutable_empty_config): """Ensure we get errors for non existent hashes.""" From 6cc9652cc8a7e8985acded96aa8e6bb8ead609ea Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Wed, 29 Mar 2023 14:50:37 -0700 Subject: [PATCH 30/39] finishing tests, removing unnecessary checks --- lib/spack/spack/spec.py | 5 +++-- lib/spack/spack/test/spec_semantics.py | 5 ++++- lib/spack/spack/test/spec_syntax.py | 3 ++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 8c43d26b7cea56..9f0c4f1d13ca8a 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -1897,8 +1897,8 @@ def replace_hash(self): spec_by_hash = self.lookup_hash() - if spec_by_hash != self or not self.eq_dag(spec_by_hash): - self._dup(spec_by_hash) + # if spec_by_hash != self or not self.eq_dag(spec_by_hash): + self._dup(spec_by_hash) def to_node_dict(self, hash=ht.dag_hash): """Create a dictionary representing the state of this Spec. @@ -4085,6 +4085,7 @@ def _cmp_node(self): yield self.compiler yield self.compiler_flags yield self.architecture + yield self.abstract_hash # this is not present on older specs yield getattr(self, "_package_hash", None) diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index e7408dc862d78f..586f58be35857c 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -242,7 +242,9 @@ def test_abstract_specs_can_constrain_each_other(self, lhs, rhs, expected): def test_constrain_specs_by_hash(self, default_mock_concretization, database): """Test that Specs specified only by their hashes can constrain eachother.""" mpich_dag_hash = "/" + database.query_one("mpich").dag_hash() - assert Spec(mpich_dag_hash[:7]).constrain(Spec(mpich_dag_hash)) is False + spec = Spec(mpich_dag_hash[:7]) + assert spec.constrain(Spec(mpich_dag_hash)) is False + assert spec.abstract_hash == mpich_dag_hash[1:] def test_mismatched_constrain_spec_by_hash(self, default_mock_concretization, database): """Test that Specs specified only by their incompatible hashes fail appropriately.""" @@ -250,6 +252,7 @@ def test_mismatched_constrain_spec_by_hash(self, default_mock_concretization, da rhs = "/" + database.query_one("callpath ^mpich2").dag_hash() with pytest.raises(spack.spec.InvalidHashError): Spec(lhs).constrain(Spec(rhs)) + with pytest.raises(spack.spec.InvalidHashError): Spec(lhs[:7]).constrain(Spec(rhs)) @pytest.mark.parametrize( diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 913cab5fd8d502..e79546ff8d9850 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -766,7 +766,8 @@ def test_invalid_hash(database, mutable_empty_config): def test_invalid_hash_dep(database, mutable_empty_config): mpich = database.query_one("mpich") hash = mpich.dag_hash() - spack.spec.Spec(f"callpath ^zlib/{hash}").replace_hash() + with pytest.raises(spack.spec.InvalidHashError): + spack.spec.Spec(f"callpath ^zlib/{hash}").replace_hash() @pytest.mark.db From da00f41ab8e425d4a1569b55a86284adf9db6adf Mon Sep 17 00:00:00 2001 From: Nathan Hanford Date: Fri, 31 Mar 2023 18:30:41 -0700 Subject: [PATCH 31/39] windows didn't like a --- lib/spack/spack/test/spec_syntax.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index e79546ff8d9850..1718d045f39d28 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -633,8 +633,8 @@ def test_spec_by_hash_tokens(text, tokens): @pytest.mark.db def test_spec_by_hash(database, monkeypatch, mutable_empty_config): mpileaks = database.query_one("mpileaks ^zmpi") - a = spack.spec.Spec("a").concretized() - monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [a]) + b = spack.spec.Spec("b").concretized() + monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [b]) hash_str = f"/{mpileaks.dag_hash()}" parsed_spec = SpecParser(hash_str).next_spec() @@ -651,10 +651,10 @@ def test_spec_by_hash(database, monkeypatch, mutable_empty_config): parsed_spec.replace_hash() assert parsed_spec == mpileaks - a_hash = f"/{a.dag_hash()}" - parsed_spec = SpecParser(a_hash).next_spec() + b_hash = f"/{b.dag_hash()}" + parsed_spec = SpecParser(b_hash).next_spec() parsed_spec.replace_hash() - assert parsed_spec == a + assert parsed_spec == b @pytest.mark.db From b488c7f69c0446c62f5795db594f0d38740091d0 Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 12:25:34 -0400 Subject: [PATCH 32/39] Single CI job run --- .github/workflows/ci.yaml | 18 +++--- .github/workflows/windows_python.yml | 94 ++++++++++++++-------------- 2 files changed, 55 insertions(+), 57 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c1305d3558d7d6..25a99797d6d6e7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -66,20 +66,20 @@ jobs: # job outputs: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idoutputs # setting environment variables from earlier steps: https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-environment-variable # - bootstrap: - if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.bootstrap == 'true' }} - needs: [ prechecks, changes ] - uses: ./.github/workflows/bootstrap.yml - unit-tests: - if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.core == 'true' }} - needs: [ prechecks, changes ] - uses: ./.github/workflows/unit_tests.yaml + # bootstrap: + # if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.bootstrap == 'true' }} + # needs: [ prechecks, changes ] + # uses: ./.github/workflows/bootstrap.yml + # unit-tests: + # if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.core == 'true' }} + # needs: [ prechecks, changes ] + # uses: ./.github/workflows/unit_tests.yaml windows: if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.core == 'true' }} needs: [ prechecks ] uses: ./.github/workflows/windows_python.yml all: - needs: [ windows, unit-tests, bootstrap ] + needs: [ windows] runs-on: ubuntu-latest steps: - name: Success diff --git a/.github/workflows/windows_python.yml b/.github/workflows/windows_python.yml index db33c868a93662..91709672b4ab6b 100644 --- a/.github/workflows/windows_python.yml +++ b/.github/workflows/windows_python.yml @@ -29,55 +29,53 @@ jobs: ./.github/workflows/setup_git.ps1 - name: Unit Test run: | - spack unit-test -x --verbose --cov --cov-config=pyproject.toml --ignore=lib/spack/spack/test/cmd + spack unit-test -x --verbose --cov --cov-config=pyproject.toml --ignore=lib/spack/spack/test/cmd lib/spack/spack/test/spec_syntax.py::test_spec_by_hash ./share/spack/qa/validate_last_exit.ps1 - coverage combine -a - coverage xml - - uses: codecov/codecov-action@d9f34f8cd5cb3b3eb79b3e4b5dae3a16df499a70 - with: - flags: unittests,windows - unit-tests-cmd: - runs-on: windows-latest - steps: - - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 - with: - fetch-depth: 0 - - uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 - with: - python-version: 3.9 - - name: Install Python packages - run: | - python -m pip install --upgrade pip six pywin32 setuptools codecov coverage pytest-cov clingo - - name: Create local develop - run: | - ./.github/workflows/setup_git.ps1 - - name: Command Unit Test - run: | - spack unit-test -x --verbose --cov --cov-config=pyproject.toml lib/spack/spack/test/cmd - ./share/spack/qa/validate_last_exit.ps1 - coverage combine -a - coverage xml - - uses: codecov/codecov-action@d9f34f8cd5cb3b3eb79b3e4b5dae3a16df499a70 - with: - flags: unittests,windows - build-abseil: - runs-on: windows-latest - steps: - - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 - with: - fetch-depth: 0 - - uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 - with: - python-version: 3.9 - - name: Install Python packages - run: | - python -m pip install --upgrade pip six pywin32 setuptools codecov coverage - - name: Build Test - run: | - spack compiler find - spack external find cmake - spack external find ninja - spack -d install abseil-cpp + # - uses: codecov/codecov-action@d9f34f8cd5cb3b3eb79b3e4b5dae3a16df499a70 + # with: + # flags: unittests,windows + # unit-tests-cmd: + # runs-on: windows-latest + # steps: + # - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 + # with: + # fetch-depth: 0 + # - uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 + # with: + # python-version: 3.9 + # - name: Install Python packages + # run: | + # python -m pip install --upgrade pip six pywin32 setuptools codecov coverage pytest-cov clingo + # - name: Create local develop + # run: | + # ./.github/workflows/setup_git.ps1 + # - name: Command Unit Test + # run: | + # spack unit-test -x --verbose --cov --cov-config=pyproject.toml lib/spack/spack/test/cmd + # ./share/spack/qa/validate_last_exit.ps1 + # coverage combine -a + # coverage xml + # - uses: codecov/codecov-action@d9f34f8cd5cb3b3eb79b3e4b5dae3a16df499a70 + # with: + # flags: unittests,windows + # build-abseil: + # runs-on: windows-latest + # steps: + # - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 + # with: + # fetch-depth: 0 + # - uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 + # with: + # python-version: 3.9 + # - name: Install Python packages + # run: | + # python -m pip install --upgrade pip six pywin32 setuptools codecov coverage + # - name: Build Test + # run: | + # spack compiler find + # spack external find cmake + # spack external find ninja + # spack -d install abseil-cpp # TODO: johnwparent - reduce the size of the installer operations # make-installer: # runs-on: windows-latest From facf80f93d8d07dd7c5d413a541bd218a84b5ec5 Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 12:27:46 -0400 Subject: [PATCH 33/39] Run CI for me --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 25a99797d6d6e7..5ec838b521682f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -75,7 +75,7 @@ jobs: # needs: [ prechecks, changes ] # uses: ./.github/workflows/unit_tests.yaml windows: - if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.core == 'true' }} + if: ${{ github.repository == 'johnwparent/spack' && needs.changes.outputs.core == 'true' }} needs: [ prechecks ] uses: ./.github/workflows/windows_python.yml all: From 263b1013bf3a318ea9d87e151d23006238fe3098 Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 12:29:23 -0400 Subject: [PATCH 34/39] only run windows --- .github/workflows/ci.yaml | 92 +++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5ec838b521682f..7c977283e60084 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,51 +15,51 @@ concurrency: cancel-in-progress: true jobs: - prechecks: - needs: [ changes ] - uses: ./.github/workflows/valid-style.yml - with: - with_coverage: ${{ needs.changes.outputs.core }} - all-prechecks: - needs: [ prechecks ] - runs-on: ubuntu-latest - steps: - - name: Success - run: "true" - # Check which files have been updated by the PR - changes: - runs-on: ubuntu-latest - # Set job outputs to values from filter step - outputs: - bootstrap: ${{ steps.filter.outputs.bootstrap }} - core: ${{ steps.filter.outputs.core }} - packages: ${{ steps.filter.outputs.packages }} - steps: - - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # @v2 - if: ${{ github.event_name == 'push' }} - with: - fetch-depth: 0 - # For pull requests it's not necessary to checkout the code - - uses: dorny/paths-filter@4512585405083f25c027a35db413c2b3b9006d50 - id: filter - with: - # See https://github.com/dorny/paths-filter/issues/56 for the syntax used below - # Don't run if we only modified packages in the - # built-in repository or documentation - filters: | - bootstrap: - - 'var/spack/repos/builtin/packages/clingo-bootstrap/**' - - 'var/spack/repos/builtin/packages/clingo/**' - - 'var/spack/repos/builtin/packages/python/**' - - 'var/spack/repos/builtin/packages/re2c/**' - - 'lib/spack/**' - - 'share/spack/**' - - '.github/workflows/bootstrap.yml' - - '.github/workflows/ci.yaml' - core: - - './!(var/**)/**' - packages: - - 'var/**' + # prechecks: + # needs: [ changes ] + # uses: ./.github/workflows/valid-style.yml + # with: + # with_coverage: ${{ needs.changes.outputs.core }} + # all-prechecks: + # needs: [ prechecks ] + # runs-on: ubuntu-latest + # steps: + # - name: Success + # run: "true" + # # Check which files have been updated by the PR + # changes: + # runs-on: ubuntu-latest + # # Set job outputs to values from filter step + # outputs: + # bootstrap: ${{ steps.filter.outputs.bootstrap }} + # core: ${{ steps.filter.outputs.core }} + # packages: ${{ steps.filter.outputs.packages }} + # steps: + # - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # @v2 + # if: ${{ github.event_name == 'push' }} + # with: + # fetch-depth: 0 + # # For pull requests it's not necessary to checkout the code + # - uses: dorny/paths-filter@4512585405083f25c027a35db413c2b3b9006d50 + # id: filter + # with: + # # See https://github.com/dorny/paths-filter/issues/56 for the syntax used below + # # Don't run if we only modified packages in the + # # built-in repository or documentation + # filters: | + # bootstrap: + # - 'var/spack/repos/builtin/packages/clingo-bootstrap/**' + # - 'var/spack/repos/builtin/packages/clingo/**' + # - 'var/spack/repos/builtin/packages/python/**' + # - 'var/spack/repos/builtin/packages/re2c/**' + # - 'lib/spack/**' + # - 'share/spack/**' + # - '.github/workflows/bootstrap.yml' + # - '.github/workflows/ci.yaml' + # core: + # - './!(var/**)/**' + # packages: + # - 'var/**' # Some links for easier reference: # # "github" context: https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#github-context @@ -75,8 +75,6 @@ jobs: # needs: [ prechecks, changes ] # uses: ./.github/workflows/unit_tests.yaml windows: - if: ${{ github.repository == 'johnwparent/spack' && needs.changes.outputs.core == 'true' }} - needs: [ prechecks ] uses: ./.github/workflows/windows_python.yml all: needs: [ windows] From 77cf9716916f6f2ed4b0dbd11497e210388a65c3 Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 12:36:13 -0400 Subject: [PATCH 35/39] attempt to get teh CI to run --- .github/workflows/windows_python.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/windows_python.yml b/.github/workflows/windows_python.yml index 91709672b4ab6b..40b9f3e15cfec0 100644 --- a/.github/workflows/windows_python.yml +++ b/.github/workflows/windows_python.yml @@ -1,8 +1,5 @@ name: windows -on: - workflow_call: - concurrency: group: windows-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true From 8428953e3c10a61d1239ecefe38c4861ff36d66a Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 12:39:34 -0400 Subject: [PATCH 36/39] gha is frustrating --- .github/workflows/windows_python.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/windows_python.yml b/.github/workflows/windows_python.yml index 40b9f3e15cfec0..b3e9b5d05fbd23 100644 --- a/.github/workflows/windows_python.yml +++ b/.github/workflows/windows_python.yml @@ -1,5 +1,7 @@ name: windows +on: push + concurrency: group: windows-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true From c9b0e04cf304f77f2ea02b1a077116c9c9c979b9 Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 12:46:11 -0400 Subject: [PATCH 37/39] Print all compilers --- lib/spack/spack/test/spec_syntax.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 1718d045f39d28..732627f442ab46 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -633,6 +633,8 @@ def test_spec_by_hash_tokens(text, tokens): @pytest.mark.db def test_spec_by_hash(database, monkeypatch, mutable_empty_config): mpileaks = database.query_one("mpileaks ^zmpi") + comp_lst = ";".join(spack.compilers.all_compilers()) + print(f"SPACK COMPILERS: {comp_lst}") b = spack.spec.Spec("b").concretized() monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [b]) From a9692b0c022996ed2414a2bfbf676819c230788f Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 13:03:58 -0400 Subject: [PATCH 38/39] compiler_to_str --- lib/spack/spack/test/spec_syntax.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 732627f442ab46..cdef7ca0d836c9 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -633,8 +633,14 @@ def test_spec_by_hash_tokens(text, tokens): @pytest.mark.db def test_spec_by_hash(database, monkeypatch, mutable_empty_config): mpileaks = database.query_one("mpileaks ^zmpi") - comp_lst = ";".join(spack.compilers.all_compilers()) + comp_lst = ";".join([str(x) for x in spack.compilers.all_compilers()]) print(f"SPACK COMPILERS: {comp_lst}") + + comp_from_conf = ";".join([str(x) for x in spack.config.get("compilers")]) + print(f"SPACK CONFIG COMPILERS: {comp_from_conf}") + + conf_file = spack.config.config.get_config_filename("user", "compilers") + print(f"CONFIG FILE: {conf_file}") b = spack.spec.Spec("b").concretized() monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [b]) From ef78a7be89bad2dd324195b743b7a64ba64df0e0 Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 18 Apr 2023 13:52:30 -0400 Subject: [PATCH 39/39] archspec_host_is_spack_test_host --- lib/spack/spack/test/spec_syntax.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index cdef7ca0d836c9..214cc8e38d90e3 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -631,16 +631,8 @@ def test_spec_by_hash_tokens(text, tokens): @pytest.mark.db -def test_spec_by_hash(database, monkeypatch, mutable_empty_config): +def test_spec_by_hash(database, monkeypatch, mutable_empty_config, archspec_host_is_spack_test_host): mpileaks = database.query_one("mpileaks ^zmpi") - comp_lst = ";".join([str(x) for x in spack.compilers.all_compilers()]) - print(f"SPACK COMPILERS: {comp_lst}") - - comp_from_conf = ";".join([str(x) for x in spack.config.get("compilers")]) - print(f"SPACK CONFIG COMPILERS: {comp_from_conf}") - - conf_file = spack.config.config.get_config_filename("user", "compilers") - print(f"CONFIG FILE: {conf_file}") b = spack.spec.Spec("b").concretized() monkeypatch.setattr(spack.binary_distribution, "update_cache_and_get_specs", lambda: [b])