diff options
author | Massimiliano Culpo <massimiliano.culpo@gmail.com> | 2024-07-05 12:00:41 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-07-05 12:00:41 +0200 |
commit | 95cf341b5064608650ccdd67f5fed35ba6c56a7d (patch) | |
tree | 582e6e97349d9e00f474224a7cc062ee3bb5a3da /lib | |
parent | a134485b1b9113b02397151be88de0f818281ecc (diff) | |
download | spack-95cf341b5064608650ccdd67f5fed35ba6c56a7d.tar.gz spack-95cf341b5064608650ccdd67f5fed35ba6c56a7d.tar.bz2 spack-95cf341b5064608650ccdd67f5fed35ba6c56a7d.tar.xz spack-95cf341b5064608650ccdd67f5fed35ba6c56a7d.zip |
Inject dependencies in repo classes (#45053)
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/caches.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/cmd/create.py | 3 | ||||
-rw-r--r-- | lib/spack/spack/cmd/edit.py | 2 | ||||
-rw-r--r-- | lib/spack/spack/cmd/repo.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/environment/environment.py | 5 | ||||
-rw-r--r-- | lib/spack/spack/installer.py | 4 | ||||
-rw-r--r-- | lib/spack/spack/package_base.py | 9 | ||||
-rw-r--r-- | lib/spack/spack/repo.py | 231 | ||||
-rw-r--r-- | lib/spack/spack/test/cmd/pkg.py | 14 | ||||
-rw-r--r-- | lib/spack/spack/test/cmd/style.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/test/concretize.py | 26 | ||||
-rw-r--r-- | lib/spack/spack/test/concretize_preferences.py | 52 | ||||
-rw-r--r-- | lib/spack/spack/test/conftest.py | 13 | ||||
-rw-r--r-- | lib/spack/spack/test/directory_layout.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/test/packages.py | 4 | ||||
-rw-r--r-- | lib/spack/spack/test/repo.py | 110 |
16 files changed, 317 insertions, 176 deletions
diff --git a/lib/spack/spack/caches.py b/lib/spack/spack/caches.py index 19c736748e..14d03a14b3 100644 --- a/lib/spack/spack/caches.py +++ b/lib/spack/spack/caches.py @@ -34,6 +34,8 @@ def _misc_cache(): return spack.util.file_cache.FileCache(path) +FileCacheType = Union[spack.util.file_cache.FileCache, llnl.util.lang.Singleton] + #: Spack's cache for small data MISC_CACHE: Union[spack.util.file_cache.FileCache, llnl.util.lang.Singleton] = ( llnl.util.lang.Singleton(_misc_cache) diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py index 49f4ae2177..0481b9d044 100644 --- a/lib/spack/spack/cmd/create.py +++ b/lib/spack/spack/cmd/create.py @@ -2,7 +2,6 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - import os import re import sys @@ -934,7 +933,7 @@ def get_repository(args, name): # Figure out where the new package should live repo_path = args.repo if repo_path is not None: - repo = spack.repo.Repo(repo_path) + repo = spack.repo.from_path(repo_path) if spec.namespace and spec.namespace != repo.namespace: tty.die( "Can't create package with namespace {0} in repo with " diff --git a/lib/spack/spack/cmd/edit.py b/lib/spack/spack/cmd/edit.py index e7d64c77ea..9951bb65a3 100644 --- a/lib/spack/spack/cmd/edit.py +++ b/lib/spack/spack/cmd/edit.py @@ -123,7 +123,7 @@ def edit(parser, args): spack.util.editor.editor(*paths) elif names: if args.repo: - repo = spack.repo.Repo(args.repo) + repo = spack.repo.from_path(args.repo) elif args.namespace: repo = spack.repo.PATH.get_repo(args.namespace) else: diff --git a/lib/spack/spack/cmd/repo.py b/lib/spack/spack/cmd/repo.py index 68af91cd91..e41e21f0a5 100644 --- a/lib/spack/spack/cmd/repo.py +++ b/lib/spack/spack/cmd/repo.py @@ -91,7 +91,7 @@ def repo_add(args): tty.die("Not a Spack repository: %s" % path) # Make sure it's actually a spack repository by constructing it. - repo = spack.repo.Repo(canon_path) + repo = spack.repo.from_path(canon_path) # If that succeeds, finally add it to the configuration. repos = spack.config.get("repos", scope=args.scope) @@ -124,7 +124,7 @@ def repo_remove(args): # If it is a namespace, remove corresponding repo for path in repos: try: - repo = spack.repo.Repo(path) + repo = spack.repo.from_path(path) if repo.namespace == namespace_or_path: repos.remove(path) spack.config.set("repos", repos, args.scope) @@ -142,7 +142,7 @@ def repo_list(args): repos = [] for r in roots: try: - repos.append(spack.repo.Repo(r)) + repos.append(spack.repo.from_path(r)) except spack.repo.RepoError: continue diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 32aff3e238..7643579f16 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -24,6 +24,7 @@ import llnl.util.tty.color as clr from llnl.util.link_tree import ConflictingSpecsError from llnl.util.symlink import readlink, symlink +import spack.caches import spack.cmd import spack.compilers import spack.concretize @@ -2542,7 +2543,7 @@ def _concretize_task(packed_arguments) -> Tuple[int, Spec, float]: def make_repo_path(root): """Make a RepoPath from the repo subdirectories in an environment.""" - path = spack.repo.RepoPath() + path = spack.repo.RepoPath(cache=spack.caches.MISC_CACHE) if os.path.isdir(root): for repo_root in os.listdir(root): @@ -2551,7 +2552,7 @@ def make_repo_path(root): if not os.path.isdir(repo_root): continue - repo = spack.repo.Repo(repo_root) + repo = spack.repo.from_path(repo_root) path.put_last(repo) return path diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 65c3e78553..dc94c95926 100644 --- a/lib/spack/spack/installer.py +++ b/lib/spack/spack/installer.py @@ -582,7 +582,7 @@ def dump_packages(spec: "spack.spec.Spec", path: str) -> None: # Create a source repo and get the pkg directory out of it. try: - source_repo = spack.repo.Repo(source_repo_root) + 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: tty.debug(f"Failed to create source repo for {node.name}: {str(err)}") @@ -593,7 +593,7 @@ def dump_packages(spec: "spack.spec.Spec", path: str) -> None: dest_repo_root = os.path.join(path, node.namespace) if not os.path.exists(dest_repo_root): spack.repo.create_repo(dest_repo_root) - repo = spack.repo.Repo(dest_repo_root) + repo = spack.repo.from_path(dest_repo_root) # Get the location of the package in the dest repo. dest_pkg_dir = repo.dirname_for_package_name(node.name) diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index 30aeb0bd7c..50ea97a9d4 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -748,11 +748,6 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass self._fetch_time = 0.0 self.win_rpath = fsys.WindowsSimulatedRPath(self) - - if self.is_extension: - pkg_cls = spack.repo.PATH.get_pkg_class(self.extendee_spec.name) - pkg_cls(self.extendee_spec)._check_extendable() - super().__init__() @classmethod @@ -2388,10 +2383,6 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass PackageBase.uninstall_by_spec(spec, force=True, deprecator=deprecator) link_fn(deprecator.prefix, spec.prefix) - def _check_extendable(self): - if not self.extendable: - raise ValueError("Package %s is not extendable!" % self.name) - def view(self): """Create a view with the prefix of this package as the root. Extensions added to this view will modify the installation prefix of diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py index 743f5a18e8..f3394a118c 100644 --- a/lib/spack/spack/repo.py +++ b/lib/spack/spack/repo.py @@ -25,7 +25,8 @@ import sys import traceback import types import uuid -from typing import Any, Dict, List, Set, Tuple, Union +import warnings +from typing import Any, Dict, Generator, List, Optional, Set, Tuple, Type, Union import llnl.path import llnl.util.filesystem as fs @@ -126,11 +127,35 @@ class SpackNamespaceLoader: class ReposFinder: - """MetaPathFinder class that loads a Python module corresponding to a Spack package + """MetaPathFinder class that loads a Python module corresponding to a Spack package. - Return a loader based on the inspection of the current global repository list. + Returns a loader based on the inspection of the current repository list. """ + def __init__(self): + self._repo_init = _path + self._repo = None + + @property + def current_repository(self): + if self._repo is None: + self._repo = self._repo_init() + return self._repo + + @current_repository.setter + def current_repository(self, value): + self._repo = value + + @contextlib.contextmanager + def switch_repo(self, substitute: "RepoType"): + """Switch the current repository list for the duration of the context manager.""" + old = self.current_repository + try: + self.current_repository = substitute + yield + finally: + self.current_repository = old + def find_spec(self, fullname, python_path, target=None): # "target" is not None only when calling importlib.reload() if target is not None: @@ -149,9 +174,14 @@ class ReposFinder: # namespaces are added to repo, and package modules are leaves. namespace, dot, module_name = fullname.rpartition(".") - # If it's a module in some repo, or if it is the repo's - # namespace, let the repo handle it. - for repo in PATH.repos: + # If it's a module in some repo, or if it is the repo's namespace, let the repo handle it. + is_repo_path = isinstance(self.current_repository, RepoPath) + if is_repo_path: + repos = self.current_repository.repos + else: + repos = [self.current_repository] + + for repo in repos: # We are using the namespace of the repo and the repo contains the package if namespace == repo.full_namespace: # With 2 nested conditionals we can call "repo.real_name" only once @@ -165,7 +195,7 @@ class ReposFinder: # No repo provides the namespace, but it is a valid prefix of # something in the RepoPath. - if PATH.by_namespace.is_prefix(fullname): + if is_repo_path and self.current_repository.by_namespace.is_prefix(fullname): return SpackNamespaceLoader() return None @@ -560,7 +590,7 @@ class RepoIndex: self, package_checker: FastPackageChecker, namespace: str, - cache: spack.util.file_cache.FileCache, + cache: spack.caches.FileCacheType, ): self.checker = package_checker self.packages_path = self.checker.packages_path @@ -648,11 +678,9 @@ class RepoPath: repos (list): list Repo objects or paths to put in this RepoPath """ - def __init__(self, *repos, **kwargs): - cache = kwargs.get("cache", spack.caches.MISC_CACHE) + def __init__(self, *repos, cache, overrides=None): self.repos = [] self.by_namespace = nm.NamespaceTrie() - self._provider_index = None self._patch_index = None self._tag_index = None @@ -661,7 +689,8 @@ class RepoPath: for repo in repos: try: if isinstance(repo, str): - repo = Repo(repo, cache=cache) + repo = Repo(repo, cache=cache, overrides=overrides) + repo.finder(self) self.put_last(repo) except RepoError as e: tty.warn( @@ -915,18 +944,28 @@ class Repo: Each package repository must have a top-level configuration file called `repo.yaml`. - Currently, `repo.yaml` this must define: + Currently, `repo.yaml` must define: `namespace`: A Python namespace where the repository's packages should live. + `subdirectory`: + An optional subdirectory name where packages are placed """ - def __init__(self, root, cache=None): + def __init__( + self, + root: str, + *, + cache: spack.caches.FileCacheType, + overrides: Optional[Dict[str, Any]] = None, + ) -> None: """Instantiate a package repository from a filesystem path. Args: root: the root directory of the repository + cache: file cache associated with this repository + overrides: dict mapping package name to class attribute overrides for that package """ # Root directory, containing _repo.yaml and package dirs # Allow roots to by spack-relative by starting with '$spack' @@ -939,20 +978,20 @@ class Repo: # Validate repository layout. self.config_file = os.path.join(self.root, repo_config_name) - check(os.path.isfile(self.config_file), "No %s found in '%s'" % (repo_config_name, root)) + check(os.path.isfile(self.config_file), f"No {repo_config_name} found in '{root}'") # Read configuration and validate namespace config = self._read_config() check( "namespace" in config, - "%s must define a namespace." % os.path.join(root, repo_config_name), + f"{os.path.join(root, repo_config_name)} must define a namespace.", ) self.namespace = config["namespace"] check( re.match(r"[a-zA-Z][a-zA-Z0-9_.]+", self.namespace), - ("Invalid namespace '%s' in repo '%s'. " % (self.namespace, self.root)) - + "Namespaces must be valid python identifiers separated by '.'", + f"Invalid namespace '{self.namespace}' in repo '{self.root}'. " + "Namespaces must be valid python identifiers separated by '.'", ) # Set up 'full_namespace' to include the super-namespace @@ -964,23 +1003,26 @@ class Repo: packages_dir = config.get("subdirectory", packages_dir_name) self.packages_path = os.path.join(self.root, packages_dir) check( - os.path.isdir(self.packages_path), - "No directory '%s' found in '%s'" % (packages_dir, root), + os.path.isdir(self.packages_path), f"No directory '{packages_dir}' found in '{root}'" ) - # These are internal cache variables. - self._modules = {} - self._classes = {} - self._instances = {} + # Class attribute overrides by package name + self.overrides = overrides or {} + + # Optional reference to a RepoPath to influence module import from spack.pkg + self._finder: Optional[RepoPath] = None # Maps that goes from package name to corresponding file stat - self._fast_package_checker = None + self._fast_package_checker: Optional[FastPackageChecker] = None # Indexes for this repository, computed lazily - self._repo_index = None - self._cache = cache or spack.caches.MISC_CACHE + self._repo_index: Optional[RepoIndex] = None + self._cache = cache + + def finder(self, value: RepoPath) -> None: + self._finder = value - def real_name(self, import_name): + def real_name(self, import_name: str) -> Optional[str]: """Allow users to import Spack packages using Python identifiers. A python identifier might map to many different Spack package @@ -999,18 +1041,21 @@ class Repo: return import_name options = nm.possible_spack_module_names(import_name) - options.remove(import_name) + try: + options.remove(import_name) + except ValueError: + pass for name in options: if name in self: return name return None - def is_prefix(self, fullname): + def is_prefix(self, fullname: str) -> bool: """True if fullname is a prefix of this Repo's namespace.""" parts = fullname.split(".") return self._names[: len(parts)] == parts - def _read_config(self): + def _read_config(self) -> Dict[str, str]: """Check for a YAML config file in this db's root directory.""" try: with open(self.config_file) as reponame_file: @@ -1021,14 +1066,14 @@ class Repo: or "repo" not in yaml_data or not isinstance(yaml_data["repo"], dict) ): - tty.die("Invalid %s in repository %s" % (repo_config_name, self.root)) + tty.die(f"Invalid {repo_config_name} in repository {self.root}") return yaml_data["repo"] except IOError: - tty.die("Error reading %s when opening %s" % (self.config_file, self.root)) + tty.die(f"Error reading {self.config_file} when opening {self.root}") - def get(self, spec): + def get(self, spec: "spack.spec.Spec") -> "spack.package_base.PackageBase": """Returns the package associated with the supplied spec.""" msg = "Repo.get can only be called on concrete specs" assert isinstance(spec, spack.spec.Spec) and spec.concrete, msg @@ -1049,16 +1094,13 @@ class Repo: # pass these through as their error messages will be fine. raise except Exception as e: - tty.debug(e) - # Make sure other errors in constructors hit the error # handler by wrapping them - if spack.config.get("config:debug"): - sys.excepthook(*sys.exc_info()) - raise FailedConstructorError(spec.fullname, *sys.exc_info()) + tty.debug(e) + raise FailedConstructorError(spec.fullname, *sys.exc_info()) from e @autospec - def dump_provenance(self, spec, path): + def dump_provenance(self, spec: "spack.spec.Spec", path: str) -> None: """Dump provenance information for a spec to a particular path. This dumps the package file and any associated patch files. @@ -1066,7 +1108,7 @@ class Repo: """ if spec.namespace and spec.namespace != self.namespace: raise UnknownPackageError( - "Repository %s does not contain package %s." % (self.namespace, spec.fullname) + f"Repository {self.namespace} does not contain package {spec.fullname}." ) package_path = self.filename_for_package_name(spec.name) @@ -1083,17 +1125,13 @@ class Repo: if os.path.exists(patch.path): fs.install(patch.path, path) else: - tty.warn("Patch file did not exist: %s" % patch.path) + warnings.warn(f"Patch file did not exist: {patch.path}") # Install the package.py file itself. fs.install(self.filename_for_package_name(spec.name), path) - def purge(self): - """Clear entire package instance cache.""" - self._instances.clear() - @property - def index(self): + def index(self) -> RepoIndex: """Construct the index for this repo lazily.""" if self._repo_index is None: self._repo_index = RepoIndex(self._pkg_checker, self.namespace, cache=self._cache) @@ -1103,42 +1141,40 @@ class Repo: return self._repo_index @property - def provider_index(self): + def provider_index(self) -> spack.provider_index.ProviderIndex: """A provider index with names *specific* to this repo.""" return self.index["providers"] @property - def tag_index(self): + def tag_index(self) -> spack.tag.TagIndex: """Index of tags and which packages they're defined on.""" return self.index["tags"] @property - def patch_index(self): + def patch_index(self) -> spack.patch.PatchCache: """Index of patches and packages they're defined on.""" return self.index["patches"] @autospec - def providers_for(self, vpkg_spec): + def providers_for(self, vpkg_spec: "spack.spec.Spec") -> List["spack.spec.Spec"]: providers = self.provider_index.providers_for(vpkg_spec) if not providers: raise UnknownPackageError(vpkg_spec.fullname) return providers @autospec - def extensions_for(self, extendee_spec): - return [ - pkg_cls(spack.spec.Spec(pkg_cls.name)) - for pkg_cls in self.all_package_classes() - if pkg_cls(spack.spec.Spec(pkg_cls.name)).extends(extendee_spec) - ] - - def dirname_for_package_name(self, pkg_name): - """Get the directory name for a particular package. This is the - directory that contains its package.py file.""" + def extensions_for( + self, extendee_spec: "spack.spec.Spec" + ) -> List["spack.package_base.PackageBase"]: + result = [pkg_cls(spack.spec.Spec(pkg_cls.name)) for pkg_cls in self.all_package_classes()] + return [x for x in result if x.extends(extendee_spec)] + + def dirname_for_package_name(self, pkg_name: str) -> str: + """Given a package name, get the directory containing its package.py file.""" _, unqualified_name = self.partition_package_name(pkg_name) return os.path.join(self.packages_path, unqualified_name) - def filename_for_package_name(self, pkg_name): + def filename_for_package_name(self, pkg_name: str) -> str: """Get the filename for the module we should load for a particular package. Packages for a Repo live in ``$root/<package_name>/package.py`` @@ -1151,23 +1187,23 @@ class Repo: return os.path.join(pkg_dir, package_file_name) @property - def _pkg_checker(self): + def _pkg_checker(self) -> FastPackageChecker: if self._fast_package_checker is None: self._fast_package_checker = FastPackageChecker(self.packages_path) return self._fast_package_checker - def all_package_names(self, include_virtuals=False): + def all_package_names(self, include_virtuals: bool = False) -> List[str]: """Returns a sorted list of all package names in the Repo.""" names = sorted(self._pkg_checker.keys()) if include_virtuals: return names return [x for x in names if not self.is_virtual(x)] - def package_path(self, name): + def package_path(self, name: str) -> str: """Get path to package.py file for this repo.""" return os.path.join(self.packages_path, name, package_file_name) - def all_package_paths(self): + def all_package_paths(self) -> Generator[str, None, None]: for name in self.all_package_names(): yield self.package_path(name) @@ -1176,7 +1212,7 @@ class Repo: v.intersection_update(*(self.tag_index[tag.lower()] for tag in tags)) return v - def all_package_classes(self): + def all_package_classes(self) -> Generator[Type["spack.package_base.PackageBase"], None, None]: """Iterator over all package *classes* in the repository. Use this with care, because loading packages is slow. @@ -1184,7 +1220,7 @@ class Repo: for name in self.all_package_names(): yield self.get_pkg_class(name) - def exists(self, pkg_name): + def exists(self, pkg_name: str) -> bool: """Whether a package with the supplied name exists.""" if pkg_name is None: return False @@ -1201,28 +1237,22 @@ class Repo: """Time a package file in this repo was last updated.""" return self._pkg_checker.last_mtime() - def is_virtual(self, pkg_name): + def is_virtual(self, pkg_name: str) -> bool: """Return True if the package with this name is virtual, False otherwise. This function use the provider index. If calling from a code block that is used to construct the provider index use the ``is_virtual_safe`` function. - - Args: - pkg_name (str): name of the package we want to check """ return pkg_name in self.provider_index - def is_virtual_safe(self, pkg_name): + def is_virtual_safe(self, pkg_name: str) -> bool: """Return True if the package with this name is virtual, False otherwise. This function doesn't use the provider index. - - Args: - pkg_name (str): name of the package we want to check """ return not self.exists(pkg_name) or self.get_pkg_class(pkg_name).virtual - def get_pkg_class(self, pkg_name): + def get_pkg_class(self, pkg_name: str) -> Type["spack.package_base.PackageBase"]: """Get the class for the package out of its module. First loads (or fetches from cache) a module for the @@ -1234,7 +1264,8 @@ class Repo: fullname = f"{self.full_namespace}.{pkg_name}" try: - module = importlib.import_module(fullname) + with REPOS_FINDER.switch_repo(self._finder or self): + module = importlib.import_module(fullname) except ImportError: raise UnknownPackageError(fullname) except Exception as e: @@ -1245,26 +1276,21 @@ class Repo: if not inspect.isclass(cls): tty.die(f"{pkg_name}.{class_name} is not a class") - new_cfg_settings = ( - spack.config.get("packages").get(pkg_name, {}).get("package_attributes", {}) - ) - + # Clear any prior changes to class attributes in case the class was loaded from the + # same repo, but with different overrides overridden_attrs = getattr(cls, "overridden_attrs", {}) attrs_exclusively_from_config = getattr(cls, "attrs_exclusively_from_config", []) - # Clear any prior changes to class attributes in case the config has - # since changed for key, val in overridden_attrs.items(): setattr(cls, key, val) for key in attrs_exclusively_from_config: delattr(cls, key) - # Keep track of every class attribute that is overridden by the config: - # if the config changes between calls to this method, we make sure to - # restore the original config values (in case the new config no longer - # sets attributes that it used to) + # Keep track of every class attribute that is overridden: if different overrides + # dictionaries are used on the same physical repo, we make sure to restore the original + # config values new_overridden_attrs = {} new_attrs_exclusively_from_config = set() - for key, val in new_cfg_settings.items(): + for key, val in self.overrides.get(pkg_name, {}).items(): if hasattr(cls, key): new_overridden_attrs[key] = getattr(cls, key) else: @@ -1291,13 +1317,13 @@ class Repo: return namespace, pkg_name - def __str__(self): - return "[Repo '%s' at '%s']" % (self.namespace, self.root) + def __str__(self) -> str: + return f"Repo '{self.namespace}' at {self.root}" - def __repr__(self): + def __repr__(self) -> str: return self.__str__() - def __contains__(self, pkg_name): + def __contains__(self, pkg_name: str) -> bool: return self.exists(pkg_name) @@ -1373,12 +1399,17 @@ def create_repo(root, namespace=None, subdir=packages_dir_name): return full_path, namespace +def from_path(path: str) -> "Repo": + """Returns a repository from the path passed as input. Injects the global misc cache.""" + return Repo(path, cache=spack.caches.MISC_CACHE) + + def create_or_construct(path, namespace=None): """Create a repository, or just return a Repo if it already exists.""" if not os.path.exists(path): fs.mkdirp(path) create_repo(path, namespace) - return Repo(path) + return from_path(path) def _path(configuration=None): @@ -1396,7 +1427,17 @@ def create(configuration): repo_dirs = configuration.get("repos") if not repo_dirs: raise NoRepoConfiguredError("Spack configuration contains no package repositories.") - return RepoPath(*repo_dirs) + + overrides = {} + for pkg_name, data in configuration.get("packages").items(): + if pkg_name == "all": + continue + value = data.get("package_attributes", {}) + if not value: + continue + overrides[pkg_name] = value + + return RepoPath(*repo_dirs, cache=spack.caches.MISC_CACHE, overrides=overrides) #: Singleton repo path instance diff --git a/lib/spack/spack/test/cmd/pkg.py b/lib/spack/spack/test/cmd/pkg.py index f37d66363e..ecf30f0518 100644 --- a/lib/spack/spack/test/cmd/pkg.py +++ b/lib/spack/spack/test/cmd/pkg.py @@ -13,6 +13,7 @@ from llnl.util.filesystem import mkdirp, working_dir import spack.cmd.pkg import spack.main import spack.repo +import spack.util.file_cache #: new fake package template pkg_template = """\ @@ -34,13 +35,14 @@ abd = set(("pkg-a", "pkg-b", "pkg-d")) # Force all tests to use a git repository *in* the mock packages repo. @pytest.fixture(scope="module") -def mock_pkg_git_repo(git, tmpdir_factory): +def mock_pkg_git_repo(git, tmp_path_factory): """Copy the builtin.mock repo and make a mutable git repo inside it.""" - tmproot = tmpdir_factory.mktemp("mock_pkg_git_repo") - repo_path = tmproot.join("builtin.mock") + root_dir = tmp_path_factory.mktemp("mock_pkg_git_repo") + repo_dir = root_dir / "builtin.mock" + shutil.copytree(spack.paths.mock_packages_path, str(repo_dir)) - shutil.copytree(spack.paths.mock_packages_path, str(repo_path)) - mock_repo = spack.repo.RepoPath(str(repo_path)) + repo_cache = spack.util.file_cache.FileCache(str(root_dir / "cache")) + mock_repo = spack.repo.RepoPath(str(repo_dir), cache=repo_cache) mock_repo_packages = mock_repo.repos[0].packages_path with working_dir(mock_repo_packages): @@ -75,7 +77,7 @@ def mock_pkg_git_repo(git, tmpdir_factory): git("rm", "-rf", "pkg-c") git("-c", "commit.gpgsign=false", "commit", "-m", "change pkg-b, remove pkg-c, add pkg-d") - with spack.repo.use_repositories(str(repo_path)): + with spack.repo.use_repositories(str(repo_dir)): yield mock_repo_packages diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py index afdb3dbb1f..5605181604 100644 --- a/lib/spack/spack/test/cmd/style.py +++ b/lib/spack/spack/test/cmd/style.py @@ -38,7 +38,7 @@ def flake8_package(tmpdir): change to the ``flake8`` mock package, yields the filename, then undoes the change on cleanup. """ - repo = spack.repo.Repo(spack.paths.mock_packages_path) + repo = spack.repo.from_path(spack.paths.mock_packages_path) filename = repo.filename_for_package_name("flake8") rel_path = os.path.dirname(os.path.relpath(filename, spack.paths.prefix)) tmp = tmpdir / rel_path / "flake8-ci-package.py" @@ -54,7 +54,7 @@ def flake8_package(tmpdir): @pytest.fixture def flake8_package_with_errors(scope="function"): """A flake8 package with errors.""" - repo = spack.repo.Repo(spack.paths.mock_packages_path) + repo = spack.repo.from_path(spack.paths.mock_packages_path) filename = repo.filename_for_package_name("flake8") tmp = filename + ".tmp" @@ -130,7 +130,7 @@ def test_changed_files_all_files(): assert os.path.join(spack.paths.module_path, "spec.py") in files # a mock package - repo = spack.repo.Repo(spack.paths.mock_packages_path) + repo = spack.repo.from_path(spack.paths.mock_packages_path) filename = repo.filename_for_package_name("flake8") assert filename in files diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 9a80f361a1..c69f680dd1 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -24,6 +24,7 @@ import spack.hash_types as ht import spack.platforms import spack.repo import spack.solver.asp +import spack.util.file_cache import spack.util.libc import spack.variant as vt from spack.concretize import find_spec @@ -168,19 +169,18 @@ def fuzz_dep_order(request, monkeypatch): @pytest.fixture() -def repo_with_changing_recipe(tmpdir_factory, mutable_mock_repo): +def repo_with_changing_recipe(tmp_path_factory, mutable_mock_repo): repo_namespace = "changing" - repo_dir = tmpdir_factory.mktemp(repo_namespace) + repo_dir = tmp_path_factory.mktemp(repo_namespace) - repo_dir.join("repo.yaml").write( + (repo_dir / "repo.yaml").write_text( """ repo: namespace: changing -""", - ensure=True, +""" ) - packages_dir = repo_dir.ensure("packages", dir=True) + packages_dir = repo_dir / "packages" root_pkg_str = """ class Root(Package): homepage = "http://www.example.com" @@ -191,7 +191,9 @@ class Root(Package): conflicts("^changing~foo") """ - packages_dir.join("root", "package.py").write(root_pkg_str, ensure=True) + package_py = packages_dir / "root" / "package.py" + package_py.parent.mkdir(parents=True) + package_py.write_text(root_pkg_str) changing_template = """ class Changing(Package): @@ -225,7 +227,9 @@ class Changing(Package): def __init__(self, repo_directory): self.repo_dir = repo_directory - self.repo = spack.repo.Repo(str(repo_directory)) + cache_dir = tmp_path_factory.mktemp("cache") + self.repo_cache = spack.util.file_cache.FileCache(str(cache_dir)) + self.repo = spack.repo.Repo(str(repo_directory), cache=self.repo_cache) def change(self, changes=None): changes = changes or {} @@ -246,10 +250,12 @@ class Changing(Package): # Change the recipe t = jinja2.Template(changing_template) changing_pkg_str = t.render(**context) - packages_dir.join("changing", "package.py").write(changing_pkg_str, ensure=True) + package_py = packages_dir / "changing" / "package.py" + package_py.parent.mkdir(parents=True, exist_ok=True) + package_py.write_text(changing_pkg_str) # Re-add the repository - self.repo = spack.repo.Repo(str(self.repo_dir)) + self.repo = spack.repo.Repo(str(self.repo_dir), cache=self.repo_cache) repository.put_first(self.repo) _changing_pkg = _ChangingPackage(repo_dir) diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index a7683bf65f..6748b845aa 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -161,21 +161,24 @@ class TestConcretizePreferences: spec = concretize("mpileaks") assert "zmpi" in spec - def test_config_set_pkg_property_url(self, mutable_mock_repo): + @pytest.mark.parametrize( + "update,expected", + [ + ( + {"url": "http://www.somewhereelse.com/mpileaks-1.0.tar.gz"}, + "http://www.somewhereelse.com/mpileaks-2.3.tar.gz", + ), + ({}, "http://www.llnl.gov/mpileaks-2.3.tar.gz"), + ], + ) + def test_config_set_pkg_property_url(self, update, expected, mock_repo_path): """Test setting an existing attribute in the package class""" - update_packages( - "mpileaks", - "package_attributes", - {"url": "http://www.somewhereelse.com/mpileaks-1.0.tar.gz"}, - ) - spec = concretize("mpileaks") - assert spec.package.fetcher.url == "http://www.somewhereelse.com/mpileaks-2.3.tar.gz" - - update_packages("mpileaks", "package_attributes", {}) - spec = concretize("mpileaks") - assert spec.package.fetcher.url == "http://www.llnl.gov/mpileaks-2.3.tar.gz" + update_packages("mpileaks", "package_attributes", update) + with spack.repo.use_repositories(mock_repo_path): + spec = concretize("mpileaks") + assert spec.package.fetcher.url == expected - def test_config_set_pkg_property_new(self, mutable_mock_repo): + def test_config_set_pkg_property_new(self, mock_repo_path): """Test that you can set arbitrary attributes on the Package class""" conf = syaml.load_config( """\ @@ -194,19 +197,20 @@ mpileaks: """ ) spack.config.set("packages", conf, scope="concretize") - - spec = concretize("mpileaks") - assert spec.package.v1 == 1 - assert spec.package.v2 is True - assert spec.package.v3 == "yesterday" - assert spec.package.v4 == "true" - assert dict(spec.package.v5) == {"x": 1, "y": 2} - assert list(spec.package.v6) == [1, 2] + with spack.repo.use_repositories(mock_repo_path): + spec = concretize("mpileaks") + assert spec.package.v1 == 1 + assert spec.package.v2 is True + assert spec.package.v3 == "yesterday" + assert spec.package.v4 == "true" + assert dict(spec.package.v5) == {"x": 1, "y": 2} + assert list(spec.package.v6) == [1, 2] update_packages("mpileaks", "package_attributes", {}) - spec = concretize("mpileaks") - with pytest.raises(AttributeError): - spec.package.v1 + with spack.repo.use_repositories(mock_repo_path): + spec = concretize("mpileaks") + with pytest.raises(AttributeError): + spec.package.v1 def test_preferred(self): """ "Test packages with some version marked as preferred=True""" diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 7a6bd0622f..94d0969d0c 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -561,7 +561,7 @@ def _use_test_platform(test_platform): # @pytest.fixture(scope="session") def mock_repo_path(): - yield spack.repo.Repo(spack.paths.mock_packages_path) + yield spack.repo.from_path(spack.paths.mock_packages_path) def _pkg_install_fn(pkg, spec, prefix): @@ -588,7 +588,7 @@ def mock_packages(mock_repo_path, mock_pkg_install, request): def mutable_mock_repo(mock_repo_path, request): """Function-scoped mock packages, for tests that need to modify them.""" ensure_configuration_fixture_run_before(request) - mock_repo = spack.repo.Repo(spack.paths.mock_packages_path) + mock_repo = spack.repo.from_path(spack.paths.mock_packages_path) with spack.repo.use_repositories(mock_repo) as mock_repo_path: yield mock_repo_path @@ -2019,7 +2019,8 @@ repo: with open(str(pkg_file), "w") as f: f.write(pkg_str) - return spack.repo.Repo(repo_path) + repo_cache = spack.util.file_cache.FileCache(str(tmpdir.join("cache"))) + return spack.repo.Repo(repo_path, cache=repo_cache) @pytest.fixture() @@ -2061,3 +2062,9 @@ def _c_compiler_always_exists(): spack.solver.asp.c_compiler_runs = _true yield spack.solver.asp.c_compiler_runs = fn + + +@pytest.fixture(scope="session") +def mock_test_cache(tmp_path_factory): + cache_dir = tmp_path_factory.mktemp("cache") + return spack.util.file_cache.FileCache(str(cache_dir)) diff --git a/lib/spack/spack/test/directory_layout.py b/lib/spack/spack/test/directory_layout.py index 6f42281827..da51de415e 100644 --- a/lib/spack/spack/test/directory_layout.py +++ b/lib/spack/spack/test/directory_layout.py @@ -146,7 +146,7 @@ def test_read_and_write_spec(temporary_store, config, mock_packages): assert not os.path.exists(install_dir) -def test_handle_unknown_package(temporary_store, config, mock_packages): +def test_handle_unknown_package(temporary_store, config, mock_packages, tmp_path): """This test ensures that spack can at least do *some* operations with packages that are installed but that it does not know about. This is actually not such an uncommon @@ -158,7 +158,9 @@ def test_handle_unknown_package(temporary_store, config, mock_packages): or query them again if the package goes away. """ layout = temporary_store.layout - mock_db = spack.repo.RepoPath(spack.paths.mock_packages_path) + + repo_cache = spack.util.file_cache.FileCache(str(tmp_path / "cache")) + mock_db = spack.repo.RepoPath(spack.paths.mock_packages_path, cache=repo_cache) not_in_mock = set.difference( set(spack.repo.all_package_names()), set(mock_db.all_package_names()) diff --git a/lib/spack/spack/test/packages.py b/lib/spack/spack/test/packages.py index f2507ee57e..d00db3ed0c 100644 --- a/lib/spack/spack/test/packages.py +++ b/lib/spack/spack/test/packages.py @@ -32,12 +32,12 @@ class TestPackage: assert pkg_cls.name == "mpich" def test_package_filename(self): - repo = spack.repo.Repo(mock_packages_path) + repo = spack.repo.from_path(mock_packages_path) filename = repo.filename_for_package_name("mpich") assert filename == os.path.join(mock_packages_path, "packages", "mpich", "package.py") def test_nonexisting_package_filename(self): - repo = spack.repo.Repo(mock_packages_path) + repo = spack.repo.from_path(mock_packages_path) filename = repo.filename_for_package_name("some-nonexisting-package") assert filename == os.path.join( mock_packages_path, "packages", "some-nonexisting-package", "package.py" diff --git a/lib/spack/spack/test/repo.py b/lib/spack/spack/test/repo.py index 877b4af243..3aa9f00698 100644 --- a/lib/spack/spack/test/repo.py +++ b/lib/spack/spack/test/repo.py @@ -12,21 +12,28 @@ import spack.repo @pytest.fixture(params=["packages", "", "foo"]) -def extra_repo(tmpdir_factory, request): +def extra_repo(tmp_path_factory, request): repo_namespace = "extra_test_repo" - repo_dir = tmpdir_factory.mktemp(repo_namespace) - repo_dir.ensure(request.param, dir=True) - - with open(str(repo_dir.join("repo.yaml")), "w") as f: - f.write( + repo_dir = tmp_path_factory.mktemp(repo_namespace) + cache_dir = tmp_path_factory.mktemp("cache") + (repo_dir / request.param).mkdir(parents=True, exist_ok=True) + if request.param == "packages": + (repo_dir / "repo.yaml").write_text( """ repo: namespace: extra_test_repo """ ) - if request.param != "packages": - f.write(f" subdirectory: '{request.param}'") - return (spack.repo.Repo(str(repo_dir)), request.param) + else: + (repo_dir / "repo.yaml").write_text( + f""" +repo: + namespace: extra_test_repo + subdirectory: '{request.param}' +""" + ) + repo_cache = spack.util.file_cache.FileCache(str(cache_dir)) + return spack.repo.Repo(str(repo_dir), cache=repo_cache), request.param def test_repo_getpkg(mutable_mock_repo): @@ -177,8 +184,11 @@ def test_repo_dump_virtuals(tmpdir, mutable_mock_repo, mock_packages, ensure_deb ([spack.paths.mock_packages_path, spack.paths.packages_path], ["builtin.mock", "builtin"]), ], ) -def test_repository_construction_doesnt_use_globals(nullify_globals, repo_paths, namespaces): - repo_path = spack.repo.RepoPath(*repo_paths) +def test_repository_construction_doesnt_use_globals( + nullify_globals, tmp_path, repo_paths, namespaces +): + repo_cache = spack.util.file_cache.FileCache(str(tmp_path / "cache")) + repo_path = spack.repo.RepoPath(*repo_paths, cache=repo_cache) assert len(repo_path.repos) == len(namespaces) assert [x.namespace for x in repo_path.repos] == namespaces @@ -188,8 +198,84 @@ def test_path_computation_with_names(method_name, mock_repo_path): """Tests that repositories can compute the correct paths when using both fully qualified names and unqualified names. """ - repo_path = spack.repo.RepoPath(mock_repo_path) + repo_path = spack.repo.RepoPath(mock_repo_path, cache=None) method = getattr(repo_path, method_name) unqualified = method("mpileaks") qualified = method("builtin.mock.mpileaks") assert qualified == unqualified + + +@pytest.mark.usefixtures("nullify_globals") +class TestRepo: + """Test that the Repo class work correctly, and does not depend on globals, + except the REPOS_FINDER. + """ + + def test_creation(self, mock_test_cache): + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + assert repo.config_file.endswith("repo.yaml") + assert repo.namespace == "builtin.mock" + + @pytest.mark.parametrize( + "name,expected", [("mpi", True), ("mpich", False), ("mpileaks", False)] + ) + def test_is_virtual(self, name, expected, mock_test_cache): + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + assert repo.is_virtual(name) is expected + assert repo.is_virtual_safe(name) is expected + + @pytest.mark.parametrize( + "module_name,expected", + [ + ("dla_future", "dla-future"), + ("num7zip", "7zip"), + # If no package is there, None is returned + ("unknown", None), + ], + ) + def test_real_name(self, module_name, expected, mock_test_cache): + """Test that we can correctly compute the 'real' name of a package, from the one + used to import the Python module. + """ + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + assert repo.real_name(module_name) == expected + + @pytest.mark.parametrize("name", ["mpileaks", "7zip", "dla-future"]) + def test_get(self, name, mock_test_cache): + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + mock_spec = spack.spec.Spec(name) + mock_spec._mark_concrete() + pkg = repo.get(mock_spec) + assert pkg.__class__ == repo.get_pkg_class(name) + + @pytest.mark.parametrize("virtual_name,expected", [("mpi", ["mpich", "zmpi"])]) + def test_providers(self, virtual_name, expected, mock_test_cache): + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + provider_names = {x.name for x in repo.providers_for(virtual_name)} + assert provider_names.issuperset(expected) + + @pytest.mark.parametrize( + "extended,expected", + [("python", ["py-extension1", "python-venv"]), ("perl", ["perl-extension"])], + ) + def test_extensions(self, extended, expected, mock_test_cache): + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + provider_names = {x.name for x in repo.extensions_for(extended)} + assert provider_names.issuperset(expected) + + def test_all_package_names(self, mock_test_cache): + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + all_names = repo.all_package_names(include_virtuals=True) + real_names = repo.all_package_names(include_virtuals=False) + assert set(all_names).issuperset(real_names) + for name in set(all_names) - set(real_names): + assert repo.is_virtual(name) + assert repo.is_virtual_safe(name) + + def test_packages_with_tags(self, mock_test_cache): + repo = spack.repo.Repo(spack.paths.mock_packages_path, cache=mock_test_cache) + r1 = repo.packages_with_tags("tag1") + r2 = repo.packages_with_tags("tag1", "tag2") + assert "mpich" in r1 and "mpich" in r2 + assert "mpich2" in r1 and "mpich2" not in r2 + assert set(r2).issubset(r1) |