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.

51 files changed, 176 insertions, 215 deletions

diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py
index abb889ee7b..408648f1c1 100644
--- a/lib/spack/llnl/util/lang.py
+++ b/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
diff --git a/lib/spack/spack/architecture.py b/lib/spack/spack/architecture.py
index 44ce51ddfd..c9217e0941 100644
--- a/lib/spack/spack/architecture.py
+++ b/lib/spack/spack/architecture.py
@@ -72,6 +72,7 @@ import spack.compilers
 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 @@ class Target(object):
                 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 @@ class Target(object):
             # 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:
diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index 3944d5450e..f0a7030822 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -21,6 +21,7 @@ from llnl.util.tty.colify import colify
 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)
diff --git a/lib/spack/spack/cmd/activate.py b/lib/spack/spack/cmd/activate.py
index a6c27b2f1d..9b96733fae 100644
--- a/lib/spack/spack/cmd/activate.py
+++ b/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], env)
+    spec = spack.cmd.disambiguate_spec(specs[0], ev.active_environment())
     if not spec.package.is_extension:
         tty.die("%s is not an extension." % spec.name)
 
diff --git a/lib/spack/spack/cmd/add.py b/lib/spack/spack/cmd/add.py
index 453b9cd886..631038ff2b 100644
--- a/lib/spack/spack/cmd/add.py
+++ b/lib/spack/spack/cmd/add.py
@@ -7,7 +7,6 @@ import llnl.util.tty as tty
 
 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):
diff --git a/lib/spack/spack/cmd/analyze.py b/lib/spack/spack/cmd/analyze.py
index 0bdf47e73f..f584674ae2 100644
--- a/lib/spack/spack/cmd/analyze.py
+++ b/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)
diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py
index 8fc0b6a04e..0444091f4e 100644
--- a/lib/spack/spack/cmd/buildcache.py
+++ b/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()
 
diff --git a/lib/spack/spack/cmd/ci.py b/lib/spack/spack/cmd/ci.py
index a605f621c3..a376d9ed95 100644
--- a/lib/spack/spack/cmd/ci.py
+++ b/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.
diff --git a/lib/spack/spack/cmd/concretize.py b/lib/spack/spack/cmd/concretize.py
index e6a8ecf6f5..433236b238 100644
--- a/lib/spack/spack/cmd/concretize.py
+++ b/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 @@ chosen, test dependencies are enabled for all packages in the environment.""")
 
 
 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
diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py
index f1bdced6e0..2fc57a52c5 100644
--- a/lib/spack/spack/cmd/config.py
+++ b/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()
 
diff --git a/lib/spack/spack/cmd/deactivate.py b/lib/spack/spack/cmd/deactivate.py
index 0afa4af82b..d67981cd9e 100644
--- a/lib/spack/spack/cmd/deactivate.py
+++ b/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
 
diff --git a/lib/spack/spack/cmd/dependencies.py b/lib/spack/spack/cmd/dependencies.py
index 2f923fece0..e4d6c3fe56 100644
--- a/lib/spack/spack/cmd/dependencies.py
+++ b/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}'
diff --git a/lib/spack/spack/cmd/dependents.py b/lib/spack/spack/cmd/dependents.py
index 6097f46ff9..76b308c295 100644
--- a/lib/spack/spack/cmd/dependents.py
+++ b/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}'
diff --git a/lib/spack/spack/cmd/deprecate.py b/lib/spack/spack/cmd/deprecate.py
index 2ab3cb17c8..216095cb8e 100644
--- a/lib/spack/spack/cmd/deprecate.py
+++ b/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:
diff --git a/lib/spack/spack/cmd/develop.py b/lib/spack/spack/cmd/develop.py
index 22bd6dad86..efba1f2fd7 100644
--- a/lib/spack/spack/cmd/develop.py
+++ b/lib/spack/spack/cmd/develop.py
@@ -9,7 +9,6 @@ import llnl.util.tty as tty
 
 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:
diff --git a/lib/spack/spack/cmd/diff.py b/lib/spack/spack/cmd/diff.py
index 6029363af8..85b73b6545 100644
--- a/lib/spack/spack/cmd/diff.py
+++ b/lib/spack/spack/cmd/diff.py
@@ -175,7 +175,7 @@ def print_difference(c, attributes="all", out=None):
 
 
 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.")
diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py
index fe40da4412..75d573f6fe 100644
--- a/lib/spack/spack/cmd/env.py
+++ b/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
diff --git a/lib/spack/spack/cmd/extensions.py b/lib/spack/spack/cmd/extensions.py
index d80eb84b60..0ef2137b1d 100644
--- a/lib/spack/spack/cmd/extensions.py
+++ b/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:
diff --git a/lib/spack/spack/cmd/fetch.py b/lib/spack/spack/cmd/fetch.py
index bb4f535180..4993c49912 100644
--- a/lib/spack/spack/cmd/fetch.py
+++ b/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()
diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py
index 875c236164..13066578e3 100644
--- a/lib/spack/spack/cmd/find.py
+++ b/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)
 
diff --git a/lib/spack/spack/cmd/gc.py b/lib/spack/spack/cmd/gc.py
index f85f27bfca..08fcd92873 100644
--- a/lib/spack/spack/cmd/gc.py
+++ b/lib/spack/spack/cmd/gc.py
@@ -7,7 +7,7 @@ import llnl.util.tty as tty
 
 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))
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index f71a788ec1..98e150af50 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/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 @@ environment variables:
         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 @@ environment variables:
 
             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 @@ environment variables:
     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)
diff --git a/lib/spack/spack/cmd/load.py b/lib/spack/spack/cmd/load.py
index 6749e169fa..8bfeac7e69 100644
--- a/lib/spack/spack/cmd/load.py
+++ b/lib/spack/spack/cmd/load.py
@@ -55,7 +55,7 @@ the dependencies"""
 
 
 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)]
 
