From dd9e28a6211990426ab3d3af1af45141c5ef4850 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Fri, 30 Aug 2024 15:07:33 +0200 Subject: [PATCH] simplify mro stuff --- lib/spack/spack/build_environment.py | 37 ++++++---------------- lib/spack/spack/build_systems/autotools.py | 5 ++- lib/spack/spack/test/build_environment.py | 12 +++---- 3 files changed, 16 insertions(+), 38 deletions(-) diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index e807bf6fd02..c0698bdad7b 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -555,7 +555,7 @@ def set_package_py_globals(pkg, context: Context = Context.BUILD): """Populate the Python module of a package with some useful global names. This makes things easier for package writers. """ - module = ModuleChangePropagator(pkg) + module = SetPackageGlobals(pkg) if context == Context.BUILD: module.std_cmake_args = spack.build_systems.cmake.CMakeBuilder.std_args(pkg) @@ -1016,7 +1016,7 @@ def set_all_package_py_globals(self): # setting globals for those. if id(spec) not in self.nodes_in_subdag: continue - dependent_module = ModuleChangePropagator(spec.package) + dependent_module = SetPackageGlobals(spec.package) pkg.setup_dependent_package(dependent_module, spec) dependent_module.propagate_changes_to_mro() @@ -1543,7 +1543,7 @@ def write_log_summary(out, log_type, log, last=None): out.write(make_log_context(warnings)) -class ModuleChangePropagator: +class SetPackageGlobals: """Wrapper class to accept changes to a package.py Python module, and propagate them in the MRO of the package. @@ -1551,41 +1551,22 @@ class ModuleChangePropagator: "setup_dependent_package" function during build environment setup. """ - _PROTECTED_NAMES = ("package", "current_module", "modules_in_mro", "_set_attributes") - def __init__(self, package: spack.package_base.PackageBase) -> None: - 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 package.__class__.__mro__: module = getattr(cls, "module", None) - if module is None or module is spack.package_base: break - - if module is self.current_module: - continue - 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) + super().__setattr__("modules_in_mro", modules_in_mro) def __getattr__(self, item): - return getattr(self.current_module, item) + return getattr(self.modules_in_mro[0], 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) + if key == "modules_in_mro": + raise AttributeError(f'Cannot set attribute "{key}" in ModuleMonkeyPatcher') + for module in self.modules_in_mro: + setattr(module, key, value) diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index b9b9ee2f2e1..74743cff6b8 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/lib/spack/spack/build_systems/autotools.py @@ -14,6 +14,7 @@ import spack.build_environment import spack.builder import spack.package_base +from spack.build_environment import SetPackageGlobals from spack.directives import build_system, conflicts, depends_on from spack.multimethod import when from spack.operating_systems.mac_os import macos_version @@ -577,9 +578,7 @@ def set_configure_or_die(self): raise RuntimeError(msg.format(self.configure_directory)) # Monkey-patch the configure script in the corresponding module - globals_for_pkg = spack.build_environment.ModuleChangePropagator(self.pkg) - globals_for_pkg.configure = Executable(self.configure_abs_path) - globals_for_pkg.propagate_changes_to_mro() + SetPackageGlobals(self.pkg).configure = Executable(self.configure_abs_path) def configure_args(self): """Return the list of all the arguments that must be passed to configure, diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index 3ae41acc9ce..33d9655b520 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -575,28 +575,26 @@ def test_build_jobs_defaults(): ) -class TestModuleMonkeyPatcher: +class TestSetPackageGlobals: def test_getting_attributes(self, default_mock_concretization): s = default_mock_concretization("libelf") - module_wrapper = spack.build_environment.ModuleChangePropagator(s.package) + module_wrapper = spack.build_environment.SetPackageGlobals(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) + module_wrapper = spack.build_environment.SetPackageGlobals(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 s.package.__class__.__mro__: - current_module = cls.module - if current_module == spack.package_base: + if cls.module == spack.package_base: break - assert current_module.SOME_ATTRIBUTE == 1 + assert cls.module.SOME_ATTRIBUTE == 1 def test_effective_deptype_build_environment(default_mock_concretization):