diff options
| -rw-r--r-- | bin/spack.bat | 28 | ||||
| -rw-r--r-- | lib/spack/spack/cmd/__init__.py | 2 | ||||
| -rw-r--r-- | lib/spack/spack/cmd/unload.py | 2 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/find.py | 2 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/load.py | 186 | ||||
| -rw-r--r-- | lib/spack/spack/util/environment.py | 4 |
6 files changed, 134 insertions, 90 deletions
diff --git a/bin/spack.bat b/bin/spack.bat index 8328013599..a5a5384e9f 100644 --- a/bin/spack.bat +++ b/bin/spack.bat @@ -188,25 +188,27 @@ if NOT "%_sp_args%"=="%_sp_args:--help=%" ( goto :end_switch :case_load -:: If args contain --sh, --csh, or -h/--help: just execute. -if defined _sp_args ( - if NOT "%_sp_args%"=="%_sp_args:--help=%" ( - goto :default_case - ) else if NOT "%_sp_args%"=="%_sp_args:-h=%" ( - goto :default_case - ) else if NOT "%_sp_args%"=="%_sp_args:--bat=%" ( - goto :default_case - ) +if NOT defined _sp_args ( + exit /B 0 +) + +:: If args contain --bat, or -h/--help: just execute. +if NOT "%_sp_args%"=="%_sp_args:--help=%" ( + goto :default_case +) else if NOT "%_sp_args%"=="%_sp_args:-h=%" ( + goto :default_case +) else if NOT "%_sp_args%"=="%_sp_args:--bat=%" ( + goto :default_case +) else if NOT "%_sp_args%"=="%_sp_args:--list=%" ( + goto :default_case ) for /f "tokens=* USEBACKQ" %%I in ( - `python "%spack%" %_sp_flags% %_sp_subcommand% --bat %_sp_args%`) do %%I + `python "%spack%" %_sp_flags% %_sp_subcommand% --bat %_sp_args%` + ) do %%I goto :end_switch -:case_unload -goto :case_load - :default_case python "%spack%" %_sp_flags% %_sp_subcommand% %_sp_args% goto :end_switch diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py index bd6a3eb768..023b7ae73e 100644 --- a/lib/spack/spack/cmd/__init__.py +++ b/lib/spack/spack/cmd/__init__.py @@ -444,7 +444,7 @@ def display_specs(specs, args=None, **kwargs): def filter_loaded_specs(specs): """Filter a list of specs returning only those that are currently loaded.""" - hashes = os.environ.get(uenv.spack_loaded_hashes_var, "").split(":") + hashes = os.environ.get(uenv.spack_loaded_hashes_var, "").split(os.pathsep) return [x for x in specs if x.dag_hash() in hashes] diff --git a/lib/spack/spack/cmd/unload.py b/lib/spack/spack/cmd/unload.py index 3bafe5e8c6..65daabcd46 100644 --- a/lib/spack/spack/cmd/unload.py +++ b/lib/spack/spack/cmd/unload.py @@ -71,7 +71,7 @@ def unload(parser, args): "Cannot specify specs on command line when unloading all specs with '--all'" ) - hashes = os.environ.get(uenv.spack_loaded_hashes_var, "").split(":") + hashes = os.environ.get(uenv.spack_loaded_hashes_var, "").split(os.pathsep) if args.specs: specs = [ spack.cmd.disambiguate_spec_from_hashes(spec, hashes) diff --git a/lib/spack/spack/test/cmd/find.py b/lib/spack/spack/test/cmd/find.py index 37a0a7ff14..95d5734451 100644 --- a/lib/spack/spack/test/cmd/find.py +++ b/lib/spack/spack/test/cmd/find.py @@ -434,7 +434,7 @@ def test_find_loaded(database, working_env): output = find("--loaded", "--group") assert output == "" - os.environ[uenv.spack_loaded_hashes_var] = ":".join( + os.environ[uenv.spack_loaded_hashes_var] = os.pathsep.join( [x.dag_hash() for x in spack.store.STORE.db.query()] ) output = find("--loaded") diff --git a/lib/spack/spack/test/cmd/load.py b/lib/spack/spack/test/cmd/load.py index e2abe72e0e..73a062ebd4 100644 --- a/lib/spack/spack/test/cmd/load.py +++ b/lib/spack/spack/test/cmd/load.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import os import re +import sys import pytest @@ -17,101 +18,125 @@ unload = SpackCommand("unload") install = SpackCommand("install") location = SpackCommand("location") -pytestmark = pytest.mark.not_on_windows("does not run on windows") - def test_manpath_trailing_colon( install_mockery, mock_fetch, mock_archive, mock_packages, working_env ): + (shell, set_command, commandsep) = ( + ("--bat", 'set "%s=%s"', "\n") + if sys.platform == "win32" + else ("--sh", "export %s=%s", ";") + ) + """Test that the commands generated by load add the MANPATH prefix inspections. Also test that Spack correctly preserves the default/existing manpath search path via a trailing colon""" install("mpileaks") - sh_out = load("--sh", "mpileaks") - lines = sh_out.split("\n") - assert any(re.match(r"export MANPATH=.*:;", ln) for ln in lines) - - os.environ["MANPATH"] = "/tmp/man:" - - sh_out = load("--sh", "mpileaks") - lines = sh_out.split("\n") - assert any(re.match(r"export MANPATH=.*:/tmp/man:;", ln) for ln in lines) - - -def test_load_recursive(install_mockery, mock_fetch, mock_archive, mock_packages, working_env): - """Test that `spack load` applies prefix inspections of its required runtime deps in - topo-order""" - install("mpileaks") - mpileaks_spec = spack.spec.Spec("mpileaks").concretized() - - # Ensure our reference variable is cleed. - os.environ["CMAKE_PREFIX_PATH"] = "/hello:/world" - - sh_out = load("--sh", "mpileaks") - csh_out = load("--csh", "mpileaks") - - def extract_cmake_prefix_path(output, prefix): - return next(cmd for cmd in output.split(";") if cmd.startswith(prefix))[ - len(prefix) : - ].split(":") + sh_out = load(shell, "mpileaks") + lines = [line.strip("\n") for line in sh_out.split(commandsep)] + assert any(re.match(set_command % ("MANPATH", ".*" + os.pathsep), ln) for ln in lines) + os.environ["MANPATH"] = "/tmp/man" + os.pathsep - # Map a prefix found in CMAKE_PREFIX_PATH back to a package name in mpileaks' DAG. - prefix_to_pkg = lambda prefix: next( - s.name for s in mpileaks_spec.traverse() if s.prefix == prefix + sh_out = load(shell, "mpileaks") + lines = [line.strip("\n") for line in sh_out.split(commandsep)] + assert any( + re.match(set_command % ("MANPATH", ".*" + os.pathsep + "/tmp/man" + os.pathsep), ln) + for ln in lines ) - paths_sh = extract_cmake_prefix_path(sh_out, prefix="export CMAKE_PREFIX_PATH=") - paths_csh = extract_cmake_prefix_path(csh_out, prefix="setenv CMAKE_PREFIX_PATH ") - # Shouldn't be a difference between loading csh / sh, so check they're the same. - assert paths_sh == paths_csh - - # We should've prepended new paths, and keep old ones. - assert paths_sh[-2:] == ["/hello", "/world"] - - # All but the last two paths are added by spack load; lookup what packages they're from. - pkgs = [prefix_to_pkg(p) for p in paths_sh[:-2]] - - # Do we have all the runtime packages? - assert set(pkgs) == set( - s.name for s in mpileaks_spec.traverse(deptype=("link", "run"), root=True) - ) - - # Finally, do we list them in topo order? - for i, pkg in enumerate(pkgs): - set(s.name for s in mpileaks_spec[pkg].traverse(direction="parents")) in set(pkgs[:i]) - - # Lastly, do we keep track that mpileaks was loaded? - assert f"export {uenv.spack_loaded_hashes_var}={mpileaks_spec.dag_hash()}" in sh_out - assert f"setenv {uenv.spack_loaded_hashes_var} {mpileaks_spec.dag_hash()}" in csh_out - - -def test_load_includes_run_env(install_mockery, mock_fetch, mock_archive, mock_packages): +def test_load_recursive(install_mockery, mock_fetch, mock_archive, mock_packages, working_env): + def test_load_shell(shell, set_command): + """Test that `spack load` applies prefix inspections of its required runtime deps in + topo-order""" + install("mpileaks") + mpileaks_spec = spack.spec.Spec("mpileaks").concretized() + + # Ensure our reference variable is clean. + os.environ["CMAKE_PREFIX_PATH"] = "/hello" + os.pathsep + "/world" + + shell_out = load(shell, "mpileaks") + + def extract_value(output, variable): + match = re.search(set_command % variable, output, flags=re.MULTILINE) + value = match.group(1) + return value.split(os.pathsep) + + # Map a prefix found in CMAKE_PREFIX_PATH back to a package name in mpileaks' DAG. + prefix_to_pkg = lambda prefix: next( + s.name for s in mpileaks_spec.traverse() if s.prefix == prefix + ) + + paths_shell = extract_value(shell_out, "CMAKE_PREFIX_PATH") + + # We should've prepended new paths, and keep old ones. + assert paths_shell[-2:] == ["/hello", "/world"] + + # All but the last two paths are added by spack load; lookup what packages they're from. + pkgs = [prefix_to_pkg(p) for p in paths_shell[:-2]] + + # Do we have all the runtime packages? + assert set(pkgs) == set( + s.name for s in mpileaks_spec.traverse(deptype=("link", "run"), root=True) + ) + + # Finally, do we list them in topo order? + for i, pkg in enumerate(pkgs): + set(s.name for s in mpileaks_spec[pkg].traverse(direction="parents")) in set(pkgs[:i]) + + # Lastly, do we keep track that mpileaks was loaded? + assert ( + extract_value(shell_out, uenv.spack_loaded_hashes_var)[0] == mpileaks_spec.dag_hash() + ) + return paths_shell + + if sys.platform == "win32": + shell, set_command = ("--bat", r'set "%s=(.*)"') + test_load_shell(shell, set_command) + else: + params = [("--sh", r"export %s=([^;]*)"), ("--csh", r"setenv %s ([^;]*)")] + shell, set_command = params[0] + paths_sh = test_load_shell(shell, set_command) + shell, set_command = params[1] + paths_csh = test_load_shell(shell, set_command) + assert paths_sh == paths_csh + + +@pytest.mark.parametrize( + "shell,set_command", + ( + [("--bat", 'set "%s=%s"')] + if sys.platform == "win32" + else [("--sh", "export %s=%s"), ("--csh", "setenv %s %s")] + ), +) +def test_load_includes_run_env( + shell, set_command, install_mockery, mock_fetch, mock_archive, mock_packages +): """Tests that environment changes from the package's `setup_run_environment` method are added to the user environment in addition to the prefix inspections""" install("mpileaks") - sh_out = load("--sh", "mpileaks") - csh_out = load("--csh", "mpileaks") + shell_out = load(shell, "mpileaks") - assert "export FOOBAR=mpileaks" in sh_out - assert "setenv FOOBAR mpileaks" in csh_out + assert set_command % ("FOOBAR", "mpileaks") in shell_out def test_load_first(install_mockery, mock_fetch, mock_archive, mock_packages): """Test with and without the --first option""" + shell = "--bat" if sys.platform == "win32" else "--sh" install("libelf@0.8.12") install("libelf@0.8.13") # Now there are two versions of libelf, which should cause an error - out = load("--sh", "libelf", fail_on_error=False) + out = load(shell, "libelf", fail_on_error=False) assert "matches multiple packages" in out assert "Use a more specific spec" in out # Using --first should avoid the error condition - load("--sh", "--first", "libelf") + load(shell, "--first", "libelf") def test_load_fails_no_shell(install_mockery, mock_fetch, mock_archive, mock_packages): @@ -122,7 +147,24 @@ def test_load_fails_no_shell(install_mockery, mock_fetch, mock_archive, mock_pac assert "To set up shell support" in out -def test_unload(install_mockery, mock_fetch, mock_archive, mock_packages, working_env): +@pytest.mark.parametrize( + "shell,set_command,unset_command", + ( + [("--bat", 'set "%s=%s"', 'set "%s="')] + if sys.platform == "win32" + else [("--sh", "export %s=%s", "unset %s"), ("--csh", "setenv %s %s", "unsetenv %s")] + ), +) +def test_unload( + shell, + set_command, + unset_command, + install_mockery, + mock_fetch, + mock_archive, + mock_packages, + working_env, +): """Tests that any variables set in the user environment are undone by the unload command""" install("mpileaks") @@ -130,16 +172,16 @@ def test_unload(install_mockery, mock_fetch, mock_archive, mock_packages, workin # Set so unload has something to do os.environ["FOOBAR"] = "mpileaks" - os.environ[uenv.spack_loaded_hashes_var] = "%s:%s" % (mpileaks_spec.dag_hash(), "garbage") + os.environ[uenv.spack_loaded_hashes_var] = ("%s" + os.pathsep + "%s") % ( + mpileaks_spec.dag_hash(), + "garbage", + ) - sh_out = unload("--sh", "mpileaks") - csh_out = unload("--csh", "mpileaks") + shell_out = unload(shell, "mpileaks") - assert "unset FOOBAR" in sh_out - assert "unsetenv FOOBAR" in csh_out + assert (unset_command % "FOOBAR") in shell_out - assert "export %s=garbage" % uenv.spack_loaded_hashes_var in sh_out - assert "setenv %s garbage" % uenv.spack_loaded_hashes_var in csh_out + assert set_command % (uenv.spack_loaded_hashes_var, "garbage") in shell_out def test_unload_fails_no_shell( diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index 1d53d33efc..40b2507905 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -679,8 +679,8 @@ class EnvironmentModifications: for modifier in actions: modifier.execute(new_env) - if "MANPATH" in new_env and not new_env["MANPATH"].endswith(":"): - new_env["MANPATH"] += ":" + if "MANPATH" in new_env and not new_env["MANPATH"].endswith(os.pathsep): + new_env["MANPATH"] += os.pathsep cmds = "" |