shell integration: make output more consistent

This cleans up the output for commands with shell support now that csh
and tcsh do not require SPACK_ROOT to be set. This also adds output
describing how to source the fish init script, and makes messages more
consistent across commands.

- [x] Remove SPACK_ROOT from csh/tcsh instructions
- [x] Add fish to shell instructions
- [x] make output consistent across commands

.github/workflows/linux_unit_tests.yaml

@@ -26,9 +26,12 @@ jobs:
     - name: Install System packages
       run: |
           sudo apt-get -y update
-          sudo apt-get install -y coreutils gfortran graphviz gnupg2 mercurial ninja-build patchelf
+          # Needed for unit tests
+          sudo apt-get install -y coreutils gfortran graphviz gnupg2 mercurial
+          sudo apt-get install -y ninja-build patchelf
           # Needed for kcov
-          sudo apt-get -y install cmake binutils-dev libcurl4-openssl-dev zlib1g-dev libdw-dev libiberty-dev
+          sudo apt-get -y install cmake binutils-dev libcurl4-openssl-dev
+          sudo apt-get -y install zlib1g-dev libdw-dev libiberty-dev
     - name: Install Python packages
       run: |
           pip install --upgrade pip six setuptools codecov coverage
@@ -69,9 +72,11 @@ jobs:
     - name: Install System packages
       run: |
           sudo apt-get -y update
-          sudo apt-get install -y coreutils gfortran gnupg2 mercurial ninja-build patchelf zsh fish
+          # Needed for shell tests
+          sudo apt-get install -y coreutils csh zsh tcsh fish dash bash
           # Needed for kcov
-          sudo apt-get -y install cmake binutils-dev libcurl4-openssl-dev zlib1g-dev libdw-dev libiberty-dev
+          sudo apt-get -y install cmake binutils-dev libcurl4-openssl-dev
+          sudo apt-get -y zlib1g-dev libdw-dev libiberty-dev
     - name: Install Python packages
       run: |
           pip install --upgrade pip six setuptools codecov coverage

.github/workflows/macos_unit_tests.yaml

@@ -26,7 +26,7 @@ jobs:
           pip install --upgrade flake8 pep8-naming
     - name: Setup Homebrew packages
       run: |
-        brew install gcc gnupg2 dash kcov
+        brew install dash fish gcc gnupg2 kcov
     - name: Run unit tests
       run: |
         git --version

lib/spack/spack/cmd/cd.py

@@ -3,8 +3,9 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-from spack.cmd.common import print_module_placeholder_help
+import llnl.util.tty as tty
 
+import spack.cmd.common
 import spack.cmd.location
 
 description = "cd to spack directories in the shell"
@@ -20,4 +21,8 @@ def setup_parser(subparser):
 
 
 def cd(parser, args):
-    print_module_placeholder_help()
+    lines = [
+        "`spack cd` requires spack's shell support.",
+        "",
+    ] + spack.cmd.common.shell_init_instructions()
+    tty.msg(*lines)

lib/spack/spack/cmd/common/__init__.py

@@ -3,35 +3,21 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-
 import spack.paths
-from llnl.util import tty
-
-
-shell_init_instructions = [
-    "To initialize spack's shell commands:",
-    "",
-    "    # for bash and zsh",
-    "    . %s/setup-env.sh" % spack.paths.share_path,
-    "",
-    "    # for csh and tcsh",
-    "    setenv SPACK_ROOT %s" % spack.paths.prefix,
-    "    source %s/setup-env.csh" % spack.paths.share_path, ""
-]
+import llnl.util.tty.color as color
 
 
-def print_module_placeholder_help():
-    """
-    For use by commands to tell user how to activate shell support.
-    """
-    msg = [
-        "This command requires spack's shell integration.", ""
-    ] + shell_init_instructions + [
-        "This exposes a 'spack' shell function, which you can use like",
-        "    $ spack load package-foo", "",
-        "Running the Spack executable directly (for example, invoking",
-        "./bin/spack) will bypass the shell function and print this",
-        "placeholder message, even if you have sourced one of the above",
-        "shell integration scripts."
+def shell_init_instructions():
+    return [
+        "To set up shell support, run the command below for your shell.",
+        "",
+        color.colorize("@*c{For bash/zsh/sh:}"),
+        "  . %s/setup-env.sh" % spack.paths.share_path,
+        "",
+        color.colorize("@*c{For csh/tcsh:}"),
+        "  source %s/setup-env.csh" % spack.paths.share_path,
+        "",
+        color.colorize("@*c{For fish:}"),
+        "  source %s/setup-env.fish" % spack.paths.share_path,
+        "",
     ]
-    tty.msg(*msg)

lib/spack/spack/cmd/env.py

@@ -85,14 +85,16 @@ def env_activate(args):
     env = args.activate_env
     if not args.shell:
         msg = [
-            "This command works best with Spack's shell support",
+            "`spack env activate` works best with spack's shell support.",
             ""
-        ] + spack.cmd.common.shell_init_instructions + [
-            'Or, if you want to use `spack env activate` without initializing',
-            'shell support, you can run one of these:',
+        ] + spack.cmd.common.shell_init_instructions() + [
+            'Or, if you want to use `spack env activate` without shell',
+            'support, you can run one of these:',
             '',
-            '    eval `spack env activate --sh %s`   # for bash/sh' % env,
-            '    eval `spack env activate --csh %s`  # for csh/tcsh' % env,
+            '    eval `spack env activate --sh   %s`  # bash/zsh/sh' % env,
+            '    eval `spack env activate --csh  %s`  # csh/tcsh' % env,
+            '    eval `spack env activate --fish %s`  # fish' % env,
+            ''
         ]
         tty.msg(*msg)
         return 1
@@ -142,14 +144,16 @@ def env_deactivate_setup_parser(subparser):
 def env_deactivate(args):
     if not args.shell:
         msg = [
-            "This command works best with Spack's shell support",
+            "`spack env deactivate` works best with spack's shell support.",
             ""
-        ] + spack.cmd.common.shell_init_instructions + [
-            'Or, if you want to use `spack env activate` without initializing',
-            'shell support, you can run one of these:',
+        ] + spack.cmd.common.shell_init_instructions() + [
+            'Or, if you want to use `spack env deactivate` without shell',
+            'support, you can run one of these:',
+            '',
+            '    eval `spack env deactivate --sh`   # bash/zsh/sh',
+            '    eval `spack env deactivate --csh`  # csh/tcsh',
+            '    eval `spack env deactivate --fish  # fish',
             '',
-            '    eval `spack env deactivate --sh`   # for bash/sh',
-            '    eval `spack env deactivate --csh`  # for csh/tcsh',
         ]
         tty.msg(*msg)
         return 1

lib/spack/spack/cmd/load.py

@@ -62,16 +62,18 @@ def load(parser, args):
              for spec in spack.cmd.parse_specs(args.specs)]
 
     if not args.shell:
