From 207c37759ca02b7f1065e2d24acce6729c4d446c Mon Sep 17 00:00:00 2001
From: Todd Gamblin <tgamblin@llnl.gov>
Date: Fri, 14 Dec 2018 10:28:30 -0800
Subject: [PATCH] env: all commands that disambiguate specs should be env-aware

- some commands were missed in the rollout of spack environments

- this makes all commands that need to disambiguate specs restrict the
  disambiguation to installed packages in the active environment, as
  users would expect
---
 lib/spack/spack/cmd/__init__.py     | 12 ++++++++++--
 lib/spack/spack/cmd/activate.py     |  4 +++-
 lib/spack/spack/cmd/add.py          |  2 +-
 lib/spack/spack/cmd/buildcache.py   | 13 ++++++++++---
 lib/spack/spack/cmd/concretize.py   |  2 +-
 lib/spack/spack/cmd/config.py       |  2 +-
 lib/spack/spack/cmd/deactivate.py   |  4 +++-
 lib/spack/spack/cmd/dependencies.py |  4 +++-
 lib/spack/spack/cmd/dependents.py   |  4 +++-
 lib/spack/spack/cmd/env.py          |  4 ++--
 lib/spack/spack/cmd/extensions.py   |  4 +++-
 lib/spack/spack/cmd/find.py         |  2 +-
 lib/spack/spack/cmd/install.py      |  4 ++--
 lib/spack/spack/cmd/location.py     |  4 +++-
 lib/spack/spack/cmd/remove.py       |  2 +-
 lib/spack/spack/cmd/stage.py        |  2 +-
 lib/spack/spack/cmd/uninstall.py    |  2 +-
 lib/spack/spack/cmd/view.py         |  4 +++-
 lib/spack/spack/environment.py      |  6 +++---
 19 files changed, 55 insertions(+), 26 deletions(-)

diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index 2963c053e4..9586d3f122 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -158,8 +158,16 @@ def elide_list(line_list, max_num=10):
         return line_list
 
 
-def disambiguate_spec(spec):
-    matching_specs = spack.store.db.query(spec)
+def disambiguate_spec(spec, env):
+    """Given a spec, figure out which installed package it refers to.
+
+    Arguments:
+        spec (spack.spec.Spec): a spec to disambiguate
+        env (spack.environment.Environment): a spack environment,
+            if one is active, or None if no environment is active
+    """
+    hashes = env.all_hashes() if env else None
+    matching_specs = spack.store.db.query(spec, hashes=hashes)
     if not matching_specs:
         tty.die("Spec '%s' matches no installed packages." % spec)
 
diff --git a/lib/spack/spack/cmd/activate.py b/lib/spack/spack/cmd/activate.py
index de6bbc3ecc..44e77d7ade 100644
--- a/lib/spack/spack/cmd/activate.py
+++ b/lib/spack/spack/cmd/activate.py
@@ -8,6 +8,7 @@
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.environment as ev
 from spack.filesystem_view import YamlFilesystemView
 
 description = "activate a package extension"
@@ -32,7 +33,8 @@ def activate(parser, args):
     if len(specs) != 1:
         tty.die("activate requires one spec.  %d given." % len(specs))
 
-    spec = spack.cmd.disambiguate_spec(specs[0])
+    env = ev.get_env(args, 'activate')
+    spec = spack.cmd.disambiguate_spec(specs[0], env)
     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 6dc903ae68..69f85be234 100644
--- a/lib/spack/spack/cmd/add.py
+++ b/lib/spack/spack/cmd/add.py
@@ -22,7 +22,7 @@ def setup_parser(subparser):
 
 
 def add(parser, args):
-    env = ev.get_env(args, 'add')
+    env = ev.get_env(args, 'add', required=True)
 
     for spec in spack.cmd.parse_specs(args.specs):
         if not env.add(spec):
diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py
index e9aa6d849e..76016f19af 100644
--- a/lib/spack/spack/cmd/buildcache.py
+++ b/lib/spack/spack/cmd/buildcache.py
@@ -8,6 +8,7 @@
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.environment as ev
 import spack.repo
 import spack.store
 import spack.spec