diff --git a/lib/spack/spack/cmd/location.py b/lib/spack/spack/cmd/location.py
index 61687f02bc..f844f51ed6 100644
--- a/lib/spack/spack/cmd/location.py
+++ b/lib/spack/spack/cmd/location.py
@@ -11,7 +11,6 @@ import llnl.util.tty as tty
 
 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
diff --git a/lib/spack/spack/cmd/mirror.py b/lib/spack/spack/cmd/mirror.py
index 84d7fa0649..fa202f09f0 100644
--- a/lib/spack/spack/cmd/mirror.py
+++ b/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:
diff --git a/lib/spack/spack/cmd/remove.py b/lib/spack/spack/cmd/remove.py
index 1f97e6e6e5..a2417089db 100644
--- a/lib/spack/spack/cmd/remove.py
+++ b/lib/spack/spack/cmd/remove.py
@@ -7,7 +7,6 @@ import llnl.util.tty as tty
 
 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:
diff --git a/lib/spack/spack/cmd/stage.py b/lib/spack/spack/cmd/stage.py
index c3a3c21e68..8903cb83e5 100644
--- a/lib/spack/spack/cmd/stage.py
+++ b/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():
diff --git a/lib/spack/spack/cmd/test.py b/lib/spack/spack/cmd/test.py
index c480b4394f..d2a5e7214b 100644
--- a/lib/spack/spack/cmd/test.py
+++ b/lib/spack/spack/cmd/test.py
@@ -155,7 +155,7 @@ environment variables:
         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)
diff --git a/lib/spack/spack/cmd/undevelop.py b/lib/spack/spack/cmd/undevelop.py
index 3f8288e09a..a26519ac38 100644
--- a/lib/spack/spack/cmd/undevelop.py
+++ b/lib/spack/spack/cmd/undevelop.py
@@ -7,7 +7,6 @@ import llnl.util.tty as tty
 
 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()
diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index ead4f8ac84..fdd9986108 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/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))
diff --git a/lib/spack/spack/cmd/verify.py b/lib/spack/spack/cmd/verify.py
index 723aa2f994..a1d8acdf15 100644
--- a/lib/spack/spack/cmd/verify.py
+++ b/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))
diff --git a/lib/spack/spack/cmd/view.py b/lib/spack/spack/cmd/view.py
index f795c17a1c..fc0c30c7e2 100644
--- a/lib/spack/spack/cmd/view.py
+++ b/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:
diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index 1048709297..2e6e067c9d 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -33,6 +33,7 @@ import llnl.util.tty as tty
 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 @@ class Concretizer(object):
         """
         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
diff --git a/lib/spack/spack/container/__init__.py b/lib/spack/spack/container/__init__.py
index c57e0d0c51..f9212a574c 100644
--- a/lib/spack/spack/container/__init__.py
+++ b/lib/spack/spack/container/__init__.py
@@ -7,7 +7,7 @@ generate container recipes from a Spack environment
 """
 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'}
diff --git a/lib/spack/spack/container/writers/__init__.py b/lib/spack/spack/container/writers/__init__.py
index 14bc5a77c4..978e999605 100644
--- a/lib/spack/spack/container/writers/__init__.py
+++ b/lib/spack/spack/container/writers/__init__.py
@@ -8,7 +8,7 @@ convenience functions.
 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
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 6fa7f62553..dbe7318517 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -39,6 +39,7 @@ from spack.filesystem_view import (
     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):
