solver: don't use tags to compute injected deps (#49723)

This commit reorders ASP setup, so that rules from
possible compilers are collected first.

This allows us to know the dependencies that may be
injected before counting the possible dependencies,
so we can account for them too.

Proceeding this way makes it easier to inject
complex runtimes, like hip.

Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
This commit is contained in:
Massimiliano Culpo 2025-04-02 09:32:25 +02:00 committed by GitHub
parent c004c8b616
commit 01471aee6b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 56 additions and 57 deletions

View File

@ -2997,8 +2997,36 @@ def setup(
"""
reuse = reuse or []
check_packages_exist(specs)
self.gen = ProblemInstanceBuilder()
node_counter = create_counter(specs, tests=self.tests, possible_graph=self.possible_graph)
# Compute possible compilers first, so we can record which dependencies they might inject
_ = spack.compilers.config.all_compilers(init_config=True)
# Get compilers from buildcache only if injected through "reuse" specs
supported_compilers = spack.compilers.config.supported_compilers()
compilers_from_reuse = {
x for x in reuse if x.name in supported_compilers and not x.external
}
candidate_compilers, self.rejected_compilers = possible_compilers(
configuration=spack.config.CONFIG
)
for x in candidate_compilers:
if x.external or x in reuse:
continue
reuse.append(x)
for dep in x.traverse(root=False, deptype="run"):
reuse.extend(dep.traverse(deptype=("link", "run")))
candidate_compilers.update(compilers_from_reuse)
self.possible_compilers = list(candidate_compilers)
self.possible_compilers.sort() # type: ignore[call-overload]
self.gen.h1("Runtimes")
injected_dependencies = self.define_runtime_constraints()
node_counter = create_counter(
specs + injected_dependencies, tests=self.tests, possible_graph=self.possible_graph
)
self.possible_virtuals = node_counter.possible_virtuals()
self.pkgs = node_counter.possible_dependencies()
self.libcs = sorted(all_libcs()) # type: ignore[type-var]
@ -3021,7 +3049,6 @@ def setup(
if node.namespace is not None:
self.explicitly_required_namespaces[node.name] = node.namespace
self.gen = ProblemInstanceBuilder()
self.gen.h1("Generic information")
if using_libc_compatibility():
for libc in self.libcs:
@ -3050,27 +3077,6 @@ def setup(
specs = tuple(specs) # ensure compatible types to add
_ = spack.compilers.config.all_compilers(init_config=True)
# Get compilers from buildcache only if injected through "reuse" specs
supported_compilers = spack.compilers.config.supported_compilers()
compilers_from_reuse = {
x for x in reuse if x.name in supported_compilers and not x.external
}
candidate_compilers, self.rejected_compilers = possible_compilers(
configuration=spack.config.CONFIG
)
for x in candidate_compilers:
if x.external or x in reuse:
continue
reuse.append(x)
for dep in x.traverse(root=False, deptype="run"):
reuse.extend(dep.traverse(deptype=("link", "run")))
candidate_compilers.update(compilers_from_reuse)
self.possible_compilers = list(candidate_compilers)
self.possible_compilers.sort() # type: ignore[call-overload]
self.gen.h1("Reusable concrete specs")
self.define_concrete_input_specs(specs, self.pkgs)
if reuse:
@ -3142,9 +3148,6 @@ def setup(
self.gen.h1("Variant Values defined in specs")
self.define_variant_values()
self.gen.h1("Runtimes")
self.define_runtime_constraints()
self.gen.h1("Version Constraints")
self.collect_virtual_constraints()
self.define_version_constraints()
@ -3178,8 +3181,10 @@ def visit(node):
path = os.path.join(parent_dir, "concretize.lp")
parse_files([path], visit)
def define_runtime_constraints(self):
"""Define the constraints to be imposed on the runtimes"""
def define_runtime_constraints(self) -> List[spack.spec.Spec]:
"""Define the constraints to be imposed on the runtimes, and returns a list of
injected packages.
"""
recorder = RuntimePropertyRecorder(self)
for compiler in self.possible_compilers:
@ -3229,6 +3234,7 @@ def define_runtime_constraints(self):
)
recorder.consume_facts()
return sorted(recorder.injected_dependencies)
def literal_specs(self, specs):
for spec in sorted(specs):
@ -3493,6 +3499,7 @@ def __init__(self, setup):
self._setup = setup
self.rules = []
self.runtime_conditions = set()
self.injected_dependencies = set()
# State of this object set in the __call__ method, and reset after
# each directive-like method
self.current_package = None
@ -3531,6 +3538,7 @@ def depends_on(self, dependency_str: str, *, when: str, type: str, description:
if dependency_spec.versions != vn.any_version:
self._setup.version_constraints.add((dependency_spec.name, dependency_spec.versions))
self.injected_dependencies.add(dependency_spec)
body_str, node_variable = self.rule_body_from(when_spec)
head_clauses = self._setup.spec_clauses(dependency_spec, body=False)
@ -3686,20 +3694,21 @@ def consume_facts(self):
"""Consume the facts collected by this object, and emits rules and
facts for the runtimes.
"""
self._setup.gen.h2("Runtimes: declarations")
runtime_pkgs = sorted(
{x.name for x in self.injected_dependencies if not spack.repo.PATH.is_virtual(x.name)}
)
for runtime_pkg in runtime_pkgs:
self._setup.gen.fact(fn.runtime(runtime_pkg))
self._setup.gen.newline()
self._setup.gen.h2("Runtimes: rules")
self._setup.gen.newline()
for rule in self.rules:
self._setup.gen.append(rule)
self._setup.gen.newline()
self._setup.gen.h2("Runtimes: conditions")
for runtime_pkg in spack.repo.PATH.packages_with_tags("runtime"):
self._setup.gen.fact(fn.runtime(runtime_pkg))
self._setup.gen.fact(fn.possible_in_link_run(runtime_pkg))
self._setup.gen.newline()
# Inject version rules for runtimes (versions are declared based
# on the available compilers)
self._setup.pkg_version_rules(runtime_pkg)
self._setup.gen.h2("Runtimes: requirements")
for imposed_spec, when_spec in sorted(self.runtime_conditions):
msg = f"{when_spec} requires {imposed_spec} at runtime"
_ = self._setup.condition(when_spec, imposed_spec=imposed_spec, msg=msg)

View File

@ -18,8 +18,6 @@
import spack.store
from spack.error import SpackError
RUNTIME_TAG = "runtime"
class PossibleGraph(NamedTuple):
real_pkgs: Set[str]
@ -50,7 +48,8 @@ def possible_dependencies(
) -> PossibleGraph:
"""Returns the set of possible dependencies, and the set of possible virtuals.
Both sets always include runtime packages, which may be injected by compilers.
Runtime packages, which may be injected by compilers, needs to be added to specs if
the dependency is not explicit in the package.py recipe.
Args:
transitive: return transitive dependencies if True, only direct dependencies if False
@ -70,14 +69,9 @@ class NoStaticAnalysis(PossibleDependencyGraph):
def __init__(self, *, configuration: spack.config.Configuration, repo: spack.repo.RepoPath):
self.configuration = configuration
self.repo = repo
self.runtime_pkgs = set(self.repo.packages_with_tags(RUNTIME_TAG))
self.runtime_virtuals = set()
self._platform_condition = spack.spec.Spec(
f"platform={spack.platforms.host()} target={archspec.cpu.host().family}:"
)
for x in self.runtime_pkgs:
pkg_class = self.repo.get_pkg_class(x)
self.runtime_virtuals.update(pkg_class.provided_virtual_names())
try:
self.libc_pkgs = [x.name for x in self.providers_for("libc")]
@ -214,8 +208,6 @@ def possible_dependencies(
for root, children in edges.items():
real_packages.update(x for x in children if self._is_possible(pkg_name=x))
virtuals.update(self.runtime_virtuals)
real_packages = real_packages | self.runtime_pkgs
return PossibleGraph(real_pkgs=real_packages, virtuals=virtuals, edges=edges)
def _package_list(self, specs: Tuple[Union[spack.spec.Spec, str], ...]) -> List[str]:
@ -470,7 +462,7 @@ def possible_packages_facts(self, gen, fn):
gen.fact(fn.max_dupes(package_name, 1))
gen.newline()
gen.h2("Packages with at multiple possible nodes (build-tools)")
gen.h2("Packages with multiple possible nodes (build-tools)")
default = spack.config.CONFIG.get("concretizer:duplicates:max_dupes:default", 2)
for package_name in sorted(self.possible_dependencies() & build_tools):
max_dupes = spack.config.CONFIG.get(

View File

@ -38,10 +38,9 @@
(["--transitive", "--deptype=link", "dtbuild1"], {"dtlink2"}),
],
)
def test_direct_dependencies(cli_args, expected, mock_runtimes):
def test_direct_dependencies(cli_args, expected, mock_packages):
out = dependencies(*cli_args)
result = set(re.split(r"\s+", out.strip()))
expected.update(mock_runtimes)
assert expected == result

View File

@ -111,14 +111,13 @@ def mpi_names(mock_inspector):
("dtbuild1", {"allowed_deps": dt.LINK}, {"dtbuild1", "dtlink2"}),
],
)
def test_possible_dependencies(pkg_name, fn_kwargs, expected, mock_runtimes, mock_inspector):
def test_possible_dependencies(pkg_name, fn_kwargs, expected, mock_inspector):
"""Tests possible nodes of mpileaks, under different scenarios."""
expected.update(mock_runtimes)
result, *_ = mock_inspector.possible_dependencies(pkg_name, **fn_kwargs)
assert expected == result
def test_possible_dependencies_virtual(mock_inspector, mock_packages, mock_runtimes, mpi_names):
def test_possible_dependencies_virtual(mock_inspector, mock_packages, mpi_names):
expected = set(mpi_names)
for name in mpi_names:
expected.update(
@ -126,7 +125,6 @@ def test_possible_dependencies_virtual(mock_inspector, mock_packages, mock_runti
for dep in mock_packages.get_pkg_class(name).dependencies_by_name()
if not mock_packages.is_virtual(dep)
)
expected.update(mock_runtimes)
expected.update(s.name for s in mock_packages.providers_for("c"))
real_pkgs, *_ = mock_inspector.possible_dependencies(
@ -146,7 +144,6 @@ def test_possible_dependencies_with_multiple_classes(
pkgs = ["dt-diamond", "mpileaks"]
expected = set(mpileaks_possible_deps)
expected.update({"dt-diamond", "dt-diamond-left", "dt-diamond-right", "dt-diamond-bottom"})
expected.update(mock_packages.packages_with_tags("runtime"))
real_pkgs, *_ = mock_inspector.possible_dependencies(*pkgs, allowed_deps=dt.ALL)
assert set(expected) == real_pkgs

View File

@ -110,9 +110,9 @@ class Corge
f.write(corge_h)
with open("%s/corge/corgegator.cc" % self.stage.source_path, "w", encoding="utf-8") as f:
f.write(corgegator_cc)
gpp = which("/usr/bin/g++")
gpp = which("g++")
if sys.platform == "darwin":
gpp = which("/usr/bin/clang++")
gpp = which("clang++")
gpp(
"-Dcorge_EXPORTS",
"-I%s" % self.stage.source_path,

View File

@ -0,0 +1 @@
../../builtin.mock/packages/compiler-wrapper

View File

@ -0,0 +1 @@
../../builtin.mock/packages/compiler-wrapper