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

@@ -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 <spec>')
+    env = ev.get_env(args, 'env remove <spec>')
 
     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 = ev.get_env(args, 'env concretize')
-    _env_concretize(env, use_repo=args.use_env_repo, force=args.force)
+    env.concretize(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.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

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',
+        '--only-concrete', action='store_true', default=False,
-        default='package,dependencies',
+        help='(with environment) only install already concretized specs')
-        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"""
-    )
     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 there are no args but an active environment or spack.yaml file
-        if os.path.exists(ev.manifest_name):
+        # then install the packages from it.
-            env = ev.Environment(os.getcwd())
+        env = ev.get_env(args, 'install', required=False)
+        if env:
+            if not args.only_concrete:
                 env.concretize()
                 env.write()
-            env.install_all()
+            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")

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:

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):