New spack.environment.active_environment api, and make spack.environment not depend on spack.cmd. (#25439)

* Refactor active environment getters

- Make `spack.environment.active_environment` a trivial getter for the active
environment, replacing `spack.environment.get_env` when the arguments are
not needed
- New method `spack.cmd.require_active_environment(cmd_name)` for 
commands that require an environment (rather than abusing 
get_env/active_environment)
- Clean up calling code to call spack.environment.active_environment or
spack.cmd.require_active_environment as appropriate
- Remove the `-e` parsing from `active_environment`, because `main.py` is
responsible for processing `-e` and already activates the environment.
- Move `spack.environment.find_environment` to
`spack.cmd.find_environment`, to avoid having spack.environment aware
of argparse.
- Refactor `spack install` command so argument parsing is all handled in the
command, no argparse in spack.environment or spack.installer
- Update documentation

* Python 2: toplevel import errors only with 'as ev'

In two files, `import spack.environment as ev` leads to errors
These errors are not well understood ("'module' object has no attribute
'environment'"). All other files standardize on the above syntax.

lib/spack/llnl/util/lang.py

@@ -933,3 +933,19 @@ class Devnull(object):
     """
     def write(self, *_):
         pass
+
+
+def elide_list(line_list, max_num=10):
+    """Takes a long list and limits it to a smaller number of elements,
+       replacing intervening elements with '...'.  For example::
+
+           elide_list([1,2,3,4,5,6], 4)
+
+       gives::
+
+           [1, 2, 3, '...', 6]
+    """
+    if len(line_list) > max_num:
+        return line_list[:max_num - 1] + ['...'] + line_list[-1:]
+    else:
+        return line_list

lib/spack/spack/architecture.py

@@ -72,6 +72,7 @@
 import spack.config
 import spack.error as serr
 import spack.paths
+import spack.spec
 import spack.util.classes
 import spack.util.executable
 import spack.version
@@ -197,7 +198,6 @@ def optimization_flags(self, compiler):
                 contains both the name and the version of the compiler we want to use
         """
         # Mixed toolchains are not supported yet
-        import spack.compilers
         if isinstance(compiler, spack.compiler.Compiler):
             if spack.compilers.is_mixed_toolchain(compiler):
                 msg = ('microarchitecture specific optimizations are not '
@@ -218,7 +218,6 @@ def optimization_flags(self, compiler):
             # of its name in compilers.yaml. Depending on where this function
             # is called we might get either a CompilerSpec or a fully fledged
             # compiler object.
-            import spack.spec
             if isinstance(compiler, spack.spec.CompilerSpec):
                 compiler = spack.compilers.compilers_for_spec(compiler).pop()
             try:

lib/spack/spack/cmd/__init__.py

@@ -21,6 +21,7 @@
 from llnl.util.tty.color import colorize
 
 import spack.config
+import spack.environment as ev
 import spack.error
 import spack.extensions
 import spack.paths
@@ -186,29 +187,13 @@ def matching_spec_from_env(spec):
     If no matching spec is found in the environment (or if no environment is
     active), this will return the given spec but concretized.
     """
-    env = spack.environment.get_env({}, cmd_name)
+    env = ev.active_environment()
     if env:
         return env.matching_spec(spec) or spec.concretized()
     else:
         return spec.concretized()
 
 
-def elide_list(line_list, max_num=10):
-    """Takes a long list and limits it to a smaller number of elements,
-       replacing intervening elements with '...'.  For example::
-
-           elide_list([1,2,3,4,5,6], 4)
-
-       gives::
-
-           [1, 2, 3, '...', 6]
-    """
-    if len(line_list) > max_num:
-        return line_list[:max_num - 1] + ['...'] + line_list[-1:]
-    else:
-        return line_list
-
-
 def disambiguate_spec(spec, env, local=False, installed=True, first=False):
     """Given a spec, figure out which installed package it refers to.
 
@@ -501,3 +486,71 @@ def extant_file(f):
     if not os.path.isfile(f):
         raise argparse.ArgumentTypeError('%s does not exist' % f)
     return f
+
+
+def require_active_env(cmd_name):
+    """Used by commands to get the active environment
+
+    If an environment is not found, print an error message that says the calling
+    command *needs* an active environment.
+
+    Arguments:
+        cmd_name (str): name of calling command
+
+    Returns:
+        (spack.environment.Environment): the active environment
+    """
+    env = ev.active_environment()
+
+    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)
+
+
+def find_environment(args):
+    """Find active environment from args or environment variable.
+
+    Check for an environment in this order:
+        1. via ``spack -e ENV`` or ``spack -D DIR`` (arguments)
+        2. via a path in the spack.environment.spack_env_var environment variable.
+
+    If an environment is found, read it in.  If not, return None.
+
+    Arguments:
+        args (argparse.Namespace): argparse namespace with command arguments
+
+    Returns:
+        (spack.environment.Environment): a found environment, or ``None``
+    """
+
+    # treat env as a name
+    env = args.env
+    if env:
+        if ev.exists(env):
+            return ev.read(env)
+
+    else:
+        # if env was specified, see if it is a directory otherwise, look
+        # at env_dir (env and env_dir are mutually exclusive)
+        env = args.env_dir
+
+        # if no argument, look for the environment variable
+        if not env:
+            env = os.environ.get(ev.spack_env_var)
+
+            # nothing was set; there's no active environment
+            if not env:
+                return None
+
+    # if we get here, env isn't the name of a spack environment; it has
+    # to be a path to an environment, or there is something wrong.
+    if ev.is_env_dir(env):
+        return ev.Environment(env)
+
+    raise ev.SpackEnvironmentError('no environment in %s' % env)

lib/spack/spack/cmd/activate.py

@@ -30,8 +30,7 @@ def activate(parser, args):
     if len(specs) != 1:
         tty.die("activate requires one spec.  %d given." % len(specs))
 
-    env = ev.get_env(args, 'activate')
+    spec = spack.cmd.disambiguate_spec(specs[0], ev.active_environment())
-    spec = spack.cmd.disambiguate_spec(specs[0], env)
     if not spec.package.is_extension:
         tty.die("%s is not an extension." % spec.name)
 

lib/spack/spack/cmd/add.py

@@ -7,7 +7,6 @@
 
 import spack.cmd
 import spack.cmd.common.arguments as arguments
-import spack.environment as ev
 
 description = 'add a spec to an environment'
 section = "environments"
@@ -22,7 +21,7 @@ def setup_parser(subparser):
 
 
 def add(parser, args):
-    env = ev.get_env(args, 'add', required=True)
+    env = spack.cmd.require_active_env(cmd_name='add')
 
     with env.write_transaction():
         for spec in spack.cmd.parse_specs(args.specs):

lib/spack/spack/cmd/analyze.py

@@ -95,7 +95,7 @@ def analyze(parser, args, **kwargs):
         sys.exit(0)
 
     # handle active environment, if any
-    env = ev.get_env(args, 'analyze')
+    env = ev.active_environment()
 
     # Get an disambiguate spec (we should only have one)
     specs = spack.cmd.parse_specs(args.spec)

lib/spack/spack/cmd/buildcache.py

@@ -457,7 +457,7 @@ def createtarball(args):
     """create a binary package from an existing install"""
 
     # restrict matching to current environment if one is active
-    env = ev.get_env(args, 'buildcache create')
+    env = ev.active_environment()
 
     output_location = None
     if args.directory:
@@ -601,7 +601,7 @@ def check_binaries(args):
     if args.spec or args.spec_yaml:
         specs = [get_concrete_spec(args)]
     else:
-        env = ev.get_env(args, 'buildcache', required=True)
+        env = spack.cmd.require_active_env(cmd_name='buildcache')
         env.concretize()
         specs = env.all_specs()
 

lib/spack/spack/cmd/ci.py

@@ -118,7 +118,7 @@ def ci_generate(args):
        for creating a build group for the generated workload and registering
        all generated jobs under that build group.  If this environment
        variable is not set, no build group will be created on CDash."""
-    env = ev.get_env(args, 'ci generate', required=True)
+    env = spack.cmd.require_active_env(cmd_name='ci generate')
 
     output_file = args.output_file
     copy_yaml_to = args.copy_to
@@ -152,7 +152,7 @@ def ci_generate(args):
 def ci_reindex(args):
     """Rebuild the buildcache index associated with the mirror in the
        active, gitlab-enabled environment. """
-    env = ev.get_env(args, 'ci rebuild-index', required=True)
+    env = spack.cmd.require_active_env(cmd_name='ci rebuild-index')
     yaml_root = ev.config_dict(env.yaml)
 
     if 'mirrors' not in yaml_root or len(yaml_root['mirrors'].values()) < 1:
@@ -169,7 +169,7 @@ def ci_rebuild(args):
     """Check a single spec against the remote mirror, and rebuild it from
        source if the mirror does not contain the full hash match of the spec
        as computed locally. """
-    env = ev.get_env(args, 'ci rebuild', required=True)
+    env = spack.cmd.require_active_env(cmd_name='ci rebuild')
 
     # Make sure the environment is "gitlab-enabled", or else there's nothing
     # to do.

lib/spack/spack/cmd/concretize.py

@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import spack.cmd
 import spack.environment as ev
 
 description = 'concretize an environment and write a lockfile'
@@ -23,7 +24,7 @@ def setup_parser(subparser):
 
 
 def concretize(parser, args):
-    env = ev.get_env(args, 'concretize', required=True)
+    env = spack.cmd.require_active_env(cmd_name='concretize')
 
     if args.test == 'all':
         tests = True

lib/spack/spack/cmd/config.py

@@ -118,7 +118,7 @@ def _get_scope_and_section(args):
 
     # w/no args and an active environment, point to env manifest
     if not section:
-        env = ev.get_env(args, 'config edit')
+        env = ev.active_environment()
         if env:
             scope = env.env_file_config_scope_name()
 

lib/spack/spack/cmd/deactivate.py

@@ -36,7 +36,7 @@ def deactivate(parser, args):
     if len(specs) != 1:
         tty.die("deactivate requires one spec.  %d given." % len(specs))
 
-    env = ev.get_env(args, 'deactivate')
+    env = ev.active_environment()
     spec = spack.cmd.disambiguate_spec(specs[0], env)
     pkg = spec.package
 

lib/spack/spack/cmd/dependencies.py

@@ -41,7 +41,7 @@ def dependencies(parser, args):
         tty.die("spack dependencies takes only one spec.")
 
     if args.installed:
-        env = ev.get_env(args, 'dependencies')
+        env = ev.active_environment()
         spec = spack.cmd.disambiguate_spec(specs[0], env)
 
         format_string = '{name}{@version}{%compiler}{/hash:7}'

lib/spack/spack/cmd/dependents.py

@@ -82,7 +82,7 @@ def dependents(parser, args):
         tty.die("spack dependents takes only one spec.")
 
     if args.installed:
-        env = ev.get_env(args, 'dependents')
+        env = ev.active_environment()
         spec = spack.cmd.disambiguate_spec(specs[0], env)
 
         format_string = '{name}{@version}{%compiler}{/hash:7}'

lib/spack/spack/cmd/deprecate.py

@@ -71,7 +71,7 @@ def setup_parser(sp):
 
 def deprecate(parser, args):
     """Deprecate one spec in favor of another"""
-    env = ev.get_env(args, 'deprecate')
+    env = ev.active_environment()
     specs = spack.cmd.parse_specs(args.specs)
 
     if len(specs) != 2:

lib/spack/spack/cmd/develop.py

@@ -9,7 +9,6 @@
 
 import spack.cmd
 import spack.cmd.common.arguments as arguments
-import spack.environment as ev
 from spack.error import SpackError
 
 description = "add a spec to an environment's dev-build information"
@@ -37,7 +36,7 @@ def setup_parser(subparser):
 
 
 def develop(parser, args):
-    env = ev.get_env(args, 'develop', required=True)
+    env = spack.cmd.require_active_env(cmd_name='develop')
 
     if not args.spec:
         if args.clone is False:

lib/spack/spack/cmd/diff.py

@@ -175,7 +175,7 @@ def group_by_type(diffset):
 
 
 def diff(parser, args):
-    env = ev.get_env(args, 'diff')
+    env = ev.active_environment()
 
     if len(args.specs) != 2:
         tty.die("You must provide two specs to diff.")

lib/spack/spack/cmd/env.py

@@ -6,7 +6,6 @@
 import os
 import shutil
 import sys
-from collections import namedtuple
 
 import llnl.util.filesystem as fs
 import llnl.util.tty as tty
@@ -106,10 +105,8 @@ def env_activate(args):
         tty.debug("Environment %s is already active" % args.activate_env)
         return
 
-    active_env = ev.get_env(namedtuple('args', ['env'])(env),
-                            'activate')
     cmds = ev.activate(
-        active_env, add_view=args.with_view, shell=args.shell,
+        ev.Environment(spack_env), add_view=args.with_view, shell=args.shell,
         prompt=env_prompt if args.prompt else None
     )
     sys.stdout.write(cmds)
@@ -315,7 +312,7 @@ def env_view_setup_parser(subparser):
 
 
 def env_view(args):
-    env = ev.get_env(args, 'env view')
+    env = ev.active_environment()
 
     if env:
         if args.action == ViewAction.regenerate:
@@ -342,7 +339,7 @@ def env_status_setup_parser(subparser):
 
 
 def env_status(args):
-    env = ev.get_env(args, 'env status')
+    env = ev.active_environment()
     if env:
         if env.path == os.getcwd():
             tty.msg('Using %s in current directory: %s'
@@ -373,7 +370,7 @@ def env_loads_setup_parser(subparser):
 
 
 def env_loads(args):
-    env = ev.get_env(args, 'env loads', required=True)
+    env = spack.cmd.require_active_env(cmd_name='env loads')
 
     # Set the module types that have been selected
     module_type = args.module_type

lib/spack/spack/cmd/extensions.py

@@ -67,7 +67,7 @@ def extensions(parser, args):
     if not spec[0].package.extendable:
         tty.die("%s is not an extendable package." % spec[0].name)
 
-    env = ev.get_env(args, 'extensions')
+    env = ev.active_environment()
     spec = cmd.disambiguate_spec(spec[0], env)
 
     if not spec.package.extendable:

lib/spack/spack/cmd/fetch.py

@@ -47,7 +47,7 @@ def fetch(parser, args):
         # fetch all uninstalled specs from it otherwise fetch all.
         # If we are also not in an environment, complain to the
         # user that we don't know what to do.
-        env = ev.get_env(args, "fetch")
+        env = ev.active_environment()
         if env:
             if args.missing:
                 specs = env.uninstalled_specs()

lib/spack/spack/cmd/find.py

@@ -220,7 +220,7 @@ def find(parser, args):
     added = set()
     removed = set()
 
-    env = ev.get_env(args, 'find')
+    env = ev.active_environment()
     if env:
         decorator, added, roots, removed = setup_env(env)
 

lib/spack/spack/cmd/gc.py

@@ -7,7 +7,7 @@
 
 import spack.cmd.common.arguments
 import spack.cmd.uninstall
-import spack.environment
+import spack.environment as ev
 import spack.store
 
 description = "remove specs that are now no longer needed"
@@ -24,7 +24,7 @@ def gc(parser, args):
 
     # Restrict garbage collection to the active environment
     # speculating over roots that are yet to be installed
-    env = spack.environment.get_env(args=None, cmd_name='gc')
+    env = ev.active_environment()
     if env:
         msg = 'Restricting the garbage collection to the "{0}" environment'
         tty.msg(msg.format(env.name))

lib/spack/spack/cmd/install.py

@@ -204,7 +204,7 @@ def install_specs(cli_args, kwargs, specs):
     """
 
     # handle active environment, if any
-    env = ev.get_env(cli_args, 'install')
+    env = ev.active_environment()
 
     try:
         if env:
@@ -324,10 +324,14 @@ def get_tests(specs):
         else:
             return False
 
+    # Parse cli arguments and construct a dictionary
+    # that will be passed to the package installer
+    update_kwargs_from_args(args, kwargs)
+
     if not args.spec and not args.specfiles:
         # if there are no args but an active environment
         # then install the packages from it.
-        env = ev.get_env(args, 'install')
+        env = ev.active_environment()
         if env:
             tests = get_tests(env.user_specs)
             kwargs['tests'] = tests
@@ -352,7 +356,7 @@ def get_tests(specs):
 
             tty.msg("Installing environment {0}".format(env.name))
             with reporter('build'):
-                env.install_all(args, **kwargs)
+                env.install_all(**kwargs)
 
             tty.debug("Regenerating environment views for {0}"
                       .format(env.name))
@@ -381,10 +385,6 @@ def get_tests(specs):
     if args.deprecated:
         spack.config.set('config:deprecated', True, scope='command_line')
 
-    # Parse cli arguments and construct a dictionary
-    # that will be passed to the package installer
-    update_kwargs_from_args(args, kwargs)
-
     # 1. Abstract specs from cli
     abstract_specs = spack.cmd.parse_specs(args.spec)
     tests = get_tests(abstract_specs)

lib/spack/spack/cmd/load.py

@@ -55,7 +55,7 @@ def setup_parser(subparser):
 
 
 def load(parser, args):
-    env = ev.get_env(args, 'load')
+    env = ev.active_environment()
     specs = [spack.cmd.disambiguate_spec(spec, env, first=args.load_first)
              for spec in spack.cmd.parse_specs(args.specs)]
 

lib/spack/spack/cmd/location.py

@@ -11,7 +11,6 @@
 
 import spack.cmd
 import spack.cmd.common.arguments as arguments
-import spack.environment
 import spack.environment as ev
 import spack.paths
 import spack.repo
@@ -73,7 +72,7 @@ def location(parser, args):
         return
 
     if args.location_env:
-        path = spack.environment.root(args.location_env)
+        path = ev.root(args.location_env)
         if not os.path.isdir(path):
             tty.die("no such environment: '%s'" % args.location_env)
         print(path)
@@ -97,7 +96,7 @@ def location(parser, args):
 
     # install_dir command matches against installed specs.
     if args.install_dir:
-        env = ev.get_env(args, 'location')
+        env = ev.active_environment()
         spec = spack.cmd.disambiguate_spec(specs[0], env)
         print(spec.prefix)
         return

lib/spack/spack/cmd/mirror.py

@@ -253,7 +253,7 @@ def _determine_specs_to_mirror(args):
                         "To mirror all packages, use the '--all' option"
                         " (this will require significant time and space).")
 
-            env = ev.get_env(args, 'mirror')
+            env = ev.active_environment()
             if env:
                 env_specs = env.all_specs()
             else:

lib/spack/spack/cmd/remove.py

@@ -7,7 +7,6 @@
 
 import spack.cmd
 import spack.cmd.common.arguments as arguments
-import spack.environment as ev
 
 description = 'remove specs from an environment'
 section = "environments"
@@ -28,7 +27,7 @@ def setup_parser(subparser):
 
 
 def remove(parser, args):
-    env = ev.get_env(args, 'remove', required=True)
+    env = spack.cmd.require_active_env(cmd_name='remove')
 
     with env.write_transaction():
         if args.all:

lib/spack/spack/cmd/stage.py

@@ -34,7 +34,7 @@ def stage(parser, args):
         spack.stage.create_stage_root(custom_path)
 
     if not args.specs:
-        env = ev.get_env(args, 'stage')
+        env = ev.active_environment()
         if env:
             tty.msg("Staging specs from environment %s" % env.name)
             for spec in env.specs_by_hash.values():

lib/spack/spack/cmd/test.py

@@ -155,7 +155,7 @@ def test_run(args):
         spack.config.set('config:fail_fast', True, scope='command_line')
 
     # Get specs to test
-    env = ev.get_env(args, 'test')
+    env = ev.active_environment()
     hashes = env.all_hashes() if env else None
 
     specs = spack.cmd.parse_specs(args.specs) if args.specs else [None]
@@ -221,7 +221,7 @@ def test_list(args):
 
     # TODO: This can be extended to have all of the output formatting options
     # from `spack find`.
-    env = ev.get_env(args, 'test')
+    env = ev.active_environment()
     hashes = env.all_hashes() if env else None
 
     specs = spack.store.db.query(hashes=hashes)

lib/spack/spack/cmd/undevelop.py

@@ -7,7 +7,6 @@
 
 import spack.cmd
 import spack.cmd.common.arguments as arguments
-import spack.environment as ev
 
 description = 'remove specs from an environment'
 section = "environments"
@@ -22,7 +21,7 @@ def setup_parser(subparser):
 
 
 def undevelop(parser, args):
-    env = ev.get_env(args, 'undevelop', required=True)
+    env = spack.cmd.require_active_env(cmd_name='undevelop')
 
     if args.all:
         specs = env.dev_specs.keys()

lib/spack/spack/cmd/uninstall.py

@@ -311,7 +311,7 @@ def get_uninstall_list(args, specs, env):
 
 
 def uninstall_specs(args, specs):
-    env = ev.get_env(args, 'uninstall')
+    env = ev.active_environment()
 
     uninstall_list, remove_list = get_uninstall_list(args, specs, env)
     anything_to_do = set(uninstall_list).union(set(remove_list))

lib/spack/spack/cmd/verify.py

@@ -74,7 +74,7 @@ def verify(parser, args):
 
     elif args.specs_or_files:
         # construct disambiguated spec list
-        env = ev.get_env(args, 'verify')
+        env = ev.active_environment()
         specs = list(map(lambda x: spack.cmd.disambiguate_spec(x, env,
                                                                local=local),
                          spec_args))

lib/spack/spack/cmd/view.py

@@ -202,7 +202,7 @@ def view(parser, args):
 
     elif args.action in actions_link:
         # only link commands need to disambiguate specs
-        env = ev.get_env(args, 'view link')
+        env = ev.active_environment()
         specs = [spack.cmd.disambiguate_spec(s, env) for s in specs]
 
     elif args.action in actions_status:

lib/spack/spack/concretize.py

@@ -33,6 +33,7 @@
 import spack.abi
 import spack.architecture
 import spack.compilers
+import spack.environment
 import spack.error
 import spack.repo
 import spack.spec
@@ -66,7 +67,7 @@ def concretize_develop(self, spec):
         """
         Add ``dev_path=*`` variant to packages built from local source.
         """
-        env = spack.environment.get_env(None, None)
+        env = spack.environment.active_environment()
         dev_info = env.dev_specs.get(spec.name, {}) if env else {}
         if not dev_info:
             return False

lib/spack/spack/container/__init__.py

@@ -7,7 +7,7 @@
 """
 import warnings
 
-import spack.environment
+import spack.environment as ev
 import spack.schema.env as env
 import spack.util.spack_yaml as syaml
 
@@ -36,7 +36,7 @@ def validate(configuration_file):
         config = syaml.load(f)
 
     # Ensure we have a "container" attribute with sensible defaults set
-    env_dict = spack.environment.config_dict(config)
+    env_dict = ev.config_dict(config)
     env_dict.setdefault('container', {
         'format': 'docker',
         'images': {'os': 'ubuntu:18.04', 'spack': 'develop'}

lib/spack/spack/container/writers/__init__.py

@@ -8,7 +8,7 @@
 import collections
 import copy
 
-import spack.environment
+import spack.environment as ev
 import spack.schema.env
 import spack.tengine as tengine
 import spack.util.spack_yaml as syaml
@@ -37,7 +37,7 @@ def create(configuration):
     Args:
         configuration: how to generate the current recipe
     """
-    name = spack.environment.config_dict(configuration)['container']['format']
+    name = ev.config_dict(configuration)['container']['format']
     return _writer_factory[name](configuration)
 
 
@@ -56,7 +56,7 @@ class PathContext(tengine.Context):
     directly via PATH.
     """
     def __init__(self, config):
-        self.config = spack.environment.config_dict(config)
+        self.config = ev.config_dict(config)
         self.container_config = self.config['container']
 
     @tengine.context_property

lib/spack/spack/environment.py

@@ -39,6 +39,7 @@
     inverse_view_func_parser,
     view_func_parser,
 )
+from spack.installer import PackageInstaller
 from spack.spec import Spec
 from spack.spec_list import InvalidSpecConstraintError, SpecList
 from spack.util.path import substitute_path_variables
@@ -263,100 +264,9 @@ def deactivate(shell='sh'):
     return cmds
 
 
-def find_environment(args):
+def active_environment():
-    """Find active environment from args, spack.yaml, or environment variable.
+    """Returns the active environment when there is any"""
-
+    return _active_environment
-    This is called in ``spack.main`` to figure out which environment to
-    activate.
-
-    Check for an environment in this order:
-        1. via ``spack -e ENV`` or ``spack -D DIR`` (arguments)
-        2. as a spack.yaml file in the current directory, or
-        3. via a path in the SPACK_ENV environment variable.
-
-    If an environment is found, read it in.  If not, return None.
-
-    Arguments:
-        args (argparse.Namespace): argparse namespace wtih command arguments
-
-    Returns:
-        (Environment): a found environment, or ``None``
-    """
-    # try arguments
-    env = getattr(args, 'env', None)
-
-    # treat env as a name
-    if env:
-        if exists(env):
-            return read(env)
-
-    else:
-        # if env was specified, see if it is a dirctory otherwise, look
-        # at env_dir (env and env_dir are mutually exclusive)
-        env = getattr(args, 'env_dir', None)
-
-        # if no argument, look for the environment variable
-        if not env:
-            env = os.environ.get(spack_env_var)
-
-            # nothing was set; there's no active environment
-            if not env:
-                return None
-
-    # if we get here, env isn't the name of a spack environment; it has
-    # to be a path to an environment, or there is something wrong.
-    if is_env_dir(env):
-        return Environment(env)
-
-    raise SpackEnvironmentError('no environment in %s' % env)
-
-
-def get_env(args, cmd_name, required=False):
-    """Used by commands to get the active environment.
-
-    This first checks for an ``env`` argument, then looks at the
-    ``active`` environment.  We check args first because Spack's
-    subcommand arguments are parsed *after* the ``-e`` and ``-D``
-    arguments to ``spack``.  So there may be an ``env`` argument that is
-    *not* the active environment, and we give it precedence.
-
-    This is used by a number of commands for determining whether there is
-    an active environment.
-
-    If an environment is not found *and* is required, print an error
-    message that says the calling command *needs* an active environment.
-
-    Arguments:
-        args (argparse.Namespace): argparse namespace wtih command arguments
-        cmd_name (str): name of calling command
-        required (bool): if ``True``, raise an exception when no environment
-            is found; if ``False``, just return ``None``
-
-    Returns:
-        (Environment): if there is an arg or active environment
-    """
-    # try argument first
-    env = getattr(args, 'env', None)
-    if env:
-        if exists(env):
-            return read(env)
-        elif is_env_dir(env):
-            return Environment(env)
-        else:
-            raise SpackEnvironmentError('no environment in %s' % env)
-
-    # try the active environment. This is set by find_environment() (above)
-    if _active_environment:
-        return _active_environment
-    elif not required:
-        return None
-    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)
 
 
 def _root(name):
@@ -1560,21 +1470,18 @@ def uninstalled_specs(self):
                     uninstalled_specs.append(spec)
         return uninstalled_specs
 
-    def install_all(self, args=None, **install_args):
+    def install_all(self, **install_args):
         """Install all concretized specs in an environment.
 
         Note: this does not regenerate the views for the environment;
         that needs to be done separately with a call to write().
 
         Args:
-            args (argparse.Namespace): argparse namespace with command arguments
             install_args (dict): keyword install arguments
         """
-        self.install_specs(None, args=args, **install_args)
+        self.install_specs(None, **install_args)
-
-    def install_specs(self, specs=None, args=None, **install_args):
-        from spack.installer import PackageInstaller
 
+    def install_specs(self, specs=None, **install_args):
         tty.debug('Assessing installation status of environment packages')
         # If "spack install" is invoked repeatedly for a large environment
         # where all specs are already installed, the operation can take
@@ -1608,15 +1515,7 @@ def install_specs(self, specs=None, args=None, **install_args):
 
         installs = []
         for spec in specs_to_install:
-            # Parse cli arguments and construct a dictionary
+            installs.append((spec.package, install_args))
-            # that will be passed to the package installer
-            kwargs = dict()
-            if install_args:
-                kwargs.update(install_args)
-            if args:
-                spack.cmd.install.update_kwargs_from_args(args, kwargs)
-
-            installs.append((spec.package, kwargs))
 
         try:
             builder = PackageInstaller(installs)

lib/spack/spack/hooks/module_file_generation.py

@@ -37,8 +37,8 @@ def _for_each_enabled(spec, method_name):
 
 
 def post_install(spec):
-    import spack.environment  # break import cycle
+    import spack.environment as ev  # break import cycle
-    if spack.environment.get_env({}, ''):
+    if ev.active_environment():
         # If the installed through an environment, we skip post_install
         # module generation and generate the modules on env_write so Spack
         # can manage interactions between env views and modules

lib/spack/spack/main.py

@@ -743,7 +743,7 @@ def main(argv=None):
 
     # activate an environment if one was specified on the command line
     if not args.no_env:
-        env = ev.find_environment(args)
+        env = spack.cmd.find_environment(args)
         if env:
             ev.activate(env, args.use_env_repo, add_view=False)
 

lib/spack/spack/modules/common.py

@@ -698,7 +698,11 @@ def environment_modifications(self):
             if use_view is True:
                 use_view = ev.default_view_name
 
-            env = ev.get_env({}, 'post_env_write_hook', required=True)
+            env = ev.active_environment()
+            if not env:
+                raise ev.SpackEnvironmentViewError("Module generation with views "
+                                                   "requires active environment")
+
             view = env.views[use_view]
 
             spec.prefix = view.get_projection_for_spec(spec)

lib/spack/spack/package.py

@@ -41,6 +41,7 @@
 import spack.dependency
 import spack.directives
 import spack.directory_layout
+import spack.environment
 import spack.error
 import spack.fetch_strategy as fs
 import spack.hooks
@@ -1537,7 +1538,7 @@ def content_hash(self, content=None):
             # should this attempt to download the source and set one? This
             # probably only happens for source repositories which are
             # referenced by branch name rather than tag or commit ID.
-            env = spack.environment.get_env(None, None)
+            env = spack.environment.active_environment()
             from_local_sources = env and env.is_develop(self.spec)
             if not self.spec.external and not from_local_sources:
                 message = 'Missing a source id for {s.name}@{s.version}'

lib/spack/spack/relocate.py

@@ -16,7 +16,6 @@
 import llnl.util.tty as tty
 
 import spack.architecture
-import spack.cmd
 import spack.repo
 import spack.spec
 import spack.util.executable as executable

lib/spack/spack/solver/asp.py

@@ -37,6 +37,7 @@
 import spack.config
 import spack.dependency
 import spack.directives
+import spack.environment as ev
 import spack.error
 import spack.package
 import spack.package_prefs
@@ -1396,7 +1397,7 @@ def setup(self, driver, specs, tests=False):
             self.preferred_versions(pkg)
 
         # Inject dev_path from environment
-        env = spack.environment.get_env(None, None)
+        env = ev.active_environment()
         if env:
             for spec in sorted(specs):
                 for dep in spec.traverse():
@@ -1625,9 +1626,8 @@ def build_specs(self, function_tuples):
         for s in self._specs.values():
             spack.spec.Spec.ensure_external_path_if_external(s)
 
-        env = spack.environment.get_env(None, None)
         for s in self._specs.values():
-            _develop_specs_from_env(s, env)
+            _develop_specs_from_env(s, ev.active_environment())
 
         for s in self._specs.values():
             s._mark_concrete()

lib/spack/spack/stage.py

@@ -18,6 +18,7 @@
 
 from six import iteritems, string_types
 
+import llnl.util.lang
 import llnl.util.tty as tty
 from llnl.util.filesystem import (
     can_access,
@@ -29,7 +30,6 @@
 )
 
 import spack.caches
-import spack.cmd
 import spack.config
 import spack.error
 import spack.fetch_strategy as fs
@@ -857,7 +857,7 @@ def get_checksums_for_versions(
     tty.msg('Found {0} version{1} of {2}:'.format(
             num_ver, '' if num_ver == 1 else 's', name),
             '',
-            *spack.cmd.elide_list(
+            *llnl.util.lang.elide_list(
                 ['{0:{1}}  {2}'.format(str(v), max_len, url_dict[v])
                  for v in sorted_versions]))
     print()

lib/spack/spack/test/cmd/bootstrap.py

@@ -7,7 +7,7 @@
 import pytest
 
 import spack.config
-import spack.environment
+import spack.environment as ev
 import spack.main
 
 _bootstrap = spack.main.SpackCommand('bootstrap')
@@ -65,7 +65,7 @@ def test_reset_in_file_scopes(mutable_config, scopes):
 def test_reset_in_environment(mutable_mock_env_path, mutable_config):
     env = spack.main.SpackCommand('env')
     env('create', 'bootstrap-test')
-    current_environment = spack.environment.read('bootstrap-test')
+    current_environment = ev.read('bootstrap-test')
 
     with current_environment:
         _bootstrap('disable')

lib/spack/spack/test/cmd/ci.py

@@ -56,7 +56,7 @@ def _set_project_dir(path):
 @pytest.fixture()
 def env_deactivate():
     yield
-    spack.environment._active_environment = None
+    ev._active_environment = None
     os.environ.pop('SPACK_ENV', None)
 
 

lib/spack/spack/test/cmd/common/arguments.py

@@ -90,9 +90,7 @@ def test_multiple_env_match_raises_error(mock_packages, mutable_mock_env_path):
     e.add('a foobar=fee')
     e.concretize()
     with e:
-        with pytest.raises(
+        with pytest.raises(ev.SpackEnvironmentError) as exc_info:
-                spack.environment.SpackEnvironmentError) as exc_info:
-
             spack.cmd.matching_spec_from_env(spack.cmd.parse_specs(['a'])[0])
 
     assert 'matches multiple specs' in exc_info.value.message

lib/spack/spack/test/cmd/env.py

@@ -53,7 +53,7 @@ def check_viewdir_removal(viewdir):
 @pytest.fixture()
 def env_deactivate():
     yield
-    spack.environment._active_environment = None
+    ev._active_environment = None
     os.environ.pop('SPACK_ENV', None)
 
 
@@ -228,7 +228,7 @@ def test_activate_adds_transitive_run_deps_to_path(
     with e:
         install('depends-on-run-env')
 
-    cmds = spack.environment.activate(e)
+    cmds = ev.activate(e)
     assert 'DEPENDENCY_ENV_VAR=1' in cmds
 
 
@@ -1143,7 +1143,7 @@ def test_env_without_view_install(
     env('create', '--without-view', 'test')
 
     test_env = ev.read('test')
-    with pytest.raises(spack.environment.SpackEnvironmentError):
+    with pytest.raises(ev.SpackEnvironmentError):
         test_env.default_view
 
     view_dir = tmpdir.join('view')

lib/spack/spack/test/config.py

@@ -14,7 +14,7 @@
 from llnl.util.filesystem import mkdirp, touch
 
 import spack.config
-import spack.environment
+import spack.environment as ev
 import spack.main
 import spack.paths
 import spack.schema.compilers
@@ -360,8 +360,8 @@ def test_substitute_config_variables(mock_low_high_config, monkeypatch):
 
     # Fake an active environment and $env is replaced properly
     fake_env_path = '/quux/quuux'
-    monkeypatch.setattr(spack.environment, 'get_env',
+    monkeypatch.setattr(ev, 'active_environment',
-                        lambda x, y: MockEnv(fake_env_path))
+                        lambda: MockEnv(fake_env_path))
     assert spack_path.canonicalize_path(
         '$env/foo/bar/baz'
     ) == os.path.join(fake_env_path, 'foo/bar/baz')

lib/spack/spack/test/conftest.py

@@ -1267,11 +1267,11 @@ def get_rev():
 @pytest.fixture()
 def mutable_mock_env_path(tmpdir_factory):
     """Fixture for mocking the internal spack environments directory."""
-    saved_path = spack.environment.env_path
+    saved_path = ev.env_path
     mock_path = tmpdir_factory.mktemp('mock-env-path')
-    spack.environment.env_path = str(mock_path)
+    ev.env_path = str(mock_path)
     yield mock_path
-    spack.environment.env_path = saved_path
+    ev.env_path = saved_path
 
 
 @pytest.fixture()

lib/spack/spack/util/path.py

@@ -85,7 +85,7 @@ def substitute_config_variables(path):
     environment yaml files.
     """
     import spack.environment as ev  # break circular
-    env = ev.get_env({}, '')
+    env = ev.active_environment()
     if env:
         replacements.update({'env': env.path})
     else:

lib/spack/spack/util/web.py

@@ -24,7 +24,6 @@
 import llnl.util.tty as tty
 from llnl.util.filesystem import mkdirp
 
-import spack.cmd
 import spack.config
 import spack.error
 import spack.url