From d2f9c7a6670d2c99dc24e9365c47482db91e7a28 Mon Sep 17 00:00:00 2001 From: Ryan Krattiger Date: Thu, 29 Jan 2026 16:05:32 -0600 Subject: [PATCH 1/6] CMakePackage: Add option to get cmake args from dependents This feature is not complete but it is a minimal implementation to provided the ability for packages to define hints for cmake. Not all hints can be passed as environment variables and cmake argument injection is not possible via existing mechanisms. As a demonstration of this feature the python hint flags are converted from the ad hoc method in the CMakePackage to using the package callback. Flags can be disabled using a common class list attribute `disable_cmake_hints_from`. --- .../spack_repo/builtin/build_systems/cmake.py | 57 ++++++++++--------- .../builtin/packages/opencv/package.py | 2 +- .../builtin/packages/python/package.py | 9 +++ 3 files changed, 41 insertions(+), 27 deletions(-) diff --git a/repos/spack_repo/builtin/build_systems/cmake.py b/repos/spack_repo/builtin/build_systems/cmake.py index f41091ba5ac..c36862820a1 100644 --- a/repos/spack_repo/builtin/build_systems/cmake.py +++ b/repos/spack_repo/builtin/build_systems/cmake.py @@ -42,24 +42,6 @@ def _extract_primary_generator(generator): return _primary_generator_extractor.match(generator).group(1) -def _maybe_set_python_hints(pkg: PackageBase, args: List[str]) -> None: - """Set the PYTHON_EXECUTABLE, Python_EXECUTABLE, and Python3_EXECUTABLE CMake variables - if the package has Python as build or link dep and ``find_python_hints`` is set to True. See - ``find_python_hints`` for context.""" - if not getattr(pkg, "find_python_hints", False) or not pkg.spec.dependencies( - "python", deptype=("build", "link") - ): - return - python_executable = pkg.spec["python"].command.path - args.extend( - [ - define("PYTHON_EXECUTABLE", python_executable), - define("Python_EXECUTABLE", python_executable), - define("Python3_EXECUTABLE", python_executable), - ] - ) - - def _supports_compilation_databases(pkg: PackageBase) -> bool: """Check if this package (and CMake) can support compilation databases.""" @@ -172,6 +154,9 @@ class CMakePackage(PackageBase): https://cmake.org/cmake/help/latest/ """ + #: Disable adding extra CMake args provided by listed depenedencies + disable_cmake_hints_from: List[str] = [] + #: This attribute is used in UI queries that need to know the build #: system base class build_system_class = "CMakePackage" @@ -179,13 +164,6 @@ class CMakePackage(PackageBase): #: Legacy buildsystem attribute used to deserialize and install old specs default_buildsystem = "cmake" - #: When this package depends on Python and ``find_python_hints`` is set to True, pass the - #: defines {Python3,Python,PYTHON}_EXECUTABLE explicitly, so that CMake locates the right - #: Python in its builtin FindPython3, FindPython, and FindPythonInterp modules. Spack does - #: CMake's job because CMake's modules by default only search for Python versions known at the - #: time of release. - find_python_hints = True - build_system("cmake") with when("build_system=cmake"): @@ -405,7 +383,34 @@ def std_args(pkg: PackageBase, generator: Optional[str] = None) -> List[str]: ) _conditional_cmake_defaults(pkg, args) - _maybe_set_python_hints(pkg, args) + + # Append extra hint/option arguments from dependencies + disable_cmake_hints_from = getattr(pkg, "disable_cmake_hints_from", []) + for dep in pkg.spec.edges_to_dependencies(): + # Skip disabled dependency cmake args + # Consider virtual names as well package names + disabled_for_pkg = dep.spec.package.name in disable_cmake_hints_from + disabled_for_virtual = False + if dep.virtuals: + disabled_for_virtual = any((v in disable_cmake_hints_from for v in dep.virtuals)) + + if disabled_for_pkg or disabled_for_virtual: + continue + + # TODO: Consider properties from virtual packages like Mpi + packages = [dep.spec.package] + for dep_pkg in packages: + if not hasattr(dep_pkg, "dependent_cmake_args"): + continue + try: + # Try calling as a function first + args.extend(dep_pkg.dependent_cmake_args(pkg)) + except TypeError: + # Fallback to calling as a str/list attribute + dep_args = dep_pkg.dependent_cmake_args + if isinstance(dep_args, str): + dep_args = [dep_args] + args.extend(dep_args) return args diff --git a/repos/spack_repo/builtin/packages/opencv/package.py b/repos/spack_repo/builtin/packages/opencv/package.py index ded7baeab0f..382e294f2e1 100644 --- a/repos/spack_repo/builtin/packages/opencv/package.py +++ b/repos/spack_repo/builtin/packages/opencv/package.py @@ -17,7 +17,7 @@ class Opencv(CMakePackage, CudaPackage): homepage = "https://opencv.org/" url = "https://github.com/opencv/opencv/archive/4.5.0.tar.gz" git = "https://github.com/opencv/opencv.git" - find_python_hints = False # opencv uses custom OpenCVDetectPython.cmake + disable_cmake_hints_from = ["python"] # opencv uses custom OpenCVDetectPython.cmake maintainers("bvanessen", "adamjstewart") diff --git a/repos/spack_repo/builtin/packages/python/package.py b/repos/spack_repo/builtin/packages/python/package.py index 3b202d21d57..c12ddd8dd17 100644 --- a/repos/spack_repo/builtin/packages/python/package.py +++ b/repos/spack_repo/builtin/packages/python/package.py @@ -13,6 +13,7 @@ from shutil import copy from typing import Dict, List +from spack_repo.builtin.build_systems.cmake import define as cmdefine from spack_repo.builtin.build_systems.generic import Package from spack.package import * @@ -1324,6 +1325,14 @@ def setup_dependent_package(self, module, dependent_spec): module.python_platlib = join_path(dependent_spec.prefix, self.platlib) module.python_purelib = join_path(dependent_spec.prefix, self.purelib) + def dependent_cmake_args(self, pkg: PackageBase) -> List[str]: + python_executable = pkg.spec["python"].command.path + return [ + cmdefine("PYTHON_EXECUTABLE", python_executable), + cmdefine("Python_EXECUTABLE", python_executable), + cmdefine("Python3_EXECUTABLE", python_executable), + ] + def add_files_to_view(self, view, merge_map, skip_if_exists=True): """Make the view a virtual environment if it isn't one already. From 88c263d62ab1cff210e9b43c95fb0197519d7b1c Mon Sep 17 00:00:00 2001 From: Ryan Krattiger Date: Tue, 3 Feb 2026 10:12:06 -0600 Subject: [PATCH 2/6] Address some review comments Remove cruft from virtual handling remove CMakePackge dep add comment about python-venv redirect --- .../spack_repo/builtin/build_systems/cmake.py | 21 +++++++------------ .../builtin/packages/python/package.py | 9 ++++---- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/repos/spack_repo/builtin/build_systems/cmake.py b/repos/spack_repo/builtin/build_systems/cmake.py index c36862820a1..44264bbff9a 100644 --- a/repos/spack_repo/builtin/build_systems/cmake.py +++ b/repos/spack_repo/builtin/build_systems/cmake.py @@ -385,8 +385,14 @@ def std_args(pkg: PackageBase, generator: Optional[str] = None) -> List[str]: _conditional_cmake_defaults(pkg, args) # Append extra hint/option arguments from dependencies + # TODO: Consider properties from virtual packages like Mpi disable_cmake_hints_from = getattr(pkg, "disable_cmake_hints_from", []) for dep in pkg.spec.edges_to_dependencies(): + # Skip packages without the callback + dep_pkg = dep.spec.package + if not hasattr(dep_pkg, "dependent_cmake_args"): + continue + # Skip disabled dependency cmake args # Consider virtual names as well package names disabled_for_pkg = dep.spec.package.name in disable_cmake_hints_from @@ -397,20 +403,7 @@ def std_args(pkg: PackageBase, generator: Optional[str] = None) -> List[str]: if disabled_for_pkg or disabled_for_virtual: continue - # TODO: Consider properties from virtual packages like Mpi - packages = [dep.spec.package] - for dep_pkg in packages: - if not hasattr(dep_pkg, "dependent_cmake_args"): - continue - try: - # Try calling as a function first - args.extend(dep_pkg.dependent_cmake_args(pkg)) - except TypeError: - # Fallback to calling as a str/list attribute - dep_args = dep_pkg.dependent_cmake_args - if isinstance(dep_args, str): - dep_args = [dep_args] - args.extend(dep_args) + args.extend(dep_pkg.dependent_cmake_args(pkg)) return args diff --git a/repos/spack_repo/builtin/packages/python/package.py b/repos/spack_repo/builtin/packages/python/package.py index c12ddd8dd17..d5678a37959 100644 --- a/repos/spack_repo/builtin/packages/python/package.py +++ b/repos/spack_repo/builtin/packages/python/package.py @@ -13,7 +13,6 @@ from shutil import copy from typing import Dict, List -from spack_repo.builtin.build_systems.cmake import define as cmdefine from spack_repo.builtin.build_systems.generic import Package from spack.package import * @@ -1326,11 +1325,13 @@ def setup_dependent_package(self, module, dependent_spec): module.python_purelib = join_path(dependent_spec.prefix, self.purelib) def dependent_cmake_args(self, pkg: PackageBase) -> List[str]: + # pkg.spec["python"] can re-direct to python-venv if pkg extends python + # ref. https://github.com/spack/spack/pull/40773 python_executable = pkg.spec["python"].command.path return [ - cmdefine("PYTHON_EXECUTABLE", python_executable), - cmdefine("Python_EXECUTABLE", python_executable), - cmdefine("Python3_EXECUTABLE", python_executable), + f"-DPYTHON_EXECUTABLE:PATH={python_executable}"), + f"-DPython_EXECUTABLE:PATH={python_executable}"), + f"-DPython3_EXECUTABLE:PATH={python_executable}"), ] def add_files_to_view(self, view, merge_map, skip_if_exists=True): From a29a91211f340d1e3977283fefd84835183b8ee9 Mon Sep 17 00:00:00 2001 From: Ryan Krattiger Date: Tue, 3 Feb 2026 10:50:19 -0600 Subject: [PATCH 3/6] Add test for `dependent_cmake_args` --- .../builtin/packages/python/package.py | 6 ++--- tests/build_systems.py | 6 +++++ .../packages/cmake_client/package.py | 2 ++ .../packages/cmake_hints/package.py | 26 +++++++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py diff --git a/repos/spack_repo/builtin/packages/python/package.py b/repos/spack_repo/builtin/packages/python/package.py index d5678a37959..b502b1e8334 100644 --- a/repos/spack_repo/builtin/packages/python/package.py +++ b/repos/spack_repo/builtin/packages/python/package.py @@ -1329,9 +1329,9 @@ def dependent_cmake_args(self, pkg: PackageBase) -> List[str]: # ref. https://github.com/spack/spack/pull/40773 python_executable = pkg.spec["python"].command.path return [ - f"-DPYTHON_EXECUTABLE:PATH={python_executable}"), - f"-DPython_EXECUTABLE:PATH={python_executable}"), - f"-DPython3_EXECUTABLE:PATH={python_executable}"), + f"-DPYTHON_EXECUTABLE:PATH={python_executable}", + f"-DPython_EXECUTABLE:PATH={python_executable}", + f"-DPython3_EXECUTABLE:PATH={python_executable}", ] def add_files_to_view(self, view, merge_map, skip_if_exists=True): diff --git a/tests/build_systems.py b/tests/build_systems.py index 795be957b87..b4fe82dec5b 100644 --- a/tests/build_systems.py +++ b/tests/build_systems.py @@ -257,6 +257,12 @@ def test_cmake_std_args(self, default_mock_concretization): s = default_mock_concretization("mpich") assert cmake.CMakeBuilder.std_args(s.package) + def test_cmake_dependent_args(self, default_mock_concretization): + # Call the function on a CMakePackage instance + s = default_mock_concretization("cmake-client +cmake_hints") + args = cmake.CMakeBuilder.std_args(s.package) + assert "-DCMAKE_HINTS_ARG:STRING=\"Foo\"" in args + def test_cmake_bad_generator(self, default_mock_concretization): s = default_mock_concretization("cmake-client") with pytest.raises(InstallError): diff --git a/tests/repos/spack_repo/builtin_mock/packages/cmake_client/package.py b/tests/repos/spack_repo/builtin_mock/packages/cmake_client/package.py index dddf55dea42..fd028deae04 100644 --- a/tests/repos/spack_repo/builtin_mock/packages/cmake_client/package.py +++ b/tests/repos/spack_repo/builtin_mock/packages/cmake_client/package.py @@ -30,8 +30,10 @@ class CmakeClient(CMakePackage): ) variant("single", description="", default="blue", values=("blue", "red", "green"), multi=False) variant("truthy", description="", default=True) + variant("cmake_hints", description="", default=False) depends_on("c", type="build") + depends_on("cmake-hints", when="+cmake_hints") callback_counter = 0 diff --git a/tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py b/tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py new file mode 100644 index 00000000000..81f31bcbb8a --- /dev/null +++ b/tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py @@ -0,0 +1,26 @@ +# Copyright Spack Project Developers. See COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import PackageBase, Package, version + +from typing import List + + +def check(condition, msg): + """Raise an install error if condition is False.""" + if not condition: + raise InstallError(msg) + + +class CmakeHints(Package): + """A dummy package that uses cmake.""" + + homepage = "https://www.example.com" + url = "https://www.example.com/cmake-hints-1.0.tar.gz" + version("1.0", md5="4cb3ff35b2472aae70f542116d616e63") + + def dependent_cmake_args(self, pkg: PackageBase) -> List[str]: + return [ + "-DCMAKE_HINTS_ARG:STRING=\"Foo\"" + ] From 6cb4746b2b8a6a2cc21c0263c34c90aa0bd2b16f Mon Sep 17 00:00:00 2001 From: Ryan Krattiger Date: Tue, 3 Feb 2026 11:05:24 -0600 Subject: [PATCH 4/6] Fix style --- .ci/gitlab/configs/ci.yaml | 2 +- tests/build_systems.py | 2 +- .../builtin_mock/packages/cmake_hints/package.py | 8 +++----- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.ci/gitlab/configs/ci.yaml b/.ci/gitlab/configs/ci.yaml index 5cf2429bf77..5f58695020e 100644 --- a/.ci/gitlab/configs/ci.yaml +++ b/.ci/gitlab/configs/ci.yaml @@ -27,7 +27,7 @@ ci: after_script: - - cat /proc/loadavg || true - cat /proc/meminfo | grep 'MemTotal\|MemFree' || true - - - time ./spack/bin/spack python ${SPACK_CI_PACKAGES_ROOT}/.ci/gitlab/scripts/common/aggregate_package_logs.spack.py + - - time ./spack python ${SPACK_CI_PACKAGES_ROOT}/.ci/gitlab/scripts/common/aggregate_package_logs.spack.py --prefix /home/software/spack:${CI_PROJECT_DIR}/opt/spack --log install_times.json ${SPACK_ARTIFACTS_ROOT}/user_data/install_times.json || true diff --git a/tests/build_systems.py b/tests/build_systems.py index b4fe82dec5b..2a88516462e 100644 --- a/tests/build_systems.py +++ b/tests/build_systems.py @@ -261,7 +261,7 @@ def test_cmake_dependent_args(self, default_mock_concretization): # Call the function on a CMakePackage instance s = default_mock_concretization("cmake-client +cmake_hints") args = cmake.CMakeBuilder.std_args(s.package) - assert "-DCMAKE_HINTS_ARG:STRING=\"Foo\"" in args + assert '-DCMAKE_HINTS_ARG:STRING="Foo"' in args def test_cmake_bad_generator(self, default_mock_concretization): s = default_mock_concretization("cmake-client") diff --git a/tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py b/tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py index 81f31bcbb8a..1a5c67028a1 100644 --- a/tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py +++ b/tests/repos/spack_repo/builtin_mock/packages/cmake_hints/package.py @@ -2,10 +2,10 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) -from spack.package import PackageBase, Package, version - from typing import List +from spack.package import Package, PackageBase, version + def check(condition, msg): """Raise an install error if condition is False.""" @@ -21,6 +21,4 @@ class CmakeHints(Package): version("1.0", md5="4cb3ff35b2472aae70f542116d616e63") def dependent_cmake_args(self, pkg: PackageBase) -> List[str]: - return [ - "-DCMAKE_HINTS_ARG:STRING=\"Foo\"" - ] + return ['-DCMAKE_HINTS_ARG:STRING="Foo"'] From 1a954aff77ef8c7daafffa3ec27ad205641ae46f Mon Sep 17 00:00:00 2001 From: Ryan Krattiger Date: Tue, 3 Feb 2026 12:13:21 -0600 Subject: [PATCH 5/6] Pass dependent pkg spec and make naming consistent --- .ci/gitlab/configs/ci.yaml | 2 +- repos/spack_repo/builtin/build_systems/cmake.py | 2 +- repos/spack_repo/builtin/packages/python/package.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.ci/gitlab/configs/ci.yaml b/.ci/gitlab/configs/ci.yaml index 5f58695020e..5cf2429bf77 100644 --- a/.ci/gitlab/configs/ci.yaml +++ b/.ci/gitlab/configs/ci.yaml @@ -27,7 +27,7 @@ ci: after_script: - - cat /proc/loadavg || true - cat /proc/meminfo | grep 'MemTotal\|MemFree' || true - - - time ./spack python ${SPACK_CI_PACKAGES_ROOT}/.ci/gitlab/scripts/common/aggregate_package_logs.spack.py + - - time ./spack/bin/spack python ${SPACK_CI_PACKAGES_ROOT}/.ci/gitlab/scripts/common/aggregate_package_logs.spack.py --prefix /home/software/spack:${CI_PROJECT_DIR}/opt/spack --log install_times.json ${SPACK_ARTIFACTS_ROOT}/user_data/install_times.json || true diff --git a/repos/spack_repo/builtin/build_systems/cmake.py b/repos/spack_repo/builtin/build_systems/cmake.py index 44264bbff9a..733b0bfd336 100644 --- a/repos/spack_repo/builtin/build_systems/cmake.py +++ b/repos/spack_repo/builtin/build_systems/cmake.py @@ -403,7 +403,7 @@ def std_args(pkg: PackageBase, generator: Optional[str] = None) -> List[str]: if disabled_for_pkg or disabled_for_virtual: continue - args.extend(dep_pkg.dependent_cmake_args(pkg)) + args.extend(dep_pkg.dependent_cmake_args(pkg.spec)) return args diff --git a/repos/spack_repo/builtin/packages/python/package.py b/repos/spack_repo/builtin/packages/python/package.py index b502b1e8334..6d28e2e2288 100644 --- a/repos/spack_repo/builtin/packages/python/package.py +++ b/repos/spack_repo/builtin/packages/python/package.py @@ -1324,10 +1324,10 @@ def setup_dependent_package(self, module, dependent_spec): module.python_platlib = join_path(dependent_spec.prefix, self.platlib) module.python_purelib = join_path(dependent_spec.prefix, self.purelib) - def dependent_cmake_args(self, pkg: PackageBase) -> List[str]: + def dependent_cmake_args(self, dependent_spec: Spec) -> List[str]: # pkg.spec["python"] can re-direct to python-venv if pkg extends python # ref. https://github.com/spack/spack/pull/40773 - python_executable = pkg.spec["python"].command.path + python_executable = dependent_spec["python"].command.path return [ f"-DPYTHON_EXECUTABLE:PATH={python_executable}", f"-DPython_EXECUTABLE:PATH={python_executable}", From 7b7a55128983e34de02c801c0f7f3683f7a82d06 Mon Sep 17 00:00:00 2001 From: Ryan Krattiger Date: Fri, 13 Feb 2026 11:31:00 -0600 Subject: [PATCH 6/6] Some cleanup from review --- repos/spack_repo/builtin/build_systems/cmake.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/repos/spack_repo/builtin/build_systems/cmake.py b/repos/spack_repo/builtin/build_systems/cmake.py index 733b0bfd336..e8972e1f451 100644 --- a/repos/spack_repo/builtin/build_systems/cmake.py +++ b/repos/spack_repo/builtin/build_systems/cmake.py @@ -154,7 +154,7 @@ class CMakePackage(PackageBase): https://cmake.org/cmake/help/latest/ """ - #: Disable adding extra CMake args provided by listed depenedencies + #: List of package names for which CMake argument injection should be disabled disable_cmake_hints_from: List[str] = [] #: This attribute is used in UI queries that need to know the build @@ -394,13 +394,11 @@ def std_args(pkg: PackageBase, generator: Optional[str] = None) -> List[str]: continue # Skip disabled dependency cmake args - # Consider virtual names as well package names - disabled_for_pkg = dep.spec.package.name in disable_cmake_hints_from - disabled_for_virtual = False - if dep.virtuals: - disabled_for_virtual = any((v in disable_cmake_hints_from for v in dep.virtuals)) + if dep_pkg.name in disable_cmake_hints_from: + continue - if disabled_for_pkg or disabled_for_virtual: + # Skip disabled virtual dependency cmake args + if any(v in disable_cmake_hints_from for v in dep.virtuals): continue args.extend(dep_pkg.dependent_cmake_args(pkg.spec))