env: move add, remove, and concretize to top-level commands

7 files changed, 155 insertions, 104 deletions

diff --git a/lib/spack/spack/cmd/add.py b/lib/spack/spack/cmd/add.py
new file mode 100644
index 0000000000..d154a6bc7a
--- /dev/null
+++ b/lib/spack/spack/cmd/add.py
@@ -0,0 +1,33 @@
+# Copyright 2013-2018 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+import argparse
+
+import llnl.util.tty as tty
+
+import spack.cmd
+import spack.environment as ev
+
+
+description = 'add a spec to an environment'
+section = "environment"
+level = "long"
+
+
+def setup_parser(subparser):
+    subparser.add_argument(
+        'specs', nargs=argparse.REMAINDER, help="specs of packages to add")
+
+
+def add(parser, args):
+    env = ev.get_env(args, 'add')
+
+    for spec in spack.cmd.parse_specs(args.specs):
+        if not env.add(spec):
+            tty.msg("Package {0} was already added to {1}"
+                    .format(spec.name, env.name))
+        else:
+            tty.msg('Adding %s to environment %s' % (spec, env.name))
+    env.write()
diff --git a/lib/spack/spack/cmd/concretize.py b/lib/spack/spack/cmd/concretize.py
new file mode 100644
index 0000000000..abc6aa57d4
--- /dev/null
+++ b/lib/spack/spack/cmd/concretize.py
@@ -0,0 +1,22 @@
+# Copyright 2013-2018 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+import spack.environment as ev
+
+description = 'concretize an environment and write a lockfile'
+section = "environment"
+level = "long"
+
+
+def setup_parser(subparser):
+    subparser.add_argument(
+        '-f', '--force', action='store_true',
+        help="Re-concretize even if already concretized.")
+
+
+def concretize(parser, args):
+    env = ev.get_env(args, 'concretize')
+    env.concretize(force=args.force)
+    env.write()
diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py
index 478b991010..14deacd14e 100644
--- a/lib/spack/spack/cmd/env.py
+++ b/lib/spack/spack/cmd/env.py
@@ -5,7 +5,6 @@
 
 import os
 import sys
-import argparse
 
 import llnl.util.tty as tty
 import llnl.util.filesystem as fs
