Spec.satisfies, Spec.intersects: drop spack.repo
Currently Spec.satisfies is stateful both for concrete and abstract
specs, even after our changes where concrete specs list virtuals as edge
attributes.
Virtuals on edges basically provide too little information:
1. They don't specify what version of the virtual is provided, so you
cannot know the return value of `s.satisfies("^mpi@3:")` without
looking in the current state of the package.
2. It's not all about edges: `mpich.satisfies("mpi")` is expected to be
true (difference is *can* provide versus *provides*).
This commit tries to reduce the statefulness by only letting satisfies
and intersects access `spec.package`, instead of looking up the package.
The idea is that it reduces the surface area with `spack.repo`, as in
principle `spec.package` can be set upon construction.
The commit is breaking for abstract specs. The idea is you cannot answer
queries about abstract specs unless you associate a repo.
This commit is contained in:
Harmen Stoppels
parent
01e16b58a3
commit
ae3bcf9937
@ -8,6 +8,7 @@
|
||||
import llnl.util.tty as tty
|
||||
|
||||
import spack.config
|
||||
import spack.error
|
||||
import spack.repo
|
||||
import spack.util.path
|
||||
from spack.cmd.common import arguments
|
||||
@ -129,7 +130,7 @@ def repo_remove(args):
|
||||
spack.config.set("repos", repos, args.scope)
|
||||
tty.msg("Removed repository %s with namespace '%s'." % (repo.root, repo.namespace))
|
||||
return
|
||||
except spack.repo.RepoError:
|
||||
except spack.error.RepoError:
|
||||
continue
|
||||
|
||||
tty.die("No repository with path or namespace: %s" % namespace_or_path)
|
||||
@ -142,7 +143,7 @@ def repo_list(args):
|
||||
for r in roots:
|
||||
try:
|
||||
repos.append(spack.repo.from_path(r))
|
||||
except spack.repo.RepoError:
|
||||
except spack.error.RepoError:
|
||||
continue
|
||||
|
||||
if sys.stdout.isatty():
|
||||
|
||||
@ -202,3 +202,11 @@ class MirrorError(SpackError):
|
||||
|
||||
def __init__(self, msg, long_msg=None):
|
||||
super().__init__(msg, long_msg)
|
||||
|
||||
|
||||
class RepoError(SpackError):
|
||||
"""Superclass for repository-related errors."""
|
||||
|
||||
|
||||
class UnknownEntityError(RepoError):
|
||||
"""Raised when we encounter a package spack doesn't have."""
|
||||
|
||||
@ -560,7 +560,7 @@ def dump_packages(spec: "spack.spec.Spec", path: str) -> None:
|
||||
try:
|
||||
source_repo = spack.repo.from_path(source_repo_root)
|
||||
source_pkg_dir = source_repo.dirname_for_package_name(node.name)
|
||||
except spack.repo.RepoError as err:
|
||||
except spack.error.RepoError as err:
|
||||
tty.debug(f"Failed to create source repo for {node.name}: {str(err)}")
|
||||
source_pkg_dir = None
|
||||
tty.warn(f"Warning: Couldn't copy in provenance for {node.name}")
|
||||
|
||||
@ -2109,7 +2109,7 @@ def uninstall_by_spec(spec, force=False, deprecator=None):
|
||||
# Try to get the package for the spec
|
||||
try:
|
||||
pkg = spec.package
|
||||
except spack.repo.UnknownEntityError:
|
||||
except spack.error.UnknownEntityError:
|
||||
pkg = None
|
||||
|
||||
# Pre-uninstall hook runs first.
|
||||
|
||||
@ -663,7 +663,7 @@ def __init__(
|
||||
repo = Repo(repo, cache=cache, overrides=overrides)
|
||||
repo.finder(self)
|
||||
self.put_last(repo)
|
||||
except RepoError as e:
|
||||
except spack.error.RepoError as e:
|
||||
tty.warn(
|
||||
f"Failed to initialize repository: '{repo}'.",
|
||||
e.message,
|
||||
@ -1241,7 +1241,7 @@ def get_pkg_class(self, pkg_name: str) -> Type["spack.package_base.PackageBase"]
|
||||
raise UnknownPackageError(fullname)
|
||||
except Exception as e:
|
||||
msg = f"cannot load package '{pkg_name}' from the '{self.namespace}' repository: {e}"
|
||||
raise RepoError(msg) from e
|
||||
raise spack.error.RepoError(msg) from e
|
||||
|
||||
cls = getattr(module, class_name)
|
||||
if not isinstance(cls, type):
|
||||
@ -1501,27 +1501,19 @@ def recipe_filename(self, name):
|
||||
return os.path.join(self.root, "packages", name, "package.py")
|
||||
|
||||
|
||||
class RepoError(spack.error.SpackError):
|
||||
"""Superclass for repository-related errors."""
|
||||
|
||||
|
||||
class NoRepoConfiguredError(RepoError):
|
||||
class NoRepoConfiguredError(spack.error.RepoError):
|
||||
"""Raised when there are no repositories configured."""
|
||||
|
||||
|
||||
class InvalidNamespaceError(RepoError):
|
||||
class InvalidNamespaceError(spack.error.RepoError):
|
||||
"""Raised when an invalid namespace is encountered."""
|
||||
|
||||
|
||||
class BadRepoError(RepoError):
|
||||
class BadRepoError(spack.error.RepoError):
|
||||
"""Raised when repo layout is invalid."""
|
||||
|
||||
|
||||
class UnknownEntityError(RepoError):
|
||||
"""Raised when we encounter a package spack doesn't have."""
|
||||
|
||||
|
||||
class UnknownPackageError(UnknownEntityError):
|
||||
class UnknownPackageError(spack.error.UnknownEntityError):
|
||||
"""Raised when we encounter a package spack doesn't have."""
|
||||
|
||||
def __init__(self, name, repo=None):
|
||||
@ -1558,7 +1550,7 @@ def __init__(self, name, repo=None):
|
||||
self.name = name
|
||||
|
||||
|
||||
class UnknownNamespaceError(UnknownEntityError):
|
||||
class UnknownNamespaceError(spack.error.UnknownEntityError):
|
||||
"""Raised when we encounter an unknown namespace"""
|
||||
|
||||
def __init__(self, namespace, name=None):
|
||||
@ -1568,7 +1560,7 @@ def __init__(self, namespace, name=None):
|
||||
super().__init__(msg, long_msg)
|
||||
|
||||
|
||||
class FailedConstructorError(RepoError):
|
||||
class FailedConstructorError(spack.error.RepoError):
|
||||
"""Raised when a package's class constructor fails."""
|
||||
|
||||
def __init__(self, name, exc_type, exc_obj, exc_tb):
|
||||
|
||||
@ -3831,7 +3831,7 @@ def _is_reusable(spec: spack.spec.Spec, packages, local: bool) -> bool:
|
||||
|
||||
try:
|
||||
provided = spack.repo.PATH.get(spec).provided_virtual_names()
|
||||
except spack.repo.RepoError:
|
||||
except spack.error.RepoError:
|
||||
provided = []
|
||||
|
||||
for name in {spec.name, *provided}:
|
||||
|
||||
@ -91,7 +91,6 @@
|
||||
import spack.hash_types as ht
|
||||
import spack.paths
|
||||
import spack.platforms
|
||||
import spack.provider_index
|
||||
import spack.repo
|
||||
import spack.spec_parser
|
||||
import spack.store
|
||||
@ -1893,9 +1892,7 @@ def root(self):
|
||||
|
||||
@property
|
||||
def package(self):
|
||||
assert self.concrete, "{0}: Spec.package can only be called on concrete specs".format(
|
||||
self.name
|
||||
)
|
||||
assert self.concrete, f"{self.name}: Spec.package can only be called on concrete specs"
|
||||
if not self._package:
|
||||
self._package = spack.repo.PATH.get(self)
|
||||
return self._package
|
||||
@ -3267,7 +3264,7 @@ def _constrain_dependencies(self, other):
|
||||
|
||||
def common_dependencies(self, other):
|
||||
"""Return names of dependencies that self an other have in common."""
|
||||
common = set(s.name for s in self.traverse(root=False))
|
||||
common = {s.name for s in self.traverse(root=False)}
|
||||
common.intersection_update(s.name for s in other.traverse(root=False))
|
||||
return common
|
||||
|
||||
@ -3315,7 +3312,7 @@ def intersects(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
|
||||
elif other.concrete:
|
||||
return other.satisfies(self)
|
||||
|
||||
# From here we know both self and other are not concrete
|
||||
# From here we know both self and other are abstract
|
||||
self_hash = self.abstract_hash
|
||||
other_hash = other.abstract_hash
|
||||
|
||||
@ -3326,32 +3323,8 @@ def intersects(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
|
||||
):
|
||||
return False
|
||||
|
||||
# If the names are different, we need to consider virtuals
|
||||
# We do not consider virtuals for two abstract specs, since no package is associated
|
||||
if self.name != other.name and self.name and other.name:
|
||||
if self.virtual and other.virtual:
|
||||
# Two virtual specs intersect only if there are providers for both
|
||||
lhs = spack.repo.PATH.providers_for(str(self))
|
||||
rhs = spack.repo.PATH.providers_for(str(other))
|
||||
intersection = [s for s in lhs if any(s.intersects(z) for z in rhs)]
|
||||
return bool(intersection)
|
||||
|
||||
# A provider can satisfy a virtual dependency.
|
||||
elif self.virtual or other.virtual:
|
||||
virtual_spec, non_virtual_spec = (self, other) if self.virtual else (other, self)
|
||||
try:
|
||||
# Here we might get an abstract spec
|
||||
pkg_cls = spack.repo.PATH.get_pkg_class(non_virtual_spec.fullname)
|
||||
pkg = pkg_cls(non_virtual_spec)
|
||||
except spack.repo.UnknownEntityError:
|
||||
# If we can't get package info on this spec, don't treat
|
||||
# it as a provider of this vdep.
|
||||
return False
|
||||
|
||||
if pkg.provides(virtual_spec.name):
|
||||
for when_spec, provided in pkg.provided.items():
|
||||
if non_virtual_spec.intersects(when_spec, deps=False):
|
||||
if any(vpkg.intersects(virtual_spec) for vpkg in provided):
|
||||
return True
|
||||
return False
|
||||
|
||||
# namespaces either match, or other doesn't require one.
|
||||
@ -3381,40 +3354,16 @@ def intersects(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
|
||||
return False
|
||||
|
||||
# If we need to descend into dependencies, do it, otherwise we're done.
|
||||
if deps:
|
||||
if not deps or not self._dependencies or not other._dependencies:
|
||||
return True
|
||||
|
||||
return self._intersects_dependencies(other)
|
||||
|
||||
return True
|
||||
|
||||
def _intersects_dependencies(self, other):
|
||||
if not other._dependencies or not self._dependencies:
|
||||
# one spec *could* eventually satisfy the other
|
||||
return True
|
||||
|
||||
# Handle first-order constraints directly
|
||||
def _intersects_dependencies(self, other: "Spec") -> bool:
|
||||
for name in self.common_dependencies(other):
|
||||
if not self[name].intersects(other[name], deps=False):
|
||||
return False
|
||||
|
||||
# For virtual dependencies, we need to dig a little deeper.
|
||||
self_index = spack.provider_index.ProviderIndex(
|
||||
repository=spack.repo.PATH, specs=self.traverse(), restrict=True
|
||||
)
|
||||
other_index = spack.provider_index.ProviderIndex(
|
||||
repository=spack.repo.PATH, specs=other.traverse(), restrict=True
|
||||
)
|
||||
|
||||
# These two loops handle cases where there is an overly restrictive
|
||||
# vpkg in one spec for a provider in the other (e.g., mpi@3: is not
|
||||
# compatible with mpich2)
|
||||
for spec in self.virtual_dependencies():
|
||||
if spec.name in other_index and not other_index.providers_for(spec):
|
||||
return False
|
||||
|
||||
for spec in other.virtual_dependencies():
|
||||
if spec.name in self_index and not self_index.providers_for(spec):
|
||||
return False
|
||||
|
||||
return True
|
||||
|
||||
def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
|
||||
@ -3439,24 +3388,22 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
|
||||
if not compare_hash or not compare_hash.startswith(other.abstract_hash):
|
||||
return False
|
||||
|
||||
# If the names are different, we need to consider virtuals
|
||||
if self.name != other.name and self.name and other.name:
|
||||
# A concrete provider can satisfy a virtual dependency.
|
||||
if not self.virtual and other.virtual:
|
||||
# If the names are different, we need to consider virtuals. We return false for two
|
||||
# abstract specs with different names because no package is associated with them.
|
||||
if other.name and self.name != other.name:
|
||||
try:
|
||||
# Here we might get an abstract spec
|
||||
pkg_cls = spack.repo.PATH.get_pkg_class(self.fullname)
|
||||
pkg = pkg_cls(self)
|
||||
except spack.repo.UnknownEntityError:
|
||||
# If we can't get package info on this spec, don't treat
|
||||
# it as a provider of this vdep.
|
||||
pkg = self.package
|
||||
except (AssertionError, spack.error.RepoError):
|
||||
# If we can't get package info on this spec, don't treat it as a provider of this
|
||||
# vdep.
|
||||
return False
|
||||
|
||||
if pkg.provides(other.name):
|
||||
for when_spec, provided in pkg.provided.items():
|
||||
if self.satisfies(when_spec, deps=False):
|
||||
if any(vpkg.intersects(other) for vpkg in provided):
|
||||
for when, provided in pkg.provided.items():
|
||||
if self.satisfies(when, deps=False):
|
||||
if any(p.intersects(other) for p in provided):
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
# namespaces either match, or other doesn't require one.
|
||||
@ -3509,10 +3456,6 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
|
||||
# will be verified later.
|
||||
lhs_edges: Dict[str, Set[DependencySpec]] = collections.defaultdict(set)
|
||||
for rhs_edge in other.traverse_edges(root=False, cover="edges"):
|
||||
# If we are checking for ^mpi we need to verify if there is any edge
|
||||
if rhs_edge.spec.virtual:
|
||||
rhs_edge.update_virtuals(virtuals=(rhs_edge.spec.name,))
|
||||
|
||||
if not rhs_edge.virtuals:
|
||||
continue
|
||||
|
||||
@ -3554,10 +3497,6 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool:
|
||||
for rhs in other.traverse(root=False)
|
||||
)
|
||||
|
||||
def virtual_dependencies(self):
|
||||
"""Return list of any virtual deps in this spec."""
|
||||
return [spec for spec in self.traverse() if spec.virtual]
|
||||
|
||||
@property # type: ignore[misc] # decorated prop not supported in mypy
|
||||
def patches(self):
|
||||
"""Return patch objects for any patch sha256 sums on this Spec.
|
||||
|
||||
@ -457,7 +457,7 @@ def bpp_path(spec):
|
||||
|
||||
def _repoerr(repo, name):
|
||||
if name == "cmake":
|
||||
raise spack.repo.RepoError(repo_err_msg)
|
||||
raise spack.error.RepoError(repo_err_msg)
|
||||
else:
|
||||
return orig_dirname(repo, name)
|
||||
|
||||
@ -476,7 +476,7 @@ def _repoerr(repo, name):
|
||||
# Now try the error path, which requires the mock directory structure
|
||||
# above
|
||||
monkeypatch.setattr(spack.repo.Repo, "dirname_for_package_name", _repoerr)
|
||||
with pytest.raises(spack.repo.RepoError, match=repo_err_msg):
|
||||
with pytest.raises(spack.error.RepoError, match=repo_err_msg):
|
||||
inst.dump_packages(spec, path)
|
||||
|
||||
out = str(capsys.readouterr()[1])
|
||||
|
||||
@ -472,9 +472,6 @@ def test_concrete_specs_which_satisfies_abstract(self, lhs, rhs, default_mock_co
|
||||
("mpileaks^mpich@2.0^callpath@1.7", "^mpich@1:3^callpath@1.4:1.6"),
|
||||
("mpileaks^mpich@4.0^callpath@1.7", "^mpich@1:3^callpath@1.4:1.6"),
|
||||
("mpileaks^mpi@3", "^mpi@1.2:1.6"),
|
||||
("mpileaks^mpi@3:", "^mpich2@1.4"),
|
||||
("mpileaks^mpi@3:", "^mpich2"),
|
||||
("mpileaks^mpi@3:", "^mpich@1.0"),
|
||||
("mpich~foo", "mpich+foo"),
|
||||
("mpich+foo", "mpich~foo"),
|
||||
("mpich foo=True", "mpich foo=False"),
|
||||
@ -705,9 +702,9 @@ def test_copy_satisfies_transitive(self):
|
||||
assert copy[s.name].satisfies(s)
|
||||
|
||||
def test_intersects_virtual(self):
|
||||
assert Spec("mpich").intersects(Spec("mpi"))
|
||||
assert Spec("mpich2").intersects(Spec("mpi"))
|
||||
assert Spec("zmpi").intersects(Spec("mpi"))
|
||||
assert spack.concretize.concretize_one("mpich").intersects("mpi")
|
||||
assert spack.concretize.concretize_one("mpich2").intersects("mpi")
|
||||
assert spack.concretize.concretize_one("zmpi").intersects("mpi")
|
||||
|
||||
def test_intersects_virtual_providers(self):
|
||||
"""Tests that we can always intersect virtual providers from abstract specs.
|
||||
@ -1829,9 +1826,6 @@ def test_abstract_contains_semantic(lhs, rhs, expected, mock_packages):
|
||||
(Spec, "cppflags=-foo", "cflags=-foo", (True, False, False)),
|
||||
# Versions
|
||||
(Spec, "@0.94h", "@:0.94i", (True, True, False)),
|
||||
# Different virtuals intersect if there is at least package providing both
|
||||
(Spec, "mpi", "lapack", (True, False, False)),
|
||||
(Spec, "mpi", "pkgconfig", (False, False, False)),
|
||||
# Intersection among target ranges for different architectures
|
||||
(Spec, "target=x86_64:", "target=ppc64le:", (False, False, False)),
|
||||
(Spec, "target=x86_64:", "target=:power9", (False, False, False)),
|
||||
|
||||
@ -11,6 +11,7 @@
|
||||
import spack.binary_distribution
|
||||
import spack.cmd
|
||||
import spack.concretize
|
||||
import spack.error
|
||||
import spack.platforms.test
|
||||
import spack.repo
|
||||
import spack.spec
|
||||
@ -1139,7 +1140,7 @@ def test_parse_filename_missing_slash_as_spec(specfile_for, tmpdir, filename):
|
||||
|
||||
# Check that if we concretize this spec, we get a good error
|
||||
# message that mentions we might've meant a file.
|
||||
with pytest.raises(spack.repo.UnknownEntityError) as exc_info:
|
||||
with pytest.raises(spack.error.UnknownEntityError) as exc_info:
|
||||
spack.concretize.concretize_one(spec)
|
||||
assert exc_info.value.long_message
|
||||
assert (
|
||||
|
||||
@ -10,6 +10,7 @@
|
||||
from llnl.util.filesystem import mkdirp, working_dir
|
||||
|
||||
import spack.caches
|
||||
import spack.error
|
||||
import spack.fetch_strategy
|
||||
import spack.paths
|
||||
import spack.repo
|
||||
@ -78,7 +79,7 @@ def pkg(self):
|
||||
try:
|
||||
pkg = spack.repo.PATH.get_pkg_class(self.pkg_name)
|
||||
pkg.git
|
||||
except (spack.repo.RepoError, AttributeError) as e:
|
||||
except (spack.error.RepoError, AttributeError) as e:
|
||||
raise VersionLookupError(f"Couldn't get the git repo for {self.pkg_name}") from e
|
||||
self._pkg = pkg
|
||||
return self._pkg
|
||||
|
||||
Loading…
Reference in New Issue
Block a user