use SYSTEM constant for dependency that uses system toolchain in dumped easyconfig#4069
Merged
Micket merged 2 commits intoeasybuilders:developfrom Aug 26, 2022
Merged
Conversation
Member
Author
|
Necessary change for easyconfigs test suite is implemented in easybuilders/easybuild-easyconfigs#16126, which should be merged ASAP after this PR gets merged... |
1 task
Micket
reviewed
Aug 25, 2022
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
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.
Currently, we usually using
Trueas 4th value in a dependency specification if the system toolchain should be used, for example:A more intuitive alternative, which has actually been silently supported already for a long time, is to use the
SYSTEMconstant instead (since EasyBuild v4.0.0, see #2877):This translates to the following under the covers:
Directly specifying the toolchain to use for a dependency through the classic
name/versiondict value has been supported for ages (at least since EasyBuild v2.7.0, but also before already); theSYSTEMconstant just makes this (significantly) less ugly.This practice currently isn't adopted in the central easyconfigs repository though, mainly because the easyconfigs test suite in which we check whether the values of easyconfig parameters for the original easyconfig and for the easyconfig file that's obtained by dumping the parsed easyconfig are equal.
This check fails when using the
SYSTEMconstant in a dependency specification as shown above, because the dumped easyconfig will use the more crypticTruevalue instead.With the changes in this PR, we will be able to use
SYSTEMrather thanTrueto indicate that the system toolchain should be used for a dependency for easyconfigs being contributed to the central easyconfigs repository...Note that this will need a change in the easyconfigs test suite analogous to the workaround that @ocaisa added to easybuilders/easybuild-easyconfigs#15900 though, since the check on the toolchain part of a dependency will fail if
Trueis still being used instead of theSYSTEMconstant (which we should keep supporting for now, we can consider deprecating that later, and start requiring that new PRs useSYSTEMrather thanTrueat some point).