csh: don't require SPACK_ROOT for sourcing setup-env.csh (#18225)

Don't require SPACK_ROOT for sourcing setup-env.csh and make output more consistent
2020-10-23 18:54:34 -07:00 · 2020-10-23 18:54:34 -07:00 · 560beb098e
commit 560beb098e
parent 4cf85ed5f7
15 changed files with 240 additions and 120 deletions
--- a/.github/workflows/linux_unit_tests.yaml
+++ b/.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 install zlib1g-dev libdw-dev libiberty-dev
    - name: Install Python packages
      run: |
          pip install --upgrade pip six setuptools codecov coverage
--- a/.github/workflows/macos_unit_tests.yaml
+++ b/.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
--- a/lib/spack/spack/cmd/cd.py
+++ b/lib/spack/spack/cmd/cd.py
@ -3,8 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)

-from spack.cmd.common import print_module_placeholder_help
-
+import spack.cmd.common
 import spack.cmd.location

 description = "cd to spack directories in the shell"
@ -20,4 +19,8 @@ def setup_parser(subparser):


 def cd(parser, args):
-    print_module_placeholder_help()
+    spec = " ".join(args.spec) if args.spec else "SPEC"
+    spack.cmd.common.shell_init_instructions(
+        "spack cd",
+        "cd `spack location --install-dir %s`" % spec
+    )
--- a/lib/spack/spack/cmd/common/init.py
+++ b/lib/spack/spack/cmd/common/init.py
@ -3,35 +3,51 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)

+import llnl.util.tty as tty
+import llnl.util.tty.color as color

 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, ""
