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
parent 2489b137d9
commit 9af5eca9ec
5 changed files with 83 additions and 26 deletions

View file

@ -24,8 +24,9 @@
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, Tuple, Type, TypeVar from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Type, TypeVar, Union
import llnl.util.filesystem as fsys import llnl.util.filesystem as fsys
import llnl.util.tty as tty import llnl.util.tty as tty
@ -682,13 +683,13 @@ def __init__(self, spec):
@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:
@ -2449,14 +2450,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
@ -2469,9 +2477,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

@ -12,6 +12,7 @@
import pprint import pprint
import re import re
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
@ -379,7 +380,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
@ -1132,6 +1133,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.
@ -1144,7 +1146,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)
@ -1336,7 +1340,10 @@ def _rule_from_str(
) )
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)
@ -1973,7 +1980,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:
@ -2092,7 +2099,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
@ -2554,8 +2561,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()
@ -2568,6 +2573,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
@ -2777,6 +2786,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 SpecBuilder: class SpecBuilder:
"""Class with actions to rebuild a spec from ASP results.""" """Class with actions to rebuild a spec from ASP results."""
@ -2836,6 +2852,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
@ -3050,14 +3069,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

@ -2184,6 +2184,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)