Deduplicate code to propagate module changes across MRO (#34157)

2 files changed, 22 insertions, 50 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 2ec1683eb8..541d8cf0f9 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -563,14 +563,18 @@ def determine_number_of_jobs(
     return min(max_cpus, config_default)
 
 
-def _set_variables_for_single_module(pkg, module):
-    """Helper function to set module variables for single module."""
+def set_module_variables_for_package(pkg):
+    """Populate the Python module of a package with some useful global names.
+    This makes things easier for package writers.
+    """
     # Put a marker on this module so that it won't execute the body of this
     # function again, since it is not needed
     marker = "_set_run_already_called"
-    if getattr(module, marker, False):
+    if getattr(pkg.module, marker, False):
         return
 
+    module = ModuleChangePropagator(pkg)
+
     jobs = determine_number_of_jobs(parallel=pkg.parallel)
 
     m = module
@@ -638,20 +642,7 @@ def _set_variables_for_single_module(pkg, module):
     # Put a marker on this module so that it won't execute the body of this
     # function again, since it is not needed
     setattr(m, marker, True)
-
-
-def set_module_variables_for_package(pkg):
-    """Populate the module scope of install() with some useful functions.
-    This makes things easier for package writers.
-    """
-    # If a user makes their own package repo, e.g.
-    # spack.pkg.mystuff.libelf.Libelf, and they inherit from an existing class
-    # like spack.pkg.original.libelf.Libelf, then set the module variables
-    # for both classes so the parent class can still use them if it gets
-    # called. parent_class_modules includes pkg.module.
-    modules = parent_class_modules(pkg.__class__)
-    for mod in modules:
-        _set_variables_for_single_module(pkg, mod)
+    module.propagate_changes_to_mro()
 
 
 def _static_to_shared_library(arch, compiler, static_lib, shared_lib=None, **kwargs):
@@ -761,25 +752,6 @@ def get_rpaths(pkg):
     return list(dedupe(filter_system_paths(rpaths)))
 
 
-def parent_class_modules(cls):
-    """
-    Get list of superclass modules that descend from spack.package_base.PackageBase
-
-    Includes cls.__module__
-    """
-    if not issubclass(cls, spack.package_base.PackageBase) or issubclass(
-        spack.package_base.PackageBase, cls
-    ):
-        return []
-    result = []
-    module = sys.modules.get(cls.__module__)
-    if module:
-        result = [module]
-    for c in cls.__bases__:
-        result.extend(parent_class_modules(c))
-    return result
-
-
 def load_external_modules(pkg):
     """Traverse a package's spec DAG and load any external modules.
 
diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py
index 48f948f277..5da0b8912b 100644
--- a/lib/spack/spack/test/build_environment.py
+++ b/lib/spack/spack/test/build_environment.py
@@ -436,20 +436,20 @@ dt-diamond-left:
     assert not (set(os.path.normpath(x) for x in link_dirs[:-2]) & external_lib_paths)
 
 
-def test_parallel_false_is_not_propagating(config, mock_packages):
-    class AttributeHolder(object):
-        pass
-
-    # Package A has parallel = False and depends on B which instead
-    # can be built in parallel
-    s = spack.spec.Spec("a foobar=bar")
-    s.concretize()
-
-    for spec in s.traverse():
-        expected_jobs = spack.config.get("config:build_jobs") if s.package.parallel else 1
-        m = AttributeHolder()
-        spack.build_environment._set_variables_for_single_module(s.package, m)
-        assert m.make_jobs == expected_jobs
+def test_parallel_false_is_not_propagating(default_mock_concretization):
+    """Test that parallel=False is not propagating to dependencies"""
+    # a foobar=bar (parallel = False)
+    # |
+    # b (parallel =True)
+    s = default_mock_concretization("a foobar=bar")
+
+    spack.build_environment.set_module_variables_for_package(s.package)
+    assert s["a"].package.module.make_jobs == 1
+
+    spack.build_environment.set_module_variables_for_package(s["b"].package)
+    assert s["b"].package.module.make_jobs == spack.build_environment.determine_number_of_jobs(
+        s["b"].package.parallel
+    )
 
 
 @pytest.mark.parametrize(