hlrs-software-stack/spack
7
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Buildcache commands cleanup, again... (#39203)

Browse source 
* Inform mypy that tty.die is noreturn

* avoid temporary allocation in env

* update spack buildcache save-specfile

* fix spack buildcache check/download/get-buildcache-name

- ensure that required args and mutually exclusive ones are marked as
  such in argparse for better error messages
- deprecate --spec-file everywhere
- use disambiguate for better error messages
This commit is contained in:
Harmen Stoppels 2023-08-03 10:44:02 +02:00 committed by GitHub
parent 41d2161b5b
commit 2069a42ba3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 116 additions and 160 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
lib/spack/llnl/util/tty/__init__.py
View file

@ -12,6 +12,7 @@
import traceback
from datetime import datetime
from sys import platform as _platform
from typing import NoReturn
if _platform != "win32":
import fcntl
@ -244,7 +245,7 @@ def warn(message, *args, **kwargs):
info("Warning: " + str(message), *args, **kwargs)
def die(message, *args, **kwargs):
def die(message, *args, **kwargs) -> NoReturn:
kwargs.setdefault("countback", 4)
error(message, *args, **kwargs)
sys.exit(1)

16
lib/spack/spack/cmd/__init__.py
View file

@ -584,14 +584,14 @@ def require_active_env(cmd_name):
if env:
return env
else:
tty.die(
"`spack %s` requires an environment" % cmd_name,
"activate an environment first:",
" spack env activate ENV",
"or use:",
" spack -e ENV %s ..." % cmd_name,
)
tty.die(
"`spack %s` requires an environment" % cmd_name,
"activate an environment first:",
" spack env activate ENV",
"or use:",
" spack -e ENV %s ..." % cmd_name,
)
def find_environment(args):

205
lib/spack/spack/cmd/buildcache.py
View file

@ -2,12 +2,14 @@
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import argparse
import glob
import json
import os
import shutil
import sys
import tempfile
from typing import List
import llnl.util.tty as tty
import llnl.util.tty.color as clr
@ -18,7 +20,6 @@
import spack.cmd.common.arguments as arguments
import spack.config
import spack.environment as ev
import spack.hash_types as ht
import spack.mirror
import spack.relocate
import spack.repo
@ -28,7 +29,6 @@
import spack.util.url as url_util
import spack.util.web as web_util
from spack.cmd import display_specs
from spack.error import SpecError
from spack.spec import Spec, save_dependency_specfiles
from spack.stage import Stage
from spack.util.string import plural
@ -38,8 +38,8 @@
level = "long"
def setup_parser(subparser):
setup_parser.parser = subparser
def setup_parser(subparser: argparse.ArgumentParser):
setattr(setup_parser, "parser", subparser)
subparsers = subparser.add_subparsers(help="buildcache sub-commands")
push = subparsers.add_parser("push", aliases=["create"], help=push_fn.__doc__)
@ -158,14 +158,12 @@ def setup_parser(subparser):
default=spack.config.default_modify_scope(),
help="configuration scope containing mirrors to check",
)
check.add_argument(
"-s", "--spec", default=None, help="check single spec instead of release specs file"
check_spec_or_specfile = check.add_mutually_exclusive_group(required=True)
check_spec_or_specfile.add_argument(
"-s", "--spec", help="check single spec instead of release specs file"
)
check.add_argument(
check_spec_or_specfile.add_argument(
"--spec-file",
default=None,
help="check single spec from json or yaml file instead of release specs file",
)
@ -173,16 +171,19 @@ def setup_parser(subparser):
# Download tarball and specfile
download = subparsers.add_parser("download", help=download_fn.__doc__)
download.add_argument(
"-s", "--spec", default=None, help="download built tarball for spec from mirror"
download_spec_or_specfile = download.add_mutually_exclusive_group(required=True)
download_spec_or_specfile.add_argument(
"-s", "--spec", help="download built tarball for spec from mirror"
)
download_spec_or_specfile.add_argument(
"--spec-file", help="download built tarball for spec (from json or yaml file) from mirror"
)
download.add_argument(
"--spec-file",
"-p",
"--path",
required=True,
default=None,
help="download built tarball for spec (from json or yaml file) from mirror",
)
download.add_argument(
"-p", "--path", default=None, help="path to directory where tarball should be downloaded"
help="path to directory where tarball should be downloaded",
)
download.set_defaults(func=download_fn)
@ -190,32 +191,32 @@ def setup_parser(subparser):
getbuildcachename = subparsers.add_parser(
"get-buildcache-name", help=get_buildcache_name_fn.__doc__
)
getbuildcachename.add_argument(
"-s", "--spec", default=None, help="spec string for which buildcache name is desired"
getbuildcachename_spec_or_specfile = getbuildcachename.add_mutually_exclusive_group(
required=True
)
getbuildcachename.add_argument(
"--spec-file",
default=None,
help="path to spec json or yaml file for which buildcache name is desired",
getbuildcachename_spec_or_specfile.add_argument(
"-s", "--spec", help="spec string for which buildcache name is desired"
)
getbuildcachename_spec_or_specfile.add_argument(
"--spec-file", help="path to spec json or yaml file for which buildcache name is desired"
)
getbuildcachename.set_defaults(func=get_buildcache_name_fn)
# Given the root spec, save the yaml of the dependent spec to a file
savespecfile = subparsers.add_parser("save-specfile", help=save_specfile_fn.__doc__)
savespecfile.add_argument("--root-spec", default=None, help="root spec of dependent spec")
savespecfile.add_argument(
"--root-specfile",
default=None,
help="path to json or yaml file containing root spec of dependent spec",
savespecfile_spec_or_specfile = savespecfile.add_mutually_exclusive_group(required=True)
savespecfile_spec_or_specfile.add_argument("--root-spec", help="root spec of dependent spec")
savespecfile_spec_or_specfile.add_argument(
"--root-specfile", help="path to json or yaml file containing root spec of dependent spec"
)
savespecfile.add_argument(
"-s",
"--specs",
default=None,
required=True,
help="list of dependent specs for which saved yaml is desired",
)
savespecfile.add_argument(
"--specfile-dir", default=None, help="path to directory where spec yamls should be saved"
"--specfile-dir", required=True, help="path to directory where spec yamls should be saved"
)
savespecfile.set_defaults(func=save_specfile_fn)
@ -257,54 +258,24 @@ def setup_parser(subparser):
update_index.set_defaults(func=update_index_fn)
def _matching_specs(specs, spec_file):
"""Return a list of matching specs read from either a spec file (JSON or YAML),
a query over the store or a query over the active environment.
"""
env = ev.active_environment()
hashes = env.all_hashes() if env else None
if spec_file:
return spack.store.specfile_matches(spec_file, hashes=hashes)
if specs:
constraints = spack.cmd.parse_specs(specs)
return spack.store.find(constraints, hashes=hashes)
if env:
return [concrete for _, concrete in env.concretized_specs()]
tty.die(
"build cache file creation requires at least one"
" installed package spec, an active environment,"
" or else a path to a json or yaml file containing a spec"
" to install"
)
def _concrete_spec_from_args(args):
spec_str, specfile_path = args.spec, args.spec_file
if not spec_str and not specfile_path:
tty.error("must provide either spec string or path to YAML or JSON specfile")
sys.exit(1)
if spec_str:
try:
constraints = spack.cmd.parse_specs(spec_str)
spec = spack.store.find(constraints)[0]
spec.concretize()
except SpecError as spec_error:
tty.error("Unable to concretize spec {0}".format(spec_str))
tty.debug(spec_error)
sys.exit(1)
return spec
return Spec.from_specfile(specfile_path)
def _matching_specs(specs: List[Spec]) -> List[Spec]:
"""Disambiguate specs and return a list of matching specs"""
return [spack.cmd.disambiguate_spec(s, ev.active_environment(), installed=any) for s in specs]
def push_fn(args):
"""create a binary package and push it to a mirror"""
if args.spec_file:
tty.warn(
"The flag `--spec-file` is deprecated and will be removed in Spack 0.22. "
"Use positional arguments instead."
)
if args.specs or args.spec_file:
specs = _matching_specs(spack.cmd.parse_specs(args.specs or args.spec_file))
else:
specs = spack.cmd.require_active_env("buildcache push").all_specs()
mirror = arguments.mirror_name_or_url(args.mirror)
if args.allow_root:
@ -315,7 +286,7 @@ def push_fn(args):
url = mirror.push_url
specs = bindist.specs_to_be_packaged(
_matching_specs(args.specs, args.spec_file),
specs,
root="package" in args.things_to_install,
dependencies="dependencies" in args.things_to_install,
)
@ -423,16 +394,21 @@ def preview_fn(args):
def check_fn(args):
"""check specs against remote binary mirror(s) to see if any need to be rebuilt
either a single spec from --spec, or else the full set of release specs. this command uses the
process exit code to indicate its result, specifically, if the exit code is non-zero, then at
least one of the indicated specs needs to be rebuilt
this command uses the process exit code to indicate its result, specifically, if the
exit code is non-zero, then at least one of the indicated specs needs to be rebuilt
"""
if args.spec or args.spec_file:
specs = [_concrete_spec_from_args(args)]
if args.spec_file:
tty.warn(
"The flag `--spec-file` is deprecated and will be removed in Spack 0.22. "
"Use --spec instead."
)
specs = spack.cmd.parse_specs(args.spec or args.spec_file)
if specs:
specs = _matching_specs(specs, specs)
else:
env = spack.cmd.require_active_env(cmd_name="buildcache")
env.concretize()
specs = env.all_specs()
specs = spack.cmd.require_active_env("buildcache check").all_specs()
if not specs:
tty.msg("No specs provided, exiting.")
@ -462,26 +438,28 @@ def download_fn(args):
code indicates that the command failed to download at least one of the required buildcache
components
"""
if not args.spec and not args.spec_file:
tty.msg("No specs provided, exiting.")
return
if args.spec_file:
tty.warn(
"The flag `--spec-file` is deprecated and will be removed in Spack 0.22. "
"Use --spec instead."
)
if not args.path:
tty.msg("No download path provided, exiting")
return
specs = _matching_specs(spack.cmd.parse_specs(args.spec or args.spec_file))
spec = _concrete_spec_from_args(args)
result = bindist.download_single_spec(spec, args.path)
if len(specs) != 1:
tty.die("a single spec argument is required to download from a buildcache")
if not result:
if not bindist.download_single_spec(specs[0], args.path):
sys.exit(1)
def get_buildcache_name_fn(args):
"""get name (prefix) of buildcache entries for this spec"""
spec = _concrete_spec_from_args(args)
buildcache_name = bindist.tarball_name(spec, "")
print("{0}".format(buildcache_name))
tty.warn("This command is deprecated and will be removed in Spack 0.22.")
specs = _matching_specs(spack.cmd.parse_specs(args.spec or args.spec_file))
if len(specs) != 1:
tty.die("a single spec argument is required to get buildcache name")
print(bindist.tarball_name(specs[0], ""))
def save_specfile_fn(args):
@ -491,29 +469,24 @@ def save_specfile_fn(args):
successful. if any errors or exceptions are encountered, or if expected command-line arguments
are not provided, then the exit code will be non-zero
"""
if not args.root_spec and not args.root_specfile:
tty.msg("No root spec provided, exiting.")
sys.exit(1)
if not args.specs:
tty.msg("No dependent specs provided, exiting.")
sys.exit(1)
if not args.specfile_dir:
tty.msg("No yaml directory provided, exiting.")
sys.exit(1)
if args.root_specfile:
with open(args.root_specfile) as fd:
root_spec_as_json = fd.read()
spec_format = "yaml" if args.root_specfile.endswith("yaml") else "json"
else:
root_spec = Spec(args.root_spec)
root_spec.concretize()
root_spec_as_json = root_spec.to_json(hash=ht.dag_hash)
spec_format = "json"
tty.warn(
"The flag `--root-specfile` is deprecated and will be removed in Spack 0.22. "
"Use --root-spec instead."
)
specs = spack.cmd.parse_specs(args.root_spec or args.root_specfile)
if len(specs) != 1:
tty.die("a single spec argument is required to save specfile")
root = specs[0]
if not root.concrete:
root.concretize()
save_dependency_specfiles(
root_spec_as_json, args.specfile_dir, args.specs.split(), spec_format
root, args.specfile_dir, dependencies=spack.cmd.parse_specs(args.specs)
)

17
lib/spack/spack/environment/environment.py
View file

@ -16,7 +16,7 @@
import urllib.parse
import urllib.request
import warnings
from typing import Any, Dict, List, Optional, Set, Tuple, Union
from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union
import llnl.util.filesystem as fs
import llnl.util.tty as tty
@ -1921,16 +1921,17 @@ def install_specs(self, specs=None, **install_args):
"Could not install log links for {0}: {1}".format(spec.name, str(e))
)
def all_specs(self):
"""Return all specs, even those a user spec would shadow."""
roots = [self.specs_by_hash[h] for h in self.concretized_order]
specs = [s for s in traverse.traverse_nodes(roots, key=traverse.by_dag_hash)]
specs.sort()
return specs
def all_specs_generator(self) -> Iterable[Spec]:
"""Returns a generator for all concrete specs"""
return traverse.traverse_nodes(self.concrete_roots(), key=traverse.by_dag_hash)
def all_specs(self) -> List[Spec]:
"""Returns a list of all concrete specs"""
return list(self.all_specs_generator())
def all_hashes(self):
"""Return hashes of all specs."""
return [s.dag_hash() for s in self.all_specs()]
return [s.dag_hash() for s in self.all_specs_generator()]
def roots(self):
"""Specs explicitly requested by the user *in this environment*.

32
lib/spack/spack/spec.py
View file

@ -56,7 +56,7 @@
import os
import re
import warnings
from typing import Tuple, Union
from typing import List, Tuple, Union
import llnl.util.filesystem as fs
import llnl.util.lang as lang
@ -5147,9 +5147,7 @@ def __missing__(self, key):
return value
def save_dependency_specfiles(
root_spec_info, output_directory, dependencies=None, spec_format="json"
):
def save_dependency_specfiles(root: Spec, output_directory: str, dependencies: List[Spec]):
"""Given a root spec (represented as a yaml object), index it with a subset
of its dependencies, and write each dependency to a separate yaml file
in the output directory. By default, all dependencies will be written
@ -5158,26 +5156,15 @@ def save_dependency_specfiles(
incoming spec is not json, that can be specified with the spec_format
parameter. This can be used to convert from yaml specfiles to the
json format."""
if spec_format == "json":
root_spec = Spec.from_json(root_spec_info)
elif spec_format == "yaml":
root_spec = Spec.from_yaml(root_spec_info)
else:
raise SpecParseError("Unrecognized spec format {0}.".format(spec_format))
dep_list = dependencies
if not dep_list:
dep_list = [dep.name for dep in root_spec.traverse()]
for spec in root.traverse():
if not any(spec.satisfies(dep) for dep in dependencies):
continue
for dep_name in dep_list:
if dep_name not in root_spec:
msg = "Dependency {0} does not exist in root spec {1}".format(dep_name, root_spec.name)
raise SpecDependencyNotFoundError(msg)
dep_spec = root_spec[dep_name]
json_path = os.path.join(output_directory, "{0}.json".format(dep_name))
json_path = os.path.join(output_directory, f"{spec.name}.json")
with open(json_path, "w") as fd:
fd.write(dep_spec.to_json(hash=ht.dag_hash))
fd.write(spec.to_json(hash=ht.dag_hash))
class SpecParseError(spack.error.SpecError):
@ -5398,11 +5385,6 @@ def __init__(self, spec, matches):
super().__init__(message, long_message)
class SpecDependencyNotFoundError(spack.error.SpecError):
"""Raised when a failure is encountered writing the dependencies of
a spec."""
class SpecDeprecatedError(spack.error.SpecError):
"""Raised when a spec concretizes to a deprecated spec or dependency."""

3
lib/spack/spack/test/spec_yaml.py
View file

@ -326,9 +326,8 @@ def test_save_dependency_spec_jsons_subset(tmpdir, config):
spec_a = Spec("a").concretized()
b_spec = spec_a["b"]
c_spec = spec_a["c"]
spec_a_json = spec_a.to_json()
save_dependency_specfiles(spec_a_json, output_path, ["b", "c"])
save_dependency_specfiles(spec_a, output_path, [Spec("b"), Spec("c")])
assert check_specs_equal(b_spec, os.path.join(output_path, "b.json"))
assert check_specs_equal(c_spec, os.path.join(output_path, "c.json"))