Rework conflicts so that "vendors" is not needed anymore

This commit is contained in:
Massimiliano Culpo 2023-08-14 14:24:00 +02:00 committed by Todd Gamblin
parent fb83f8ef31
commit d3aca68e8f
9 changed files with 39 additions and 88 deletions

View file

@ -33,7 +33,6 @@ class OpenMpi(Package):
import functools import functools
import os.path import os.path
import re import re
import warnings
from typing import Any, Callable, List, Optional, Set, Tuple, Union from typing import Any, Callable, List, Optional, Set, Tuple, Union
import llnl.util.lang import llnl.util.lang
@ -69,7 +68,6 @@ class OpenMpi(Package):
"resource", "resource",
"build_system", "build_system",
"requires", "requires",
"vendors",
] ]
#: These are variant names used by Spack internally; packages can't use them #: These are variant names used by Spack internally; packages can't use them
@ -520,20 +518,8 @@ def _execute_conflicts(pkg):
if not when_spec: if not when_spec:
return return
# TODO: (remove after v0.21)
conflict_key = conflict_spec
s = spack.spec.Spec(conflict_spec)
if s.name and s.name != pkg.name:
warning_msg = (
f"the conflict in package '{pkg.name}' on '{conflict_spec}' should "
f"start with a '^' sigil. Not using it is deprecated as of v0.21 and"
f" will be disallowed in v0.22"
)
warnings.warn(warning_msg)
conflict_key = "^" + conflict_spec
# Save in a list the conflicts and the associated custom messages # Save in a list the conflicts and the associated custom messages
when_spec_list = pkg.conflicts.setdefault(conflict_key, []) when_spec_list = pkg.conflicts.setdefault(conflict_spec, [])
msg_with_name = f"{pkg.name}: {msg}" if msg is not None else msg msg_with_name = f"{pkg.name}: {msg}" if msg is not None else msg
when_spec_list.append((when_spec, msg_with_name)) when_spec_list.append((when_spec, msg_with_name))
@ -917,29 +903,6 @@ def _execute_requires(pkg):
return _execute_requires return _execute_requires
@directive("vendors")
def vendors(spec, when=None):
"""Declares that a package has an internal copy of another package.
Currently, the effect is to forbid having the two packages in the same
"unification set".
Args:
spec: spec being vendored
when: optional constraint that triggers vendoring
"""
def _execute_vendors(pkg):
when_spec = make_when_spec(when)
if not when_spec:
return
when_spec_list = pkg.vendors.setdefault(spec, [])
when_spec_list.append(when_spec)
return _execute_vendors
class DirectiveError(spack.error.SpackError): class DirectiveError(spack.error.SpackError):
"""This is raised when something is wrong with a package directive.""" """This is raised when something is wrong with a package directive."""

View file

