From 47e70c5c3af3ae66f648f7fecb65cedfa8e9b0aa Mon Sep 17 00:00:00 2001
From: Greg Becker <becker33@llnl.gov>
Date: Sun, 27 Oct 2024 11:35:10 -0700
Subject: 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.
---
 lib/spack/spack/solver/asp.py      | 20 ++++++++++++++++++--
 lib/spack/spack/test/concretize.py | 24 ++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

(limited to 'lib')

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",
-- 
cgit v1.2.3-70-g09d2