From e178c58847eec3fd2d6cdd7f646a5a2cd7a5e8c4 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 7 May 2024 09:06:51 +0200 Subject: [PATCH] Respect requests when filtering reused specs (#44042) Some specs which were excluded from reuse, are currently added back to the solve when we traverse dependencies of other reusable specs. This fixes the issue by keeping track of what we can explicitly reuse. --- lib/spack/spack/solver/asp.py | 15 ++++++++++- lib/spack/spack/test/concretize.py | 40 ++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index e3dd3ca6df..6b2205a6bb 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -944,14 +944,26 @@ class ConcreteSpecsByHash(collections.abc.Mapping): def __init__(self) -> None: self.data: Dict[str, spack.spec.Spec] = {} + self.explicit: Set[str] = set() def __getitem__(self, dag_hash: str) -> spack.spec.Spec: return self.data[dag_hash] + def explicit_items(self) -> Iterator[Tuple[str, spack.spec.Spec]]: + """Iterate on items that have been added explicitly, and not just as a dependency + of other nodes. + """ + for h, s in self.items(): + # We need to make an exception for gcc-runtime, until we can splice it. + if h in self.explicit or s.name == "gcc-runtime": + yield h, s + def add(self, spec: spack.spec.Spec) -> bool: """Adds a new concrete spec to the mapping. Returns True if the spec was just added, False if the spec was already in the mapping. + Calling this function marks the spec as added explicitly. + Args: spec: spec to be added @@ -966,6 +978,7 @@ def add(self, spec: spack.spec.Spec) -> bool: raise ValueError(msg) dag_hash = spec.dag_hash() + self.explicit.add(dag_hash) if dag_hash in self.data: return False @@ -2349,7 +2362,7 @@ def register_concrete_spec(self, spec, possible): def concrete_specs(self): """Emit facts for reusable specs""" - for h, spec in self.reusable_and_possible.items(): + for h, spec in self.reusable_and_possible.explicit_items(): # this indicates that there is a spec like this installed self.gen.fact(fn.installed_hash(spec.name, h)) # this describes what constraints it imposes on the solve diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 009d9b6a36..f0b92c98f5 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -2463,6 +2463,46 @@ def test_spec_with_build_dep_from_json(self, tmp_path): s = Spec(f"dtuse ^{str(json_file)}").concretized() assert s["dttop"].dag_hash() == build_dep.dag_hash() + @pytest.mark.regression("44040") + def test_exclude_specs_from_reuse(self, monkeypatch): + """Tests that we can exclude a spec from reuse when concretizing, and that the spec + is not added back to the solve as a dependency of another reusable spec. + + The expected spec is: + + o callpath@1.0 + |\ + | |\ + o | | mpich@3.0.4 + |/ / + | o dyninst@8.2 + |/| + | |\ + | | o libdwarf@20130729 + | |/| + |/|/ + | o libelf@0.8.13 + |/ + o glibc@2.31 + """ + # Prepare a mock mirror that returns an old version of dyninst + request_str = "callpath ^mpich" + reused = Spec(f"{request_str} ^dyninst@8.1.1").concretized() + monkeypatch.setattr(spack.solver.asp, "_specs_from_mirror", lambda: [reused]) + + # Exclude dyninst from reuse, so we expect that the old version is not taken into account + with spack.config.override( + "concretizer:reuse", {"from": [{"type": "buildcache", "exclude": ["dyninst"]}]} + ): + result = Spec(request_str).concretized() + + assert result.dag_hash() != reused.dag_hash() + assert result["mpich"].dag_hash() == reused["mpich"].dag_hash() + assert result["dyninst"].dag_hash() != reused["dyninst"].dag_hash() + assert result["dyninst"].satisfies("@=8.2") + for dep in result["dyninst"].traverse(root=False): + assert dep.dag_hash() == reused[dep.name].dag_hash() + @pytest.fixture() def duplicates_test_repository():