diff --git a/lib/spack/spack/cmd/repo.py b/lib/spack/spack/cmd/repo.py index 1420067d083..79f571d3ac9 100644 --- a/lib/spack/spack/cmd/repo.py +++ b/lib/spack/spack/cmd/repo.py @@ -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(): diff --git a/lib/spack/spack/error.py b/lib/spack/spack/error.py index 92eb5f951ab..57070dac081 100644 --- a/lib/spack/spack/error.py +++ b/lib/spack/spack/error.py @@ -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.""" diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 37a1301733a..14679a6eb4f 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -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}") diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index dd35bfe287d..3e6be884c71 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -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. diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py index e4a945f0bfe..d3797380a00 100644 --- a/lib/spack/spack/repo.py +++ b/lib/spack/spack/repo.py @@ -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): diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index ce0f0a48c58..3b3d65ed9c0 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -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}: diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 67bd7c57767..ebeab1c6e71 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -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: - 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 + if not deps or not self._dependencies or not other._dependencies: return True - # Handle first-order constraints directly + return self._intersects_dependencies(other) + + 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: - 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. - return False + # 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: + 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, provided in pkg.provided.items(): + if self.satisfies(when, deps=False): + if any(p.intersects(other) for p in provided): + return True - 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): - 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. diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index ee716069f65..f85ca1feabf 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -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]) diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index a4b046fb443..b9530e56e18 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -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)), diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 4f58be0b2c3..612be83703a 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -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 ( diff --git a/lib/spack/spack/version/git_ref_lookup.py b/lib/spack/spack/version/git_ref_lookup.py index 3e9542167c5..c1c034f6ad3 100644 --- a/lib/spack/spack/version/git_ref_lookup.py +++ b/lib/spack/spack/version/git_ref_lookup.py @@ -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