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.
This commit is contained in:
		| @@ -1661,7 +1661,7 @@ def dedupe_hardlinks_if_necessary(root, buildinfo): | |||||||
|         buildinfo[key] = new_list |         buildinfo[key] = new_list | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| def relocate_package(spec, allow_root): | def relocate_package(spec): | ||||||
|     """ |     """ | ||||||
|     Relocate the given package |     Relocate the given package | ||||||
|     """ |     """ | ||||||
| @@ -1679,7 +1679,7 @@ def relocate_package(spec, allow_root): | |||||||
|     old_spack_prefix = str(buildinfo.get("spackprefix")) |     old_spack_prefix = str(buildinfo.get("spackprefix")) | ||||||
|     old_rel_prefix = buildinfo.get("relative_prefix") |     old_rel_prefix = buildinfo.get("relative_prefix") | ||||||
|     old_prefix = os.path.join(old_layout_root, old_rel_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 |     # In the past prefix_to_hash was the default and externals were not dropped, so prefixes | ||||||
|     # were not unique. |     # were not unique. | ||||||
| @@ -1852,7 +1852,7 @@ def _extract_inner_tarball(spec, filename, extract_to, unsigned, remote_checksum | |||||||
|     return tarfile_path |     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 |     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) |     os.remove(specfile_path) | ||||||
| 
 | 
 | ||||||
|     try: |     try: | ||||||
|         relocate_package(spec, allow_root) |         relocate_package(spec) | ||||||
|     except Exception as e: |     except Exception as e: | ||||||
|         shutil.rmtree(spec.prefix) |         shutil.rmtree(spec.prefix) | ||||||
|         raise e |         raise e | ||||||
| @@ -1967,7 +1967,7 @@ def extract_tarball(spec, download_result, allow_root=False, unsigned=False, for | |||||||
|         _delete_staged_downloads(download_result) |         _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. |     """Install the root node of a concrete spec from a buildcache. | ||||||
| 
 | 
 | ||||||
|     Checking the sha256 sum of a node before installation is usually needed only |     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: |     Args: | ||||||
|         spec: spec to be installed (note that only the root node will be installed) |         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 |         unsigned (bool): if True allows installing unsigned binaries | ||||||
|         force (bool): force installation if the spec is already present in the |         force (bool): force installation if the spec is already present in the | ||||||
|             local store |             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 |     # don't print long padded paths while extracting/relocating binaries | ||||||
|     with spack.util.path.filter_padding(): |     with spack.util.path.filter_padding(): | ||||||
|         tty.msg('Installing "{0}" from a buildcache'.format(spec.format())) |         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.hooks.post_install(spec, False) | ||||||
|         spack.store.db.add(spec, spack.store.layout) |         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. |     """Install a single concrete spec from a buildcache. | ||||||
| 
 | 
 | ||||||
|     Args: |     Args: | ||||||
|         spec (spack.spec.Spec): spec to be installed |         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 |         unsigned (bool): if True allows installing unsigned binaries | ||||||
|         force (bool): force installation if the spec is already present in the |         force (bool): force installation if the spec is already present in the | ||||||
|             local store |             local store | ||||||
|     """ |     """ | ||||||
|     for node in spec.traverse(root=True, order="post", deptype=("link", "run")): |     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): | def try_direct_fetch(spec, mirrors=None): | ||||||
|   | |||||||
| @@ -200,7 +200,7 @@ def _install_by_hash( | |||||||
|                 matches = spack.store.find([spec_str], multiple=False, query_fn=query) |                 matches = spack.store.find([spec_str], multiple=False, query_fn=query) | ||||||
|                 for match in matches: |                 for match in matches: | ||||||
|                     spack.binary_distribution.install_root_node( |                     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( |     def _install_and_test( | ||||||
|   | |||||||
| @@ -43,11 +43,12 @@ def setup_parser(subparser): | |||||||
|     subparsers = subparser.add_subparsers(help="buildcache sub-commands") |     subparsers = subparser.add_subparsers(help="buildcache sub-commands") | ||||||
| 
 | 
 | ||||||
|     create = subparsers.add_parser("create", help=create_fn.__doc__) |     create = subparsers.add_parser("create", help=create_fn.__doc__) | ||||||
|  |     # TODO: remove from Spack 0.21 | ||||||
|     create.add_argument( |     create.add_argument( | ||||||
|         "-r", |         "-r", | ||||||
|         "--rel", |         "--rel", | ||||||
|         action="store_true", |         action="store_true", | ||||||
|         help="make all rpaths relative before creating tarballs.", |         help="make all rpaths relative before creating tarballs. (deprecated)", | ||||||
|     ) |     ) | ||||||
|     create.add_argument( |     create.add_argument( | ||||||
|         "-f", "--force", action="store_true", help="overwrite tarball if it exists." |         "-f", "--force", action="store_true", help="overwrite tarball if it exists." | ||||||
| @@ -130,11 +131,12 @@ def setup_parser(subparser): | |||||||
|     install.add_argument( |     install.add_argument( | ||||||
|         "-m", "--multiple", action="store_true", help="allow all matching packages " |         "-m", "--multiple", action="store_true", help="allow all matching packages " | ||||||
|     ) |     ) | ||||||
|  |     # TODO: remove from Spack 0.21 | ||||||
|     install.add_argument( |     install.add_argument( | ||||||
|         "-a", |         "-a", | ||||||
|         "--allow-root", |         "--allow-root", | ||||||
|         action="store_true", |         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( |     install.add_argument( | ||||||
|         "-u", |         "-u", | ||||||
| @@ -449,6 +451,9 @@ def create_fn(args): | |||||||
|             "Spack 0.21, use positional arguments instead." |             "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 |     # TODO: remove this in 0.21. If we have mirror_flag, the first | ||||||
|     # spec is in the positional mirror arg due to argparse limitations. |     # spec is in the positional mirror arg due to argparse limitations. | ||||||
|     input_specs = args.specs |     input_specs = args.specs | ||||||
| @@ -521,12 +526,13 @@ def install_fn(args): | |||||||
|     if not args.specs: |     if not args.specs: | ||||||
|         tty.die("a spec argument is required to install from a buildcache") |         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) |     query = bindist.BinaryCacheQuery(all_architectures=args.otherarch) | ||||||
|     matches = spack.store.find(args.specs, multiple=args.multiple, query_fn=query) |     matches = spack.store.find(args.specs, multiple=args.multiple, query_fn=query) | ||||||
|     for match in matches: |     for match in matches: | ||||||
|         bindist.install_single_spec( |         bindist.install_single_spec(match, unsigned=args.unsigned, force=args.force) | ||||||
|             match, allow_root=args.allow_root, unsigned=args.unsigned, force=args.force |  | ||||||
|         ) |  | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| def list_fn(args): | def list_fn(args): | ||||||
|   | |||||||
| @@ -391,7 +391,7 @@ def _process_binary_cache_tarball( | |||||||
| 
 | 
 | ||||||
|     with timer.measure("install"), spack.util.path.filter_padding(): |     with timer.measure("install"), spack.util.path.filter_padding(): | ||||||
|         binary_distribution.extract_tarball( |         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 |         pkg.installed_from_binary_cache = True | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Harmen Stoppels
					Harmen Stoppels