netcdf: fix bugs introduced with multiple build systems split#36825
Merged
haampie merged 2 commits intospack:developfrom Apr 14, 2023
Merged
netcdf: fix bugs introduced with multiple build systems split#36825haampie merged 2 commits intospack:developfrom
haampie merged 2 commits intospack:developfrom
Conversation
fixes spack#36689 Modifications: - [x] The "base" builder class should be last in the MRO - [x] `filter_compiler_wrappers` needs to be moved to builders - [x] Decorating a function from a mixin class require using the correct metaclass for the mixin
skosukhin
reviewed
Apr 13, 2023
|
|
||
|
|
||
| class BackupStep: | ||
| class BackupStep(metaclass=spack.builder.PhaseCallbacksMeta): |
Member
There was a problem hiding this comment.
I would prefer BackupStep and Setup classes to be merged into one. Possibly, under a generic name like BaseBuilder. But I can do that later.
Member
Author
There was a problem hiding this comment.
That's fine with me. The caveat here is that if a mixin has some @run_after or @run_before decorated method, then it must inherit from the correct metaclass.
Member
|
@adamjstewart I guess we should update |
Member
|
Works for me. Thanks! |
Member
|
@skosukhin can you open a PR for proj? Don't have time to test but can review. |
skosukhin
reviewed
Apr 14, 2023
haampie
approved these changes
Apr 14, 2023
RikkiButler20
pushed a commit
to RikkiButler20/spack
that referenced
this pull request
Apr 25, 2023
…36825) Fixes spack#36689 - The "base" builder class should be last in the MRO - `filter_compiler_wrappers` needs to be moved to builders - Decorating a function from a mixin class require using the correct metaclass for the mixin
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #36689
Modifications:
filter_compiler_wrappersneeds to be moved to builders