Fix using fully-qualified namespaces from root specs (#41957)

Explicitly requested namespaces are annotated during
the setup phase, and used to retrieve the correct package
class.

An attribute for the namespace has been added for each node.

Currently, a single namespace per package is allowed
during concretization.
This commit is contained in:
Massimiliano Culpo 2024-01-16 11:47:32 +01:00 committed by GitHub
parent f449832d6f
commit ddae696cf8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 82 additions and 25 deletions

View File

@ -24,6 +24,7 @@
import textwrap import textwrap
import time import time
import traceback import traceback
import typing
import warnings import warnings
from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Type, TypeVar, Union from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Type, TypeVar, Union
@ -732,13 +733,13 @@ def dependencies_by_name(cls, when: bool = False):
@classmethod @classmethod
def possible_dependencies( def possible_dependencies(
cls, cls,
transitive=True, transitive: bool = True,
expand_virtuals=True, expand_virtuals: bool = True,
depflag: dt.DepFlag = dt.ALL, depflag: dt.DepFlag = dt.ALL,
visited=None, visited: Optional[dict] = None,
missing=None, missing: Optional[dict] = None,
virtuals=None, virtuals: Optional[set] = None,
): ) -> Dict[str, Set[str]]:
"""Return dict of possible dependencies of this package. """Return dict of possible dependencies of this package.
Args: Args:
@ -2493,14 +2494,21 @@ def flatten_dependencies(spec, flat_dir):
dep_files.merge(flat_dir + "/" + name) dep_files.merge(flat_dir + "/" + name)
def possible_dependencies(*pkg_or_spec, **kwargs): def possible_dependencies(
*pkg_or_spec: Union[str, spack.spec.Spec, typing.Type[PackageBase]],
transitive: bool = True,
expand_virtuals: bool = True,
depflag: dt.DepFlag = dt.ALL,
missing: Optional[dict] = None,
virtuals: Optional[set] = None,
) -> Dict[str, Set[str]]:
"""Get the possible dependencies of a number of packages. """Get the possible dependencies of a number of packages.
See ``PackageBase.possible_dependencies`` for details. See ``PackageBase.possible_dependencies`` for details.
""" """
packages = [] packages = []
for pos in pkg_or_spec: for pos in pkg_or_spec:
if isinstance(pos, PackageMeta): if isinstance(pos, PackageMeta) and issubclass(pos, PackageBase):
packages.append(pos) packages.append(pos)
continue continue
@ -2513,9 +2521,16 @@ def possible_dependencies(*pkg_or_spec, **kwargs):
else: else:
packages.append(pos.package_class) packages.append(pos.package_class)
visited = {} visited: Dict[str, Set[str]] = {}
for pkg in packages: for pkg in packages:
pkg.possible_dependencies(visited=visited, **kwargs) pkg.possible_dependencies(
visited=visited,
transitive=transitive,
expand_virtuals=expand_virtuals,
depflag=depflag,
missing=missing,
virtuals=virtuals,
)
return visited return visited

View File

@ -13,6 +13,7 @@
import re import re
import sys import sys
import types import types
import typing
import warnings import warnings
from typing import Callable, Dict, List, NamedTuple, Optional, Sequence, Set, Tuple, Union from typing import Callable, Dict, List, NamedTuple, Optional, Sequence, Set, Tuple, Union
@ -418,7 +419,7 @@ def check_packages_exist(specs):
for spec in specs: for spec in specs:
for s in spec.traverse(): for s in spec.traverse():
try: try:
check_passed = repo.exists(s.name) or repo.is_virtual(s.name) check_passed = repo.repo_for_pkg(s).exists(s.name) or repo.is_virtual(s.name)
except Exception as e: except Exception as e:
msg = "Cannot find package: {0}".format(str(e)) msg = "Cannot find package: {0}".format(str(e))
check_passed = False check_passed = False
@ -1175,6 +1176,7 @@ def __init__(self, tests=False):
# Set during the call to setup # Set during the call to setup
self.pkgs = None self.pkgs = None
self.explicitly_required_namespaces = {}
def pkg_version_rules(self, pkg): def pkg_version_rules(self, pkg):
"""Output declared versions of a package. """Output declared versions of a package.
@ -1187,7 +1189,9 @@ def key_fn(version):
# Origins are sorted by "provenance" first, see the Provenance enumeration above # Origins are sorted by "provenance" first, see the Provenance enumeration above
return version.origin, version.idx return version.origin, version.idx
pkg = packagize(pkg) if isinstance(pkg, str):
pkg = self.pkg_class(pkg)
declared_versions = self.declared_versions[pkg.name] declared_versions = self.declared_versions[pkg.name]
partially_sorted_versions = sorted(set(declared_versions), key=key_fn) partially_sorted_versions = sorted(set(declared_versions), key=key_fn)
@ -1406,7 +1410,10 @@ def reject_requirement_constraint(
return False return False
def pkg_rules(self, pkg, tests): def pkg_rules(self, pkg, tests):
pkg = packagize(pkg) pkg = self.pkg_class(pkg)
# Namespace of the package
self.gen.fact(fn.pkg_fact(pkg.name, fn.namespace(pkg.namespace)))
# versions # versions
self.pkg_version_rules(pkg) self.pkg_version_rules(pkg)
@ -2038,7 +2045,7 @@ class Body:
if not spec.concrete: if not spec.concrete:
reserved_names = spack.directives.reserved_names reserved_names = spack.directives.reserved_names
if not spec.virtual and vname not in reserved_names: if not spec.virtual and vname not in reserved_names:
pkg_cls = spack.repo.PATH.get_pkg_class(spec.name) pkg_cls = self.pkg_class(spec.name)
try: try:
variant_def, _ = pkg_cls.variants[vname] variant_def, _ = pkg_cls.variants[vname]
except KeyError: except KeyError:
@ -2159,7 +2166,7 @@ def define_package_versions_and_validate_preferences(
"""Declare any versions in specs not declared in packages.""" """Declare any versions in specs not declared in packages."""
packages_yaml = spack.config.get("packages") packages_yaml = spack.config.get("packages")
for pkg_name in possible_pkgs: for pkg_name in possible_pkgs:
pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) pkg_cls = self.pkg_class(pkg_name)
# All the versions from the corresponding package.py file. Since concepts # All the versions from the corresponding package.py file. Since concepts
# like being a "develop" version or being preferred exist only at a # like being a "develop" version or being preferred exist only at a
@ -2621,8 +2628,6 @@ def setup(
""" """
check_packages_exist(specs) check_packages_exist(specs)
self.possible_virtuals = set(x.name for x in specs if x.virtual)
node_counter = _create_counter(specs, tests=self.tests) node_counter = _create_counter(specs, tests=self.tests)
self.possible_virtuals = node_counter.possible_virtuals() self.possible_virtuals = node_counter.possible_virtuals()
self.pkgs = node_counter.possible_dependencies() self.pkgs = node_counter.possible_dependencies()
@ -2638,6 +2643,10 @@ def setup(
if missing_deps: if missing_deps:
raise spack.spec.InvalidDependencyError(spec.name, missing_deps) raise spack.spec.InvalidDependencyError(spec.name, missing_deps)
for node in spack.traverse.traverse_nodes(specs):
if node.namespace is not None:
self.explicitly_required_namespaces[node.name] = node.namespace
# driver is used by all the functions below to add facts and # driver is used by all the functions below to add facts and
# rules to generate an ASP program. # rules to generate an ASP program.
self.gen = driver self.gen = driver
@ -2866,6 +2875,13 @@ def _specs_from_requires(self, pkg_name, section):
for s in spec_group[key]: for s in spec_group[key]:
yield _spec_with_default_name(s, pkg_name) yield _spec_with_default_name(s, pkg_name)
def pkg_class(self, pkg_name: str) -> typing.Type["spack.package_base.PackageBase"]:
request = pkg_name
if pkg_name in self.explicitly_required_namespaces:
namespace = self.explicitly_required_namespaces[pkg_name]
request = f"{namespace}.{pkg_name}"
return spack.repo.PATH.get_pkg_class(request)
class RuntimePropertyRecorder: class RuntimePropertyRecorder:
"""An object of this class is injected in callbacks to compilers, to let them declare """An object of this class is injected in callbacks to compilers, to let them declare
@ -3077,6 +3093,9 @@ def _arch(self, node):
self._specs[node].architecture = arch self._specs[node].architecture = arch
return arch return arch
def namespace(self, node, namespace):
self._specs[node].namespace = namespace
def node_platform(self, node, platform): def node_platform(self, node, platform):
self._arch(node).platform = platform self._arch(node).platform = platform
@ -3291,14 +3310,6 @@ def build_specs(self, function_tuples):
action(*args) action(*args)
# namespace assignment is done after the fact, as it is not
# currently part of the solve
for spec in self._specs.values():
if spec.namespace:
continue
repo = spack.repo.PATH.repo_for_pkg(spec)
spec.namespace = repo.namespace
# fix flags after all specs are constructed # fix flags after all specs are constructed
self.reorder_flags() self.reorder_flags()

View File

@ -45,6 +45,9 @@
:- attr("depends_on", node(min_dupe_id, Package), node(ID, _), "link"), ID != min_dupe_id, unification_set("root", node(min_dupe_id, Package)), internal_error("link dependency out of the root unification set"). :- attr("depends_on", node(min_dupe_id, Package), node(ID, _), "link"), ID != min_dupe_id, unification_set("root", node(min_dupe_id, Package)), internal_error("link dependency out of the root unification set").
:- attr("depends_on", node(min_dupe_id, Package), node(ID, _), "run"), ID != min_dupe_id, unification_set("root", node(min_dupe_id, Package)), internal_error("run dependency out of the root unification set"). :- attr("depends_on", node(min_dupe_id, Package), node(ID, _), "run"), ID != min_dupe_id, unification_set("root", node(min_dupe_id, Package)), internal_error("run dependency out of the root unification set").
% Namespaces are statically assigned by a package fact
attr("namespace", node(ID, Package), Namespace) :- attr("node", node(ID, Package)), pkg_fact(Package, namespace(Namespace)).
% Rules on "unification sets", i.e. on sets of nodes allowing a single configuration of any given package % Rules on "unification sets", i.e. on sets of nodes allowing a single configuration of any given package
unify(SetID, PackageName) :- unification_set(SetID, node(_, PackageName)). unify(SetID, PackageName) :- unification_set(SetID, node(_, PackageName)).
:- 2 { unification_set(SetID, node(_, PackageName)) }, unify(SetID, PackageName). :- 2 { unification_set(SetID, node(_, PackageName)) }, unify(SetID, PackageName).

View File

@ -2207,6 +2207,33 @@ def test_reuse_python_from_cli_and_extension_from_db(self, mutable_database):
assert with_reuse.dag_hash() == without_reuse.dag_hash() assert with_reuse.dag_hash() == without_reuse.dag_hash()
@pytest.mark.regression("35536")
@pytest.mark.parametrize(
"spec_str,expected_namespaces",
[
# Single node with fully qualified namespace
("builtin.mock.gmake", {"gmake": "builtin.mock"}),
# Dependency with fully qualified namespace
("hdf5 ^builtin.mock.gmake", {"gmake": "builtin.mock", "hdf5": "duplicates.test"}),
("hdf5 ^gmake", {"gmake": "duplicates.test", "hdf5": "duplicates.test"}),
],
)
@pytest.mark.only_clingo("Uses specs requiring multiple gmake specs")
def test_select_lower_priority_package_from_repository_stack(
self, spec_str, expected_namespaces
):
"""Tests that a user can explicitly select a lower priority, fully qualified dependency
from cli.
"""
# 'builtin.mock" and "duplicates.test" share a 'gmake' package
additional_repo = os.path.join(spack.paths.repos_path, "duplicates.test")
with spack.repo.use_repositories(additional_repo, override=False):
s = Spec(spec_str).concretized()
for name, namespace in expected_namespaces.items():
assert s[name].concrete
assert s[name].namespace == namespace
@pytest.fixture() @pytest.fixture()
def duplicates_test_repository(): def duplicates_test_repository():

View File

@ -13,6 +13,7 @@ class Gmake(Package):
url = "https://ftpmirror.gnu.org/make/make-4.4.tar.gz" url = "https://ftpmirror.gnu.org/make/make-4.4.tar.gz"
version("4.4", sha256="ce35865411f0490368a8fc383f29071de6690cbadc27704734978221f25e2bed") version("4.4", sha256="ce35865411f0490368a8fc383f29071de6690cbadc27704734978221f25e2bed")
version("3.0", sha256="ce35865411f0490368a8fc383f29071de6690cbadc27704734978221f25e2bed")
def do_stage(self): def do_stage(self):
mkdirp(self.stage.source_path) mkdirp(self.stage.source_path)