From 6d030ba1372f7fc1de3f3d8d87a47f203ef036f8 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Thu, 28 Oct 2021 20:39:25 +0200 Subject: [PATCH] Deactivate previous env before activating new one (#25409) * Deactivate previous env before activating new one Currently on develop you can run `spack env activate` multiple times to switch between environments, but they leave traces, even though Spack only supports one active environment at a time. Currently: ```console $ spack env create a $ spack env create b $ spack env activate -p a [a] $ spack env activate -p b [b] [a] $ spack env activate -p b [a] [b] [a] $ spack env activate -p a [a] [b] [c] $ echo $MANPATH | tr ":" "\n" /path/to/environments/a/.spack-env/view/share/man /path/to/environments/a/.spack-env/view/man /path/to/environments/b/.spack-env/view/share/man /path/to/environments/b/.spack-env/view/man ``` This PR fixes that: ```console $ spack env activate -p a [a] $ spack env activate -p b [b] $ spack env activate -p a [a] $ echo $MANPATH | tr ":" "\n" /path/to/environments/a/.spack-env/view/share/man /path/to/environments/a/.spack-env/view/man ``` --- lib/spack/spack/cmd/env.py | 32 ++++++++++-------- lib/spack/spack/environment/shell.py | 6 +++- share/spack/qa/setup-env-test.fish | 50 +++++++++++++++++++++++++--- share/spack/qa/setup-env-test.sh | 12 +++++-- share/spack/qa/test-framework.sh | 30 +++++++++++++++++ 5 files changed, 108 insertions(+), 22 deletions(-) diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index 5b9caa0877..b88e1281bc 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -23,6 +23,7 @@ import spack.environment.shell import spack.schema.env import spack.util.string as string +from spack.util.environment import EnvironmentModifications description = "manage virtual environments" section = "environments" @@ -115,42 +116,45 @@ def env_activate(args): # Temporary environment if args.temp: env = create_temp_env_directory() - spack_env = os.path.abspath(env) - short_name = os.path.basename(spack_env) + env_path = os.path.abspath(env) + short_name = os.path.basename(env_path) ev.Environment(env).write(regenerate=False) # Named environment elif ev.exists(env_name_or_dir) and not args.dir: - spack_env = ev.root(env_name_or_dir) + env_path = ev.root(env_name_or_dir) short_name = env_name_or_dir # Environment directory elif ev.is_env_dir(env_name_or_dir): - spack_env = os.path.abspath(env_name_or_dir) - short_name = os.path.basename(spack_env) + env_path = os.path.abspath(env_name_or_dir) + short_name = os.path.basename(env_path) else: tty.die("No such environment: '%s'" % env_name_or_dir) env_prompt = '[%s]' % short_name - if spack_env == os.environ.get('SPACK_ENV'): - tty.debug("Environment is already active") - return + # We only support one active environment at a time, so deactivate the current one. + if ev.active_environment() is None: + cmds = '' + env_mods = EnvironmentModifications() + else: + cmds = spack.environment.shell.deactivate_header(shell=args.shell) + env_mods = spack.environment.shell.deactivate() # Activate new environment - active_env = ev.Environment(spack_env) - cmds = spack.environment.shell.activate_header( + active_env = ev.Environment(env_path) + cmds += spack.environment.shell.activate_header( env=active_env, shell=args.shell, prompt=env_prompt if args.prompt else None ) - env_mods = spack.environment.shell.activate( + env_mods.extend(spack.environment.shell.activate( env=active_env, add_view=args.with_view - ) + )) cmds += env_mods.shell_modifications(args.shell) - sys.stdout.write(cmds) @@ -184,7 +188,7 @@ def env_deactivate(args): tty.die('Calling spack env deactivate with --env, --env-dir and --no-env ' 'is ambiguous') - if 'SPACK_ENV' not in os.environ: + if ev.active_environment() is None: tty.die('No environment is currently active.') cmds = spack.environment.shell.deactivate_header(args.shell) diff --git a/lib/spack/spack/environment/shell.py b/lib/spack/spack/environment/shell.py index 1ffd6cd171..96b3a8c31c 100644 --- a/lib/spack/spack/environment/shell.py +++ b/lib/spack/spack/environment/shell.py @@ -134,7 +134,11 @@ def activate(env, use_env_repo=False, add_view=True): def deactivate(): """ - Deactivate an environment and collect corresponding environment modifications + Deactivate an environment and collect corresponding environment modifications. + + Note: unloads the environment in its current state, not in the state it was + loaded in, meaning that specs that were removed from the spack environment + after activation are not unloaded. Returns: spack.util.environment.EnvironmentModifications: Environment variables diff --git a/share/spack/qa/setup-env-test.fish b/share/spack/qa/setup-env-test.fish index 5cd257aa86..c48ecf4bc4 100755 --- a/share/spack/qa/setup-env-test.fish +++ b/share/spack/qa/setup-env-test.fish @@ -88,7 +88,7 @@ end function spt_succeeds printf "'$argv' succeeds ... " - set -l output (eval $argv 2>&1) + set -l output ($argv 2>&1) # Save the command result set cmd_status $status @@ -115,7 +115,7 @@ end function spt_fails printf "'$argv' fails ... " - set -l output (eval $argv 2>&1) + set -l output ($argv 2>&1) if test $status -eq 0 fail @@ -143,7 +143,7 @@ function spt_contains printf "'$remaining_args' output contains '$target_string' ... " - set -l output (eval $remaining_args 2>&1) + set -l output ($remaining_args 2>&1) # Save the command result set cmd_status $status @@ -166,6 +166,41 @@ function spt_contains end +# +# Ensure that a string is not in the output of a command. The command must have a 0 exit +# status to guard against false positives. Suppresses output on success. +# On failure, echo the exit code and output. +# +function spt_does_not_contain + set -l target_string $argv[1] + set -l remaining_args $argv[2..-1] + + printf "'$remaining_args' does not contain '$target_string' ... " + + set -l output ($remaining_args 2>&1) + + # Save the command result + set cmd_status $status + + if test $cmd_status -ne 0 + fail + echo_red "Command exited with error $cmd_status." + else if not echo "$output" | string match -q -r ".*$target_string.*" + pass + return + else + fail + echo_red "'$target_string' was in the output." + end + if test -n "$output" + echo_msg "Output:" + echo "$output" + else + echo_msg "No output." + end +end + + # # Ensure that a variable is set. # @@ -255,12 +290,13 @@ spack -m install --fake a # create a test environment for testing environment commands echo "Creating a mock environment" spack env create spack_test_env +spack env create spack_test_2_env # ensure that we uninstall b on exit function spt_cleanup -p %self echo "Removing test environment before exiting." spack env deactivate 2>&1 > /dev/null - spack env rm -y spack_test_env + spack env rm -y spack_test_env spack_test_2_env title "Cleanup" echo "Removing test packages before exiting." @@ -366,6 +402,12 @@ is_set SPACK_ENV spack env deactivate is_not_set SPACK_ENV +echo "Testing spack env activate repeatedly" +spack env activate spack_test_env +spack env activate spack_test_2_env +spt_contains 'spack_test_2_env' 'fish' '-c' 'echo $PATH' +spt_does_not_contain 'spack_test_env' 'fish' '-c' 'echo $PATH' + # # NOTE: `--prompt` on fish does nothing => currently not implemented. # diff --git a/share/spack/qa/setup-env-test.sh b/share/spack/qa/setup-env-test.sh index 88e30aefeb..bddf7ca1af 100755 --- a/share/spack/qa/setup-env-test.sh +++ b/share/spack/qa/setup-env-test.sh @@ -70,13 +70,13 @@ b_module=$(spack -m module tcl find b) # Create a test environment for testing environment commands echo "Creating a mock environment" spack env create spack_test_env -test_env_location=$(spack location -e spack_test_env) +spack env create spack_test_2_env # Ensure that we uninstall b on exit cleanup() { echo "Removing test environment before exiting." spack env deactivate 2>&1 > /dev/null - spack env rm -y spack_test_env + spack env rm -y spack_test_env spack_test_2_env title "Cleanup" echo "Removing test packages before exiting." @@ -178,4 +178,10 @@ echo "Testing 'spack env activate --temp'" spack env activate --temp is_set SPACK_ENV spack env deactivate -is_not_set SPACK_ENV \ No newline at end of file +is_not_set SPACK_ENV + +echo "Testing spack env activate repeatedly" +spack env activate spack_test_env +spack env activate spack_test_2_env +contains "spack_test_2_env" sh -c 'echo $PATH' +does_not_contain "spack_test_env" sh -c 'echo $PATH' diff --git a/share/spack/qa/test-framework.sh b/share/spack/qa/test-framework.sh index 4df37cd906..92b82644ca 100755 --- a/share/spack/qa/test-framework.sh +++ b/share/spack/qa/test-framework.sh @@ -132,6 +132,36 @@ contains() { fi } +# +# Ensure that a string is not in the output of a command. The command must have a 0 exit +# status to guard against false positives. Suppresses output on success. +# On failure, echo the exit code and output. +# +does_not_contain() { + string="$1" + shift + + printf "'%s' output does not contain '$string' ... " "$*" + output=$("$@" 2>&1) + err="$?" + + if [ "$err" != 0 ]; then + fail + elif [ "${output#*$string}" = "${output}" ]; then + pass + return + else + fail + echo_red "'$string' was in the output." + fi + if [ -n "$output" ]; then + echo_msg "Output:" + echo "$output" + else + echo_msg "No output." + fi +} + # # Ensure that a variable is set. #