@ -1030,7 +1030,10 @@ def conflict_rules(self, pkg):
no_constraint_msg = "{0}: conflicts with '{1}'" no_constraint_msg = "{0}: conflicts with '{1}'"
for trigger, constraints in pkg.conflicts.items(): for trigger, constraints in pkg.conflicts.items():
trigger_msg = "conflict trigger %s" % str(trigger) trigger_msg = "conflict trigger %s" % str(trigger)
trigger_id = self.condition(spack.spec.Spec(trigger), name=pkg.name, msg=trigger_msg) trigger_spec = spack.spec.Spec(trigger)
trigger_id = self.condition(
trigger_spec, name=trigger_spec.name or pkg.name, msg=trigger_msg
)
for constraint, conflict_msg in constraints: for constraint, conflict_msg in constraints:
if conflict_msg is None: if conflict_msg is None:
@ -1045,15 +1048,6 @@ def conflict_rules(self, pkg):
) )
self.gen.newline() self.gen.newline()
def vendor_rules(self, pkg):
"""Facts about vendored packages."""
for vendored_spec_str, constraints in pkg.vendors.items():
vendored_spec = spack.spec.Spec(vendored_spec_str)
for constraint in constraints:
constraint_id = self.condition(constraint, name=pkg.name)
self.gen.fact(fn.pkg_fact(pkg.name, fn.vendors(constraint_id, vendored_spec.name)))
self.gen.newline()
def compiler_facts(self): def compiler_facts(self):
"""Facts about available compilers.""" """Facts about available compilers."""
@ -1203,9 +1197,6 @@ def pkg_rules(self, pkg, tests):
# conflicts # conflicts
self.conflict_rules(pkg) self.conflict_rules(pkg)
# vendoring
self.vendor_rules(pkg)
# default compilers for this package # default compilers for this package
self.package_compiler_defaults(pkg) self.package_compiler_defaults(pkg)
@ -1224,19 +1215,20 @@ def pkg_rules(self, pkg, tests):
self.package_requirement_rules(pkg) self.package_requirement_rules(pkg)
# trigger and effect tables # trigger and effect tables
self.trigger_rules(pkg.name) self.trigger_rules()
self.effect_rules(pkg.name) self.effect_rules(pkg.name)
def trigger_rules(self, name): def trigger_rules(self):
self.gen.h2("Trigger conditions") self.gen.h2("Trigger conditions")
cache = self._trigger_cache[name] for name in self._trigger_cache:
for spec_str, (trigger_id, requirements) in cache.items(): cache = self._trigger_cache[name]
self.gen.fact(fn.pkg_fact(name, fn.trigger_id(trigger_id))) for spec_str, (trigger_id, requirements) in cache.items():
self.gen.fact(fn.pkg_fact(name, fn.trigger_msg(spec_str))) self.gen.fact(fn.pkg_fact(name, fn.trigger_id(trigger_id)))
for predicate in requirements: self.gen.fact(fn.pkg_fact(name, fn.trigger_msg(spec_str)))
self.gen.fact(fn.condition_requirement(trigger_id, *predicate.args)) for predicate in requirements:
self.gen.newline() self.gen.fact(fn.condition_requirement(trigger_id, *predicate.args))
cache.clear() self.gen.newline()
self._trigger_cache.clear()
def effect_rules(self, name): def effect_rules(self, name):
self.gen.h2("Imposed requirements") self.gen.h2("Imposed requirements")
@ -1488,7 +1480,7 @@ def provider_requirements(self):
virtual_str, requirements, kind=RequirementKind.VIRTUAL virtual_str, requirements, kind=RequirementKind.VIRTUAL
) )
self.emit_facts_from_requirement_rules(rules) self.emit_facts_from_requirement_rules(rules)
self.trigger_rules(virtual_str) self.trigger_rules()
self.effect_rules(virtual_str) self.effect_rules(virtual_str)
def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]): def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]):
@ -1607,7 +1599,7 @@ def external_packages(self):
self.possible_versions[spec.name].add(spec.version) self.possible_versions[spec.name].add(spec.version)
self.gen.newline() self.gen.newline()
self.trigger_rules(pkg_name) self.trigger_rules()
def preferred_variants(self, pkg_name): def preferred_variants(self, pkg_name):
"""Facts on concretization preferences, as read from packages.yaml""" """Facts on concretization preferences, as read from packages.yaml"""
@ -2434,7 +2426,7 @@ def setup(self, driver, specs, reuse=None):
# Inject dev_path from environment # Inject dev_path from environment
for ds in dev_specs: for ds in dev_specs:
self.condition(spack.spec.Spec(ds.name), ds, msg="%s is a develop spec" % ds.name) self.condition(spack.spec.Spec(ds.name), ds, msg="%s is a develop spec" % ds.name)
self.trigger_rules(ds.name) self.trigger_rules()
self.effect_rules(ds.name) self.effect_rules(ds.name)
self.gen.h1("Spec Constraints") self.gen.h1("Spec Constraints")

View file

@ -441,21 +441,14 @@ error(10, "'{0}' is not a valid dependency for any package in the DAG", Package)
error(1, Msg) error(1, Msg)
:- attr("node", node(ID, Package)), :- attr("node", node(ID, Package)),
pkg_fact(Package, conflict(TriggerID, ConstraintID, Msg)), pkg_fact(Package, conflict(TriggerID, ConstraintID, Msg)),
condition_holds(TriggerID, node(ID, Package)), % node(ID1, TriggerPackage) is node(ID2, Package) in most, but not all, cases
condition_holds(ConstraintID, node(ID, Package)), condition_holds(TriggerID, node(ID1, TriggerPackage)),
condition_holds(ConstraintID, node(ID2, Package)),
unification_set(X, node(ID2, Package)),
unification_set(X, node(ID1, TriggerPackage)),
not external(node(ID, Package)), % ignore conflicts for externals not external(node(ID, Package)), % ignore conflicts for externals
not attr("hash", node(ID, Package), _). % ignore conflicts for installed packages not attr("hash", node(ID, Package), _). % ignore conflicts for installed packages
%-----------------------------------------------------------------------------
% Vendoring
%-----------------------------------------------------------------------------
error(1, "{0} vendors an internal copy of {1}, so it cannot be in the same unification set as {1}", Package, VendoredPackage)
:- pkg_fact(Package, vendors(ConditionID, VendoredPackage)),
attr("node", node(ID, Package)),
condition_holds(ConditionID, node(ID, Package)),
unification_set(X, node(ID, Package)),
unification_set(X, node(_, VendoredPackage)).
%----------------------------------------------------------------------------- %-----------------------------------------------------------------------------
% Virtual dependencies % Virtual dependencies
%----------------------------------------------------------------------------- %-----------------------------------------------------------------------------

