diff options
| -rw-r--r-- | lib/spack/spack/paths.py | 5 | ||||
| -rw-r--r-- | lib/spack/spack/test/util/executable.py | 20 | ||||
| -rw-r--r-- | lib/spack/spack/util/environment.py | 31 | ||||
| -rw-r--r-- | lib/spack/spack/util/executable.py | 85 | ||||
| -rw-r--r-- | var/spack/repos/builtin.mock/packages/cmake-client/package.py | 2 | ||||
| -rw-r--r-- | var/spack/repos/builtin/packages/oommf/package.py | 12 |
6 files changed, 93 insertions, 62 deletions
diff --git a/lib/spack/spack/paths.py b/lib/spack/spack/paths.py index 18656b76c2..4e66adf11a 100644 --- a/lib/spack/spack/paths.py +++ b/lib/spack/spack/paths.py @@ -10,11 +10,12 @@ throughout Spack and should bring in a minimal number of external dependencies. """ import os +from pathlib import PurePath import llnl.util.filesystem #: This file lives in $prefix/lib/spack/spack/__file__ -prefix = llnl.util.filesystem.ancestor(__file__, 4) +prefix = str(PurePath(llnl.util.filesystem.ancestor(__file__, 4))) #: synonym for prefix spack_root = prefix @@ -88,7 +89,7 @@ def _get_user_cache_path(): return os.path.expanduser(os.getenv("SPACK_USER_CACHE_PATH") or "~%s.spack" % os.sep) -user_cache_path = _get_user_cache_path() +user_cache_path = str(PurePath(_get_user_cache_path())) #: junit, cdash, etc. reports about builds reports_path = os.path.join(user_cache_path, "reports") diff --git a/lib/spack/spack/test/util/executable.py b/lib/spack/spack/test/util/executable.py index b4e4b4888e..839cf04bfb 100644 --- a/lib/spack/spack/test/util/executable.py +++ b/lib/spack/spack/test/util/executable.py @@ -5,6 +5,7 @@ import os import sys +from pathlib import PurePath import pytest @@ -16,13 +17,16 @@ from spack.hooks.sbang import filter_shebangs_in_directory def test_read_unicode(tmpdir, working_env): - script_name = "print_unicode.py" - # read the unicode back in and see whether things work - if sys.platform == "win32": - script = ex.Executable("%s %s" % (sys.executable, script_name)) - else: - script = ex.Executable("./%s" % script_name) with tmpdir.as_cwd(): + script_name = "print_unicode.py" + # read the unicode back in and see whether things work + if sys.platform == "win32": + script = ex.Executable("%s" % (sys.executable)) + script_args = [script_name] + else: + script = ex.Executable("./%s" % script_name) + script_args = [] + os.environ["LD_LIBRARY_PATH"] = spack.main.spack_ld_library_path # make a script that prints some unicode with open(script_name, "w") as f: @@ -38,7 +42,7 @@ print(u'\\xc3') fs.set_executable(script_name) filter_shebangs_in_directory(".", [script_name]) - assert "\xc3" == script(output=str).strip() + assert "\xc3" == script(*script_args, output=str).strip() def test_which_relative_path_with_slash(tmpdir, working_env): @@ -68,7 +72,7 @@ def test_which_with_slash_ignores_path(tmpdir, working_env): path = str(tmpdir.join("exe")) wrong_path = str(tmpdir.join("bin", "exe")) - os.environ["PATH"] = os.path.dirname(wrong_path) + os.environ["PATH"] = str(PurePath(wrong_path).parent) with tmpdir.as_cwd(): if sys.platform == "win32": diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py index da6832e4dd..30322c320a 100644 --- a/lib/spack/spack/util/environment.py +++ b/lib/spack/spack/util/environment.py @@ -1066,8 +1066,8 @@ def environment_after_sourcing_files( Keyword Args: env (dict): the initial environment (default: current environment) - shell (str): the shell to use (default: ``/bin/bash``) - shell_options (str): options passed to the shell (default: ``-c``) + shell (str): the shell to use (default: ``/bin/bash`` or ``cmd.exe`` (Windows)) + shell_options (str): options passed to the shell (default: ``-c`` or ``/C`` (Windows)) source_command (str): the command to run (default: ``source``) suppress_output (str): redirect used to suppress output of command (default: ``&> /dev/null``) @@ -1075,15 +1075,23 @@ def environment_after_sourcing_files( only when the previous command succeeds (default: ``&&``) """ # Set the shell executable that will be used to source files - shell_cmd = kwargs.get("shell", "/bin/bash") - shell_options = kwargs.get("shell_options", "-c") - source_command = kwargs.get("source_command", "source") - suppress_output = kwargs.get("suppress_output", "&> /dev/null") + if sys.platform == "win32": + shell_cmd = kwargs.get("shell", "cmd.exe") + shell_options = kwargs.get("shell_options", "/C") + suppress_output = kwargs.get("suppress_output", "") + source_command = kwargs.get("source_command", "") + else: + shell_cmd = kwargs.get("shell", "/bin/bash") + shell_options = kwargs.get("shell_options", "-c") + suppress_output = kwargs.get("suppress_output", "&> /dev/null") + source_command = kwargs.get("source_command", "source") concatenate_on_success = kwargs.get("concatenate_on_success", "&&") - shell = Executable(" ".join([shell_cmd, shell_options])) + shell = Executable(shell_cmd) def _source_single_file(file_and_args, environment): + shell_options_list = shell_options.split() + source_file = [source_command] source_file.extend(x for x in file_and_args) source_file = " ".join(source_file) @@ -1101,7 +1109,14 @@ def environment_after_sourcing_files( source_file_arguments = " ".join( [source_file, suppress_output, concatenate_on_success, dump_environment_cmd] ) - output = shell(source_file_arguments, output=str, env=environment, ignore_quotes=True) + output = shell( + *shell_options_list, + source_file_arguments, + output=str, + env=environment, + ignore_quotes=True, + ) + return json.loads(output) current_environment = kwargs.get("env", dict(os.environ)) diff --git a/lib/spack/spack/util/executable.py b/lib/spack/spack/util/executable.py index 788348879a..a3f3d16c77 100644 --- a/lib/spack/spack/util/executable.py +++ b/lib/spack/spack/util/executable.py @@ -5,14 +5,13 @@ import os import re -import shlex import subprocess import sys +from pathlib import Path, PurePath import llnl.util.tty as tty import spack.error -from spack.util.path import Path, format_os_path, path_to_os_path, system_path_filter __all__ = ["Executable", "which", "ProcessError"] @@ -21,11 +20,12 @@ class Executable: """Class representing a program that can be run on the command line.""" def __init__(self, name): - # necesary here for the shlex call to succeed - name = format_os_path(name, mode=Path.unix) - self.exe = shlex.split(str(name)) - # filter back to platform dependent path - self.exe = path_to_os_path(*self.exe) + file_path = str(Path(name)) + if sys.platform != "win32" and name.startswith("."): + # pathlib strips the ./ from relative paths so it must be added back + file_path = os.path.join(".", file_path) + self.exe = [file_path] + self.default_env = {} from spack.util.environment import EnvironmentModifications # no cycle @@ -35,12 +35,10 @@ class Executable: if not self.exe: raise ProcessError("Cannot construct executable for '%s'" % name) - @system_path_filter def add_default_arg(self, arg): """Add a default argument to the command.""" self.exe.append(arg) - @system_path_filter def add_default_env(self, key, value): """Set an environment variable when the command is run. @@ -70,7 +68,7 @@ class Executable: Returns: str: The basename of the executable """ - return os.path.basename(self.path) + return PurePath(self.path).name @property def path(self): @@ -79,7 +77,7 @@ class Executable: Returns: str: The path to the executable """ - return self.exe[0] + return str(PurePath(self.exe[0])) def __call__(self, *args, **kwargs): """Run this executable in a subprocess. @@ -235,7 +233,11 @@ class Executable: return result except OSError as e: - raise ProcessError("%s: %s" % (self.exe[0], e.strerror), "Command: " + cmd_line_string) + message = "Command: " + cmd_line_string + if " " in self.exe[0]: + message += "\nDid you mean to add a space to the command?" + + raise ProcessError("%s: %s" % (self.exe[0], e.strerror), message) except subprocess.CalledProcessError as e: if fail_on_error: @@ -269,39 +271,48 @@ class Executable: return " ".join(self.exe) -@system_path_filter def which_string(*args, **kwargs): """Like ``which()``, but return a string instead of an ``Executable``.""" path = kwargs.get("path", os.environ.get("PATH", "")) required = kwargs.get("required", False) + if isinstance(path, list): + paths = [Path(str(x)) for x in path] + if isinstance(path, str): - path = path.split(os.pathsep) + paths = [Path(x) for x in path.split(os.pathsep)] - for name in args: - win_candidates = [] - if sys.platform == "win32" and (not name.endswith(".exe") and not name.endswith(".bat")): - win_candidates = [name + ext for ext in [".exe", ".bat"]] - candidate_names = [name] if not win_candidates else win_candidates + def get_candidate_items(search_item): + if sys.platform == "win32" and not search_item.suffix: + return [search_item.parent / (search_item.name + ext) for ext in [".exe", ".bat"]] + return [Path(search_item)] + + def add_extra_search_paths(paths): + with_parents = [] + with_parents.extend(paths) if sys.platform == "win32": - new_path = path[:] - for p in path: - if os.path.basename(p) == "bin": - new_path.append(os.path.dirname(p)) - path = new_path - - for candidate_name in candidate_names: - if os.path.sep in candidate_name: - exe = os.path.abspath(candidate_name) - if os.path.isfile(exe) and os.access(exe, os.X_OK): - return exe - else: - for directory in path: - directory = path_to_os_path(directory).pop() - exe = os.path.join(directory, candidate_name) - if os.path.isfile(exe) and os.access(exe, os.X_OK): - return exe + for p in paths: + if p.name == "bin": + with_parents.append(p.parent) + return with_parents + + for search_item in args: + search_paths = [] + search_paths.extend(paths) + if search_item.startswith("."): + # we do this because pathlib will strip any leading ./ + search_paths.insert(0, Path.cwd()) + search_paths = add_extra_search_paths(search_paths) + + search_item = Path(search_item) + candidate_items = get_candidate_items(Path(search_item)) + + for candidate_item in candidate_items: + for directory in search_paths: + exe = directory / candidate_item + if exe.is_file() and os.access(str(exe), os.X_OK): + return str(exe) if required: raise CommandNotFoundError("spack requires '%s'. Make sure it is in your path." % args[0]) @@ -326,7 +337,7 @@ def which(*args, **kwargs): Executable: The first executable that is found in the path """ exe = which_string(*args, **kwargs) - return Executable(shlex.quote(exe)) if exe else None + return Executable(exe) if exe else None class ProcessError(spack.error.SpackError): diff --git a/var/spack/repos/builtin.mock/packages/cmake-client/package.py b/var/spack/repos/builtin.mock/packages/cmake-client/package.py index 2ce5a98dd1..a0694f2d6d 100644 --- a/var/spack/repos/builtin.mock/packages/cmake-client/package.py +++ b/var/spack/repos/builtin.mock/packages/cmake-client/package.py @@ -109,7 +109,7 @@ class CmakeClient(CMakePackage): print(cmake) print(cmake.exe) check( - cmake.exe[0].startswith(spec["cmake"].prefix.bin), + cmake.path.startswith(spec["cmake"].prefix.bin), "Wrong cmake was in environment: %s" % cmake, ) diff --git a/var/spack/repos/builtin/packages/oommf/package.py b/var/spack/repos/builtin/packages/oommf/package.py index f7127f073e..a3a4a6aeac 100644 --- a/var/spack/repos/builtin/packages/oommf/package.py +++ b/var/spack/repos/builtin/packages/oommf/package.py @@ -168,15 +168,15 @@ class Oommf(Package): def configure(self, spec, prefix): # change into directory with source code with working_dir(self.get_oommf_source_root()): - configure = Executable("./oommf.tcl pimake distclean") - configure() - configure2 = Executable("./oommf.tcl pimake upgrade") - configure2() + configure = Executable("./oommf.tcl") + configure("pimake", "distclean") + configure2 = Executable("./oommf.tcl") + configure2("pimake", "upgrade") def build(self, spec, prefix): with working_dir(self.get_oommf_source_root()): - make = Executable("./oommf.tcl pimake ") - make() + make = Executable("./oommf.tcl") + make("pimake") def install(self, spec, prefix): # keep a copy of all the tcl files and everything oommf created. |