From e63b45b29309c42aa1c58af9138634f04483c007 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sat, 27 Oct 2018 12:05:03 -0700 Subject: [PATCH] env: moved all `spack env install` functionality into `spack install` - moved get_env from cmd/env.py to environment.py - spack install will now install into the active environment when no arguments are provided. It looks: 1. at the command line 2. for a local spack.yaml file 3. for any currently activated environment --- lib/spack/spack/cmd/env.py | 83 +++------------------------- lib/spack/spack/cmd/install.py | 98 +++++++++++++++++---------------- lib/spack/spack/environment.py | 47 ++++++++++++++++ lib/spack/spack/test/cmd/env.py | 16 ++++-- 4 files changed, 115 insertions(+), 129 deletions(-) diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index c0687a56037..478b9910101 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -39,46 +39,10 @@ ['status', 'st'], 'loads', 'stage', - 'install', 'uninstall', ] -def get_env(args, cmd_name, fail_on_error=True): - """Get target environment from args, or from environment variables. - - This is used by a number of commands for handling the environment - argument. - - Check whether an environment was passed via arguments, then whether - it was passed via SPACK_ENV. If an environment is found, read it in. - If not, print an error message referencing the calling command. - - Arguments: - args (Namespace): argparse namespace wtih command arguments - cmd_name (str): name of calling command - - """ - env = args.env - if not env: - if ev.active: - return ev.active - elif not fail_on_error: - return None - tty.die( - '`spack env %s` requires an environment' % cmd_name, - 'activate an environment first:', - ' spack env activate ENV', - 'or use:', - ' spack -e ENV %s ...' % cmd_name) - - environment = ev.disambiguate(env) - - if not environment: - tty.die('no such environment: %s' % env) - return environment - - # # env activate # @@ -316,7 +280,7 @@ def env_add_setup_parser(subparser): def env_add(args): - env = get_env(args, 'add') + env = ev.get_env(args, 'env add') for spec in spack.cmd.parse_specs(args.specs): if not env.add(spec): @@ -340,7 +304,7 @@ def env_remove_setup_parser(subparser): def env_remove(args): - env = get_env(args, 'remove ') + env = ev.get_env(args, 'env remove ') if args.all: env.clear() @@ -364,41 +328,10 @@ def env_concretize_setup_parser(subparser): def env_concretize(args): - env = get_env(args, 'status') - _env_concretize(env, use_repo=args.use_env_repo, force=args.force) - - -def _env_concretize(env, use_repo=False, force=False): - """Function body separated out to aid in testing.""" - env.concretize(force=force) + env = ev.get_env(args, 'env concretize') + env.concretize(force=args.force) env.write() - -# REMOVE -# env install -# -def env_install_setup_parser(subparser): - """concretize and install all specs in an environment""" - subparser.add_argument( - 'env', nargs='?', help='install all packages in this environment') - subparser.add_argument( - '--only-concrete', action='store_true', default=False, - help='only install already concretized specs') - spack.cmd.install.add_common_arguments(subparser) - - -def env_install(args): - env = get_env(args, 'status') - - # concretize unless otherwise specified - if not args.only_concrete: - env.concretize() - env.write() - - # install all specs in the environment - env.install_all(args) - - # REMOVE # env uninstall # @@ -410,7 +343,7 @@ def env_uninstall_setup_parser(subparser): def env_uninstall(args): - env = get_env(args, 'uninstall') + env = ev.get_env(args, 'env uninstall') env.uninstall(args) @@ -427,7 +360,7 @@ def env_status_setup_parser(subparser): def env_status(args): - env = get_env(args, 'status', fail_on_error=False) + env = ev.get_env(args, 'env status', required=False) if not env: tty.msg('No active environment') return @@ -450,7 +383,7 @@ def env_stage_setup_parser(subparser): def env_stage(args): - env = get_env(args, 'stage') + env = ev.get_env(args, 'env stage') for spec in env.specs_by_hash.values(): for dep in spec.traverse(): dep.package.do_stage() @@ -470,7 +403,7 @@ def env_loads_setup_parser(subparser): def env_loads(args): - env = get_env(args, 'loads') + env = ev.get_env(args, 'env loads') # Set the module types that have been selected module_type = args.module_type diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index a01c1a9e0a8..d1aaf20f1b2 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -26,7 +26,41 @@ level = "short" -def add_common_arguments(subparser): +def update_kwargs_from_args(args, kwargs): + """Parse cli arguments and construct a dictionary + that will be passed to Package.do_install API""" + + kwargs.update({ + 'keep_prefix': args.keep_prefix, + 'keep_stage': args.keep_stage, + 'restage': not args.dont_restage, + 'install_source': args.install_source, + 'make_jobs': args.jobs, + 'verbose': args.verbose, + 'fake': args.fake, + 'dirty': args.dirty, + 'use_cache': args.use_cache + }) + if hasattr(args, 'setup'): + setups = set() + for arglist_s in args.setup: + for arg in [x.strip() for x in arglist_s.split(',')]: + setups.add(arg) + kwargs['setup'] = setups + tty.msg('Setup={0}'.format(kwargs['setup'])) + + +def setup_parser(subparser): + subparser.add_argument( + '--only', + default='package,dependencies', + dest='things_to_install', + choices=['package', 'dependencies'], + help="""select the mode of installation. +the default is to install the package along with all its dependencies. +alternatively one can decide to install only the package or only +the dependencies""" + ) arguments.add_common_arguments(subparser, ['jobs', 'install_status']) subparser.add_argument( '--overwrite', action='store_true', @@ -56,52 +90,17 @@ def add_common_arguments(subparser): subparser.add_argument( '--fake', action='store_true', help="fake install for debug purposes.") - - cd_group = subparser.add_mutually_exclusive_group() - arguments.add_common_arguments(cd_group, ['clean', 'dirty']) - - -def update_kwargs_from_args(args, kwargs): - """Parse cli arguments and construct a dictionary - that will be passed to Package.do_install API""" - - kwargs.update({ - 'keep_prefix': args.keep_prefix, - 'keep_stage': args.keep_stage, - 'restage': not args.dont_restage, - 'install_source': args.install_source, - 'make_jobs': args.jobs, - 'verbose': args.verbose, - 'fake': args.fake, - 'dirty': args.dirty, - 'use_cache': args.use_cache - }) - if hasattr(args, 'setup'): - setups = set() - for arglist_s in args.setup: - for arg in [x.strip() for x in arglist_s.split(',')]: - setups.add(arg) - kwargs['setup'] = setups - tty.msg('Setup={0}'.format(kwargs['setup'])) - - -def setup_parser(subparser): - add_common_arguments(subparser) subparser.add_argument( - '--only', - default='package,dependencies', - dest='things_to_install', - choices=['package', 'dependencies'], - help="""select the mode of installation. -the default is to install the package along with all its dependencies. -alternatively one can decide to install only the package or only -the dependencies""" - ) + '--only-concrete', action='store_true', default=False, + help='(with environment) only install already concretized specs') subparser.add_argument( '-f', '--file', action='append', default=[], dest='specfiles', metavar='SPEC_YAML_FILE', help="install from file. Read specs to install from .yaml files") + cd_group = subparser.add_mutually_exclusive_group() + arguments.add_common_arguments(cd_group, ['clean', 'dirty']) + subparser.add_argument( 'package', nargs=argparse.REMAINDER, @@ -155,7 +154,7 @@ def install_spec(cli_args, kwargs, spec): # handle active environment, if any def install(spec, kwargs): - env = spack.environment.active + env = ev.get_env(cli_args, 'install', required=False) if env: env.install(spec, kwargs) env.write() @@ -187,12 +186,15 @@ def install(spec, kwargs): def install(parser, args, **kwargs): if not args.package and not args.specfiles: - # if there is a spack.yaml file, then install the packages in it. - if os.path.exists(ev.manifest_name): - env = ev.Environment(os.getcwd()) - env.concretize() - env.write() - env.install_all() + # if there are no args but an active environment or spack.yaml file + # then install the packages from it. + env = ev.get_env(args, 'install', required=False) + if env: + if not args.only_concrete: + env.concretize() + env.write() + tty.msg("Installing environment %s" % env.name) + env.install_all(args) return else: tty.die("install requires a package argument or a spack.yaml file") diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 1326c382b97..8ba9ac976e8 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -132,6 +132,53 @@ def deactivate(): active = None +def get_env(args, cmd_name, required=True): + """Get target environment from args, environment variables, or spack.yaml. + + This is used by a number of commands for determining whether there is + an active environment. + + Check for an environment: + 1. via spack -e ENV + 2. as a spack.yaml file in the current directory, or + 3. via the SPACK_ENV environment variable. + + If an environment is found, read it in. If not, print an error + message referencing the calling command. + + Arguments: + args (Namespace): argparse namespace wtih command arguments + cmd_name (str): name of calling command + + """ + # try arguments + env = args.env + + # try a manifest file in the current directory + if not env: + if os.path.exists(manifest_name): + env = os.getcwd() + + # try the active environment via SPACK_ENV environment variable + if not env: + if active: + return active + elif not required: + return None + 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) + + environment = disambiguate(env) + + if not environment: + tty.die('no such environment: %s' % env) + return environment + + def disambiguate(env, env_dir=None): """Used to determine whether an environment is named or a directory.""" if env: diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 1dc6695353a..87ba5249ef8 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -12,7 +12,7 @@ import spack.modules import spack.environment as ev -from spack.cmd.env import _env_concretize, _env_create +from spack.cmd.env import _env_create from spack.spec import Spec from spack.main import SpackCommand @@ -23,6 +23,7 @@ env = SpackCommand('env') +install = SpackCommand('install') def test_add(): @@ -117,7 +118,6 @@ def test_env_install_single_spec(install_mockery, mock_fetch): def test_env_install_same_spec_twice(install_mockery, mock_fetch, capfd): env('create', 'test') - install = SpackCommand('install') e = ev.read('test') with capfd.disabled(): @@ -193,7 +193,9 @@ def test_to_lockfile_dict(): def test_env_repo(): e = ev.create('test') e.add('mpileaks') - _env_concretize(e) + e.write() + + env('concretize', 'test') package = e.repo.get('mpileaks') assert package.name == 'mpileaks' @@ -486,8 +488,12 @@ def test_env_loads(install_mockery, mock_fetch): with ev.read('test'): env('add', 'mpileaks') + env('concretize', 'test') - env('install', '--fake', 'test') + + with ev.read('test'): + install('--fake') + env('loads', 'test') e = ev.read('test') @@ -535,8 +541,6 @@ def test_env_commands_die_with_no_env_arg(): env('loads') with pytest.raises(spack.main.SpackCommandError): env('stage') - with pytest.raises(spack.main.SpackCommandError): - env('install') with pytest.raises(spack.main.SpackCommandError): env('uninstall') with pytest.raises(spack.main.SpackCommandError):