remove std_cmake_args, std_pip_args, std_meson_args (#50462)

The "magic" globals `std_cmake_args`, `std_pip_args` and `std_meson_args` were deprecated in Spack 0.23 and removed in this commit, because they are no longer static and don't make sense to be defined for packages that do not depend on cmake, pip or meson.

Additionally, removing them ensures that `build_environment.py` no longer depends on builders, which will soon be moved out of `spack` into the `spack_repo` package.

The audit that scans whether these globals are used is not removed.
This commit is contained in:
Harmen Stoppels 2025-05-14 09:51:42 +02:00 committed by GitHub
parent de4a9e867d
commit 00d65b75a1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 9 additions and 61 deletions

View File

@ -2253,22 +2253,15 @@ RPATHs in Spack are handled in one of three ways:
set in standard variables like ``CC``, ``CXX``, ``F77``, and ``FC``, set in standard variables like ``CC``, ``CXX``, ``F77``, and ``FC``,
so most build systems (autotools and many gmake systems) pick them so most build systems (autotools and many gmake systems) pick them
up and use them. up and use them.
#. CMake also respects Spack's compiler wrappers, but many CMake #. CMake has its own RPATH handling, and distinguishes between build and
builds have logic to overwrite RPATHs when binaries are install RPATHs. By default, during the build it registers RPATHs to
installed. Spack provides the ``std_cmake_args`` variable, which all libraries it links to, so that just-built executables can be run
includes parameters necessary for CMake build use the right during the build itself. Upon installation, these RPATHs are cleared,
installation RPATH. It can be used like this when ``cmake`` is unless the user defines the install RPATHs. When inheriting from
invoked: ``CMakePackage``, Spack handles this automatically, and sets
``CMAKE_INSTALL_RPATH_USE_LINK_PATH`` and ``CMAKE_INSTALL_RPATH``,
.. code-block:: python so that libraries of dependencies and the package's own libraries
can be found at runtime.
class MyPackage(Package):
...
def install(self, spec, prefix):
cmake("..", *std_cmake_args)
make()
make("install")
#. If you need to modify the build to add your own RPATHs, you can #. If you need to modify the build to add your own RPATHs, you can
use the ``self.rpath`` property of your package, which will use the ``self.rpath`` property of your package, which will
return a list of all the RPATHs that Spack will use when it return a list of all the RPATHs that Spack will use when it

View File

@ -68,9 +68,6 @@
from llnl.util.symlink import symlink from llnl.util.symlink import symlink
from llnl.util.tty.color import cescape, colorize from llnl.util.tty.color import cescape, colorize
import spack.build_systems.cmake
import spack.build_systems.meson
import spack.build_systems.python
import spack.builder import spack.builder
import spack.compilers.libraries import spack.compilers.libraries
import spack.config import spack.config
@ -567,9 +564,6 @@ def set_package_py_globals(pkg, context: Context = Context.BUILD):
jobs = spack.config.determine_number_of_jobs(parallel=pkg.parallel) jobs = spack.config.determine_number_of_jobs(parallel=pkg.parallel)
module.make_jobs = jobs module.make_jobs = jobs
if context == Context.BUILD:
module.std_meson_args = spack.build_systems.meson.MesonBuilder.std_args(pkg)
module.std_pip_args = spack.build_systems.python.PythonPipBuilder.std_args(pkg)
module.make = DeprecatedExecutable(pkg.name, "make", "gmake") module.make = DeprecatedExecutable(pkg.name, "make", "gmake")
module.gmake = DeprecatedExecutable(pkg.name, "gmake", "gmake") module.gmake = DeprecatedExecutable(pkg.name, "gmake", "gmake")
@ -998,15 +992,6 @@ def set_all_package_py_globals(self):
pkg.setup_dependent_package(dependent_module, spec) pkg.setup_dependent_package(dependent_module, spec)
dependent_module.propagate_changes_to_mro() dependent_module.propagate_changes_to_mro()
if self.context == Context.BUILD:
pkg = self.specs[0].package
module = ModuleChangePropagator(pkg)
# std_cmake_args is not sufficiently static to be defined
# in set_package_py_globals and is deprecated so its handled
# here as a special case
module.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
module.propagate_changes_to_mro()
def get_env_modifications(self) -> EnvironmentModifications: def get_env_modifications(self) -> EnvironmentModifications:
"""Returns the environment variable modifications for the given input specs and context. """Returns the environment variable modifications for the given input specs and context.
Environment modifications include: Environment modifications include:

View File

@ -691,36 +691,6 @@ def test_clear_compiler_related_runtime_variables_of_build_deps(default_mock_con
assert result["ANOTHER_VAR"] == "this-should-be-present" assert result["ANOTHER_VAR"] == "this-should-be-present"
@pytest.mark.parametrize("context", [Context.BUILD, Context.RUN])
def test_build_system_globals_only_set_on_root_during_build(default_mock_concretization, context):
"""Test whether when setting up a build environment, the build related globals are set only
in the top level spec.
TODO: Since module instances are globals themselves, and Spack defines properties on them, they
persist across tests. In principle this is not terrible, cause the variables are mostly static.
But obviously it can lead to very hard to find bugs... We should get rid of those globals and
define them instead as a property on the package instance.
"""
root = spack.concretize.concretize_one("mpileaks")
build_variables = ("std_cmake_args", "std_meson_args", "std_pip_args")
# See todo above, we clear out any properties that may have been set by the previous test.
# Commenting this loop will make the test fail. I'm leaving it here as a reminder that those
# globals were always a bad idea, and we should pass them to the package instance.
for spec in root.traverse():
for variable in build_variables:
spec.package.module.__dict__.pop(variable, None)
spack.build_environment.SetupContext(root, context=context).set_all_package_py_globals()
# Excpect the globals to be set at the root in a build context only.
should_be_set = lambda depth: context == Context.BUILD and depth == 0
for depth, spec in root.traverse(depth=True, root=True):
for variable in build_variables:
assert hasattr(spec.package.module, variable) == should_be_set(depth)
def test_rpath_with_duplicate_link_deps(): def test_rpath_with_duplicate_link_deps():
"""If we have two instances of one package in the same link sub-dag, only the newest version is """If we have two instances of one package in the same link sub-dag, only the newest version is
rpath'ed. This is for runtime support without splicing.""" rpath'ed. This is for runtime support without splicing."""