diff options
author | Greg Becker <becker33@llnl.gov> | 2024-10-27 11:35:10 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-10-27 11:35:10 -0700 |
commit | 47e70c5c3af3ae66f648f7fecb65cedfa8e9b0aa (patch) | |
tree | 907dc970536b7e32a0e3f81789772554064d6e19 /lib | |
parent | fea2171672737687195572d920cc911d3c194229 (diff) | |
download | spack-47e70c5c3af3ae66f648f7fecb65cedfa8e9b0aa.tar.gz spack-47e70c5c3af3ae66f648f7fecb65cedfa8e9b0aa.tar.bz2 spack-47e70c5c3af3ae66f648f7fecb65cedfa8e9b0aa.tar.xz spack-47e70c5c3af3ae66f648f7fecb65cedfa8e9b0aa.zip |
explicit splice: do not fail for bad config replacement if target not matched (#46925)
Originally, concretization failed if the splice config points to an invalid replacement.
This PR defers the check until we know the splice is needed, so that irrelevant splices
with bad config cannot stop concretization.
While I was at it, I improved an error message from an assert to a ValueError.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/solver/asp.py | 20 | ||||
-rw-r--r-- | lib/spack/spack/test/concretize.py | 24 |
2 files changed, 42 insertions, 2 deletions
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 92734e9afd..940aae0a72 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -3829,8 +3829,16 @@ class SpecBuilder: for splice_set in splice_config: target = splice_set["target"] replacement = spack.spec.Spec(splice_set["replacement"]) - assert replacement.abstract_hash - replacement.replace_hash() + + if not replacement.abstract_hash: + location = getattr( + splice_set["replacement"], "_start_mark", " at unknown line number" + ) + msg = f"Explicit splice replacement '{replacement}' does not include a hash.\n" + msg += f"{location}\n\n" + msg += " Splice replacements must be specified by hash" + raise InvalidSpliceError(msg) + transitive = splice_set.get("transitive", False) splice_triples.append((target, replacement, transitive)) @@ -3841,6 +3849,10 @@ class SpecBuilder: if target in current_spec: # matches root or non-root # e.g. mvapich2%gcc + + # The first iteration, we need to replace the abstract hash + if not replacement.concrete: + replacement.replace_hash() current_spec = current_spec.splice(replacement, transitive) new_key = NodeArgument(id=key.id, pkg=current_spec.name) specs[new_key] = current_spec @@ -4226,3 +4238,7 @@ class SolverError(InternalConcretizerError): # Add attribute expected of the superclass interface self.required = None self.constraint_type = None + + +class InvalidSpliceError(spack.error.SpackError): + """For cases in which the splice configuration is invalid.""" diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index b56f05b601..553d8fd642 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -2306,6 +2306,30 @@ class TestConcretize: assert "hdf5 ^zmpi" in captured.err assert str(spec) in captured.err + def test_explicit_splice_fails_nonexistent(mutable_config, mock_packages, mock_store): + splice_info = {"target": "mpi", "replacement": "mpich/doesnotexist"} + spack.config.CONFIG.set("concretizer", {"splice": {"explicit": [splice_info]}}) + + with pytest.raises(spack.spec.InvalidHashError): + _ = spack.spec.Spec("hdf5^zmpi").concretized() + + def test_explicit_splice_fails_no_hash(mutable_config, mock_packages, mock_store): + splice_info = {"target": "mpi", "replacement": "mpich"} + spack.config.CONFIG.set("concretizer", {"splice": {"explicit": [splice_info]}}) + + with pytest.raises(spack.solver.asp.InvalidSpliceError, match="must be specified by hash"): + _ = spack.spec.Spec("hdf5^zmpi").concretized() + + def test_explicit_splice_non_match_nonexistent_succeeds( + mutable_config, mock_packages, mock_store + ): + """When we have a nonexistent splice configured but are not using it, don't fail.""" + splice_info = {"target": "will_not_match", "replacement": "nonexistent/doesnotexist"} + spack.config.CONFIG.set("concretizer", {"splice": {"explicit": [splice_info]}}) + spec = spack.spec.Spec("zlib").concretized() + # the main test is that it does not raise + assert not spec.spliced + @pytest.mark.db @pytest.mark.parametrize( "spec_str,mpi_name", |