deprecate buildcache create --rel, buildcache install --allow-root (#37285)

`buildcache create --rel`: deprecate this because there is no point in
making things relative before tarballing; on install you need to expand
`$ORIGIN` / `@loader_path` / relative symlinks anyways because some
dependencies may actually be in an upstream, or have different
projections.

`buildcache install --allow-root`: this flag was propagated through a
lot of functions but was ultimately unused.

4 files changed, 21 insertions, 19 deletions

diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py
index 1b89e8bf44..e974eb2237 100644
--- a/lib/spack/spack/binary_distribution.py
+++ b/lib/spack/spack/binary_distribution.py
@@ -1661,7 +1661,7 @@ def dedupe_hardlinks_if_necessary(root, buildinfo):
         buildinfo[key] = new_list
 
 
-def relocate_package(spec, allow_root):
+def relocate_package(spec):
     """
     Relocate the given package
     """
@@ -1679,7 +1679,7 @@ def relocate_package(spec, allow_root):
     old_spack_prefix = str(buildinfo.get("spackprefix"))
     old_rel_prefix = buildinfo.get("relative_prefix")
     old_prefix = os.path.join(old_layout_root, old_rel_prefix)
-    rel = buildinfo.get("relative_rpaths")
+    rel = buildinfo.get("relative_rpaths", False)
 
     # In the past prefix_to_hash was the default and externals were not dropped, so prefixes
     # were not unique.
@@ -1852,7 +1852,7 @@ def _extract_inner_tarball(spec, filename, extract_to, unsigned, remote_checksum
     return tarfile_path
 
 
-def extract_tarball(spec, download_result, allow_root=False, unsigned=False, force=False):
+def extract_tarball(spec, download_result, unsigned=False, force=False):
     """
     extract binary tarball for given package into install area
     """
@@ -1948,7 +1948,7 @@ def extract_tarball(spec, download_result, allow_root=False, unsigned=False, for
     os.remove(specfile_path)
 
     try:
-        relocate_package(spec, allow_root)
+        relocate_package(spec)
     except Exception as e:
         shutil.rmtree(spec.prefix)
         raise e
@@ -1967,7 +1967,7 @@ def extract_tarball(spec, download_result, allow_root=False, unsigned=False, for
         _delete_staged_downloads(download_result)
 
 
-def install_root_node(spec, allow_root, unsigned=False, force=False, sha256=None):
+def install_root_node(spec, unsigned=False, force=False, sha256=None):
     """Install the root node of a concrete spec from a buildcache.
 
     Checking the sha256 sum of a node before installation is usually needed only
@@ -1976,8 +1976,6 @@ def install_root_node(spec, allow_root, unsigned=False, force=False, sha256=None
 
     Args:
         spec: spec to be installed (note that only the root node will be installed)
-        allow_root (bool): allows the root directory to be present in binaries
-            (may affect relocation)
         unsigned (bool): if True allows installing unsigned binaries
         force (bool): force installation if the spec is already present in the
             local store
@@ -2013,24 +2011,22 @@ def install_root_node(spec, allow_root, unsigned=False, force=False, sha256=None
     # don't print long padded paths while extracting/relocating binaries
     with spack.util.path.filter_padding():
         tty.msg('Installing "{0}" from a buildcache'.format(spec.format()))
-        extract_tarball(spec, download_result, allow_root, unsigned, force)
+        extract_tarball(spec, download_result, unsigned, force)
         spack.hooks.post_install(spec, False)
         spack.store.db.add(spec, spack.store.layout)
 
 
-def install_single_spec(spec, allow_root=False, unsigned=False, force=False):
+def install_single_spec(spec, unsigned=False, force=False):
     """Install a single concrete spec from a buildcache.
 
     Args:
         spec (spack.spec.Spec): spec to be installed
-        allow_root (bool): allows the root directory to be present in binaries
-            (may affect relocation)
         unsigned (bool): if True allows installing unsigned binaries
         force (bool): force installation if the spec is already present in the
             local store
     """
     for node in spec.traverse(root=True, order="post", deptype=("link", "run")):
-        install_root_node(node, allow_root=allow_root, unsigned=unsigned, force=force)
+        install_root_node(node, unsigned=unsigned, force=force)
 
 
 def try_direct_fetch(spec, mirrors=None):
diff --git a/lib/spack/spack/bootstrap/core.py b/lib/spack/spack/bootstrap/core.py
index 90263075cb..e545ea5854 100644
--- a/lib/spack/spack/bootstrap/core.py
+++ b/lib/spack/spack/bootstrap/core.py
@@ -200,7 +200,7 @@ class BuildcacheBootstrapper(Bootstrapper):
                 matches = spack.store.find([spec_str], multiple=False, query_fn=query)
                 for match in matches:
                     spack.binary_distribution.install_root_node(
-                        match, allow_root=True, unsigned=True, force=True, sha256=pkg_sha256
+                        match, unsigned=True, force=True, sha256=pkg_sha256
                     )
 
     def _install_and_test(
diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py
index f8ebd034e9..2251508014 100644
--- a/lib/spack/spack/cmd/buildcache.py
+++ b/lib/spack/spack/cmd/buildcache.py
@@ -43,11 +43,12 @@ def setup_parser(subparser):
     subparsers = subparser.add_subparsers(help="buildcache sub-commands")
 
     create = subparsers.add_parser("create", help=create_fn.__doc__)
+    # TODO: remove from Spack 0.21
     create.add_argument(
         "-r",
         "--rel",
         action="store_true",
-        help="make all rpaths relative before creating tarballs.",
+        help="make all rpaths relative before creating tarballs. (deprecated)",
     )
     create.add_argument(
         "-f", "--force", action="store_true", help="overwrite tarball if it exists."
@@ -130,11 +131,12 @@ def setup_parser(subparser):
     install.add_argument(
         "-m", "--multiple", action="store_true", help="allow all matching packages "
     )
+    # TODO: remove from Spack 0.21
     install.add_argument(
         "-a",
         "--allow-root",
         action="store_true",
-        help="allow install root string in binary files after RPATH substitution",
+        help="allow install root string in binary files after RPATH substitution. (deprecated)",
     )
     install.add_argument(
         "-u",
@@ -449,6 +451,9 @@ def create_fn(args):
             "Spack 0.21, use positional arguments instead."
         )
 
+    if args.rel:
+        tty.warn("The --rel flag is deprecated and will be removed in Spack 0.21")
+
     # TODO: remove this in 0.21. If we have mirror_flag, the first
     # spec is in the positional mirror arg due to argparse limitations.
     input_specs = args.specs
@@ -521,12 +526,13 @@ def install_fn(args):
     if not args.specs:
         tty.die("a spec argument is required to install from a buildcache")
 
+    if args.allow_root:
+        tty.warn("The --allow-root flag is deprecated and will be removed in Spack 0.21")
+
     query = bindist.BinaryCacheQuery(all_architectures=args.otherarch)
     matches = spack.store.find(args.specs, multiple=args.multiple, query_fn=query)
     for match in matches:
-        bindist.install_single_spec(
-            match, allow_root=args.allow_root, unsigned=args.unsigned, force=args.force
-        )
+        bindist.install_single_spec(match, unsigned=args.unsigned, force=args.force)
 
 
 def list_fn(args):
diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py
index 4908fa7788..1c56a08980 100644
--- a/lib/spack/spack/installer.py
+++ b/lib/spack/spack/installer.py
@@ -391,7 +391,7 @@ def _process_binary_cache_tarball(
 
     with timer.measure("install"), spack.util.path.filter_padding():
         binary_distribution.extract_tarball(
-            pkg.spec, download_result, allow_root=False, unsigned=unsigned, force=False
+            pkg.spec, download_result, unsigned=unsigned, force=False
         )
 
         pkg.installed_from_binary_cache = True