@@ -33,9 +32,6 @@ subcommands = [
     'create',
     'destroy',
     ['list', 'ls'],
-    'add',
-    ['remove', 'rm'],
-    'concretize',
     ['status', 'st'],
     'loads',
     'stage',
@@ -270,68 +266,6 @@ def env_list(args):
     colify(color_names, indent=4)
 
 
-#
-# env add
-#
-def env_add_setup_parser(subparser):
-    """add a spec to an environment"""
-    subparser.add_argument(
-        'specs', nargs=argparse.REMAINDER, help="spec of the package to add")
-
-
-def env_add(args):
-    env = ev.get_env(args, 'env add')
-
-    for spec in spack.cmd.parse_specs(args.specs):
-        if not env.add(spec):
-            tty.msg("Package {0} was already added to {1}"
-                    .format(spec.name, env.name))
-        else:
-            tty.msg('Adding %s to environment %s' % (spec, env.name))
-    env.write()
-
-
-#
-# env remove
-#
-def env_remove_setup_parser(subparser):
-    """remove a spec from an environment"""
-    subparser.add_argument(
-        '-a', '--all', action='store_true', dest='all',
-        help="Remove all specs from (clear) the environment")
-    subparser.add_argument(
-        'specs', nargs=argparse.REMAINDER, help="specs to be removed")
-
-
-def env_remove(args):
-    env = ev.get_env(args, 'env remove <spec>')
-
-    if args.all:
-        env.clear()
-    else:
-        for spec in spack.cmd.parse_specs(args.specs):
-            tty.msg('Removing %s from environment %s' % (spec, env.name))
-            env.remove(spec)
-    env.write()
-
-
-#
-# env concretize
-#
-def env_concretize_setup_parser(subparser):
-    """concretize user specs and write lockfile"""
-    subparser.add_argument(
-        'env', nargs='?', help='concretize all packages for this environment')
-    subparser.add_argument(
-        '-f', '--force', action='store_true',
-        help="Re-concretize even if already concretized.")
-
-
-def env_concretize(args):
-    env = ev.get_env(args, 'env concretize')
-    env.concretize(force=args.force)
-    env.write()
-
 # REMOVE
 # env uninstall
 #
@@ -456,7 +390,7 @@ def setup_parser(subparser):
         setup_parser_cmd(subsubparser)
 
 
-def env(parser, args, **kwargs):
+def env(parser, args):
     """Look for a function called environment_<name> and call it."""
     action = subcommand_functions[args.env_command]
     action(args)
diff --git a/lib/spack/spack/cmd/remove.py b/lib/spack/spack/cmd/remove.py
new file mode 100644
index 0000000000..5e4a6e5939
--- /dev/null
+++ b/lib/spack/spack/cmd/remove.py
@@ -0,0 +1,39 @@
+# Copyright 2013-2018 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+import argparse
+
+import llnl.util.tty as tty
+
+import spack.cmd
+import spack.environment as ev
+
+
+description = 'remove specs from an environment'
+section = "environment"
+level = "long"
+
+
+def setup_parser(subparser):
+    subparser.add_argument(
+        '-a', '--all', action='store_true',
+        help="remove all specs from (clear) the environment")
+    subparser.add_argument(
+        '-f', '--force', action='store_true',
+        help="remove concretized spec (if any) immediately")
+    subparser.add_argument(
+        'specs', nargs=argparse.REMAINDER, help="specs to be removed")
+
+
+def remove(parser, args):
+    env = ev.get_env(args, 'remove')
+
+    if args.all:
+        env.clear()
+    else:
+        for spec in spack.cmd.parse_specs(args.specs):
+            tty.msg('Removing %s from environment %s' % (spec, env.name))
+            env.remove(spec, force=args.force)
+    env.write()
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 8ba9ac976e..87f4d31026 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -466,17 +466,31 @@ class Environment(object):
             self.user_specs.append(spec)
         return bool(not existing)
 
-    def remove(self, query_spec):
+    def remove(self, query_spec, force=False):
         """Remove specs from an environment that match a query_spec"""
         query_spec = Spec(query_spec)
-        matches = [s for s in self.user_specs if s.satisfies(query_spec)]
+
+        # try abstract specs first
+        matches = []
+        if not query_spec.concrete:
+            matches = [s for s in self.user_specs if s.satisfies(query_spec)]
+
+        if not matches:
+            # concrete specs match against concrete specs in the env
+            specs_hashes = zip(
+                self.concretized_user_specs, self.concretized_order)
+            matches = [
+                s for s, h in specs_hashes
+                if s.satisfies(query_spec) or query_spec.dag_hash() == h]
 
         if not matches:
             raise EnvError("Not found: {0}".format(query_spec))
 
         for spec in matches:
-            self.user_specs.remove(spec)
-            if spec in self.concretized_user_specs:
+            if spec in self.user_specs:
+                self.user_specs.remove(spec)
+
+            if force and spec in self.concretized_user_specs:
                 i = self.concretized_user_specs.index(spec)
                 del self.concretized_user_specs[i]
 
diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py
index 672812adea..ea1ec0df96 100644
--- a/lib/spack/spack/main.py
+++ b/lib/spack/spack/main.py
@@ -38,6 +38,11 @@ from spack.error import SpackError
 #: names of profile statistics
 stat_names = pstats.Stats.sort_arg_dict_default
 
+#: top-level aliases for Spack commands
+aliases = {
+    'rm': 'remove'
+}
+
 #: help levels in order of detail (i.e., number of commands shown)
 levels = ['short', 'long']
 
@@ -174,8 +179,8 @@ class SpackArgumentParser(argparse.ArgumentParser):
             cmd_set = set(c for c in commands)
 
             # make a dict of commands of interest
-            cmds = dict((a.metavar, a) for a in self.actions
-                        if a.metavar in cmd_set)
+            cmds = dict((a.dest, a) for a in self.actions
+                        if a.dest in cmd_set)
 
             # add commands to a group in order, and add the group
             group = argparse._ArgumentGroup(self, title=title)
@@ -271,8 +276,13 @@ class SpackArgumentParser(argparse.ArgumentParser):
         # each command module implements a parser() function, to which we
         # pass its subparser for setup.
         module = spack.cmd.get_module(cmd_name)
+
+        # build a list of aliases
+        alias_list = [k for k, v in aliases.items() if v == cmd_name]
+
         subparser = self.subparsers.add_parser(
-            cmd_name, help=module.description, description=module.description)
+            cmd_name, aliases=alias_list,
+            help=module.description, description=module.description)
         module.setup_parser(subparser)
 
         # return the callable function for the command
