External finding: update default paths; treat .bat as executable on Windows (#39850)

.bat or .exe files can be considered executable on Windows. This PR
expands the regex for detectable packages to allow for the detection
of packages that vendor .bat wrappers (intel mpi for example).

Additional changes:

* Outside of Windows, when searching for executables `path_hints=None`
  was used to indicate that default path hints should be provided,
  and `[]` was taken to mean that no defaults should be chosen
  (in that case, nothing is searched); behavior on Windows has
  now been updated to match.
* Above logic for handling of `path_hints=[]`  has also been extended
  to library search (for both Linux and Windows).
* All exceptions for external packages were documented as timeout
  errors: this commit adds a distinction for other types of errors
  in warning messages to the user.
This commit is contained in:
John W. Parent 2023-10-27 13:40:44 -04:00 committed by GitHub
parent 195f965076
commit 069762cd37
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 89 additions and 82 deletions

View File

@ -39,12 +39,21 @@
DETECTION_TIMEOUT = 120
def common_windows_package_paths() -> List[str]:
def common_windows_package_paths(pkg_cls=None) -> List[str]:
"""Get the paths for common package installation location on Windows
that are outside the PATH
Returns [] on unix
"""
if sys.platform != "win32":
return []
paths = WindowsCompilerExternalPaths.find_windows_compiler_bundled_packages()
paths.extend(find_win32_additional_install_paths())
paths.extend(WindowsKitExternalPaths.find_windows_kit_bin_paths())
paths.extend(WindowsKitExternalPaths.find_windows_kit_reg_installed_roots_paths())
paths.extend(WindowsKitExternalPaths.find_windows_kit_reg_sdk_paths())
if pkg_cls:
paths.extend(compute_windows_user_path_for_package(pkg_cls))
paths.extend(compute_windows_program_path_for_package(pkg_cls))
return paths
@ -62,8 +71,6 @@ def executables_in_path(path_hints: List[str]) -> Dict[str, str]:
path_hints: list of paths to be searched. If None the list will be
constructed based on the PATH environment variable.
"""
if sys.platform == "win32":
path_hints.extend(common_windows_package_paths())
search_paths = llnl.util.filesystem.search_paths_for_executables(*path_hints)
return path_to_dict(search_paths)
@ -88,27 +95,39 @@ def libraries_in_ld_and_system_library_path(
DYLD_LIBRARY_PATH, and DYLD_FALLBACK_LIBRARY_PATH environment
variables as well as the standard system library paths.
"""
path_hints = (
path_hints
or spack.util.environment.get_path("LD_LIBRARY_PATH")
default_lib_search_paths = (
spack.util.environment.get_path("LD_LIBRARY_PATH")
+ spack.util.environment.get_path("DYLD_LIBRARY_PATH")
+ spack.util.environment.get_path("DYLD_FALLBACK_LIBRARY_PATH")
+ spack.util.ld_so_conf.host_dynamic_linker_search_paths()
)
path_hints = path_hints if path_hints is not None else default_lib_search_paths
search_paths = llnl.util.filesystem.search_paths_for_libraries(*path_hints)
return path_to_dict(search_paths)
def libraries_in_windows_paths(path_hints: List[str]) -> Dict[str, str]:
path_hints.extend(spack.util.environment.get_path("PATH"))
search_paths = llnl.util.filesystem.search_paths_for_libraries(*path_hints)
def libraries_in_windows_paths(path_hints: Optional[List[str]] = None) -> Dict[str, str]:
"""Get the paths of all libraries available from the system PATH paths.
For more details, see `libraries_in_ld_and_system_library_path` regarding
return type and contents.
Args:
path_hints: list of paths to be searched. If None the list will be
constructed based on the set of PATH environment
variables as well as the standard system library paths.
"""
search_hints = (
path_hints if path_hints is not None else spack.util.environment.get_path("PATH")
)
search_paths = llnl.util.filesystem.search_paths_for_libraries(*search_hints)
# on Windows, some libraries (.dlls) are found in the bin directory or sometimes
# at the search root. Add both of those options to the search scheme
search_paths.extend(llnl.util.filesystem.search_paths_for_executables(*path_hints))
search_paths.extend(llnl.util.filesystem.search_paths_for_executables(*search_hints))
if path_hints is None:
# if no user provided path was given, add defaults to the search
search_paths.extend(WindowsKitExternalPaths.find_windows_kit_lib_paths())
search_paths.extend(WindowsKitExternalPaths.find_windows_kit_bin_paths())
search_paths.extend(WindowsKitExternalPaths.find_windows_kit_reg_installed_roots_paths())
search_paths.extend(WindowsKitExternalPaths.find_windows_kit_reg_sdk_paths())
# SDK and WGL should be handled by above, however on occasion the WDK is in an atypical
# location, so we handle that case specifically.
search_paths.extend(WindowsKitExternalPaths.find_windows_driver_development_kit_paths())
@ -125,19 +144,8 @@ def _group_by_prefix(paths: Set[str]) -> Dict[str, Set[str]]:
class Finder:
"""Inspects the file-system looking for packages. Guesses places where to look using PATH."""
def path_hints(
self, *, pkg: "spack.package_base.PackageBase", initial_guess: Optional[List[str]] = None
) -> List[str]:
"""Returns the list of paths to be searched.
Args:
pkg: package being detected
initial_guess: initial list of paths from caller
"""
result = initial_guess or []
result.extend(compute_windows_user_path_for_package(pkg))
result.extend(compute_windows_program_path_for_package(pkg))
return result
def default_path_hints(self) -> List[str]:
return []
def search_patterns(self, *, pkg: "spack.package_base.PackageBase") -> List[str]:
"""Returns the list of patterns used to match candidate files.
@ -245,6 +253,8 @@ def find(
Args:
pkg_name: package being detected
initial_guess: initial list of paths to search from the caller
if None, default paths are searched. If this
is an empty list, nothing will be searched.
"""
import spack.repo
@ -252,13 +262,18 @@ def find(
patterns = self.search_patterns(pkg=pkg_cls)
if not patterns:
return []
path_hints = self.path_hints(pkg=pkg_cls, initial_guess=initial_guess)
candidates = self.candidate_files(patterns=patterns, paths=path_hints)
if initial_guess is None:
initial_guess = self.default_path_hints()
initial_guess.extend(common_windows_package_paths(pkg_cls))
candidates = self.candidate_files(patterns=patterns, paths=initial_guess)
result = self.detect_specs(pkg=pkg_cls, paths=candidates)
return result
class ExecutablesFinder(Finder):
def default_path_hints(self) -> List[str]:
return spack.util.environment.get_path("PATH")
def search_patterns(self, *, pkg: "spack.package_base.PackageBase") -> List[str]:
result = []
if hasattr(pkg, "executables") and hasattr(pkg, "platform_executables"):
@ -298,7 +313,7 @@ def candidate_files(self, *, patterns: List[str], paths: List[str]) -> List[str]
libraries_by_path = (
libraries_in_ld_and_system_library_path(path_hints=paths)
if sys.platform != "win32"
else libraries_in_windows_paths(paths)
else libraries_in_windows_paths(path_hints=paths)
)
patterns = [re.compile(x) for x in patterns]
result = []
@ -334,21 +349,16 @@ def by_path(
# TODO: Packages should be able to define both .libraries and .executables in the future
# TODO: determine_spec_details should get all relevant libraries and executables in one call
executables_finder, libraries_finder = ExecutablesFinder(), LibrariesFinder()
executables_path_guess = (
spack.util.environment.get_path("PATH") if path_hints is None else path_hints
)
libraries_path_guess = [] if path_hints is None else path_hints
detected_specs_by_package: Dict[str, Tuple[concurrent.futures.Future, ...]] = {}
result = collections.defaultdict(list)
with concurrent.futures.ProcessPoolExecutor(max_workers=max_workers) as executor:
for pkg in packages_to_search:
executable_future = executor.submit(
executables_finder.find, pkg_name=pkg, initial_guess=executables_path_guess
executables_finder.find, pkg_name=pkg, initial_guess=path_hints
)
library_future = executor.submit(
libraries_finder.find, pkg_name=pkg, initial_guess=libraries_path_guess
libraries_finder.find, pkg_name=pkg, initial_guess=path_hints
)
detected_specs_by_package[pkg] = executable_future, library_future
@ -359,9 +369,13 @@ def by_path(
if detected:
_, unqualified_name = spack.repo.partition_package_name(pkg_name)
result[unqualified_name].extend(detected)
except Exception:
except concurrent.futures.TimeoutError:
llnl.util.tty.debug(
f"[EXTERNAL DETECTION] Skipping {pkg_name}: timeout reached"
)
except Exception as e:
llnl.util.tty.debug(
f"[EXTERNAL DETECTION] Skipping {pkg_name}: exception occured {e}"
)
return result

View File

@ -28,21 +28,12 @@ def _mock_search(path_hints=None):
return _factory
@pytest.fixture
def _platform_executables(monkeypatch):
def _win_exe_ext():
return ".bat"
monkeypatch.setattr(spack.util.path, "win_exe_ext", _win_exe_ext)
def define_plat_exe(exe):
if sys.platform == "win32":
exe += ".bat"
return exe
@pytest.mark.xfail(sys.platform == "win32", reason="https://github.com/spack/spack/pull/39850")
def test_find_external_single_package(mock_executable):
cmake_path = mock_executable("cmake", output="echo cmake version 1.foo")
search_dir = cmake_path.parent.parent
@ -54,7 +45,7 @@ def test_find_external_single_package(mock_executable):
assert len(detected_spec) == 1 and detected_spec[0].spec == Spec("cmake@1.foo")
def test_find_external_two_instances_same_package(mock_executable, _platform_executables):
def test_find_external_two_instances_same_package(mock_executable):
# Each of these cmake instances is created in a different prefix
# In Windows, quoted strings are echo'd with quotes includes
# we need to avoid that for proper regex.
@ -236,32 +227,7 @@ def test_list_detectable_packages(mutable_config, mutable_mock_repo):
assert external.returncode == 0
@pytest.mark.xfail(sys.platform == "win32", reason="https://github.com/spack/spack/pull/39850")
def test_packages_yaml_format(mock_executable, mutable_config, monkeypatch, _platform_executables):
# Prepare an environment to detect a fake gcc
gcc_exe = mock_executable("gcc", output="echo 4.2.1")
prefix = os.path.dirname(gcc_exe)
monkeypatch.setenv("PATH", prefix)
# Find the external spec
external("find", "gcc")
# Check entries in 'packages.yaml'
packages_yaml = spack.config.get("packages")
assert "gcc" in packages_yaml
assert "externals" in packages_yaml["gcc"]
externals = packages_yaml["gcc"]["externals"]
assert len(externals) == 1
external_gcc = externals[0]
assert external_gcc["spec"] == "gcc@4.2.1 languages=c"
assert external_gcc["prefix"] == os.path.dirname(prefix)
assert "extra_attributes" in external_gcc
extra_attributes = external_gcc["extra_attributes"]
assert "prefix" not in extra_attributes
assert extra_attributes["compilers"]["c"] == str(gcc_exe)
def test_overriding_prefix(mock_executable, mutable_config, monkeypatch, _platform_executables):
def test_overriding_prefix(mock_executable, mutable_config, monkeypatch):
gcc_exe = mock_executable("gcc", output="echo 4.2.1")
search_dir = gcc_exe.parent
@ -282,10 +248,7 @@ def _determine_variants(cls, exes, version_str):
assert gcc.external_path == os.path.sep + os.path.join("opt", "gcc", "bin")
@pytest.mark.xfail(sys.platform == "win32", reason="https://github.com/spack/spack/pull/39850")
def test_new_entries_are_reported_correctly(
mock_executable, mutable_config, monkeypatch, _platform_executables
):
def test_new_entries_are_reported_correctly(mock_executable, mutable_config, monkeypatch):
# Prepare an environment to detect a fake gcc
gcc_exe = mock_executable("gcc", output="echo 4.2.1")
prefix = os.path.dirname(gcc_exe)

View File

@ -0,0 +1,30 @@
# Copyright 2013-2023 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)
import collections
import spack.detection
import spack.spec
def test_detection_update_config(mutable_config):
# mock detected package
detected_packages = collections.defaultdict(list)
detected_packages["cmake"] = [
spack.detection.common.DetectedPackage(
spec=spack.spec.Spec("cmake@3.27.5"), prefix="/usr/bin"
)
]
# update config for new package
spack.detection.common.update_configuration(detected_packages)
# Check entries in 'packages.yaml'
packages_yaml = spack.config.get("packages")
assert "cmake" in packages_yaml
assert "externals" in packages_yaml["cmake"]
externals = packages_yaml["cmake"]["externals"]
assert len(externals) == 1
external_gcc = externals[0]
assert external_gcc["spec"] == "cmake@3.27.5"
assert external_gcc["prefix"] == "/usr/bin"

View File

@ -98,7 +98,7 @@ def replacements():
def win_exe_ext():
return ".exe"
return r"(?:\.bat|\.exe)"
def sanitize_filename(filename: str) -> str: