diff options
| author | Harmen Stoppels <me@harmenstoppels.nl> | 2024-07-11 15:29:56 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-07-11 15:29:56 +0200 |
| commit | 5c539732204a04b1ca179890c22384aaf0b4fe60 (patch) | |
| tree | 9758e6b1d592e1dddcce897ea067b25b7294df58 | |
| parent | 278a38f4af4fcc45cf0e47a9986dd18b21a3bf70 (diff) | |
| download | spack-5c539732204a04b1ca179890c22384aaf0b4fe60.tar.gz spack-5c539732204a04b1ca179890c22384aaf0b4fe60.tar.bz2 spack-5c539732204a04b1ca179890c22384aaf0b4fe60.tar.xz spack-5c539732204a04b1ca179890c22384aaf0b4fe60.zip | |
concretize.lp: drop 0 weight of external providers (#45025)
If an external happens to be a provider of anything, the solver would
set its weight to 0, meaning that it is most preferred, even if
packages.yaml config disagrees.
That was done so that `spack external find mpich` would be sufficent to
pick it up as mpi provider.
That may have made sense for mpi specifically, but doesn't make sense
for other virtuals. For example `glibc` provides iconv, and is an
external by design, but it's better to use libiconv as a separate
package as a provider.
Therefore, drop this rule, and instead let users add config:
```
mpi:
require: [mpich]
```
or
```
mpi:
buildable: false
```
which is well-documented.
| -rw-r--r-- | lib/spack/spack/solver/concretize.lp | 13 | ||||
| -rw-r--r-- | lib/spack/spack/test/concretize.py | 19 | ||||
| -rw-r--r-- | lib/spack/spack/test/conftest.py | 2 |
3 files changed, 13 insertions, 21 deletions
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index ee8f26fd4e..595efb1f19 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -611,25 +611,18 @@ do_not_impose(EffectID, node(X, Package)) % Virtual dependency weights %----------------------------------------------------------------------------- -% A provider may have different possible weights depending on whether it's an external -% or not, or on preferences expressed in packages.yaml etc. This rule ensures that +% A provider has different possible weights depending on its preference. This rule ensures that % we select the weight, among the possible ones, that minimizes the overall objective function. 1 { provider_weight(DependencyNode, VirtualNode, Weight) : possible_provider_weight(DependencyNode, VirtualNode, Weight, _) } 1 :- provider(DependencyNode, VirtualNode), internal_error("Package provider weights must be unique"). -% A provider that is an external can use a weight of 0 -possible_provider_weight(DependencyNode, VirtualNode, 0, "external") - :- provider(DependencyNode, VirtualNode), - external(DependencyNode). - -% A provider mentioned in the default configuration can use a weight -% according to its priority in the list of providers +% Any configured provider has a weight based on index in the preference list possible_provider_weight(node(ProviderID, Provider), node(VirtualID, Virtual), Weight, "default") :- provider(node(ProviderID, Provider), node(VirtualID, Virtual)), default_provider_preference(Virtual, Provider, Weight). -% Any provider can use 100 as a weight, which is very high and discourage its use +% Any non-configured provider has a default weight of 100 possible_provider_weight(node(ProviderID, Provider), VirtualNode, 100, "fallback") :- provider(node(ProviderID, Provider), VirtualNode). diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index fb261ff0d5..ee204dd909 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -688,7 +688,8 @@ class TestConcretize: with pytest.raises(spack.error.SpecError): spec.concretize() - def test_external_and_virtual(self): + def test_external_and_virtual(self, mutable_config): + mutable_config.set("packages:stuff", {"buildable": False}) spec = Spec("externaltest") spec.concretize() assert spec["externaltool"].external_path == os.path.sep + os.path.join( @@ -1170,16 +1171,14 @@ class TestConcretize: assert s.external assert "stuff" not in s - def test_transitive_conditional_virtual_dependency(self): + def test_transitive_conditional_virtual_dependency(self, mutable_config): + """Test that an external is used as provider if the virtual is non-buildable""" + mutable_config.set("packages:stuff", {"buildable": False}) s = Spec("transitive-conditional-virtual-dependency").concretized() - # The default for conditional-virtual-dependency is to have - # +stuff~mpi, so check that these defaults are respected - assert "+stuff" in s["conditional-virtual-dependency"] - assert "~mpi" in s["conditional-virtual-dependency"] - - # 'stuff' is provided by an external package, so check it's present - assert "externalvirtual" in s + # Test that the default +stuff~mpi is maintained, and the right provider is selected + assert s.satisfies("^conditional-virtual-dependency +stuff~mpi") + assert s.satisfies("^[virtuals=stuff] externalvirtual") @pytest.mark.regression("20040") @pytest.mark.only_clingo("Use case not supported by the original concretizer") @@ -1785,7 +1784,7 @@ class TestConcretize: [ (["libelf", "libelf@0.8.10"], 1, 1), (["libdwarf%gcc", "libelf%clang"], 2, 1), - (["libdwarf%gcc", "libdwarf%clang"], 3, 2), + (["libdwarf%gcc", "libdwarf%clang"], 3, 1), (["libdwarf^libelf@0.8.12", "libdwarf^libelf@0.8.13"], 4, 1), (["hdf5", "zmpi"], 3, 1), (["hdf5", "mpich"], 2, 1), diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 62b257a64e..cebd89e49e 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -857,7 +857,7 @@ def _populate(mock_db): _install("mpileaks ^mpich") _install("mpileaks ^mpich2") _install("mpileaks ^zmpi") - _install("externaltest") + _install("externaltest ^externalvirtual") _install("trivial-smoke-test") |