env: clean up command access to the active environment

- all commands (except `spack find`, through `ConstraintAction`) now go
  through get_env() to get the active environment

- ev.active was hard to read -- and the name wasn't descriptive.
  - rename it to _active_environment to be more descriptive and to strongly
    indicate that spack.environment manages it

5 files changed, 51 insertions, 40 deletions

diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py
index 671dde4fa0..b3b8773e00 100644
--- a/lib/spack/spack/cmd/common/arguments.py
+++ b/lib/spack/spack/cmd/common/arguments.py
@@ -8,7 +8,7 @@ import argparse
 
 import spack.cmd
 import spack.config
-import spack.environment
+import spack.environment as ev
 import spack.modules
 import spack.spec
 import spack.store
@@ -54,7 +54,7 @@ class ConstraintAction(argparse.Action):
 
         # If an environment is provided, we'll restrict the search to
         # only its installed packages.
-        env = spack.environment.active
+        env = ev._active_environment
         if env:
             kwargs['hashes'] = set(env.all_hashes())
 
diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py
index b40790af71..c8b9c169b5 100644
--- a/lib/spack/spack/cmd/env.py
+++ b/lib/spack/spack/cmd/env.py
@@ -258,7 +258,7 @@ def env_list(args):
 
     color_names = []
     for name in names:
-        if ev.active and name == ev.active.name:
+        if ev.active(name):
             name = colorize('@*g{%s}' % name)
         color_names.append(name)
 
diff --git a/lib/spack/spack/cmd/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index cb02aa4f41..10f56fc4a0 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -61,18 +61,20 @@ def setup_parser(subparser):
         help="specs of packages to uninstall")
 
 
-def find_matching_specs(specs, allow_multiple_matches=False, force=False):
+def find_matching_specs(env, specs, allow_multiple_matches=False, force=False):
     """Returns a list of specs matching the not necessarily
        concretized specs given from cli
 
     Args:
-        specs: list of specs to be matched against installed packages
-        allow_multiple_matches : if True multiple matches are admitted
+        env (Environment): active environment, or ``None`` if there is not one
+        specs (list): list of specs to be matched against installed packages
+        allow_multiple_matches (bool): if True multiple matches are admitted
 
     Return:
         list of specs
     """
-    hashes = ev.active.all_hashes() if ev.active else None
+    # constrain uninstall resolution to current environment if one is active
+    hashes = env.all_hashes() if env else None
 
     # List of specs that match expressions given via command line
     specs_from_cli = []
@@ -90,8 +92,8 @@ def find_matching_specs(specs, allow_multiple_matches=False, force=False):
 
         # No installed package matches the query
         if len(matching) == 0 and spec is not any:
-            if ev.active:
-                pkg_type = "packages in environment '%s'" % ev.active.name
+            if env:
+                pkg_type = "packages in environment '%s'" % env.name
             else:
                 pkg_type = 'installed packages'
             tty.die('{0} does not match any {1}.'.format(spec, pkg_type))
@@ -147,13 +149,14 @@ def dependent_environments(specs):
     return dependents
 
 
-def do_uninstall(specs, force):
+def do_uninstall(env, specs, force):
     """
     Uninstalls all the specs in a list.
 
     Args:
-        specs: list of specs to be uninstalled
-        force: force uninstallation (boolean)
+        env (Environment): active environment, or ``None`` if there is not one
+        specs (list): list of specs to be uninstalled
+        force (bool): force uninstallation (boolean)
     """
     packages = []
     for item in specs:
@@ -165,11 +168,13 @@ def do_uninstall(specs, force):
             # want to uninstall.
             spack.package.Package.uninstall_by_spec(item, force=True)
 
-        if ev.active:
+        if env:
             try:
-                ev.active.remove(item, force=True)
+                # try removing the spec from the current active
+                # environment. this will fail if the spec is not a root
+                env.remove(item, force=True)
             except ev.SpackEnvironmentError:
-                pass  # ignore errors from specs that are not roots
+                pass  # ignore non-root specs
 
     # Sort packages to be uninstalled by the number of installed dependents
     # This ensures we do things in the right order
@@ -183,14 +188,14 @@ def do_uninstall(specs, force):
         item.do_uninstall(force=force)
 
     # write any changes made to the active environment
-    if ev.active:
-        ev.active.write()
+    if env:
+        env.write()
 
 
-def get_uninstall_list(args, specs):
+def get_uninstall_list(args, specs, env):
     # Gets the list of installed specs that match the ones give via cli
     # takes care of '-a' is given in the cli
