From 8fc1ba2d7a27109a8f5a4836c23c2dacd3d2dd10 Mon Sep 17 00:00:00 2001
From: Richarda Butler <39577672+RikkiButler20@users.noreply.github.com>
Date: Fri, 3 Nov 2023 12:09:39 -0700
Subject: [PATCH] Bugfix: propagation of multivalued variants (#39833)

Don't encourage use of default value if propagating a multivalued variant.
---
 lib/spack/spack/solver/concretize.lp                 |  1 +
 lib/spack/spack/test/concretize.py                   | 12 ++++++++++++
 var/spack/repos/builtin.mock/packages/b/package.py   |  4 ++++
 .../packages/multivalue-variant/package.py           |  2 +-
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 2207fa9f9a..340e1b04ee 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -881,6 +881,7 @@ variant_default_not_used(node(ID, Package), Variant, Value)
   :- variant_default_value(Package, Variant, Value),
      node_has_variant(node(ID, Package), Variant),
      not attr("variant_value", node(ID, Package), Variant, Value),
+     not attr("variant_propagate", node(ID, Package), Variant, _, _),
      attr("node", node(ID, Package)).
 
 % The variant is set in an external spec
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 915f6ca39b..0af689ddd5 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -472,6 +472,18 @@ def test_concretize_propagated_variant_is_not_passed_to_dependent(self):
 
         assert spec.satisfies("^openblas+shared")
 
+    @pytest.mark.only_clingo("Original concretizer is allowed to forego variant propagation")
+    def test_concretize_propagate_multivalue_variant(self):
+        """Test that multivalue variants are propagating the specified value(s)
+        to their dependecies. The dependencies should not have the default value"""
+        spec = Spec("multivalue-variant foo==baz,fee")
+        spec.concretize()
+
+        assert spec.satisfies("^a foo=baz,fee")
+        assert spec.satisfies("^b foo=baz,fee")
+        assert not spec.satisfies("^a foo=bar")
+        assert not spec.satisfies("^b foo=bar")
+
     def test_no_matching_compiler_specs(self, mock_low_high_config):
         # only relevant when not building compilers as needed
         with spack.concretize.enable_compiler_existence_check():
diff --git a/var/spack/repos/builtin.mock/packages/b/package.py b/var/spack/repos/builtin.mock/packages/b/package.py
index 06d8286085..1685711825 100644
--- a/var/spack/repos/builtin.mock/packages/b/package.py
+++ b/var/spack/repos/builtin.mock/packages/b/package.py
@@ -15,4 +15,8 @@ class B(Package):
     version("1.0", md5="0123456789abcdef0123456789abcdef")
     version("0.9", md5="abcd456789abcdef0123456789abcdef")
 
+    variant(
+        "foo", description="", values=any_combination_of("bar", "baz", "fee").with_default("bar")
+    )
+
     depends_on("test-dependency", type="test")
diff --git a/var/spack/repos/builtin.mock/packages/multivalue-variant/package.py b/var/spack/repos/builtin.mock/packages/multivalue-variant/package.py
index 1366463241..b0f7ac9501 100644
--- a/var/spack/repos/builtin.mock/packages/multivalue-variant/package.py
+++ b/var/spack/repos/builtin.mock/packages/multivalue-variant/package.py
@@ -19,7 +19,7 @@ class MultivalueVariant(Package):
     variant(
         "foo",
         description="Multi-valued variant",
-        values=any_combination_of("bar", "baz", "barbaz"),
+        values=any_combination_of("bar", "baz", "barbaz", "fee"),
     )
 
     variant(