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.
This commit is contained in:
parent
cd6bb9e159
commit
544a121248
5 changed files with 91 additions and 1 deletions
|
@ -1484,7 +1484,7 @@ def _concretize_separately(self, tests=False):
|
||||||
for uspec, uspec_constraints in zip(self.user_specs, self.user_specs.specs_as_constraints):
|
for uspec, uspec_constraints in zip(self.user_specs, self.user_specs.specs_as_constraints):
|
||||||
if uspec not in old_concretized_user_specs:
|
if uspec not in old_concretized_user_specs:
|
||||||
root_specs.append(uspec)
|
root_specs.append(uspec)
|
||||||
args.append((i, uspec_constraints, tests))
|
args.append((i, [str(x) for x in uspec_constraints], tests))
|
||||||
i += 1
|
i += 1
|
||||||
|
|
||||||
# Ensure we don't try to bootstrap clingo in parallel
|
# 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]:
|
def _concretize_task(packed_arguments) -> Tuple[int, Spec, float]:
|
||||||
index, spec_constraints, tests = packed_arguments
|
index, spec_constraints, tests = packed_arguments
|
||||||
|
spec_constraints = [Spec(x) for x in spec_constraints]
|
||||||
with tty.SuppressOutput(msg_enabled=False):
|
with tty.SuppressOutput(msg_enabled=False):
|
||||||
start = time.time()
|
start = time.time()
|
||||||
spec = _concretize_from_constraints(spec_constraints, tests)
|
spec = _concretize_from_constraints(spec_constraints, tests)
|
||||||
|
|
|
@ -690,3 +690,29 @@ def test_removing_spec_from_manifest_with_exact_duplicates(
|
||||||
assert "zlib" in manifest.read_text()
|
assert "zlib" in manifest.read_text()
|
||||||
with ev.Environment(tmp_path) as env:
|
with ev.Environment(tmp_path) as env:
|
||||||
assert len(env.user_specs) == 1
|
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")
|
||||||
|
|
|
@ -525,6 +525,31 @@ def _specfile_for(spec_str, filename):
|
||||||
],
|
],
|
||||||
"zlib@git.foo/bar",
|
"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):
|
def test_parse_single_spec(spec_str, tokens, expected_roundtrip):
|
||||||
|
|
|
@ -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="")
|
21
var/spack/repos/builtin.mock/packages/parent-foo/package.py
Normal file
21
var/spack/repos/builtin.mock/packages/parent-foo/package.py
Normal file
|
@ -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")
|
Loading…
Reference in a new issue