-    """Find active environment from args, spack.yaml, or environment variable.
-
-    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 active_environment():
+    """Returns the active environment when there is any"""
+    return _active_environment
 
 
 def _root(name):
@@ -1560,21 +1470,18 @@ class Environment(object):
                     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)
-
-    def install_specs(self, specs=None, args=None, **install_args):
-        from spack.installer import PackageInstaller
+        self.install_specs(None, **install_args)
 
+    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 @@ class Environment(object):
 
         installs = []
         for spec in specs_to_install:
-            # Parse cli arguments and construct a dictionary
-            # 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))
+            installs.append((spec.package, install_args))
 
         try:
             builder = PackageInstaller(installs)
diff --git a/lib/spack/spack/hooks/module_file_generation.py b/lib/spack/spack/hooks/module_file_generation.py
index b95040aed2..fa04a5a3a5 100644
--- a/lib/spack/spack/hooks/module_file_generation.py
+++ b/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
-    if spack.environment.get_env({}, ''):
+    import spack.environment as ev  # break import cycle
+    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
diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py
index d29988627e..ee5b3d5343 100644
--- a/lib/spack/spack/main.py
+++ b/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)
 
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 5d9ea36648..526d8a5cec 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -698,7 +698,11 @@ class BaseContext(tengine.Context):
             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)
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 40b77ee124..10f4606972 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -41,6 +41,7 @@ import spack.config
 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 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
             # 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}'
diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py
index b77e254d4f..107b7cc87a 100644
--- a/lib/spack/spack/relocate.py
+++ b/lib/spack/spack/relocate.py
@@ -16,7 +16,6 @@ import llnl.util.lang
 import llnl.util.tty as tty
 
 import spack.architecture
-import spack.cmd
 import spack.repo
 import spack.spec
 import spack.util.executable as executable
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 3c090bdbbc..d32713da69 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -37,6 +37,7 @@ import spack.compilers
 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 @@ class SpackSolverSetup(object):
             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 @@ class SpecBuilder(object):
         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()
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index e4d34a20df..14413669f1 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -18,6 +18,7 @@ from typing import Dict  # novm
 
 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 @@ from llnl.util.filesystem import (
 )
 
 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()
diff --git a/lib/spack/spack/test/cmd/bootstrap.py b/lib/spack/spack/test/cmd/bootstrap.py
index 652721d721..3904b1019a 100644
--- a/lib/spack/spack/test/cmd/bootstrap.py
+++ b/lib/spack/spack/test/cmd/bootstrap.py
@@ -7,7 +7,7 @@ import os.path
 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')
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index 4a40d1fc01..a9891796ac 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -56,7 +56,7 @@ def project_dir_env():
 @pytest.fixture()
 def env_deactivate():
     yield
-    spack.environment._active_environment = None
+    ev._active_environment = None
     os.environ.pop('SPACK_ENV', None)
 
 
diff --git a/lib/spack/spack/test/cmd/common/arguments.py b/lib/spack/spack/test/cmd/common/arguments.py
index 8797626d60..d327939978 100644
--- a/lib/spack/spack/test/cmd/common/arguments.py
+++ b/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(
-                spack.environment.SpackEnvironmentError) as exc_info:
-
+        with pytest.raises(ev.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
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 8ead966655..ea1c660804 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/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')
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index 94966c9fac..704e9994ed 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -14,7 +14,7 @@ from six import StringIO
 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',
-                        lambda x, y: MockEnv(fake_env_path))
+    monkeypatch.setattr(ev, 'active_environment',
+                        lambda: MockEnv(fake_env_path))
     assert spack_path.canonicalize_path(
         '$env/foo/bar/baz'
     ) == os.path.join(fake_env_path, 'foo/bar/baz')
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index d579fe616f..3f7a9f3a47 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -1267,11 +1267,11 @@ def mock_svn_repository(tmpdir_factory):
 @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()
diff --git a/lib/spack/spack/util/path.py b/lib/spack/spack/util/path.py
index 04d45a012b..49bb65f7d3 100644
--- a/lib/spack/spack/util/path.py
+++ b/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:
diff --git a/lib/spack/spack/util/web.py b/lib/spack/spack/util/web.py
index 34140ed74a..0922508155 100644
--- a/lib/spack/spack/util/web.py
+++ b/lib/spack/spack/util/web.py
@@ -24,7 +24,6 @@ import llnl.util.lang
 import llnl.util.tty as tty
 from llnl.util.filesystem import mkdirp
 
-import spack.cmd
 import spack.config
 import spack.error
 import spack.url