buildcache sync: manifest-glob with arbitrary destination (#41284)

* buildcache sync: manifest-glob with arbitrary destination

The current implementation of the --manifest-glob is a bit restrictive
requiring the destination to be known by the generation stage of CI.
This allows specifying an arbitrary destination mirror URL.

* Add unit test for buildcache sync with manifest

* Fix test and arguments for manifest-glob with override destination

* Add testing path for unused mirror argument

lib/spack/spack/cmd/buildcache.py

@@ -275,23 +275,37 @@ def setup_parser(subparser: argparse.ArgumentParser):
 
     # Sync buildcache entries from one mirror to another
     sync = subparsers.add_parser("sync", help=sync_fn.__doc__)
-    sync.add_argument(
-        "--manifest-glob", help="a quoted glob pattern identifying copy manifest files"
+
+    sync_manifest_source = sync.add_argument_group(
+        "Manifest Source",
+        "Specify a list of build cache objects to sync using manifest file(s)."
+        'This option takes the place of the "source mirror" for synchronization'
+        'and optionally takes a "destination mirror" ',
     )
-    sync.add_argument(
+    sync_manifest_source.add_argument(
+        "--manifest-glob", help="a quoted glob pattern identifying CI rebuild manifest files"
+    )
+    sync_source_mirror = sync.add_argument_group(
+        "Named Source",
+        "Specify a single registered source mirror to synchronize from. This option requires"
+        "the specification of a destination mirror.",
+    )
+    sync_source_mirror.add_argument(
         "src_mirror",
         metavar="source mirror",
-        type=arguments.mirror_name_or_url,
         nargs="?",
+        type=arguments.mirror_name_or_url,
         help="source mirror name, path, or URL",
     )
+
     sync.add_argument(
         "dest_mirror",
         metavar="destination mirror",
-        type=arguments.mirror_name_or_url,
         nargs="?",
+        type=arguments.mirror_name_or_url,
         help="destination mirror name, path, or URL",
     )
+
     sync.set_defaults(func=sync_fn)
 
     # Update buildcache index without copying any additional packages
@@ -1070,7 +1084,17 @@ def sync_fn(args):
     requires an active environment in order to know which specs to sync
     """
     if args.manifest_glob:
-        manifest_copy(glob.glob(args.manifest_glob))
+        # Passing the args.src_mirror here because it is not possible to
+        # have the destination be required when specifying a named source
+        # mirror and optional for the --manifest-glob argument. In the case
+        # of manifest glob sync, the source mirror positional argument is the
+        # destination mirror if it is specified. If there are two mirrors
+        # specified, the second is ignored and the first is the override
+        # destination.
+        if args.dest_mirror:
+            tty.warn(f"Ignoring unused arguemnt: {args.dest_mirror.name}")
+
+        manifest_copy(glob.glob(args.manifest_glob), args.src_mirror)
         return 0
 
     if args.src_mirror is None or args.dest_mirror is None:
@@ -1121,7 +1145,7 @@ def sync_fn(args):
         shutil.rmtree(tmpdir)
 
 
-def manifest_copy(manifest_file_list):
+def manifest_copy(manifest_file_list, dest_mirror=None):
     """Read manifest files containing information about specific specs to copy
     from source to destination, remove duplicates since any binary packge for
     a given hash should be the same as any other, and copy all files specified
@@ -1135,10 +1159,17 @@ def manifest_copy(manifest_file_list):
                 # Last duplicate hash wins
                 deduped_manifest[spec_hash] = copy_list
 
+    build_cache_dir = bindist.build_cache_relative_path()
     for spec_hash, copy_list in deduped_manifest.items():
         for copy_file in copy_list:
-            tty.debug("copying {0} to {1}".format(copy_file["src"], copy_file["dest"]))
-            copy_buildcache_file(copy_file["src"], copy_file["dest"])
+            dest = copy_file["dest"]
+            if dest_mirror:
+                src_relative_path = os.path.join(
+                    build_cache_dir, copy_file["src"].rsplit(build_cache_dir, 1)[1].lstrip("/")
+                )
+                dest = url_util.join(dest_mirror.push_url, src_relative_path)
+            tty.debug("copying {0} to {1}".format(copy_file["src"], dest))
+            copy_buildcache_file(copy_file["src"], dest)
 
 
 def update_index(mirror: spack.mirror.Mirror, update_keys=False):

lib/spack/spack/test/cmd/buildcache.py

@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import errno
+import json
 import os
 import shutil
 
@@ -234,10 +235,71 @@ def verify_mirror_contents():
         # Use mirror names to specify mirrors
         mirror("add", "src", src_mirror_url)
         mirror("add", "dest", dest_mirror_url)
+        mirror("add", "ignored", "file:///dummy/io")
 
         buildcache("sync", "src", "dest")
 
         verify_mirror_contents()
+        shutil.rmtree(dest_mirror_dir)
+
+        def manifest_insert(manifest, spec, dest_url):
+            manifest[spec.dag_hash()] = [
+                {
+                    "src": spack.util.url.join(
+                        src_mirror_url,
+                        spack.binary_distribution.build_cache_relative_path(),
+                        spack.binary_distribution.tarball_name(spec, ".spec.json"),
+                    ),
+                    "dest": spack.util.url.join(
+                        dest_url,
+                        spack.binary_distribution.build_cache_relative_path(),
+                        spack.binary_distribution.tarball_name(spec, ".spec.json"),
+                    ),
+                },
+                {
+                    "src": spack.util.url.join(
+                        src_mirror_url,
+                        spack.binary_distribution.build_cache_relative_path(),
+                        spack.binary_distribution.tarball_path_name(spec, ".spack"),
+                    ),
+                    "dest": spack.util.url.join(
+                        dest_url,
+                        spack.binary_distribution.build_cache_relative_path(),
+                        spack.binary_distribution.tarball_path_name(spec, ".spack"),
+                    ),
+                },
+            ]
+
+        manifest_file = os.path.join(tmpdir.strpath, "manifest_dest.json")
+        with open(manifest_file, "w") as fd:
+            test_env = ev.active_environment()
+
+            manifest = {}
+            for spec in test_env.specs_by_hash.values():
+                manifest_insert(manifest, spec, dest_mirror_url)
+            json.dump(manifest, fd)
+
+        buildcache("sync", "--manifest-glob", manifest_file)
+
+        verify_mirror_contents()
+        shutil.rmtree(dest_mirror_dir)
+
+        manifest_file = os.path.join(tmpdir.strpath, "manifest_bad_dest.json")
+        with open(manifest_file, "w") as fd:
+            manifest = {}
+            for spec in test_env.specs_by_hash.values():
+                manifest_insert(
+                    manifest, spec, spack.util.url.join(dest_mirror_url, "invalid_path")
+                )
+            json.dump(manifest, fd)
+
+        # Trigger the warning
+        output = buildcache("sync", "--manifest-glob", manifest_file, "dest", "ignored")
+
+        assert "Ignoring unused arguemnt: ignored" in output
+
+        verify_mirror_contents()
+        shutil.rmtree(dest_mirror_dir)
 
 
 def test_buildcache_create_install(