install cmd: --no-add in an env installs w/out concretize and add

In an active concretize environment, support installing one or more
cli specs only if they are already present in the environment.  The
`--no-add` option is the default for root specs, but optional for
dependency specs.  I.e. if you `spack install <depspec>` in an
environment, the dependency-only spec `depspec` will be added as a
root of the environment before being installed.  In addition,
`spack install --no-add <spec>` fails if it does not find an
unambiguous match for `spec`.

4 files changed, 196 insertions, 8 deletions

diff --git a/lib/spack/docs/environments.rst b/lib/spack/docs/environments.rst
index 76b974ff62..12f6f4121c 100644
--- a/lib/spack/docs/environments.rst
+++ b/lib/spack/docs/environments.rst
@@ -356,6 +356,18 @@ command also stores a Spack repo containing the ``package.py`` file
 used at install time for each package in the ``repos/`` directory in
 the Environment.
 
+The ``--no-add`` option can be used in a concrete environment to tell
+spack to install specs already present in the environment but not to
+add any new root specs to the environment.  For root specs provided
+to ``spack install`` on the command line, ``--no-add`` is the default,
+while for dependency specs on the other hand, it is optional.  In other
+words, if there is an unambiguous match in the active concrete environment
+for a root spec provided to ``spack install`` on the command line, spack
+does not require you to specify the ``--no-add` option to prevent the spec
+from being added again.  At the same time, a spec that already exists in the
+environment, but only as a dependency, will be added to the environment as a
+root spec without the ``--no-add`` option.
+
 ^^^^^^^
 Loading
 ^^^^^^^
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index 26d824c128..53e924cb9e 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -140,6 +140,10 @@ remote spec matches that of the local spec""")
         '--only-concrete', action='store_true', default=False,
         help='(with environment) only install already concretized specs')
     subparser.add_argument(
+        '--no-add', action='store_true', default=False,
+        help="""(with environment) only install specs provided as argument
+if they are already in the concretized environment""")
+    subparser.add_argument(
         '-f', '--file', action='append', default=[],
         dest='specfiles', metavar='SPEC_YAML_FILE',
         help="install from file. Read specs to install from .yaml files")
@@ -205,11 +209,66 @@ def install_specs(cli_args, kwargs, specs):
 
     try:
         if env:
+            specs_to_install = []
+            specs_to_add = []
             for abstract, concrete in specs:
-                with env.write_transaction():
-                    concrete = env.concretize_and_add(abstract, concrete)
-                    env.write(regenerate_views=False)
-            env.install_all(cli_args, **kwargs)
+                # This won't find specs added to the env since last
+                # concretize, therefore should we consider enforcing
+                # concretization of the env before allowing to install
+                # specs?
+                m_spec = env.matching_spec(abstract)
+
+                # If there is any ambiguity in the above call to matching_spec
+                # (i.e. if more than one spec in the environment matches), then
+                # SpackEnvironmentError is rasied, with a message listing the
+                # the matches.  Getting to this point means there were either
+                # no matches or exactly one match.
+
+                if not m_spec:
+                    tty.debug('{0} matched nothing in the env'.format(
+                        abstract.name))
+                    # no matches in the env
+                    if cli_args.no_add:
+                        msg = ('You asked to install {0} without adding it ' +
+                               '(--no-add), but no such spec exists in ' +
+                               'environment').format(abstract.name)
+                        tty.die(msg)
+                    else:
+                        tty.debug('adding {0} as a root'.format(abstract.name))
+                        specs_to_add.append((abstract, concrete))
+
+                    continue
+
+                tty.debug('exactly one match for {0} in env -> {1}'.format(
+                    m_spec.name, m_spec.dag_hash()))
+
+                if m_spec in env.roots() or cli_args.no_add:
+                    # either the single match is a root spec (and --no-add is
+                    # the default for roots) or --no-add was stated explictly
+                    tty.debug('just install {0}'.format(m_spec.name))
+                    specs_to_install.append(m_spec)
+                else:
+                    # the single match is not a root (i.e. it's a dependency),
+                    # and --no-add was not specified, so we'll add it as a
+                    # root before installing
+                    tty.debug('add {0} then install it'.format(m_spec.name))
+                    specs_to_add.append((abstract, concrete))
+
+            if specs_to_add:
+                tty.debug('Adding the following specs as roots:')
+                for abstract, concrete in specs_to_add:
+                    tty.debug('  {0}'.format(abstract.name))
+                    with env.write_transaction():
+                        specs_to_install.append(
+                            env.concretize_and_add(abstract, concrete))
+                        env.write(regenerate_views=False)
+
+            # Install the validated list of cli specs
+            if specs_to_install:
+                tty.debug('Installing the following cli specs:')
+                for s in specs_to_install:
+                    tty.debug('  {0}'.format(s.name))
+                env.install_specs(specs_to_install, args=cli_args, **kwargs)
         else:
             installs = [(concrete.package, kwargs) for _, concrete in specs]
             builder = PackageInstaller(installs)
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index ab02f46222..46541fb32a 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -1440,21 +1440,31 @@ class Environment(object):
             args (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
+
         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
         # a large amount of time due to repeatedly acquiring and releasing
         # locks, this does an initial check across all specs within a single
-        # DB read transaction to reduce time spent in this case.
-        specs_to_install = self.uninstalled_specs()
+        # DB read transaction to reduce time spent in this case. In the next
+        # three lines we remove any already-installed root specs from the list
+        # to install.  However, uninstalled_specs() only considers root specs,
+        # so this will allow dep specs to be unnecessarily re-installed.
+        uninstalled_roots = self.uninstalled_specs()
+        specs_to_install = specs or uninstalled_roots
+        specs_to_install = [s for s in specs_to_install
+                            if s not in self.roots() or s in uninstalled_roots]
 
         if not specs_to_install:
             tty.msg('All of the packages are already installed')
             return
 
-        tty.debug('Processing {0} uninstalled specs'
-                  .format(len(specs_to_install)))
+        tty.debug('Processing {0} uninstalled specs'.format(
+            len(specs_to_install)))
 
         install_args['overwrite'] = install_args.get(
             'overwrite', []) + self._get_overwrite_specs()
diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py
index 73d701c06f..cbdea73931 100644
--- a/lib/spack/spack/test/cmd/install.py
+++ b/lib/spack/spack/test/cmd/install.py
@@ -32,6 +32,7 @@ add = SpackCommand('add')
 mirror = SpackCommand('mirror')
 uninstall = SpackCommand('uninstall')
 buildcache = SpackCommand('buildcache')
+find = SpackCommand('find')
 
 
 @pytest.fixture()
@@ -712,6 +713,112 @@ def test_install_only_dependencies_of_all_in_env(
                 assert os.path.exists(dep.prefix)
 
 
+def test_install_no_add_in_env(tmpdir, mock_fetch, install_mockery,
+                               mutable_mock_env_path):
+    # To test behavior of --no-add option, we create the following environment:
+    #
+    #     mpileaks
+    #         ^callpath
+    #             ^dyninst
+    #                 ^libelf@0.8.13     # or latest, really
+    #                 ^libdwarf
+    #         ^mpich
+    #     libelf@0.8.10
+    #     a~bvv
+    #         ^b
+    #     a
+    #         ^b
+    e = ev.create('test')
+    e.add('mpileaks')
+    e.add('libelf@0.8.10')  # so env has both root and dep libelf specs
+    e.add('a')
+    e.add('a ~bvv')
+    e.concretize()
+    env_specs = e.all_specs()
+
+    a_spec = None
+    b_spec = None
+    mpi_spec = None
+
+    # First find and remember some target concrete specs in the environment
+    for e_spec in env_specs:
+        if e_spec.satisfies(Spec('a ~bvv')):
+            a_spec = e_spec
+        elif e_spec.name == 'b':
+            b_spec = e_spec
+        elif e_spec.satisfies(Spec('mpi')):
+            mpi_spec = e_spec
+
+    assert(a_spec)
+    assert(a_spec.concrete)
+
+    assert(b_spec)
+    assert(b_spec.concrete)
+    assert(b_spec not in e.roots())
+
+    assert(mpi_spec)
+    assert(mpi_spec.concrete)
+
+    # Activate the environment
+    with e:
+        # Assert using --no-add with a spec not in the env fails
+        inst_out = install(
+            '--no-add', 'boost', fail_on_error=False, output=str)
+
+        assert('no such spec exists in environment' in inst_out)
+
+        # Ensure using --no-add with an ambiguous spec fails
+        with pytest.raises(ev.SpackEnvironmentError) as err:
+            inst_out = install(
+                '--no-add', 'a', output=str)
+
+        assert('a matches multiple specs in the env' in str(err))
+
+        # With "--no-add", install an unambiguous dependency spec (that already
+        # exists as a dep in the environment) using --no-add and make sure it
+        # gets installed (w/ deps), but is not added to the environment.
+        install('--no-add', 'dyninst')
+
+        find_output = find('-l', output=str)
+        assert('dyninst' in find_output)
+        assert('libdwarf' in find_output)
+        assert('libelf' in find_output)
+        assert('callpath' not in find_output)
+
+        post_install_specs = e.all_specs()
+        assert all([s in env_specs for s in post_install_specs])
+
+        # Make sure we can install a concrete dependency spec from a spec.yaml
+        # file on disk, using the ``--no-add` option, and the spec is installed
+        # but not added as a root
+        mpi_spec_yaml_path = tmpdir.join('{0}.yaml'.format(mpi_spec.name))
+        with open(mpi_spec_yaml_path.strpath, 'w') as fd:
+            fd.write(mpi_spec.to_yaml(hash=ht.full_hash))
+
+        install('--no-add', '-f', mpi_spec_yaml_path.strpath)
+        assert(mpi_spec not in e.roots())
+
+        find_output = find('-l', output=str)
+        assert(mpi_spec.name in find_output)
+
+        # Without "--no-add", install an unambiguous depependency spec (that
+        # already exists as a dep in the environment) without --no-add and make
+        # sure it is added as a root of the environment as well as installed.
+        assert(b_spec not in e.roots())
+
+        install('b')
+
+        assert(b_spec in e.roots())
+        assert(b_spec not in e.uninstalled_specs())
+
+        # Without "--no-add", install a novel spec and make sure it is added
+        # as a root and installed.
+        install('bowtie')
+
+        assert(any([s.name == 'bowtie' for s in e.roots()]))
+        assert(not any([s.name == 'bowtie' for s in e.uninstalled_specs()]))
+
+
 def test_install_help_does_not_show_cdash_options(capsys):
     """
     Make sure `spack install --help` does not describe CDash arguments