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.

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