@@ -81,7 +82,8 @@ def setup_parser(subparser):
     dlkeys.set_defaults(func=getkeys)
 
 
-def find_matching_specs(pkgs, allow_multiple_matches=False, force=False):
+def find_matching_specs(
+        pkgs, allow_multiple_matches=False, force=False, env=None):
     """Returns a list of specs matching the not necessarily
        concretized specs given from cli
 
@@ -92,12 +94,14 @@ def find_matching_specs(pkgs, allow_multiple_matches=False, force=False):
     Return:
         list of specs
     """
+    hashes = env.all_hashes() if env else None
+
     # List of specs that match expressions given via command line
     specs_from_cli = []
     has_errors = False
     specs = spack.cmd.parse_specs(pkgs)
     for spec in specs:
-        matching = spack.store.db.query(spec)
+        matching = spack.store.db.query(spec, hashes=hashes)
         # For each spec provided, make sure it refers to only one package.
         # Fail and ask user to be unambiguous if it doesn't
         if not allow_multiple_matches and len(matching) > 1:
@@ -179,7 +183,10 @@ def createtarball(args):
     if args.key:
         signkey = args.key
 
-    matches = find_matching_specs(pkgs, False, False)
+    # restrict matching to current environment if one is active
+    env = ev.get_env(args, 'buildcache create')
+
+    matches = find_matching_specs(pkgs, False, False, env=env)
     for match in matches:
         if match.external or match.virtual:
             tty.msg('skipping external or virtual spec %s' %
diff --git a/lib/spack/spack/cmd/concretize.py b/lib/spack/spack/cmd/concretize.py
index 728718e502..e8443c61a9 100644
--- a/lib/spack/spack/cmd/concretize.py
+++ b/lib/spack/spack/cmd/concretize.py
@@ -17,6 +17,6 @@ def setup_parser(subparser):
 
 
 def concretize(parser, args):
-    env = ev.get_env(args, 'concretize')
+    env = ev.get_env(args, 'concretize', required=True)
     env.concretize(force=args.force)
     env.write()
diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py
index 4fd21768c8..ae38e256b5 100644
--- a/lib/spack/spack/cmd/config.py
+++ b/lib/spack/spack/cmd/config.py
@@ -64,7 +64,7 @@ def _get_scope_and_section(args):
 
     # w/no args and an active environment, point to env manifest
     if not args.section:
-        env = ev.get_env(args, 'config edit', required=False)
+        env = ev.get_env(args, 'config edit')
         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 ef450b1c94..c1d793f159 100644
--- a/lib/spack/spack/cmd/deactivate.py
+++ b/lib/spack/spack/cmd/deactivate.py
@@ -7,6 +7,7 @@
 import llnl.util.tty as tty
 
 import spack.cmd
+import spack.environment as ev
 import spack.store
 from spack.filesystem_view import YamlFilesystemView
 from spack.graph import topological_sort
@@ -37,7 +38,8 @@ def deactivate(parser, args):
     if len(specs) != 1:
         tty.die("deactivate requires one spec.  %d given." % len(specs))
 
-    spec = spack.cmd.disambiguate_spec(specs[0])
+    env = ev.get_env(args, 'deactivate')
+    spec = spack.cmd.disambiguate_spec(specs[0], env)
     pkg = spec.package
 
     if args.view:
diff --git a/lib/spack/spack/cmd/dependencies.py b/lib/spack/spack/cmd/dependencies.py
index fe66bef1df..08c76b3a01 100644
--- a/lib/spack/spack/cmd/dependencies.py
+++ b/lib/spack/spack/cmd/dependencies.py
@@ -8,6 +8,7 @@
 import llnl.util.tty as tty
 from llnl.util.tty.colify import colify
 
+import spack.environment as ev
 import spack.store
 import spack.repo
 import spack.cmd
@@ -38,7 +39,8 @@ def dependencies(parser, args):
         tty.die("spack dependencies takes only one spec.")
 
     if args.installed:
-        spec = spack.cmd.disambiguate_spec(specs[0])
+        env = ev.get_env(args, 'dependencies')
+        spec = spack.cmd.disambiguate_spec(specs[0], env)
 
         tty.msg("Dependencies of %s" % spec.format('$_$@$%@$/', color=True))
         deps = spack.store.db.installed_relatives(
diff --git a/lib/spack/spack/cmd/dependents.py b/lib/spack/spack/cmd/dependents.py
index 46397ed5f0..937dd2cd3a 100644
--- a/lib/spack/spack/cmd/dependents.py
+++ b/lib/spack/spack/cmd/dependents.py
@@ -8,6 +8,7 @@
 import llnl.util.tty as tty
 from llnl.util.tty.colify import colify
 
+import spack.environment as ev
 import spack.repo
 import spack.store
 import spack.cmd
@@ -81,7 +82,8 @@ def dependents(parser, args):
         tty.die("spack dependents takes only one spec.")
 
     if args.installed:
-        spec = spack.cmd.disambiguate_spec(specs[0])
+        env = ev.get_env(args, 'dependents')
+        spec = spack.cmd.disambiguate_spec(specs[0], env)
 
         tty.msg("Dependents of %s" % spec.cformat('$_$@$%@$/'))
         deps = spack.store.db.installed_relatives(
diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py
index c8b9c169b5..5c24de05b1 100644
--- a/lib/spack/spack/cmd/env.py
+++ b/lib/spack/spack/cmd/env.py
@@ -280,7 +280,7 @@ def env_status_setup_parser(subparser):
 
 
 def env_status(args):
-    env = ev.get_env(args, 'env status', required=False)
+    env = ev.get_env(args, 'env status')
     if env:
         if env.path == os.getcwd():
             tty.msg('Using %s in current directory: %s'
@@ -305,7 +305,7 @@ def env_loads_setup_parser(subparser):
 
 
 def env_loads(args):
-    env = ev.get_env(args, 'env loads')
+    env = ev.get_env(args, 'env loads', required=True)
 
     # 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 a564248d63..f505cf908d 100644
--- a/lib/spack/spack/cmd/extensions.py
+++ b/lib/spack/spack/cmd/extensions.py
@@ -8,6 +8,7 @@
 import llnl.util.tty as tty
 from llnl.util.tty.colify import colify
 
+import spack.environment as ev
 import spack.cmd
 import spack.cmd.find
 import spack.repo
@@ -75,7 +76,8 @@ def extensions(parser, args):
     if not spec[0].package.extendable:
         tty.die("%s is not an extendable package." % spec[0].name)
 
-    spec = spack.cmd.disambiguate_spec(spec[0])
+    env = ev.get_env(args, 'extensions')
+    spec = spack.cmd.disambiguate_spec(spec[0], env)
 
     if not spec.package.extendable:
         tty.die("%s does not have extensions." % spec.short_spec)
diff --git a/lib/spack/spack/cmd/find.py b/lib/spack/spack/cmd/find.py
index b1ea65dfc1..cc1d7f5184 100644
--- a/lib/spack/spack/cmd/find.py
+++ b/lib/spack/spack/cmd/find.py
@@ -151,7 +151,7 @@ def find(parser, args):
     added = set()
     removed = set()
 
-    env = ev.get_env(args, 'find', required=False)
+    env = ev.get_env(args, 'find')
     if env:
         decorator, added, roots, removed = setup_env(env)
 
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index d42988303f..628c2d3564 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -160,7 +160,7 @@ def install_spec(cli_args, kwargs, abstract_spec, spec):
 
     # handle active environment, if any
     def install(spec, kwargs):
-        env = ev.get_env(cli_args, 'install', required=False)
+        env = ev.get_env(cli_args, 'install')
         if env:
             env.install(abstract_spec, spec, **kwargs)
             env.write()
@@ -194,7 +194,7 @@ def install(parser, args, **kwargs):
     if not args.package and not args.specfiles:
         # if there are no args but an active environment or spack.yaml file
         # then install the packages from it.
-        env = ev.get_env(args, 'install', required=False)
+        env = ev.get_env(args, 'install')
         if env:
             if not args.only_concrete:
                 env.concretize()
diff --git a/lib/spack/spack/cmd/location.py b/lib/spack/spack/cmd/location.py
index a878f63df8..a9a70142e3 100644
--- a/lib/spack/spack/cmd/location.py
+++ b/lib/spack/spack/cmd/location.py
@@ -9,6 +9,7 @@
 import argparse
 import llnl.util.tty as tty
 
+import spack.environment as ev
 import spack.cmd
 import spack.environment
 import spack.paths
@@ -86,7 +87,8 @@ def location(parser, args):
 
         if args.install_dir:
             # install_dir command matches against installed specs.
-            spec = spack.cmd.disambiguate_spec(specs[0])
+            env = ev.get_env(args, 'location')
+            spec = spack.cmd.disambiguate_spec(specs[0], env)
             print(spec.prefix)
 
         else:
diff --git a/lib/spack/spack/cmd/remove.py b/lib/spack/spack/cmd/remove.py
index ec0b2cd218..656d70f51e 100644
--- a/lib/spack/spack/cmd/remove.py
+++ b/lib/spack/spack/cmd/remove.py
@@ -28,7 +28,7 @@ def setup_parser(subparser):
 
 
 def remove(parser, args):
-    env = ev.get_env(args, 'remove')
+    env = ev.get_env(args, 'remove', required=True)
 
     if args.all:
         env.clear()
diff --git a/lib/spack/spack/cmd/stage.py b/lib/spack/spack/cmd/stage.py
index 68f2486b0d..3488548e6e 100644
--- a/lib/spack/spack/cmd/stage.py
+++ b/lib/spack/spack/cmd/stage.py
@@ -29,7 +29,7 @@ def setup_parser(subparser):
 
 def stage(parser, args):
     if not args.specs:
-        env = ev.get_env(args, 'stage', required=False)
+        env = ev.get_env(args, 'stage')
         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/uninstall.py b/lib/spack/spack/cmd/uninstall.py
index 81e8437999..3105831ef8 100644
--- a/lib/spack/spack/cmd/uninstall.py
+++ b/lib/spack/spack/cmd/uninstall.py
@@ -301,7 +301,7 @@ def get_uninstall_list(args, specs, env):
 
 
 def uninstall_specs(args, specs):
-    env = ev.get_env(args, 'uninstall', required=False)
+    env = ev.get_env(args, 'uninstall')
 
     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/view.py b/lib/spack/spack/cmd/view.py
index 9768458f02..c95bdf6e5f 100644
--- a/lib/spack/spack/cmd/view.py
+++ b/lib/spack/spack/cmd/view.py
@@ -38,6 +38,7 @@
 import llnl.util.tty as tty
 from llnl.util.link_tree import MergeConflictError
 
+import spack.environment as ev
 import spack.cmd
 import spack.store
 from spack.filesystem_view import YamlFilesystemView
@@ -172,7 +173,8 @@ def view(parser, args):
 
     elif args.action in actions_link:
         # only link commands need to disambiguate specs
-        specs = [spack.cmd.disambiguate_spec(s) for s in specs]
+        env = ev.get_env(args, 'view link')
+        specs = [spack.cmd.disambiguate_spec(s, env) for s in specs]
 
     elif args.action in actions_status:
         # no specs implies all
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 40d63eaa0e..18bf998438 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -184,7 +184,7 @@ def find_environment(args):
     raise SpackEnvironmentError('no environment in %s' % env)
 
 
-def get_env(args, cmd_name, required=True):
+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
@@ -202,8 +202,8 @@ def get_env(args, cmd_name, required=True):
     Arguments:
         args (Namespace): argparse namespace wtih command arguments
         cmd_name (str): name of calling command
-        required (bool): if ``False``, return ``None`` if no environment
-            is found instead of raising an exception.
+        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
-- 
GitLab