diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 3a825929307..a3e04fcac03 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -128,8 +128,6 @@ class Provenance(enum.IntEnum): SPEC = enum.auto() # A dev spec literal DEV_SPEC = enum.auto() - # An external spec declaration - EXTERNAL = enum.auto() # The 'packages' section of the configuration PACKAGES_YAML = enum.auto() # A package requirement @@ -138,6 +136,8 @@ class Provenance(enum.IntEnum): PACKAGE_PY = enum.auto() # An installed spec INSTALLED = enum.auto() + # An external spec declaration + EXTERNAL = enum.auto() # lower provenance for installed git refs so concretizer prefers StandardVersion installs INSTALLED_GIT_VERSION = enum.auto() # A runtime injected from another package (e.g. a compiler) diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 697a6d4d08d..3ed05e81691 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -1638,7 +1638,12 @@ build(PackageNode) :- attr("node", PackageNode), not concrete(PackageNode). % 200 - 299 Shifted priorities for build nodes; correspond to priorities 0 - 99. % 100 - 199 Unshifted priorities. Currently only includes minimizing #builds and minimizing dupes. % 0 - 99 Priorities for non-built nodes. -build_priority(PackageNode, 200) :- build(PackageNode), attr("node", PackageNode). + +treat_node_as_concrete(node(X, Package)) :- external(node(X, Package)). +treat_node_as_concrete(node(X, Package)) :- attr("node", node(X, Package)), runtime(Package). + +build_priority(PackageNode, 200) :- build(PackageNode), attr("node", PackageNode), not treat_node_as_concrete(PackageNode). +build_priority(PackageNode, 0) :- build(PackageNode), attr("node", PackageNode), treat_node_as_concrete(PackageNode). build_priority(PackageNode, 0) :- not build(PackageNode), attr("node", PackageNode). % don't assign versions from installed packages unless reuse is enabled @@ -1695,7 +1700,7 @@ opt_criterion(310, "requirement weight"). % Try hard to reuse installed packages (i.e., minimize the number built) opt_criterion(110, "number of packages to build (vs. reuse)"). #minimize { 0@110: #true }. -#minimize { 1@110,PackageNode : build(PackageNode) }. +#minimize { 1@110,PackageNode : build(PackageNode), not treat_node_as_concrete(PackageNode) }. opt_criterion(100, "number of nodes from the same package"). #minimize { 0@100: #true }. @@ -1860,48 +1865,59 @@ opt_criterion(10, "target mismatches"). not runtime(Dependency) }. -opt_criterion(5, "non-preferred targets"). -#minimize{ 0@205: #true }. -#minimize{ 0@5: #true }. +opt_criterion(7, "non-preferred targets"). +#minimize{ 0@207: #true }. +#minimize{ 0@7: #true }. #minimize{ - Weight@5+Priority,node(X, Package) + Weight@7+Priority,node(X, Package) : node_target_weight(node(X, Package), Weight), build_priority(node(X, Package), Priority), not runtime(Package) }. -opt_criterion(4, "preferred providers (language runtimes)"). -#minimize{ 0@204: #true }. -#minimize{ 0@4: #true }. +opt_criterion(5, "preferred providers (language runtimes)"). +#minimize{ 0@205: #true }. +#minimize{ 0@5: #true }. #minimize{ - Weight@4+Priority,ProviderNode,X,Virtual + Weight@5+Priority,ProviderNode,X,Virtual : provider_weight(ProviderNode, node(X, Virtual), Weight), language_runtime(Virtual), build_priority(ProviderNode, Priority) }. % Choose more recent versions for runtimes -opt_criterion(3, "version badness (runtimes)"). -#minimize{ 0@203: #true }. -#minimize{ 0@3: #true }. +opt_criterion(4, "version badness (runtimes)"). +#minimize{ 0@204: #true }. +#minimize{ 0@4: #true }. #minimize{ - Weight@3,node(X, Package) + Weight@4,node(X, Package) : version_weight(node(X, Package), Weight), runtime(Package) }. % Choose best target for runtimes -opt_criterion(2, "non-preferred targets (runtimes)"). -#minimize{ 0@202: #true }. -#minimize{ 0@2: #true }. +opt_criterion(3, "non-preferred targets (runtimes)"). +#minimize{ 0@203: #true }. +#minimize{ 0@3: #true }. #minimize{ - Weight@2,node(X, Package) + Weight@3,node(X, Package) : node_target_weight(node(X, Package), Weight), runtime(Package) }. % Choose more recent versions for nodes -opt_criterion(1, "edge wiring"). +opt_criterion(2, "providers on edges"). +#minimize{ 0@202: #true }. +#minimize{ 0@2: #true }. +#minimize{ + Weight@2,ParentNode,ProviderNode,Virtual + : provider_weight(ProviderNode, Virtual, Weight), + not attr("root", ProviderNode), + depends_on(ParentNode, ProviderNode) +}. + +% Choose more recent versions for nodes +opt_criterion(1, "version badness on edges"). #minimize{ 0@201: #true }. #minimize{ 0@1: #true }. #minimize{ @@ -1912,14 +1928,6 @@ opt_criterion(1, "edge wiring"). }. -#minimize{ 0@201: #true }. -#minimize{ 0@1: #true }. -#minimize{ - Weight@1,ParentNode,ProviderNode,Virtual - : provider_weight(ProviderNode, Virtual, Weight), - not attr("root", ProviderNode), - depends_on(ParentNode, ProviderNode) -}. %----------- % Notes diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py index 0399d8d7ef2..722174072ed 100644 --- a/lib/spack/spack/test/concretization/core.py +++ b/lib/spack/spack/test/concretization/core.py @@ -188,6 +188,7 @@ class Root(Package): url = "http://www.example.com/root-1.0.tar.gz" version("1.0", sha256="abcde") + depends_on("middle") depends_on("changing") conflicts("^changing~foo") @@ -196,6 +197,20 @@ class Root(Package): package_py.parent.mkdir(parents=True) package_py.write_text(root_pkg_str) + middle_pkg_str = """ +from spack.package import * + +class Middle(Package): + homepage = "http://www.example.com" + url = "http://www.example.com/root-1.0.tar.gz" + + version("1.0", sha256="abcde") + depends_on("changing") +""" + package_py = packages_dir / "middle" / "package.py" + package_py.parent.mkdir(parents=True) + package_py.write_text(middle_pkg_str) + changing_template = """ from spack.package import * @@ -445,9 +460,7 @@ def test_mixing_compilers_only_affects_subdag(self): if "c" not in x or not x.name.startswith("dt-diamond"): continue expected_gcc = x.name != "dt-diamond" - assert ( - bool(x.dependencies(name="llvm", deptype="build")) is not expected_gcc - ), x.long_spec + assert bool(x.dependencies(name="llvm", deptype="build")) is not expected_gcc, x.tree() assert bool(x.dependencies(name="gcc", deptype="build")) is expected_gcc assert x.satisfies("%clang") is not expected_gcc assert x.satisfies("%gcc") is expected_gcc @@ -1007,11 +1020,17 @@ def test_concretize_anonymous_dep(self, spec_str): ("bowtie@1.4.0", "%gcc@10.2.1"), # Version with conflicts and no valid gcc select another compiler ("bowtie@1.3.0", "%clang@15.0.0"), - # If a higher gcc is available, with a worse os, still prefer that - ("bowtie@1.2.2", "%gcc@11.1.0"), + # If a higher gcc is available, with a worse os, still prefer that, + # assuming the two operating systems are compatible + ("bowtie@1.2.2 %gcc", "%gcc@11.1.0"), ], ) - def test_compiler_conflicts_in_package_py(self, spec_str, expected_str, gcc11_with_flags): + def test_compiler_conflicts_in_package_py( + self, spec_str, expected_str, gcc11_with_flags, mutable_config + ): + mutable_config.set( + "concretizer:os_compatible", {"debian6": ["redhat6"], "redhat6": ["debian6"]} + ) with spack.config.override("packages", {"gcc": {"externals": [gcc11_with_flags]}}): s = spack.concretize.concretize_one(spec_str) assert s.satisfies(expected_str) @@ -1224,20 +1243,6 @@ def test_activating_test_dependencies(self, spec_str, tests_arg, with_dep, witho node = s[pkg_name] assert not node.dependencies(deptype="test"), msg.format(pkg_name) - @pytest.mark.regression("20019") - def test_compiler_match_is_preferred_to_newer_version(self, compiler_factory): - # This spec depends on openblas. Openblas has a conflict - # that doesn't allow newer versions with gcc@4.4.0. Check - # that an old version of openblas is selected, rather than - # a different compiler for just that node. - with spack.config.override( - "packages", {"gcc": {"externals": [compiler_factory(spec="gcc@10.1.0 os=redhat6")]}} - ): - spec_str = "simple-inheritance+openblas os=redhat6 %gcc@10.1.0" - s = spack.concretize.concretize_one(spec_str) - assert "openblas@0.2.15" in s - assert s["openblas"].satisfies("%gcc@10.1.0") - @pytest.mark.regression("19981") def test_target_ranges_in_conflicts(self): with pytest.raises(spack.error.SpackError): @@ -1863,7 +1868,7 @@ def test_misleading_error_message_on_version(self, mutable_database): @pytest.mark.regression("31148") def test_version_weight_and_provenance(self): - """Test package preferences during coconcretization.""" + """Test package preferences during concretization.""" reusable_specs = [ spack.concretize.concretize_one(spec_str) for spec_str in ("pkg-b@0.9", "pkg-b@1.0") ] @@ -1873,17 +1878,18 @@ def test_version_weight_and_provenance(self): solver = spack.solver.asp.Solver() setup = spack.solver.asp.SpackSolverSetup() result, _, _ = solver.driver.solve(setup, [root_spec], reuse=reusable_specs) - # The result here should have a single spec to build ('pkg-a') - # and it should be using pkg-b@1.0 with a version badness of 2 - # The provenance is: + # Version badness should be > 0 only for reused specs. For instance, for pkg-b + # the version provenance is: + # # version_declared("pkg-b","1.0",0,"package_py"). # version_declared("pkg-b","0.9",1,"package_py"). # version_declared("pkg-b","1.0",2,"installed"). # version_declared("pkg-b","0.9",3,"installed"). - # - # Depending on the target, it may also use gnuconfig + v_weights = [x for x in result.criteria if x[2] == "version badness (non roots)"][0] + reused_weights, built_weights, _ = v_weights + assert reused_weights > 2 and built_weights == 0 + result_spec = result.specs[0] - assert (2, 0, "version badness (non roots)") in result.criteria assert result_spec.satisfies("^pkg-b@1.0") assert result_spec["pkg-b"].dag_hash() == reusable_specs[1].dag_hash() @@ -1930,7 +1936,9 @@ def test_git_ref_version_succeeds_with_unknown_version(self, git_ref): 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.""" + """Tests that external specs that are in the DB can be reused, if they result in a + better optimization score. + """ external_conf = { "changing": { "buildable": False, @@ -1940,22 +1948,23 @@ def test_installed_externals_are_reused( spack.config.set("packages", external_conf) # Install the external spec - external1 = spack.concretize.concretize_one("changing@1.0") - PackageInstaller([external1.package], fake=True, explicit=True).install() - assert external1.external + middle_pkg = spack.concretize.concretize_one("middle") + PackageInstaller([middle_pkg.package], fake=True, explicit=True).install() + assert middle_pkg["changing"].external + changing_external = middle_pkg["changing"] # Modify the package.py file repo_with_changing_recipe.change({"delete_variant": True}) # Try to concretize the external without reuse and confirm the hash changed with spack.config.override("concretizer:reuse", False): - external2 = spack.concretize.concretize_one("changing@1.0") - assert external2.dag_hash() != external1.dag_hash() + root_no_reuse = spack.concretize.concretize_one("root") + assert root_no_reuse["changing"].dag_hash() != changing_external.dag_hash() # ... while with reuse we have the same hash with spack.config.override("concretizer:reuse", True): - external3 = spack.concretize.concretize_one("changing@1.0") - assert external3.dag_hash() == external1.dag_hash() + root_with_reuse = spack.concretize.concretize_one("root") + assert root_with_reuse["changing"].dag_hash() == changing_external.dag_hash() @pytest.mark.regression("31484") def test_user_can_select_externals_with_require(self, mutable_database, tmp_path): @@ -3440,3 +3449,24 @@ def test_using_externals_with_compilers(mutable_config, mock_packages, tmp_path) libelf = s["libelf"] assert libelf.external and libelf.satisfies("%gcc") + + +@pytest.mark.regression("50161") +def test_installed_compiler_and_better_external( + install_mockery, do_not_check_runtimes_on_reuse, mutable_config +): + """Tests that we always prefer a higher-priority external compiler, when we have a + lower-priority compiler installed, and we try to concretize a spec without specifying + the compiler dependency. + """ + pkg_b = spack.concretize.concretize_one(spack.spec.Spec("pkg-b %clang")) + PackageInstaller([pkg_b.package], fake=True, explicit=True).install() + + with spack.config.override("concretizer:reuse", False): + pkg_a = spack.concretize.concretize_one("pkg-a") + assert pkg_a["c"].satisfies("gcc@10"), pkg_a.tree() + assert pkg_a["pkg-b"]["c"].satisfies("gcc@10") + + with spack.config.override("concretizer:reuse", False): + mpileaks = spack.concretize.concretize_one("mpileaks") + assert mpileaks.satisfies("%gcc@10") diff --git a/lib/spack/spack/test/concretization/requirements.py b/lib/spack/spack/test/concretization/requirements.py index c83a59d674e..cae5cb760a0 100644 --- a/lib/spack/spack/test/concretization/requirements.py +++ b/lib/spack/spack/test/concretization/requirements.py @@ -1270,13 +1270,10 @@ def test_virtual_requirement_respects_any_of(concretize_scope, mock_packages): packages: all: require: - - "%gcc" - pkg-a: - require: - - "%gcc@10" + - "%gcc@9" """, True, - ["%gcc@10"], + ["%gcc@9"], ), ], ) @@ -1303,4 +1300,4 @@ def test_requirements_on_compilers_and_reuse( assert is_pkgb_reused == expected_reuse for c in expected_contraints: - assert pkga.satisfies(c), print(pkga.tree()) + assert pkga.satisfies(c)