Skip to content

Commit

Permalink
concretize.lp: fix issue with reuse of conditional variants (#43676)
Browse files Browse the repository at this point in the history
Currently if you request pkg +example where example is a conditional
variant, and you have a pkg in the database for which the condition
did not hold (so no +example nor ~example), the solver would reuse it
regardless, not imposing +example.

The change rules out exactly one thing: variant_set without variant_value,
which in practice could only happen when not node_has_variant (i.e. when
under the current package.py rules the variant's when condition did not
trigger).
  • Loading branch information
haampie committed Apr 16, 2024
1 parent 9a6c013 commit 9ff5a30
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
8 changes: 2 additions & 6 deletions lib/spack/spack/solver/concretize.lp
Original file line number Diff line number Diff line change
Expand Up @@ -891,12 +891,8 @@ error(100, "{0} variant '{1}' cannot have values '{2}' and '{3}' as they come fr
Set1 < Set2, % see[1]
build(node(ID, Package)).

% variant_set is an explicitly set variant value. If it's not 'set',
% we revert to the default value. If it is set, we force the set value
attr("variant_value", PackageNode, Variant, Value)
:- attr("node", PackageNode),
node_has_variant(PackageNode, Variant),
attr("variant_set", PackageNode, Variant, Value).
:- attr("variant_set", node(ID, Package), Variant, Value),
not attr("variant_value", node(ID, Package), Variant, Value).

% The rules below allow us to prefer default values for variants
% whenever possible. If a variant is set in a spec, or if it is
Expand Down
16 changes: 16 additions & 0 deletions lib/spack/spack/test/concretize.py
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,22 @@ def test_reuse_installed_packages_when_package_def_changes(
# Structure and package hash will be different without reuse
assert root.dag_hash() != new_root_without_reuse.dag_hash()

@pytest.mark.only_clingo("Use case not supported by the original concretizer")
@pytest.mark.regression("43663")
def test_no_reuse_when_variant_condition_does_not_hold(self, mutable_database, mock_packages):
spack.config.set("concretizer:reuse", True)

# Install a spec for which the `version_based` variant condition does not hold
old = Spec("conditional-variant-pkg @1").concretized()
old.package.do_install(fake=True, explicit=True)

# Then explicitly require a spec with `+version_based`, which shouldn't reuse previous spec
new1 = Spec("conditional-variant-pkg +version_based").concretized()
assert new1.satisfies("@2 +version_based")

new2 = Spec("conditional-variant-pkg +two_whens").concretized()
assert new2.satisfies("@2 +two_whens +version_based")

@pytest.mark.only_clingo("Use case not supported by the original concretizer")
def test_reuse_with_flags(self, mutable_database, mutable_config):
spack.config.set("concretizer:reuse", True)
Expand Down

0 comments on commit 9ff5a30

Please sign in to comment.