From 697719c1811c2f47b8e032f6106f9e2faee34abd Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Sat, 7 Mar 2020 04:38:08 -0800 Subject: Only use stable versions for public mirror (#15100) * add --skip-unstable-versions option to 'spack mirror create' which skips sources/resource for packages if their version is not stable (i.e. if they are the head of a git branch rather than a fixed commit) * '--skip-unstable-versions' should skip all VCS sources/resources, not just those which are not cachable --- lib/spack/spack/caches.py | 5 ++-- lib/spack/spack/cmd/mirror.py | 8 ++++-- lib/spack/spack/fetch_strategy.py | 9 +++++++ lib/spack/spack/mirror.py | 29 +++++++++++----------- lib/spack/spack/package.py | 4 +-- lib/spack/spack/patch.py | 6 ++--- lib/spack/spack/stage.py | 21 +++++++++++----- lib/spack/spack/test/cmd/mirror.py | 23 +++++++++++++++++ lib/spack/spack/test/mirror.py | 2 +- share/spack/spack-completion.bash | 2 +- .../trivial-pkg-with-valid-hash/package.py | 17 +++++++++++++ 11 files changed, 93 insertions(+), 33 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/trivial-pkg-with-valid-hash/package.py diff --git a/lib/spack/spack/caches.py b/lib/spack/spack/caches.py index 98fa8d5795..49624c06b2 100644 --- a/lib/spack/spack/caches.py +++ b/lib/spack/spack/caches.py @@ -50,8 +50,9 @@ def _fetch_cache(): class MirrorCache(object): - def __init__(self, root): + def __init__(self, root, skip_unstable_versions): self.root = os.path.abspath(root) + self.skip_unstable_versions = skip_unstable_versions def store(self, fetcher, relative_dest): """Fetch and relocate the fetcher's target into our mirror cache.""" @@ -84,5 +85,3 @@ class MirrorCache(object): #: Spack's local cache for downloaded source archives fetch_cache = llnl.util.lang.Singleton(_fetch_cache) - -mirror_cache = None diff --git a/lib/spack/spack/cmd/mirror.py b/lib/spack/spack/cmd/mirror.py index 5206927895..1473550a56 100644 --- a/lib/spack/spack/cmd/mirror.py +++ b/lib/spack/spack/cmd/mirror.py @@ -45,7 +45,10 @@ def setup_parser(subparser): " (this requires significant time and space)") create_parser.add_argument( '-f', '--file', help="file with specs of packages to put in mirror") - + create_parser.add_argument( + '--skip-unstable-versions', action='store_true', + help="don't cache versions unless they identify a stable (unchanging)" + " source code") create_parser.add_argument( '-D', '--dependencies', action='store_true', help="also fetch all dependencies") @@ -308,7 +311,8 @@ def mirror_create(args): existed = web_util.url_exists(directory) # Actually do the work to create the mirror - present, mirrored, error = spack.mirror.create(directory, mirror_specs) + present, mirrored, error = spack.mirror.create( + directory, mirror_specs, args.skip_unstable_versions) p, m, e = len(present), len(mirrored), len(error) verb = "updated" if existed else "created" diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py index 4ca6a886d4..d923e2b58e 100644 --- a/lib/spack/spack/fetch_strategy.py +++ b/lib/spack/spack/fetch_strategy.py @@ -1179,6 +1179,15 @@ class S3FetchStrategy(URLFetchStrategy): raise FailedDownloadError(self.url) +def stable_target(fetcher): + """Returns whether the fetcher target is expected to have a stable + checksum. This is only true if the target is a preexisting archive + file.""" + if isinstance(fetcher, URLFetchStrategy) and fetcher.cachable: + return True + return False + + def from_url(url): """Given a URL, find an appropriate fetch strategy for it. Currently just gives you a URLFetchStrategy that uses curl. diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py index 3f6b2b6a0e..045ca5ffec 100644 --- a/lib/spack/spack/mirror.py +++ b/lib/spack/spack/mirror.py @@ -401,7 +401,7 @@ def get_matching_versions(specs, num_versions=1): return matching -def create(path, specs): +def create(path, specs, skip_unstable_versions=False): """Create a directory to be used as a spack mirror, and fill it with package archives. @@ -409,6 +409,9 @@ def create(path, specs): path: Path to create a mirror directory hierarchy in. specs: Any package versions matching these specs will be added \ to the mirror. + skip_unstable_versions: if true, this skips adding resources when + they do not have a stable archive checksum (as determined by + ``fetch_strategy.stable_target``) Return Value: Returns a tuple of lists: (present, mirrored, error) @@ -440,16 +443,14 @@ def create(path, specs): raise MirrorError( "Cannot create directory '%s':" % mirror_root, str(e)) - mirror_cache = spack.caches.MirrorCache(mirror_root) + mirror_cache = spack.caches.MirrorCache( + mirror_root, skip_unstable_versions=skip_unstable_versions) mirror_stats = MirrorStats() - try: - spack.caches.mirror_cache = mirror_cache - # Iterate through packages and download all safe tarballs for each - for spec in specs: - mirror_stats.next_spec(spec) - add_single_spec(spec, mirror_root, mirror_stats) - finally: - spack.caches.mirror_cache = None + + # Iterate through packages and download all safe tarballs for each + for spec in specs: + mirror_stats.next_spec(spec) + _add_single_spec(spec, mirror_cache, mirror_stats) return mirror_stats.stats() @@ -495,7 +496,7 @@ class MirrorStats(object): self.errors.add(self.current_spec) -def add_single_spec(spec, mirror_root, mirror_stats): +def _add_single_spec(spec, mirror, mirror_stats): tty.msg("Adding package {pkg} to mirror".format( pkg=spec.format("{name}{@version}") )) @@ -503,10 +504,10 @@ def add_single_spec(spec, mirror_root, mirror_stats): while num_retries > 0: try: with spec.package.stage as pkg_stage: - pkg_stage.cache_mirror(mirror_stats) + pkg_stage.cache_mirror(mirror, mirror_stats) for patch in spec.package.all_patches(): - if patch.cache(): - patch.cache().cache_mirror(mirror_stats) + if patch.stage: + patch.stage.cache_mirror(mirror, mirror_stats) patch.clean() exception = None break diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index b7efc3989b..56bdab22c1 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1135,8 +1135,8 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): for patch in self.spec.patches: patch.fetch() - if patch.cache(): - patch.cache().cache_local() + if patch.stage: + patch.stage.cache_local() def do_stage(self, mirror_only=False): """Unpacks and expands the fetched tarball.""" diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py index bcb45387a8..3a3c1507e1 100644 --- a/lib/spack/spack/patch.py +++ b/lib/spack/spack/patch.py @@ -85,7 +85,8 @@ class Patch(object): apply_patch(stage, self.path, self.level, self.working_dir) - def cache(self): + @property + def stage(self): return None def to_dict(self): @@ -248,9 +249,6 @@ class UrlPatch(Patch): self._stage.create() return self._stage - def cache(self): - return self.stage - def clean(self): self.stage.destroy() diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index b445638228..dd05131391 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -493,8 +493,14 @@ class Stage(object): spack.caches.fetch_cache.store( self.fetcher, self.mirror_paths.storage_path) - def cache_mirror(self, stats): - """Perform a fetch if the resource is not already cached""" + def cache_mirror(self, mirror, stats): + """Perform a fetch if the resource is not already cached + + Arguments: + mirror (MirrorCache): the mirror to cache this Stage's resource in + stats (MirrorStats): this is updated depending on whether the + caching operation succeeded or failed + """ if isinstance(self.default_fetcher, fs.BundleFetchStrategy): # BundleFetchStrategy has no source to fetch. The associated # fetcher does nothing but the associated stage may still exist. @@ -505,20 +511,23 @@ class Stage(object): # must examine the type of the fetcher. return - dst_root = spack.caches.mirror_cache.root + if (mirror.skip_unstable_versions and + not fs.stable_target(self.default_fetcher)): + return + absolute_storage_path = os.path.join( - dst_root, self.mirror_paths.storage_path) + mirror.root, self.mirror_paths.storage_path) if os.path.exists(absolute_storage_path): stats.already_existed(absolute_storage_path) else: self.fetch() self.check() - spack.caches.mirror_cache.store( + mirror.store( self.fetcher, self.mirror_paths.storage_path) stats.added(absolute_storage_path) - spack.caches.mirror_cache.symlink(self.mirror_paths) + mirror.symlink(self.mirror_paths) def expand_archive(self): """Changes to the stage directory and attempt to expand the downloaded diff --git a/lib/spack/spack/test/cmd/mirror.py b/lib/spack/spack/test/cmd/mirror.py index d62d7df432..4bb4fad224 100644 --- a/lib/spack/spack/test/cmd/mirror.py +++ b/lib/spack/spack/test/cmd/mirror.py @@ -66,6 +66,29 @@ def test_mirror_from_env(tmpdir, mock_packages, mock_fetch, config, assert mirror_res == expected +@pytest.fixture +def source_for_pkg_with_hash(mock_packages, tmpdir): + pkg = spack.repo.get('trivial-pkg-with-valid-hash') + local_url_basename = os.path.basename(pkg.url) + local_path = os.path.join(str(tmpdir), local_url_basename) + with open(local_path, 'w') as f: + f.write(pkg.hashed_content) + local_url = "file://" + local_path + pkg.versions[spack.version.Version('1.0')]['url'] = local_url + + +def test_mirror_skip_unstable(tmpdir_factory, mock_packages, config, + source_for_pkg_with_hash): + mirror_dir = str(tmpdir_factory.mktemp('mirror-dir')) + + specs = [spack.spec.Spec(x).concretized() for x in + ['git-test', 'trivial-pkg-with-valid-hash']] + spack.mirror.create(mirror_dir, specs, skip_unstable_versions=True) + + assert (set(os.listdir(mirror_dir)) - set(['_source-cache']) == + set(['trivial-pkg-with-valid-hash'])) + + def test_mirror_crud(tmp_scope, capsys): with capsys.disabled(): mirror('add', '--scope', tmp_scope, 'mirror', 'http://spack.io') diff --git a/lib/spack/spack/test/mirror.py b/lib/spack/spack/test/mirror.py index 08b32f74f1..570c329b71 100644 --- a/lib/spack/spack/test/mirror.py +++ b/lib/spack/spack/test/mirror.py @@ -213,7 +213,7 @@ def test_mirror_cache_symlinks(tmpdir): """ cosmetic_path = 'zlib/zlib-1.2.11.tar.gz' global_path = '_source-cache/archive/c3/c3e5.tar.gz' - cache = spack.caches.MirrorCache(str(tmpdir)) + cache = spack.caches.MirrorCache(str(tmpdir), False) reference = spack.mirror.MirrorReference(cosmetic_path, global_path) cache.store(MockFetcher(), reference.storage_path) diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 869d63be31..38788438f7 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -1025,7 +1025,7 @@ _spack_mirror() { _spack_mirror_create() { if $list_options then - SPACK_COMPREPLY="-h --help -d --directory -a --all -f --file -D --dependencies -n --versions-per-spec" + SPACK_COMPREPLY="-h --help -d --directory -a --all -f --file --skip-unstable-versions -D --dependencies -n --versions-per-spec" else _all_packages fi diff --git a/var/spack/repos/builtin.mock/packages/trivial-pkg-with-valid-hash/package.py b/var/spack/repos/builtin.mock/packages/trivial-pkg-with-valid-hash/package.py new file mode 100644 index 0000000000..52fd3d99a6 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/trivial-pkg-with-valid-hash/package.py @@ -0,0 +1,17 @@ +# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack import * + + +class TrivialPkgWithValidHash(Package): + url = "http://www.unit-test-should-replace-this-url/trivial_install-1.0" + + version('1.0', '6ae8a75555209fd6c44157c0aed8016e763ff435a19cf186f76863140143ff72', expand=False) + + hashed_content = "test content" + + def install(self, spec, prefix): + pass -- cgit v1.2.3-70-g09d2