-    uninstall_list = find_matching_specs(specs, args.all, args.force)
+    uninstall_list = find_matching_specs(env, specs, args.all, args.force)
 
     # Takes care of '-R'
     spec_dependents = installed_dependents(uninstall_list)
@@ -231,7 +236,7 @@ def get_uninstall_list(args, specs):
             msgs.append(
                 'use `spack env remove` to remove environments, or '
                 '`spack remove` to remove specs from environments.')
-        if ev.active:
+        if env:
             msgs.append('consider using `spack remove` to remove the spec '
                         'from this environment')
         print()
@@ -246,7 +251,8 @@ def get_uninstall_list(args, specs):
 
 
 def uninstall_specs(args, specs):
-    uninstall_list = get_uninstall_list(args, specs)
+    env = ev.get_env(args, 'uninstall', required=False)
+    uninstall_list = get_uninstall_list(args, specs, env)
 
     if not uninstall_list:
         tty.warn('There are no package to uninstall.')
@@ -260,7 +266,7 @@ def uninstall_specs(args, specs):
             tty.die('Will not uninstall any packages.')
 
     # Uninstall everything on the list
-    do_uninstall(uninstall_list, args.force)
+    do_uninstall(env, uninstall_list, args.force)
 
 
 def uninstall(parser, args):
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index cfb3cd2c28..208eb485ae 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -28,7 +28,7 @@ spack_env_var = 'SPACK_ENV'
 
 
 #: currently activated environment
-active = None
+_active_environment = None
 
 
 #: path where environments are stored in the spack tree
@@ -98,14 +98,14 @@ def activate(env, use_env_repo=False):
     TODO: Add support for views here.  Activation should set up the shell
     TODO: environment to use the activated spack environment.
     """
-    global active
+    global _active_environment
 
-    active = env
-    prepare_config_scope(active)
+    _active_environment = env
+    prepare_config_scope(_active_environment)
     if use_env_repo:
-        spack.repo.path.put_first(active.repo)
+        spack.repo.path.put_first(_active_environment.repo)
 
-    tty.debug("Using environmennt '%s'" % active.name)
+    tty.debug("Using environmennt '%s'" % _active_environment.name)
 
 
 def deactivate():
@@ -116,19 +116,19 @@ def deactivate():
         environment was active.
 
     """
-    global active
+    global _active_environment
 
-    if not active:
+    if not _active_environment:
         return
 
-    deactivate_config_scope(active)
+    deactivate_config_scope(_active_environment)
 
     # use _repo so we only remove if a repo was actually constructed
-    if active._repo:
-        spack.repo.path.remove(active._repo)
+    if _active_environment._repo:
+        spack.repo.path.remove(_active_environment._repo)
 
-    tty.debug("Deactivated environmennt '%s'" % active.name)
-    active = None
+    tty.debug("Deactivated environmennt '%s'" % _active_environment.name)
+    _active_environment = None
 
 
 def find_environment(args):
@@ -219,8 +219,8 @@ def get_env(args, cmd_name, required=True):
             raise SpackEnvironmentError('no environment in %s' % env)
 
     # try the active environment. This is set by find_environment() (above)
-    if active:
-        return active
+    if _active_environment:
+        return _active_environment
     elif not required:
         return None
     else:
@@ -245,6 +245,11 @@ def exists(name):
     return os.path.isdir(root(name))
 
 
+def active(name):
+    """True if the named environment is active."""
+    return _active_environment and name == _active_environment.name
+
+
 def is_env_dir(path):
     """Whether a directory contains a spack environment."""
     return os.path.isdir(path) and os.path.exists(
@@ -407,7 +412,7 @@ class Environment(object):
     @property
     def active(self):
         """True if this environment is currently active."""
-        return active and self.path == active.path
+        return _active_environment and self.path == _active_environment.path
 
     @property
     def manifest_path(self):
@@ -854,7 +859,7 @@ class Environment(object):
             _write_yaml(self.yaml, f)
 
     def __enter__(self):
-        self._previous_active = active
+        self._previous_active = _active_environment
         activate(self)
         return
 
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index 9fe53b8f66..4405335cf4 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -44,7 +44,7 @@ from spack.version import Version
 @pytest.fixture(scope='session', autouse=True)
 def clean_user_environment():
     env_var = ev.spack_env_var in os.environ
-    active = ev.active
+    active = ev._active_environment
 
     if env_var:
         spack_env_value = os.environ.pop(ev.spack_env_var)