From fdfda72371e568ce2dcfdac559cda5fb6c195758 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Mon, 28 Nov 2022 14:18:26 +0100 Subject: Use a module-like object to propagate changes in the MRO, when setting build env (#34059) This fixes an issue introduced in #32340, which changed the semantics of the "module" object passed to the "setup_dependent_package" callback. --- lib/spack/spack/build_environment.py | 68 +++++++++++++++++++++++-------- lib/spack/spack/test/build_environment.py | 27 +++++++++++- 2 files changed, 77 insertions(+), 18 deletions(-) diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index f5f1f1ccd4..2ec1683eb8 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -41,7 +41,6 @@ import shutil import sys import traceback import types -from typing import List, Tuple import llnl.util.tty as tty from llnl.util.filesystem import install, install_tree, mkdirp @@ -1001,22 +1000,9 @@ def modifications_from_dependencies( if set_package_py_globals: set_module_variables_for_package(dpkg) - # Allow dependencies to modify the module - # Get list of modules that may need updating - modules = [] - for cls in inspect.getmro(type(spec.package)): - module = cls.module - if module == spack.package_base: - break - modules.append(module) - - # Execute changes as if on a single module - # copy dict to ensure prior changes are available - changes = spack.util.pattern.Bunch() - dpkg.setup_dependent_package(changes, spec) - - for module in modules: - module.__dict__.update(changes.__dict__) + current_module = ModuleChangePropagator(spec.package) + dpkg.setup_dependent_package(current_module, spec) + current_module.propagate_changes_to_mro() if context == "build": builder = spack.builder.create(dpkg) @@ -1462,3 +1448,51 @@ def write_log_summary(out, log_type, log, last=None): # If no errors are found but warnings are, display warnings out.write("\n%s found in %s log:\n" % (plural(nwar, "warning"), log_type)) out.write(make_log_context(warnings)) + + +class ModuleChangePropagator: + """Wrapper class to accept changes to a package.py Python module, and propagate them in the + MRO of the package. + + It is mainly used as a substitute of the ``package.py`` module, when calling the + "setup_dependent_package" function during build environment setup. + """ + + _PROTECTED_NAMES = ("package", "current_module", "modules_in_mro", "_set_attributes") + + def __init__(self, package): + self._set_self_attributes("package", package) + self._set_self_attributes("current_module", package.module) + + #: Modules for the classes in the MRO up to PackageBase + modules_in_mro = [] + for cls in inspect.getmro(type(package)): + module = cls.module + + if module == self.current_module: + continue + + if module == spack.package_base: + break + + modules_in_mro.append(module) + self._set_self_attributes("modules_in_mro", modules_in_mro) + self._set_self_attributes("_set_attributes", {}) + + def _set_self_attributes(self, key, value): + super().__setattr__(key, value) + + def __getattr__(self, item): + return getattr(self.current_module, item) + + def __setattr__(self, key, value): + if key in ModuleChangePropagator._PROTECTED_NAMES: + msg = f'Cannot set attribute "{key}" in ModuleMonkeyPatcher' + return AttributeError(msg) + + setattr(self.current_module, key, value) + self._set_attributes[key] = value + + def propagate_changes_to_mro(self): + for module_in_mro in self.modules_in_mro: + module_in_mro.__dict__.update(self._set_attributes) diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index 9b894a9351..48f948f277 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -2,7 +2,7 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - +import inspect import os import platform import posixpath @@ -14,6 +14,7 @@ from llnl.util.filesystem import HeaderList, LibraryList import spack.build_environment import spack.config +import spack.package_base import spack.spec import spack.util.spack_yaml as syaml from spack.build_environment import ( @@ -521,3 +522,27 @@ def test_dirty_disable_module_unload(config, mock_packages, working_env, mock_mo assert mock_module_cmd.calls assert any(("unload", "cray-libsci") == item[0] for item in mock_module_cmd.calls) assert any(("unload", "cray-mpich") == item[0] for item in mock_module_cmd.calls) + + +class TestModuleMonkeyPatcher: + def test_getting_attributes(self, default_mock_concretization): + s = default_mock_concretization("libelf") + module_wrapper = spack.build_environment.ModuleChangePropagator(s.package) + assert module_wrapper.Libelf == s.package.module.Libelf + + def test_setting_attributes(self, default_mock_concretization): + s = default_mock_concretization("libelf") + module = s.package.module + module_wrapper = spack.build_environment.ModuleChangePropagator(s.package) + + # Setting an attribute has an immediate effect + module_wrapper.SOME_ATTRIBUTE = 1 + assert module.SOME_ATTRIBUTE == 1 + + # We can also propagate the settings to classes in the MRO + module_wrapper.propagate_changes_to_mro() + for cls in inspect.getmro(type(s.package)): + current_module = cls.module + if current_module == spack.package_base: + break + assert current_module.SOME_ATTRIBUTE == 1 -- cgit v1.2.3-70-g09d2