Build logic fix: reorder definition of package module variables (#46992)
#44327 made sure to always run `set_package_py_globals` on all packages before running `setup_dependent_package` for any package, so that packages implementing the latter could depend on variables like `spack_cc` being defined. This ran into an undocumented dependency: `std_cmake_args` is set in `set_package_py_globals` and makes use of `cmake_prefix_paths` (if it is defined in the package); `py-torch`es implementation of `cmake_prefix_paths` depends on a variable set by `setup_dependent_package` (`python_platlib`). This generally restores #44327, and corrects the resulting issue by moving assignment of `std_cmake_args` to after both actions have been run.
This commit is contained in:
parent
230bc7010a
commit
31cfcafeba
@ -617,13 +617,11 @@ def set_package_py_globals(pkg, context: Context = Context.BUILD):
|
||||
"""
|
||||
module = ModuleChangePropagator(pkg)
|
||||
|
||||
if context == Context.BUILD:
|
||||
module.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg)
|
||||
module.std_meson_args = spack.build_systems.meson.MesonBuilder.std_args(pkg)
|
||||
module.std_pip_args = spack.build_systems.python.PythonPipBuilder.std_args(pkg)
|
||||
|
||||
jobs = spack.config.determine_number_of_jobs(parallel=pkg.parallel)
|
||||
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)
|
||||
|
||||
# TODO: make these build deps that can be installed if not found.
|
||||
module.make = MakeExecutable("make", jobs)
|
||||
@ -1048,6 +1046,12 @@ def set_all_package_py_globals(self):
|
||||
# This includes runtime dependencies, also runtime deps of direct build deps.
|
||||
set_package_py_globals(pkg, context=Context.RUN)
|
||||
|
||||
# Looping over the set of packages a second time
|
||||
# ensures all globals are loaded into the module space prior to
|
||||
# any package setup. This guarantees package setup methods have
|
||||
# access to expected module level definitions such as "spack_cc"
|
||||
for dspec, flag in chain(self.external, self.nonexternal):
|
||||
pkg = dspec.package
|
||||
for spec in dspec.dependents():
|
||||
# Note: some specs have dependents that are unreachable from the root, so avoid
|
||||
# setting globals for those.
|
||||
@ -1057,6 +1061,15 @@ def set_all_package_py_globals(self):
|
||||
pkg.setup_dependent_package(dependent_module, spec)
|
||||
dependent_module.propagate_changes_to_mro()
|
||||
|
||||
pkg = self.specs[0].package
|
||||
if self.context == Context.BUILD:
|
||||
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:
|
||||
"""Returns the environment variable modifications for the given input specs and context.
|
||||
Environment modifications include:
|
||||
|
@ -516,6 +516,30 @@ def test_setting_dtags_based_on_config(config_setting, expected_flag, config, mo
|
||||
assert dtags_to_add.value == expected_flag
|
||||
|
||||
|
||||
def test_module_globals_available_at_setup_dependent_time(
|
||||
monkeypatch, mutable_config, mock_packages, working_env
|
||||
):
|
||||
"""Spack built package externaltest depends on an external package
|
||||
externaltool. Externaltool's setup_dependent_package needs to be able to
|
||||
access globals on the dependent"""
|
||||
|
||||
def setup_dependent_package(module, dependent_spec):
|
||||
# Make sure set_package_py_globals was already called on
|
||||
# dependents
|
||||
# ninja is always set by the setup context and is not None
|
||||
dependent_module = dependent_spec.package.module
|
||||
assert hasattr(dependent_module, "ninja")
|
||||
assert dependent_module.ninja is not None
|
||||
dependent_spec.package.test_attr = True
|
||||
|
||||
externaltool = spack.spec.Spec("externaltest").concretized()
|
||||
monkeypatch.setattr(
|
||||
externaltool["externaltool"].package, "setup_dependent_package", setup_dependent_package
|
||||
)
|
||||
spack.build_environment.setup_package(externaltool.package, False)
|
||||
assert externaltool.package.test_attr
|
||||
|
||||
|
||||
def test_build_jobs_sequential_is_sequential():
|
||||
assert (
|
||||
spack.config.determine_number_of_jobs(
|
||||
|
Loading…
Reference in New Issue
Block a user