From 886bcbc2e5093ff40d4da434542778852cd663f4 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Mon, 10 Feb 2025 10:29:04 +0100 Subject: [PATCH] Fix spack.repo - spack.package_base circularity spack.repo now imports spack.package_base, but not the other way around. --- lib/spack/spack/builder.py | 4 +- lib/spack/spack/cmd/repo.py | 5 +- lib/spack/spack/cmd/style.py | 4 +- lib/spack/spack/directives_meta.py | 4 +- lib/spack/spack/error.py | 8 +++ lib/spack/spack/installer.py | 2 +- lib/spack/spack/package_base.py | 19 ++++--- lib/spack/spack/repo.py | 68 +++++------------------ lib/spack/spack/repo_utils.py | 36 ++++++++++++ lib/spack/spack/solver/asp.py | 2 +- lib/spack/spack/spec.py | 4 +- lib/spack/spack/version/git_ref_lookup.py | 3 +- 12 files changed, 84 insertions(+), 75 deletions(-) create mode 100644 lib/spack/spack/repo_utils.py diff --git a/lib/spack/spack/builder.py b/lib/spack/spack/builder.py index 43f9423038b..dd8f882303d 100644 --- a/lib/spack/spack/builder.py +++ b/lib/spack/spack/builder.py @@ -11,7 +11,7 @@ import spack.multimethod import spack.package_base import spack.phase_callbacks -import spack.repo +import spack.repo_utils import spack.spec import spack.util.environment @@ -59,7 +59,7 @@ def __call__(self, spec, prefix): def get_builder_class(pkg, name: str) -> Optional[Type["Builder"]]: """Return the builder class if a package module defines it.""" cls = getattr(pkg.module, name, None) - if cls and cls.__module__.startswith(spack.repo.ROOT_PYTHON_NAMESPACE): + if cls and cls.__module__.startswith(spack.repo_utils.ROOT_PYTHON_NAMESPACE): return cls return None 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/cmd/style.py b/lib/spack/spack/cmd/style.py index 82df6ad0f80..adf6af94a34 100644 --- a/lib/spack/spack/cmd/style.py +++ b/lib/spack/spack/cmd/style.py @@ -14,7 +14,7 @@ from llnl.util.filesystem import working_dir import spack.paths -import spack.repo +import spack.repo_utils import spack.util.git from spack.util.executable import Executable, which @@ -369,7 +369,7 @@ def run_black(black_cmd, file_list, args): def _module_part(root: str, expr: str): parts = expr.split(".") # spack.pkg is for repositories, don't try to resolve it here. - if ".".join(parts[:2]) == spack.repo.ROOT_PYTHON_NAMESPACE: + if ".".join(parts[:2]) == spack.repo_utils.ROOT_PYTHON_NAMESPACE: return None while parts: f1 = os.path.join(root, "lib", "spack", *parts) + ".py" diff --git a/lib/spack/spack/directives_meta.py b/lib/spack/spack/directives_meta.py index caa69bb6bf3..dfe33f22537 100644 --- a/lib/spack/spack/directives_meta.py +++ b/lib/spack/spack/directives_meta.py @@ -9,7 +9,7 @@ import llnl.util.lang import spack.error -import spack.repo +import spack.repo_utils import spack.spec #: Names of possible directives. This list is mostly populated using the @directive decorator. @@ -65,7 +65,7 @@ def __init__(cls: "DirectiveMeta", name: str, bases: tuple, attr_dict: dict): # The instance is being initialized: if it is a package we must ensure # that the directives are called to set it up. - if cls.__module__.startswith(spack.repo.ROOT_PYTHON_NAMESPACE): + if cls.__module__.startswith(spack.repo_utils.ROOT_PYTHON_NAMESPACE): # Ensure the presence of the dictionaries associated with the directives. # All dictionaries are defaultdicts that create lists for missing keys. for d in DirectiveMeta._directive_dict_names: 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 a4f7234e1f0..e600ec42d97 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -36,14 +36,14 @@ import spack.deptypes as dt import spack.directives_meta import spack.error -import spack.fetch_strategy as fs +import spack.fetch_strategy import spack.hooks import spack.mirrors.layout import spack.mirrors.mirror import spack.multimethod import spack.patch import spack.phase_callbacks -import spack.repo +import spack.repo_utils import spack.spec import spack.store import spack.url @@ -838,7 +838,7 @@ def module(cls): @classproperty def namespace(cls): """Spack namespace for the package, which identifies its repo.""" - return spack.repo.namespace_from_fullname(cls.__module__) + return spack.repo_utils.namespace_from_fullname(cls.__module__) @classproperty def fullname(cls): @@ -1098,7 +1098,7 @@ def _make_resource_stage(self, root_stage, resource): ) def _download_search(self): - dynamic_fetcher = fs.from_list_url(self) + dynamic_fetcher = spack.fetch_strategy.from_list_url(self) return [dynamic_fetcher] if dynamic_fetcher else [] def _make_root_stage(self, fetcher): @@ -1267,7 +1267,7 @@ def fetcher(self): if not self.spec.versions.concrete: raise ValueError("Cannot retrieve fetcher for package without concrete version.") if not self._fetcher: - self._fetcher = fs.for_package_version(self) + self._fetcher = spack.fetch_strategy.for_package_version(self) return self._fetcher @fetcher.setter @@ -1659,8 +1659,11 @@ def content_hash(self, content: Optional[bytes] = None) -> str: # TODO: resources if self.spec.versions.concrete: try: - source_id = fs.for_package_version(self).source_id() - except (fs.ExtrapolationError, fs.InvalidArgsError): + source_id = spack.fetch_strategy.for_package_version(self).source_id() + except ( + spack.fetch_strategy.ExtrapolationError, + spack.fetch_strategy.InvalidArgsError, + ): # ExtrapolationError happens if the package has no fetchers defined. # InvalidArgsError happens when there are version directives with args, # but none of them identifies an actual fetcher. @@ -2010,7 +2013,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..f11e277b6c3 100644 --- a/lib/spack/spack/repo.py +++ b/lib/spack/spack/repo.py @@ -35,8 +35,10 @@ import spack.caches import spack.config import spack.error +import spack.package_base import spack.patch import spack.provider_index +import spack.repo_utils import spack.spec import spack.tag import spack.tengine @@ -46,39 +48,6 @@ import spack.util.path import spack.util.spack_yaml as syaml -#: Package modules are imported as spack.pkg.. -ROOT_PYTHON_NAMESPACE = "spack.pkg" - - -def python_package_for_repo(namespace): - """Returns the full namespace of a repository, given its relative one - - For instance: - - python_package_for_repo('builtin') == 'spack.pkg.builtin' - - Args: - namespace (str): repo namespace - """ - return "{0}.{1}".format(ROOT_PYTHON_NAMESPACE, namespace) - - -def namespace_from_fullname(fullname): - """Return the repository namespace only for the full module name. - - For instance: - - namespace_from_fullname('spack.pkg.builtin.hdf5') == 'builtin' - - Args: - fullname (str): full name for the Python module - """ - namespace, dot, module = fullname.rpartition(".") - prefix_and_dot = "{0}.".format(ROOT_PYTHON_NAMESPACE) - if namespace.startswith(prefix_and_dot): - namespace = namespace[len(prefix_and_dot) :] - return namespace - class SpackNamespaceLoader: def create_module(self, spec): @@ -124,7 +93,7 @@ def find_spec(self, fullname, python_path, target=None): raise RuntimeError('cannot reload module "{0}"'.format(fullname)) # Preferred API from https://peps.python.org/pep-0451/ - if not fullname.startswith(ROOT_PYTHON_NAMESPACE): + if not fullname.startswith(spack.repo_utils.ROOT_PYTHON_NAMESPACE): return None loader = self.compute_loader(fullname) @@ -303,7 +272,6 @@ def is_package_file(filename): # Package files are named `package.py` and are not in lib/spack/spack # We have to remove the file extension because it can be .py and can be # .pyc depending on context, and can differ between the files - import spack.package_base # break cycle filename_noext = os.path.splitext(filename)[0] packagebase_filename_noext = os.path.splitext(inspect.getfile(spack.package_base.PackageBase))[ @@ -663,7 +631,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, @@ -705,7 +673,7 @@ def remove(self, repo): def get_repo(self, namespace: str) -> "Repo": """Get a repository by namespace.""" - full_namespace = python_package_for_repo(namespace) + full_namespace = spack.repo_utils.python_package_for_repo(namespace) if full_namespace not in self.by_namespace: raise UnknownNamespaceError(namespace) return self.by_namespace[full_namespace] @@ -816,7 +784,7 @@ def repo_for_pkg(self, spec: Union[str, "spack.spec.Spec"]) -> "Repo": # If the spec already has a namespace, then return the # corresponding repo if we know about it. if namespace: - fullspace = python_package_for_repo(namespace) + fullspace = spack.repo_utils.python_package_for_repo(namespace) if fullspace not in self.by_namespace: raise UnknownNamespaceError(namespace, name=name) return self.by_namespace[fullspace] @@ -966,7 +934,7 @@ def check(condition, msg): ) # Set up 'full_namespace' to include the super-namespace - self.full_namespace = python_package_for_repo(self.namespace) + self.full_namespace = spack.repo_utils.python_package_for_repo(self.namespace) # Keep name components around for checking prefixes. self._names = self.full_namespace.split(".") @@ -1241,7 +1209,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 +1469,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 +1518,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 +1528,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/repo_utils.py b/lib/spack/spack/repo_utils.py new file mode 100644 index 00000000000..cc24bac5c50 --- /dev/null +++ b/lib/spack/spack/repo_utils.py @@ -0,0 +1,36 @@ +# Copyright Spack Project Developers. See COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +#: Package modules are imported as spack.pkg.. +ROOT_PYTHON_NAMESPACE = "spack.pkg" + + +def namespace_from_fullname(fullname): + """Return the repository namespace only for the full module name. + + For instance: + + namespace_from_fullname('spack.pkg.builtin.hdf5') == 'builtin' + + Args: + fullname (str): full name for the Python module + """ + namespace, dot, module = fullname.rpartition(".") + prefix_and_dot = "{0}.".format(ROOT_PYTHON_NAMESPACE) + if namespace.startswith(prefix_and_dot): + namespace = namespace[len(prefix_and_dot) :] + return namespace + + +def python_package_for_repo(namespace): + """Returns the full namespace of a repository, given its relative one + + For instance: + + python_package_for_repo('builtin') == 'spack.pkg.builtin' + + Args: + namespace (str): repo namespace + """ + return "{0}.{1}".format(ROOT_PYTHON_NAMESPACE, namespace) diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 564a610ae9f..1df80dbc1cf 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -3814,7 +3814,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..25065cae0ac 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3342,7 +3342,7 @@ def intersects(self, other: Union[str, "Spec"], deps: bool = True) -> bool: # 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: + except spack.error.UnknownEntityError: # If we can't get package info on this spec, don't treat # it as a provider of this vdep. return False @@ -3447,7 +3447,7 @@ def satisfies(self, other: Union[str, "Spec"], deps: bool = True) -> bool: # 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: + except spack.error.UnknownEntityError: # If we can't get package info on this spec, don't treat # it as a provider of this vdep. return False 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