From 5f598214336716d5dd54bbf30327ea3fb9cea0e5 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Tue, 4 Oct 2022 13:06:50 -0700 Subject: BuildEnvironment: accumulate module changes to poke to all relevant modules (#32340) Currently, module changes from `setup_dependent_package` are applied only to the module of the package class, but not to any parent classes' modules between the package class module and `spack.package_base`. In this PR, we create a custom class to accumulate module changes, and apply those changes to each class that requires it. This design allows us to code for a single module, while applying the changes to multiple modules as needed under the hood, without requiring the user to reason about package inheritance. --- lib/spack/spack/build_environment.py | 19 ++++++++++++++++++- lib/spack/spack/test/build_environment.py | 8 ++++++++ .../packages/cmake-client-inheritor/package.py | 11 +++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 var/spack/repos/builtin.mock/packages/cmake-client-inheritor/package.py diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 93ff1512a7..63b1309a22 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -64,6 +64,7 @@ import spack.store import spack.subprocess_context import spack.user_environment import spack.util.path +import spack.util.pattern from spack.error import NoHeadersError, NoLibrariesError from spack.installer import InstallError from spack.util.cpus import cpus_available @@ -993,8 +994,24 @@ def modifications_from_dependencies( dpkg = dep.package if set_package_py_globals: set_module_variables_for_package(dpkg) + # Allow dependencies to modify the module - dpkg.setup_dependent_package(spec.package.module, spec) + # 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__) + if context == "build": dpkg.setup_dependent_build_environment(env, spec) else: diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index 41a5475c89..6ba8fc056f 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -177,6 +177,14 @@ def test_cc_not_changed_by_modules(monkeypatch, working_env): assert os.environ["ANOTHER_VAR"] == "THIS_IS_SET" +def test_setup_dependent_package_inherited_modules( + config, working_env, mock_packages, install_mockery, mock_fetch +): + # This will raise on regression + s = spack.spec.Spec("cmake-client-inheritor").concretized() + s.package.do_install() + + @pytest.mark.parametrize( "initial,modifications,expected", [ diff --git a/var/spack/repos/builtin.mock/packages/cmake-client-inheritor/package.py b/var/spack/repos/builtin.mock/packages/cmake-client-inheritor/package.py new file mode 100644 index 0000000000..bc058cc4fa --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/cmake-client-inheritor/package.py @@ -0,0 +1,11 @@ +# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack.package import * # noqa: F401 +from spack.pkg.builtin.mock.cmake_client import CmakeClient + + +class CmakeClientInheritor(CmakeClient): + """A dumy package that inherits from one using cmake.""" -- cgit v1.2.3-70-g09d2