diff options
| -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 | ||||
| -rw-r--r-- | var/spack/repos/builder.test/packages/builder-and-mixins/package.py | 32 |
4 files changed, 65 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 diff --git a/var/spack/repos/builder.test/packages/builder-and-mixins/package.py b/var/spack/repos/builder.test/packages/builder-and-mixins/package.py new file mode 100644 index 0000000000..2e12b0e806 --- /dev/null +++ b/var/spack/repos/builder.test/packages/builder-and-mixins/package.py @@ -0,0 +1,32 @@ +# Copyright 2013-2024 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) +import spack.builder +from spack.build_systems import generic +from spack.package import * + + +class BuilderAndMixins(Package): + """This package defines a mixin for its builder""" + + homepage = "http://www.example.com" + url = "http://www.example.com/a-1.0.tar.gz" + + version("2.0", md5="abcdef0123456789abcdef0123456789") + version("1.0", md5="0123456789abcdef0123456789abcdef") + + +class BuilderMixin(metaclass=spack.builder.PhaseCallbacksMeta): + @run_before("install") + def before_install(self): + pass + + @run_after("install") + def after_install(self): + pass + + +class GenericBuilder(BuilderMixin, generic.GenericBuilder): + def install(self, pkg, spec, prefix): + pass |