diff options
| -rw-r--r-- | lib/spack/spack/solver/asp.py | 54 | ||||
| -rw-r--r-- | lib/spack/spack/test/concretize.py | 36 | ||||
| -rw-r--r-- | lib/spack/spack/test/cray_manifest.py | 21 |
3 files changed, 78 insertions, 33 deletions
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 13fe2ef156..b00982b4fb 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -3134,14 +3134,38 @@ def _develop_specs_from_env(spec, env): spec.constrain(dev_info["spec"]) -def _is_reusable_external(packages, spec: spack.spec.Spec) -> bool: - """Returns true iff spec is an external that can be reused. +def _is_reusable(spec: spack.spec.Spec, packages, local: bool) -> bool: + """A spec is reusable if it's not a dev spec, it's imported from the cray manifest, it's not + external, or it's external with matching packages.yaml entry. The latter prevents two issues: + + 1. Externals in build caches: avoid installing an external on the build machine not + available on the target machine + 2. Local externals: avoid reusing an external if the local config changes. This helps in + particular when a user removes an external from packages.yaml, and expects that that + takes effect immediately. Arguments: - packages: the packages configuration spec: the spec to check + packages: the packages configuration """ - for name in {spec.name, *(p.name for p in spec.package.provided)}: + if "dev_path" in spec.variants: + return False + + if not spec.external: + return True + + # Cray external manifest externals are always reusable + if local: + _, record = spack.store.STORE.db.query_by_spec_hash(spec.dag_hash()) + if record and record.origin == "external-db": + return True + + try: + provided = [p.name for p in spec.package.provided] + except spack.repo.RepoError: + provided = [] + + for name in {spec.name, *provided}: for entry in packages.get(name, {}).get("externals", []): if ( spec.satisfies(entry["spec"]) @@ -3188,29 +3212,21 @@ class Solver: def _reusable_specs(self, specs): reusable_specs = [] if self.reuse: + packages = spack.config.get("packages") # Specs from the local Database with spack.store.STORE.db.read_transaction(): reusable_specs.extend( - [ - s - for s in spack.store.STORE.db.query(installed=True) - if not s.satisfies("dev_path=*") - ] + s + for s in spack.store.STORE.db.query(installed=True) + if _is_reusable(s, packages, local=True) ) # Specs from buildcaches try: - # Specs in a build cache that depend on externals are reusable as long as local - # config has matching externals. This should guard against picking up binaries - # linked against externals not available locally, while still supporting the use - # case of distributing binaries across machines with similar externals. - packages = spack.config.get("packages") reusable_specs.extend( - [ - s - for s in spack.binary_distribution.update_cache_and_get_specs() - if not s.external or _is_reusable_external(packages, s) - ] + s + for s in spack.binary_distribution.update_cache_and_get_specs() + if _is_reusable(s, packages, local=False) ) except (spack.binary_distribution.FetchCacheError, IndexError): # this is raised when no mirrors had indices. diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 2eb75edb6c..fd5bd7d9c5 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -1817,12 +1817,14 @@ class TestConcretize: @pytest.mark.regression("31484") @pytest.mark.only_clingo("Use case not supported by the original concretizer") - def test_installed_externals_are_reused(self, mutable_database, repo_with_changing_recipe): + def test_installed_externals_are_reused( + self, mutable_database, repo_with_changing_recipe, tmp_path + ): """Test that external specs that are in the DB can be reused.""" external_conf = { "changing": { "buildable": False, - "externals": [{"spec": "changing@1.0", "prefix": "/usr"}], + "externals": [{"spec": "changing@1.0", "prefix": str(tmp_path)}], } } spack.config.set("packages", external_conf) @@ -1847,12 +1849,12 @@ class TestConcretize: @pytest.mark.regression("31484") @pytest.mark.only_clingo("Use case not supported by the original concretizer") - def test_user_can_select_externals_with_require(self, mutable_database): + def test_user_can_select_externals_with_require(self, mutable_database, tmp_path): """Test that users have means to select an external even in presence of reusable specs.""" external_conf = { "mpi": {"buildable": False}, "multi-provider-mpi": { - "externals": [{"spec": "multi-provider-mpi@2.0.0", "prefix": "/usr"}] + "externals": [{"spec": "multi-provider-mpi@2.0.0", "prefix": str(tmp_path)}] }, } spack.config.set("packages", external_conf) @@ -2434,7 +2436,8 @@ def test_reusable_externals_match(mock_packages, tmpdir): spec.external_path = tmpdir.strpath spec.external_modules = ["mpich/4.1"] spec._mark_concrete() - assert spack.solver.asp._is_reusable_external( + assert spack.solver.asp._is_reusable( + spec, { "mpich": { "externals": [ @@ -2442,7 +2445,7 @@ def test_reusable_externals_match(mock_packages, tmpdir): ] } }, - spec, + local=False, ) @@ -2451,7 +2454,8 @@ def test_reusable_externals_match_virtual(mock_packages, tmpdir): spec.external_path = tmpdir.strpath spec.external_modules = ["mpich/4.1"] spec._mark_concrete() - assert spack.solver.asp._is_reusable_external( + assert spack.solver.asp._is_reusable( + spec, { "mpi": { "externals": [ @@ -2459,7 +2463,7 @@ def test_reusable_externals_match_virtual(mock_packages, tmpdir): ] } }, - spec, + local=False, ) @@ -2468,7 +2472,8 @@ def test_reusable_externals_different_prefix(mock_packages, tmpdir): spec.external_path = "/other/path" spec.external_modules = ["mpich/4.1"] spec._mark_concrete() - assert not spack.solver.asp._is_reusable_external( + assert not spack.solver.asp._is_reusable( + spec, { "mpich": { "externals": [ @@ -2476,7 +2481,7 @@ def test_reusable_externals_different_prefix(mock_packages, tmpdir): ] } }, - spec, + local=False, ) @@ -2486,7 +2491,8 @@ def test_reusable_externals_different_modules(mock_packages, tmpdir, modules): spec.external_path = tmpdir.strpath spec.external_modules = modules spec._mark_concrete() - assert not spack.solver.asp._is_reusable_external( + assert not spack.solver.asp._is_reusable( + spec, { "mpich": { "externals": [ @@ -2494,7 +2500,7 @@ def test_reusable_externals_different_modules(mock_packages, tmpdir, modules): ] } }, - spec, + local=False, ) @@ -2502,6 +2508,8 @@ def test_reusable_externals_different_spec(mock_packages, tmpdir): spec = Spec("mpich@4.1%gcc@13.1.0~debug build_system=generic arch=linux-ubuntu23.04-zen2") spec.external_path = tmpdir.strpath spec._mark_concrete() - assert not spack.solver.asp._is_reusable_external( - {"mpich": {"externals": [{"spec": "mpich@4.1 +debug", "prefix": tmpdir.strpath}]}}, spec + assert not spack.solver.asp._is_reusable( + spec, + {"mpich": {"externals": [{"spec": "mpich@4.1 +debug", "prefix": tmpdir.strpath}]}}, + local=False, ) diff --git a/lib/spack/spack/test/cray_manifest.py b/lib/spack/spack/test/cray_manifest.py index 123e2ac3f1..dd19919ebc 100644 --- a/lib/spack/spack/test/cray_manifest.py +++ b/lib/spack/spack/test/cray_manifest.py @@ -19,6 +19,7 @@ import spack.cmd import spack.compilers import spack.config import spack.cray_manifest as cray_manifest +import spack.solver.asp import spack.spec import spack.store from spack.cray_manifest import compiler_from_entry, entries_to_specs @@ -488,3 +489,23 @@ def test_find_external_nonempty_default_manifest_dir( spack.cmd.external._collect_and_consume_cray_manifest_files(ignore_default_dir=False) specs = spack.store.STORE.db.query("hwloc") assert any(x.dag_hash() == "hwlocfakehashaaa" for x in specs) + + +def test_reusable_externals_cray_manifest( + tmpdir, mutable_config, mock_packages, temporary_store, manifest_content +): + """The concretizer should be able to reuse specs imported from a manifest without a + externals config entry in packages.yaml""" + with tmpdir.as_cwd(): + with open("external-db.json", "w") as f: + json.dump(manifest_content, f) + cray_manifest.read(path="external-db.json", apply_updates=True) + + # Get any imported spec + spec = temporary_store.db.query_local()[0] + + # Reusable if imported locally + assert spack.solver.asp._is_reusable(spec, packages={}, local=True) + + # If cray manifest entries end up in a build cache somehow, they are not reusable + assert not spack.solver.asp._is_reusable(spec, packages={}, local=False) |