View file

@ -561,10 +561,12 @@ def test_environment_config_scheme_used(tmp_path, unify_in_config):
("vendorsb@=1.1", True, None), ("vendorsb@=1.1", True, None),
], ],
) )
def test_vendors_directive( def test_conflicts_with_packages_that_are_not_dependencies(
spec_str, expected_raise, expected_spec, tmp_path, mock_packages, config spec_str, expected_raise, expected_spec, tmp_path, mock_packages, config
): ):
"""Tests that we cannot concretize two specs together, if one vendors the other.""" """Tests that we cannot concretize two specs together, if one conflicts with the other,
even though they don't have a dependency relation.
"""
if spack.config.get("config:concretizer") == "original": if spack.config.get("config:concretizer") == "original":
pytest.xfail("Known failure of the original concretizer") pytest.xfail("Known failure of the original concretizer")

View file

@ -7,7 +7,7 @@
class Vendorsb(Package): class Vendorsb(Package):
"""A package that vendors another""" """A package that vendors another, and thus conflicts with it"""
homepage = "http://www.example.com" homepage = "http://www.example.com"
url = "http://www.example.com/b-1.0.tar.gz" url = "http://www.example.com/b-1.0.tar.gz"
@ -15,4 +15,5 @@ class Vendorsb(Package):
version("1.1", md5="0123456789abcdef0123456789abcdef") version("1.1", md5="0123456789abcdef0123456789abcdef")
version("1.0", md5="0123456789abcdef0123456789abcdef") version("1.0", md5="0123456789abcdef0123456789abcdef")
vendors("b", when="@=1.1") # b is not a dependency
conflicts("b", when="@=1.1")

View file

@ -40,7 +40,7 @@ class Memkind(AutotoolsPackage):
# memkind includes a copy of jemalloc; see # memkind includes a copy of jemalloc; see
# <https://github.com/memkind/memkind#jemalloc>. # <https://github.com/memkind/memkind#jemalloc>.
vendors("jemalloc") conflicts("jemalloc")
# https://github.com/spack/spack/issues/37292 # https://github.com/spack/spack/issues/37292
parallel = False parallel = False

View file

@ -95,8 +95,8 @@ class Palace(CMakePackage):
depends_on("arpack-ng~shared", when="~shared") depends_on("arpack-ng~shared", when="~shared")
# Palace always builds its own internal MFEM, GSLIB # Palace always builds its own internal MFEM, GSLIB
vendors("mfem") conflicts("mfem")
vendors("gslib") conflicts("gslib")
# More dependency variant conflicts # More dependency variant conflicts
conflicts("^hypre+int64", msg="Palace uses HYPRE's mixedint option for 64 bit integers") conflicts("^hypre+int64", msg="Palace uses HYPRE's mixedint option for 64 bit integers")

View file

@ -70,8 +70,8 @@ class Scotch(CMakePackage, MakefilePackage):
# Vendored dependency of METIS/ParMETIS conflicts with standard # Vendored dependency of METIS/ParMETIS conflicts with standard
# installations # installations
vendors("metis", when="+metis") conflicts("metis", when="+metis")
vendors("parmetis", when="+metis") conflicts("parmetis", when="+metis")
parallel = False parallel = False

View file

@ -29,9 +29,9 @@ class Votca(CMakePackage):
) )
variant("xtp", default=True, description="Build xtp parts of votca") variant("xtp", default=True, description="Build xtp parts of votca")
vendors("votca-tools") conflicts("votca-tools")
vendors("votca-csg") conflicts("votca-csg")
vendors("votca-xtp") conflicts("votca-xtp")
depends_on("cmake@3.13:", type="build") depends_on("cmake@3.13:", type="build")
depends_on("expat") depends_on("expat")