diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/builder.py | 25 | ||||
| -rw-r--r-- | lib/spack/spack/test/builder.py | 17 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/list.py | 5 |
3 files changed, 33 insertions, 14 deletions
diff --git a/lib/spack/spack/builder.py b/lib/spack/spack/builder.py index 4119062ab6..00734d5533 100644 --- a/lib/spack/spack/builder.py +++ b/lib/spack/spack/builder.py @@ -9,6 +9,8 @@ import functools import inspect from typing import List, Optional, Tuple +from llnl.util import lang + import spack.build_environment #: Builder classes, as registered by the "builder" decorator @@ -231,24 +233,27 @@ class PhaseCallbacksMeta(type): for temporary_stage in (_RUN_BEFORE, _RUN_AFTER): staged_callbacks = temporary_stage.callbacks - # We don't have callbacks in this class, move on - if not staged_callbacks: + # Here we have an adapter from an old-style package. This means there is no + # hierarchy of builders, and every callback that had to be combined between + # *Package and *Builder has been combined already by _PackageAdapterMeta + if name == "Adapter": continue - # If we are here we have callbacks. To get a complete list, get first what - # was attached to parent classes, then prepend what we have registered here. + # If we are here we have callbacks. To get a complete list, we accumulate all the + # callbacks from base classes, we deduplicate them, then prepend what we have + # registered here. # # The order should be: # 1. Callbacks are registered in order within the same class # 2. Callbacks defined in derived classes precede those defined in base # classes + callbacks_from_base = [] for base in bases: - callbacks_from_base = getattr(base, temporary_stage.attribute_name, None) - if callbacks_from_base: - break - else: - callbacks_from_base = [] - + current_callbacks = getattr(base, temporary_stage.attribute_name, None) + if not current_callbacks: + continue + callbacks_from_base.extend(current_callbacks) + callbacks_from_base = list(lang.dedupe(callbacks_from_base)) # Set the callbacks in this class and flush the temporary stage attr_dict[temporary_stage.attribute_name] = staged_callbacks[:] + callbacks_from_base del temporary_stage.callbacks[:] diff --git a/lib/spack/spack/test/builder.py b/lib/spack/spack/test/builder.py index 9a99e6ee08..4bd128c3bf 100644 --- a/lib/spack/spack/test/builder.py +++ b/lib/spack/spack/test/builder.py @@ -164,3 +164,20 @@ def test_install_time_test_callback(tmpdir, config, mock_packages, mock_stage): with open(s.package.tester.test_log_file, "r") as f: results = f.read().replace("\n", " ") assert "PyTestCallback test" in results + + +@pytest.mark.regression("43097") +@pytest.mark.usefixtures("builder_test_repository", "config") +def test_mixins_with_builders(working_env): + """Tests that run_after and run_before callbacks are accumulated correctly, + when mixins are used with builders. + """ + s = spack.spec.Spec("builder-and-mixins").concretized() + builder = spack.builder.create(s.package) + + # Check that callbacks added by the mixin are in the list + assert any(fn.__name__ == "before_install" for _, fn in builder.run_before_callbacks) + assert any(fn.__name__ == "after_install" for _, fn in builder.run_after_callbacks) + + # Check that callback from the GenericBuilder are in the list too + assert any(fn.__name__ == "sanity_check_prefix" for _, fn in builder.run_after_callbacks) diff --git a/lib/spack/spack/test/cmd/list.py b/lib/spack/spack/test/cmd/list.py index a46e690cd2..4a92504673 100644 --- a/lib/spack/spack/test/cmd/list.py +++ b/lib/spack/spack/test/cmd/list.py @@ -144,12 +144,9 @@ def test_list_repos(): os.path.join(spack.paths.repos_path, "builder.test"), ): total_pkgs = len(list().strip().split()) - mock_pkgs = len(list("-r", "builtin.mock").strip().split()) builder_pkgs = len(list("-r", "builder.test").strip().split()) + both_repos = len(list("-r", "builtin.mock", "-r", "builder.test").strip().split()) - assert builder_pkgs == 8 assert total_pkgs > mock_pkgs > builder_pkgs - - both_repos = len(list("-r", "builtin.mock", "-r", "builder.test").strip().split()) assert both_repos == total_pkgs |