@@ -647,6 +657,8 @@ def main(argv=None):
         # Try to load the particular command the caller asked for.  If there
         # is no module for it, just die.
         cmd_name = args.command[0]
+        cmd_name = aliases.get(cmd_name, cmd_name)
+
         try:
             command = parser.add_command(cmd_name)
         except ImportError:
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 87ba5249ef..c3f01883b9 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -21,9 +21,11 @@ from spack.main import SpackCommand
 pytestmark = pytest.mark.usefixtures(
     'mutable_mock_env_path', 'config', 'mutable_mock_packages')
 
-
-env = SpackCommand('env')
-install = SpackCommand('install')
+env         = SpackCommand('env')
+install     = SpackCommand('install')
+add         = SpackCommand('add')
+remove      = SpackCommand('remove')
+concretize  = SpackCommand('concretize')
 
 
 def test_add():
@@ -137,32 +139,34 @@ def test_remove_after_concretize():
     e.concretize()
 
     e.remove('mpileaks')
+    assert Spec('mpileaks') not in e.user_specs
+    env_specs = e._get_environment_specs()
+    assert any(s.name == 'mpileaks' for s in env_specs)
+
+    e.add('mpileaks')
+    assert any(s.name == 'mpileaks' for s in e.user_specs)
+
+    e.remove('mpileaks', force=True)
+    assert Spec('mpileaks') not in e.user_specs
     env_specs = e._get_environment_specs()
-    assert not any(x.name == 'mpileaks' for x in env_specs)
+    assert not any(s.name == 'mpileaks' for s in env_specs)
 
 
 def test_remove_command():
     env('create', 'test')
 
     with ev.read('test'):
-        env('add', 'mpileaks')
+        add('mpileaks')
     assert 'mpileaks' in env('status', 'test')
 
     with ev.read('test'):
-        env('remove', 'mpileaks')
+        remove('mpileaks')
     assert 'mpileaks' not in env('status', 'test')
 
     with ev.read('test'):
-        env('add', 'mpileaks')
+        add('mpileaks')
     assert 'mpileaks' in env('status', 'test')
 
-    env('concretize', 'test')
-    assert 'mpileaks' in env('status', 'test')
-
-    with ev.read('test'):
-        env('remove', 'mpileaks')
-    assert 'mpileaks' not in env('status', 'test')
-
 
 def test_environment_status():
     e = ev.create('test')
@@ -195,7 +199,8 @@ def test_env_repo():
     e.add('mpileaks')
     e.write()
 
-    env('concretize', 'test')
+    with ev.read('test'):
+        concretize()
 
     package = e.repo.get('mpileaks')
     assert package.name == 'mpileaks'
@@ -487,14 +492,12 @@ def test_env_loads(install_mockery, mock_fetch):
     env('create', 'test')
 
     with ev.read('test'):
-        env('add', 'mpileaks')
-
-    env('concretize', 'test')
-
-    with ev.read('test'):
+        add('mpileaks')
+        concretize()
         install('--fake')
 
-    env('loads', 'test')
+    with ev.read('test'):
+        env('loads', 'test')
 
     e = ev.read('test')
 
@@ -510,9 +513,9 @@ def test_env_loads(install_mockery, mock_fetch):
 def test_env_stage(mock_stage, mock_fetch, install_mockery):
     env('create', 'test')
     with ev.read('test'):
-        print env('add', 'mpileaks')
-        print env('add', 'zmpi')
-    env('concretize', 'test')
+        add('mpileaks')
+        add('zmpi')
+        concretize()
     env('stage', 'test')
 
     root = str(mock_stage)
@@ -536,17 +539,11 @@ def test_env_commands_die_with_no_env_arg():
 
     # these have an optional env arg and raise errors via tty.die
     with pytest.raises(spack.main.SpackCommandError):
-        env('concretize')
-    with pytest.raises(spack.main.SpackCommandError):
         env('loads')
     with pytest.raises(spack.main.SpackCommandError):
         env('stage')
     with pytest.raises(spack.main.SpackCommandError):
         env('uninstall')
-    with pytest.raises(spack.main.SpackCommandError):
-        env('add')
-    with pytest.raises(spack.main.SpackCommandError):
-        env('remove')
 
     # This should NOT raise an error with no environment
     # it just tells the user there isn't an environment