make `spack fetch` work with environments (#19166)

* make `spack fetch` work with environments
* previously: `spack fetch` required the explicit statement of
              the specs to be fetched, even when in an environment
* now: if there is no spec(s) provided to `spack fetch` we check
       if an environment is active and if yes we fetch all
       uninstalled specs.

4 files changed, 117 insertions, 20 deletions

diff --git a/lib/spack/spack/cmd/fetch.py b/lib/spack/spack/cmd/fetch.py
index 962bcefce6..bb4f535180 100644
--- a/lib/spack/spack/cmd/fetch.py
+++ b/lib/spack/spack/cmd/fetch.py
@@ -8,6 +8,7 @@ import llnl.util.tty as tty
 import spack.cmd
 import spack.cmd.common.arguments as arguments
 import spack.config
+import spack.environment as ev
 import spack.repo
 
 description = "fetch archives for packages"
@@ -18,25 +19,54 @@ level = "long"
 def setup_parser(subparser):
     arguments.add_common_arguments(subparser, ['no_checksum', 'deprecated'])
     subparser.add_argument(
-        '-m', '--missing', action='store_true',
-        help="fetch only missing (not yet installed) dependencies")
+        "-m",
+        "--missing",
+        action="store_true",
+        help="fetch only missing (not yet installed) dependencies",
+    )
     subparser.add_argument(
-        '-D', '--dependencies', action='store_true',
-        help="also fetch all dependencies")
-    arguments.add_common_arguments(subparser, ['specs'])
+        "-D",
+        "--dependencies",
+        action="store_true",
+        help="also fetch all dependencies",
+    )
+    arguments.add_common_arguments(subparser, ["specs"])
+    subparser.epilog = (
+        "With an active environment, the specs "
+        "parameter can be omitted. In this case all (uninstalled"
+        ", in case of --missing) specs from the environment are fetched"
+    )
 
 
 def fetch(parser, args):
-    if not args.specs:
-        tty.die("fetch requires at least one package argument")
+    if args.specs:
+        specs = spack.cmd.parse_specs(args.specs, concretize=True)
+    else:
+        # No specs were given explicitly, check if we are in an
+        # environment. If yes, check the missing argument, if yes
+        # 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")
+        if env:
+            if args.missing:
+                specs = env.uninstalled_specs()
+            else:
+                specs = env.all_specs()
+            if specs == []:
+                tty.die(
+                    "No uninstalled specs in environment. Did you "
+                    "run `spack concretize` yet?"
+                )
+        else:
+            tty.die("fetch requires at least one spec argument")
 
     if args.no_checksum:
-        spack.config.set('config:checksum', False, scope='command_line')
+        spack.config.set("config:checksum", False, scope="command_line")
 
     if args.deprecated:
         spack.config.set('config:deprecated', True, scope='command_line')
 
-    specs = spack.cmd.parse_specs(args.specs, concretize=True)
     for spec in specs:
         if args.missing or args.dependencies:
             for s in spec.traverse():
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 5b8e944250..8f930d67c8 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -1397,6 +1397,21 @@ class Environment(object):
                     os.remove(build_log_link)
                 os.symlink(spec.package.build_log_path, build_log_link)
 
+    def uninstalled_specs(self):
+        """Return a list of all uninstalled (and non-dev) specs."""
+        # Do the installed check across all specs within a single
+        # DB read transaction to reduce time spent in lock acquisition.
+        uninstalled_specs = []
+        with spack.store.db.read_transaction():
+            for concretized_hash in self.concretized_order:
+                spec = self.specs_by_hash[concretized_hash]
+                if not spec.package.installed or (
+                        spec.satisfies('dev_path=*') or
+                        spec.satisfies('^dev_path=*')
+                ):
+                    uninstalled_specs.append(spec)
+        return uninstalled_specs
+
     def install_all(self, args=None, **install_args):
         """Install all concretized specs in an environment.
 
@@ -1407,22 +1422,13 @@ class Environment(object):
             args (Namespace): argparse namespace with command arguments
             install_args (dict): keyword install arguments
         """
+        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.
-        tty.debug('Assessing installation status of environment packages')
-        specs_to_install = []
-        with spack.store.db.read_transaction():
-            for concretized_hash in self.concretized_order:
-                spec = self.specs_by_hash[concretized_hash]
-                if not spec.package.installed or (
-                        spec.satisfies('dev_path=*') or
-                        spec.satisfies('^dev_path=*')
-                ):
-                    # If it's a dev build it could need to be reinstalled
-                    specs_to_install.append(spec)
+        specs_to_install = self.uninstalled_specs()
 
         if not specs_to_install:
             tty.msg('All of the packages are already installed')
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index f83fcc12dd..e2df94cbfd 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -139,6 +139,19 @@ def test_concretize():
     assert any(x.name == 'mpileaks' for x in env_specs)
 
 
+def test_env_uninstalled_specs(install_mockery, mock_fetch):
+    e = ev.create('test')
+    e.add('cmake-client')
+    e.concretize()
+    assert any(s.name == 'cmake-client' for s in e.uninstalled_specs())
+    e.install_all()
+    assert not any(s.name == 'cmake-client' for s in e.uninstalled_specs())
+    e.add('mpileaks')
+    e.concretize()
+    assert not any(s.name == 'cmake-client' for s in e.uninstalled_specs())
+    assert any(s.name == 'mpileaks' for s in e.uninstalled_specs())
+
+
 def test_env_install_all(install_mockery, mock_fetch):
     e = ev.create('test')
     e.add('cmake-client')
diff --git a/lib/spack/spack/test/cmd/fetch.py b/lib/spack/spack/test/cmd/fetch.py
new file mode 100644
index 0000000000..d2f17eeb1b
--- /dev/null
+++ b/lib/spack/spack/test/cmd/fetch.py
@@ -0,0 +1,48 @@
+# Copyright 2013-2020 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 pytest
+
+import spack.environment as ev
+
+from spack.main import SpackCommand, SpackCommandError
+
+
+# everything here uses the mock_env_path
+pytestmark = pytest.mark.usefixtures(
+    "mutable_mock_env_path", "config", "mutable_mock_repo"
+)
+
+
+@pytest.mark.disable_clean_stage_check
+def test_fetch_in_env(
+    tmpdir, mock_archive, mock_stage, mock_fetch, install_mockery
+):
+    SpackCommand("env")("create", "test")
+    with ev.read("test"):
+        SpackCommand("add")("python")
+        with pytest.raises(SpackCommandError):
+            SpackCommand("fetch")()
+        SpackCommand("concretize")()
+        SpackCommand("fetch")()
+
+
+@pytest.mark.disable_clean_stage_check
+def test_fetch_single_spec(
+    tmpdir, mock_archive, mock_stage, mock_fetch, install_mockery
+):
+    SpackCommand("fetch")("mpileaks")
+
+
+@pytest.mark.disable_clean_stage_check
+def test_fetch_multiple_specs(
+    tmpdir, mock_archive, mock_stage, mock_fetch, install_mockery
+):
+    SpackCommand("fetch")("mpileaks", "gcc@10.2.0", "python")
+
+
+def test_fetch_no_argument():
+    with pytest.raises(SpackCommandError):
+        SpackCommand("fetch")()