Fix interaction of spec literals that propagate variants with unify:false (#40789)

* Add tests to ensure variant propagation syntax can round-trip to/from string

* Add a regression test for the bug in 35298

* Reconstruct the spec constraints in the worker process

Specs do not preserve any information on propagation of variants
when round-tripping to/from JSON (which we use to pickle), but
preserve it when round-tripping to/from strings.

Therefore, we pass a spec literal to the worker and reconstruct
the Spec objects there.

5 files changed, 91 insertions, 1 deletions

diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 9998161df2..cd2a5a7533 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -1484,7 +1484,7 @@ class Environment:
         for uspec, uspec_constraints in zip(self.user_specs, self.user_specs.specs_as_constraints):
             if uspec not in old_concretized_user_specs:
                 root_specs.append(uspec)
-                args.append((i, uspec_constraints, tests))
+                args.append((i, [str(x) for x in uspec_constraints], tests))
                 i += 1
 
         # Ensure we don't try to bootstrap clingo in parallel
@@ -2403,6 +2403,7 @@ def _concretize_from_constraints(spec_constraints, tests=False):
 
 def _concretize_task(packed_arguments) -> Tuple[int, Spec, float]:
     index, spec_constraints, tests = packed_arguments
+    spec_constraints = [Spec(x) for x in spec_constraints]
     with tty.SuppressOutput(msg_enabled=False):
         start = time.time()
         spec = _concretize_from_constraints(spec_constraints, tests)
diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py
index e88af08761..f6b89e2108 100644
--- a/lib/spack/spack/test/env.py
+++ b/lib/spack/spack/test/env.py
@@ -690,3 +690,29 @@ def test_removing_spec_from_manifest_with_exact_duplicates(
     assert "zlib" in manifest.read_text()
     with ev.Environment(tmp_path) as env:
         assert len(env.user_specs) == 1
+
+
+@pytest.mark.regression("35298")
+@pytest.mark.only_clingo("Propagation not supported in the original concretizer")
+def test_variant_propagation_with_unify_false(tmp_path, mock_packages):
+    """Spack distributes concretizations to different processes, when unify:false is selected and
+    the number of roots is 2 or more. When that happens, the specs to be concretized need to be
+    properly reconstructed on the worker process, if variant propagation was requested.
+    """
+    manifest = tmp_path / "spack.yaml"
+    manifest.write_text(
+        """
+    spack:
+      specs:
+      - parent-foo ++foo
+      - c
+      concretizer:
+        unify: false
+    """
+    )
+    with ev.Environment(tmp_path) as env:
+        env.concretize()
+
+    root = env.matching_spec("parent-foo")
+    for node in root.traverse():
+        assert node.satisfies("+foo")
diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py
index d731fcd31c..e7a760dc93 100644
--- a/lib/spack/spack/test/spec_syntax.py
+++ b/lib/spack/spack/test/spec_syntax.py
@@ -525,6 +525,31 @@ def specfile_for(default_mock_concretization):
             ],
             "zlib@git.foo/bar",
         ),
+        # Variant propagation
+        (
+            "zlib ++foo",
+            [
+                Token(TokenType.UNQUALIFIED_PACKAGE_NAME, "zlib"),
+                Token(TokenType.PROPAGATED_BOOL_VARIANT, "++foo"),
+            ],
+            "zlib++foo",
+        ),
+        (
+            "zlib ~~foo",
+            [
+                Token(TokenType.UNQUALIFIED_PACKAGE_NAME, "zlib"),
+                Token(TokenType.PROPAGATED_BOOL_VARIANT, "~~foo"),
+            ],
+            "zlib~~foo",
+        ),
+        (
+            "zlib foo==bar",
+            [
+                Token(TokenType.UNQUALIFIED_PACKAGE_NAME, "zlib"),
+                Token(TokenType.PROPAGATED_KEY_VALUE_PAIR, "foo==bar"),
+            ],
+            "zlib foo==bar",
+        ),
     ],
 )
 def test_parse_single_spec(spec_str, tokens, expected_roundtrip):
diff --git a/var/spack/repos/builtin.mock/packages/client-not-foo/package.py b/var/spack/repos/builtin.mock/packages/client-not-foo/package.py
new file mode 100644
index 0000000000..03c9374b3a
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/client-not-foo/package.py
@@ -0,0 +1,17 @@
+# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class ClientNotFoo(Package):
+    """This package has a variant "foo", which is False by default."""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/c-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    variant("foo", default=False, description="")
diff --git a/var/spack/repos/builtin.mock/packages/parent-foo/package.py b/var/spack/repos/builtin.mock/packages/parent-foo/package.py
new file mode 100644
index 0000000000..61d15231f7
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/parent-foo/package.py
@@ -0,0 +1,21 @@
+# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.package import *
+
+
+class ParentFoo(Package):
+    """This package has a variant "foo", which is True by default, and depends on another
+    package which has the same variant defaulting to False.
+    """
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/c-1.0.tar.gz"
+
+    version("1.0", md5="0123456789abcdef0123456789abcdef")
+
+    variant("foo", default=True, description="")
+
+    depends_on("client-not-foo")