-]
+def shell_init_instructions(cmd, equivalent):
+    """Print out instructions for users to initialize shell support.

-
-def print_module_placeholder_help():
-    """
-    For use by commands to tell user how to activate shell support.
+    Arguments:
+        cmd (str): the command the user tried to run that requires
+            shell support in order to work
+        equivalent (str): a command they can run instead, without
+            enabling shell support
    """
+
+    shell_specific = "{sh_arg}" in equivalent
+
    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."
+        "`%s` requires spack's shell support." % cmd,
+        "",
+        "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,
+        "",
+        "Or, if you do not want to use shell support, run " + (
+            "one of these" if shell_specific else "this") + " instead:",
+        "",
    ]
-    tty.msg(*msg)
+
+    if shell_specific:
+        msg += [
+            equivalent.format(sh_arg="--sh  ") + "  # bash/zsh/sh",
+            equivalent.format(sh_arg="--csh ") + "  # csh/tcsh",
+            equivalent.format(sh_arg="--fish") + "  # fish",
+        ]
+    else:
+        msg += ["  " + equivalent]
+
+    msg += ['']
+    tty.error(*msg)
--- a/lib/spack/spack/cmd/env.py
+++ b/lib/spack/spack/cmd/env.py
@ -84,17 +84,10 @@ def env_activate_setup_parser(subparser):
 def env_activate(args):
    env = args.activate_env
    if not args.shell:
-        msg = [
-            "This command 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:',
-            '',
-            '    eval `spack env activate --sh %s`   # for bash/sh' % env,
-            '    eval `spack env activate --csh %s`  # for csh/tcsh' % env,
-        ]
-        tty.msg(*msg)
+        spack.cmd.common.shell_init_instructions(
+            "spack env activate",
+            "    eval `spack env activate {sh_arg} %s`" % env,
+        )
        return 1

    if ev.exists(env) and not args.dir:
@ -141,17 +134,10 @@ 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.cmd.common.shell_init_instructions + [
-            'Or, if you want to use `spack env activate` without initializing',
-            'shell support, you can run one of these:',
-            '',
-            '    eval `spack env deactivate --sh`   # for bash/sh',
-            '    eval `spack env deactivate --csh`  # for csh/tcsh',
-        ]
-        tty.msg(*msg)
+        spack.cmd.common.shell_init_instructions(
+            "spack env deactivate",
+            "    eval `spack env deactivate {sh_arg}`",
+        )
        return 1

    if 'SPACK_ENV' not in os.environ:
--- a/lib/spack/spack/cmd/load.py
+++ b/lib/spack/spack/cmd/load.py
@ -5,8 +5,6 @@

 import sys

-import llnl.util.tty as tty
-
 import spack.cmd
 import spack.cmd.common.arguments as arguments
 import spack.environment as ev
@ -62,18 +60,11 @@ def load(parser, args):
             for spec in spack.cmd.parse_specs(args.specs)]

    if not args.shell:
-        specs_string = ' '.join(args.specs)
-        msg = [
-            "This command 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:',
-            '',
-            '    eval `spack load --sh %s`   # for bash/sh' % specs_string,
-            '    eval `spack load --csh %s`  # for csh/tcsh' % specs_string,
-        ]
-        tty.msg(*msg)
+        specs_str = ' '.join(args.specs) or "SPECS"
+        spack.cmd.common.shell_init_instructions(
+            "spack load",
+            "    eval `spack load {sh_arg}` %s" % specs_str,
+        )
        return 1

    with spack.store.db.read_transaction():
--- a/lib/spack/spack/cmd/unload.py
+++ b/lib/spack/spack/cmd/unload.py
@ -6,8 +6,6 @@
 import sys
 import os

-import llnl.util.tty as tty
-
 import spack.cmd
 import spack.cmd.common.arguments as arguments
 import spack.util.environment
@ -53,17 +51,12 @@ def unload(parser, args):
        specs = spack.store.db.query(hashes=hashes)

    if not args.shell:
-        msg = [
-            "This command 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:',
-            '',
-            '    eval `spack unload --sh %s`   # for bash/sh' % args.specs,
-            '    eval `spack unload --csh %s`  # for csh/tcsh' % args.specs,
-        ]
-        tty.msg(*msg)
+        specs_str = ' '.join(args.specs) or "SPECS"
+
+        spack.cmd.common.shell_init_instructions(
+            "spack unload",
+            "    eval `spack unload {sh_arg}` %s" % specs_str,
+        )
        return 1

    env_mod = spack.util.environment.EnvironmentModifications()
--- a/lib/spack/spack/test/cmd/cd.py
+++ b/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
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@ -1185,7 +1185,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):
--- a/lib/spack/spack/test/cmd/load.py
+++ b/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
--- a/share/spack/csh/pathadd.csh
+++ b/share/spack/csh/pathadd.csh
@ -12,17 +12,26 @@
 # otherwise append to that variable.
 set _pa_varname = PATH;
 set _pa_new_path = $_pa_args[1];
-[ $#_pa_args -gt 1 ] && set _pa_varname = $_pa_args[1] && set _pa_new_path = $_pa_args[2];
+
+if ($#_pa_args > 1) then
+    set _pa_varname = $_pa_args[1]
+    set _pa_new_path = $_pa_args[2]
+endif

 # Check whether the variable is set yet.
 set _pa_old_value = ""
 eval set _pa_set = '$?'$_pa_varname
-[ $_pa_set -eq 1 ] && eval set _pa_old_value='$'$_pa_varname;
+if ($_pa_set == 1) then
+    eval set _pa_old_value='$'$_pa_varname
+endif

 # Do the actual prepending here, if it is a dir and not already in the path
 if ( -d $_pa_new_path && \:$_pa_old_value\: !~ *\:$_pa_new_path\:* ) then
-    [ -n "$_pa_old_value" ] && setenv $_pa_varname $_pa_new_path\:$_pa_old_value
-    [ -z "$_pa_old_value" ] && setenv $_pa_varname $_pa_new_path
+    if ("x$_pa_old_value" == "x") then
+        setenv $_pa_varname $_pa_new_path
+    else
+        setenv $_pa_varname $_pa_new_path\:$_pa_old_value
+    endif
 endif

 unset _pa_args _pa_new_path _pa_old_value _pa_set _pa_varname
--- a/share/spack/csh/spack.csh
+++ b/share/spack/csh/spack.csh
@ -57,8 +57,12 @@ endif
 # Set up args -- we want a subcommand and a spec.
 set _sp_subcommand=""
 set _sp_spec=""
-[ $#_sp_args -gt 0 ] && set _sp_subcommand = ($_sp_args[1])
-[ $#_sp_args -gt 1 ] && set _sp_spec = ($_sp_args[2-])
+if ($#_sp_args > 0) then
+    set _sp_subcommand = ($_sp_args[1])
+endif
+if ($#_sp_args > 1) then
+    set _sp_spec = ($_sp_args[2-])
+endif

 # Run subcommand
 switch ($_sp_subcommand)
@ -66,7 +70,9 @@ case cd:
    shift _sp_args  # get rid of 'cd'

    set _sp_arg=""
-    [ $#_sp_args -gt 0 ] && set _sp_arg = ($_sp_args[1])
+    if ($#_sp_args > 0) then
+        set _sp_arg = ($_sp_args[1])
+    endif
    shift _sp_args

    if ( "$_sp_arg" == "-h" || "$_sp_args" == "--help" ) then
@ -79,7 +85,9 @@ case env:
    shift _sp_args  # get rid of 'env'

    set _sp_arg=""
-    [ $#_sp_args -gt 0 ] && set _sp_arg = ($_sp_args[1])
+    if ($#_sp_args > 0) then
+        set _sp_arg = ($_sp_args[1])
+    endif

    if ( "$_sp_arg" == "-h" || "$_sp_arg" == "--help" ) then
        \spack env -h
@ -87,7 +95,9 @@ case env:
        switch ($_sp_arg)
            case activate:
                set _sp_env_arg=""
-                [ $#_sp_args -gt 1 ] && set _sp_env_arg = ($_sp_args[2])
+                if ($#_sp_args > 1) then
+                    set _sp_env_arg = ($_sp_args[2])
+                endif

                # Space needed here to differentiate between `-h`
                # argument and environments with "-h" in the name.
@ -106,7 +116,9 @@ case env:
                breaksw
            case deactivate:
                set _sp_env_arg=""
-                [ $#_sp_args -gt 1 ] && set _sp_env_arg = ($_sp_args[2])
+                if ($#_sp_args > 1) then
+                    set _sp_env_arg = ($_sp_args[2])
+                endif

                # Space needed here to differentiate between `--sh`
                # argument and environments with "--sh" in the name.
--- a/share/spack/qa/run-shell-tests
+++ b/share/spack/qa/run-shell-tests
@ -47,3 +47,7 @@ dash "$QA_DIR/setup-env-test.sh"

 # Run fish tests
 fish "$QA_DIR/setup-env-test.fish"
+
+# run csh and tcsh tests
+csh  "$QA_DIR/setup-env-test.csh"
+tcsh "$QA_DIR/setup-env-test.csh"
--- a/share/spack/qa/setup-env-test.csh
+++ b/share/spack/qa/setup-env-test.csh
@ -0,0 +1,76 @@
+#!/bin/csh
+#
+# 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)
+
+#
+# This tests that Spack's setup-env.csh init script works.
+#
+# There are limited tests here so far, as we haven't ported the unit test
+# functions we have for sh/bash/zsh/fish to csh.
+#
+
+# -----------------------------------------------------------------------
+# Setup test environment and do some preliminary checks
+# -----------------------------------------------------------------------
+
+# find spack but don't call it SPACK_ROOT -- we want to ensure that
+# setup-env.csh sets that.
+set QA_DIR = `dirname $0`
+set SPACK_DIR = `cd $QA_DIR/../../.. && pwd`
+
+# Make sure no environment is active, and SPACK_ROOT is not set
+unsetenv SPACK_ENV
+unsetenv SPACK_ROOT
+
+# Source setup-env.sh before tests
+source "$SPACK_DIR/share/spack/setup-env.csh"
+
+echo -n "SPACK_ROOT is set..."
+if (! $?SPACK_ROOT) then
+    echo "FAIL"
+    echo "Error: SPACK_ROOT not set by setup-env.csh"
+    exit 1
+else
+    echo "SUCCESS"
+endif
+
+echo -n "SPACK_ROOT is set correctly..."
+if ("$SPACK_ROOT" != "$SPACK_DIR") then
+    echo "FAIL"
+    echo "Error: SPACK_ROOT not set correctly by setup-env.csh"
+    echo "    Expected: '$SPACK_DIR'"
+    echo "    Found:    '$SPACK_ROOT'"
+    exit 1
+else
+    echo "SUCCESS"
+endif
+
+echo -n "spack is in the path..."
+set spack_script = `which \spack`
+if ("$spack_script" != "$SPACK_DIR/bin/spack") then
+    echo "FAIL"
+    echo "Error: could not find spack after sourcing."
+    echo "    Expected: '$SPACK_DIR/bin/spack'"
+    echo "    Found:    '$spack_script'"
+    exit 1
+else
+    echo "SUCCESS"
+endif
+
+echo -n "spack is aliased to something after sourcing..."
+set spack_alias = `which spack`
+if ("$spack_alias" !~ 'spack: aliased to '*) then
+    echo "FAIL"
+    echo "Error: spack not aliased after sourcing."
+    echo "    Expected: 'spack: aliased to [...]'"
+    echo "    Found:    '$spack_alias'"
+    exit 1
+else
+    echo "SUCCESS"
+endif
+
+echo "SUCCESS"
+exit 0
--- a/share/spack/setup-env.csh
+++ b/share/spack/setup-env.csh
@ -3,14 +3,12 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)

-
 #
 # This file is part of Spack and sets up the spack environment for
 # csh and tcsh.  This includes environment modules and lmod support, and
 # it also puts spack in your path.  Source it like this:
 #
-#    setenv SPACK_ROOT /path/to/spack
-#    source $SPACK_ROOT/share/spack/setup-env.csh
+#    source /path/to/spack/share/spack/setup-env.csh
 #

 # prevent infinite recursion when spack shells out (e.g., on cray for modules)
@ -19,32 +17,59 @@ if ($?_sp_initializing) then
 endif
 setenv _sp_initializing true

-if ($?SPACK_ROOT) then
-    set _spack_source_file = $SPACK_ROOT/share/spack/setup-env.csh
-    set _spack_share_dir   = $SPACK_ROOT/share/spack
+# If SPACK_ROOT is not set, we'll try to find it ourselves.
+# csh/tcsh don't have a built-in way to do this, but both keep files
+# they are sourcing open. We use /proc on linux and lsof on macs to
+# find this script's full path in the current process's open files.
+if (! $?SPACK_ROOT) then
+    # figure out a command to list open files
+    if (-d /proc/$$/fd) then
+        set _sp_lsof = "ls -l /proc/$$/fd"
+    else
+        which lsof > /dev/null
+        if ($? == 0) then
+            set _sp_lsof = "lsof -p $$"
+        endif
+    endif

-    # Command aliases point at separate source files
-    alias spack          'set _sp_args = (\!*); source $_spack_share_dir/csh/spack.csh'
-    alias spacktivate    'spack env activate'
-    alias _spack_pathadd 'set _pa_args = (\!*) && source $_spack_share_dir/csh/pathadd.csh'
+    # filter this script out of list of open files
+    if ( $?_sp_lsof ) then
+        set _sp_source_file = `$_sp_lsof | sed -e 's/^[^/]*//' | grep "/setup-env.csh"`
+    endif

-    # Set variables needed by this script
-    _spack_pathadd PATH "$SPACK_ROOT/bin"
-    eval `spack --print-shell-vars csh`
+    # This script is in $SPACK_ROOT/share/spack; get the root with dirname
+    if ($?_sp_source_file) then
+        set _sp_share_spack = `dirname "$_sp_source_file"`
+        set _sp_share = `dirname "$_sp_share_spack"`
+        setenv SPACK_ROOT `dirname "$_sp_share"`
+    endif

-    # Set up module search paths in the user environment
-    set tcl_roots = `echo $_sp_tcl_roots:q | sed 's/:/ /g'`
-    set compatible_sys_types = `echo $_sp_compatible_sys_types:q | sed 's/:/ /g'`
-    foreach tcl_root ($tcl_roots:q)
-        foreach systype ($compatible_sys_types:q)
-            _spack_pathadd MODULEPATH "$tcl_root/$systype"
-        end
-    end
-
-else
-    echo "ERROR: Sourcing spack setup-env.csh requires setting SPACK_ROOT to "
-    echo "       the root of your spack installation."
+    if (! $?SPACK_ROOT) then
+        echo "==> Error: setup-env.csh couldn't figure out where spack lives."
+        echo "    Set SPACK_ROOT to the root of your spack installation and try again."
+        exit 1
+    endif
 endif

+# Command aliases point at separate source files
+set _spack_source_file = $SPACK_ROOT/share/spack/setup-env.csh
+set _spack_share_dir = $SPACK_ROOT/share/spack
+alias spack          'set _sp_args = (\!*); source $_spack_share_dir/csh/spack.csh'
+alias spacktivate    'spack env activate'
+alias _spack_pathadd 'set _pa_args = (\!*) && source $_spack_share_dir/csh/pathadd.csh'
+
+# Set variables needed by this script
+_spack_pathadd PATH "$SPACK_ROOT/bin"
+eval `spack --print-shell-vars csh`
+
+# Set up module search paths in the user environment
+set tcl_roots = `echo $_sp_tcl_roots:q | sed 's/:/ /g'`
+set compatible_sys_types = `echo $_sp_compatible_sys_types:q | sed 's/:/ /g'`
+foreach tcl_root ($tcl_roots:q)
+    foreach systype ($compatible_sys_types:q)
+        _spack_pathadd MODULEPATH "$tcl_root/$systype"
+    end
+end
+
 # done: unset sentinel variable as we're no longer initializing
 unsetenv _sp_initializing