-        specs_string = ' '.join(args.specs)
+        specs_str = ' '.join(args.specs) or "SPECS"
         msg = [
-            "This command works best with Spack's shell support",
+            "`spack load` works best with spack's shell support.",
             ""
-        ] + spack.cmd.common.shell_init_instructions + [
-            'Or, if you want to use `spack load` without initializing',
-            'shell support, you can run one of these:',
+        ] + spack.cmd.common.shell_init_instructions() + [
+            'Or, if you want to use `spack load` without shell',
+            'support, you can run one of these:',
+            '',
+            '    eval `spack load --sh   %s`  # bash/zsh/sh' % specs_str,
+            '    eval `spack load --csh  %s`  # csh/tcsh' % specs_str,
+            '    eval `spack load --fish %s`  # fish' % specs_str,
             '',
-            '    eval `spack load --sh %s`   # for bash/sh' % specs_string,
-            '    eval `spack load --csh %s`  # for csh/tcsh' % specs_string,
         ]
         tty.msg(*msg)
         return 1

lib/spack/spack/cmd/unload.py

@@ -53,15 +53,18 @@ def unload(parser, args):
         specs = spack.store.db.query(hashes=hashes)
 
     if not args.shell:
+        specs_str = ' '.join(args.specs) or "SPECS"
         msg = [
-            "This command works best with Spack's shell support",
+            "`spack unload` works best with spack's shell support.",
             ""
-        ] + spack.cmd.common.shell_init_instructions + [
-            'Or, if you want to use `spack unload` without initializing',
-            'shell support, you can run one of these:',
+        ] + spack.cmd.common.shell_init_instructions() + [
+            'Or, if you want to use `spack unload` without shell',
+            'support, you can run one of these:',
+            '',
+            '    eval `spack unload --sh   %s`  # bash/zsh/sh' % specs_str,
+            '    eval `spack unload --csh  %s`  # csh/tcsh' % specs_str,
+            '    eval `spack unload --fish %s`  # fish' % specs_str,
             '',
-            '    eval `spack unload --sh %s`   # for bash/sh' % args.specs,
-            '    eval `spack unload --csh %s`  # for csh/tcsh' % args.specs,
         ]
         tty.msg(*msg)
         return 1

lib/spack/spack/test/cmd/cd.py

@@ -14,4 +14,4 @@ def test_cd():
 
     out = cd()
 
-    assert "To initialize spack's shell commands:" in out
+    assert "To set up shell support" in out

lib/spack/spack/test/cmd/env.py

@@ -1110,7 +1110,7 @@ def test_env_activate_view_fails(
         tmpdir, mock_stage, mock_fetch, install_mockery, env_deactivate):
     """Sanity check on env activate to make sure it requires shell support"""
     out = env('activate', 'test')
-    assert "To initialize spack's shell commands:" in out
+    assert "To set up shell support" in out
 
 
 def test_stack_yaml_definitions(tmpdir):

lib/spack/spack/test/cmd/load.py

@@ -102,7 +102,7 @@ def test_load_fails_no_shell(install_mockery, mock_fetch, mock_archive,
     install('mpileaks')
 
     out = load('mpileaks', fail_on_error=False)
-    assert "To initialize spack's shell commands" in out
+    assert "To set up shell support" in out
 
 
 def test_unload(install_mockery, mock_fetch, mock_archive, mock_packages,
@@ -135,4 +135,4 @@ def test_unload_fails_no_shell(install_mockery, mock_fetch, mock_archive,
     os.environ[uenv.spack_loaded_hashes_var] = mpileaks_spec.dag_hash()
 
     out = unload('mpileaks', fail_on_error=False)
-    assert "To initialize spack's shell commands" in out
+    assert "To set up shell support" in out