Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use source toolchain version when passing only --try-toolchain #4395

Merged
merged 2 commits into from
Dec 5, 2023

Conversation

Flamefire
Copy link
Contributor

process_software_build_specs always adds a toolchain key even when only the name of the toolchain is available. This makes the code in tweak ignore the code path in using toolchain_name and/or toolchain_version with the fallback to using the values of the source toolchain.
This in turn leads to failures further down when a value of None is used where an actual version is expected.

Fix by only adding the toolchain key when we have both the name and version and hence a complete, valid toolchain spec.

CC @boegel who added the code using a default of None for the version in #810 Maybe you remember the reasoning?

`process_software_build_specs` ialways adds a `toolchain` key
even when only the name of the toolchain is available.
This makes the code in `tweak` ignore the code path in using
`toolchain_name` and/or `toolchain_version` with the fallback to using
the values of the source toolchain.
This in turn leads to failures further down when a value of `None` is
used where an actual version is expected.

Fix by only adding the `toolchain` key when we have both the name and
version and hence a complete, valid toolchain spec.
Copy link
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ocaisa ocaisa added the bug fix label Dec 5, 2023
@ocaisa ocaisa added this to the next release (4.9.0?) milestone Dec 5, 2023
@ocaisa ocaisa merged commit badc27c into easybuilders:develop Dec 5, 2023
40 checks passed
@Flamefire Flamefire deleted the fix-try-toolchain branch December 5, 2023 14:14
@easybuilders easybuilders deleted a comment from boegelbot Dec 6, 2023
@boegel
Copy link
Member

boegel commented Dec 6, 2023

CC @boegel who added the code using a default of None for the version in #810 Maybe you remember the reasoning?

Not sure what the reasoning was here, doesn't make much sense indeed, your fix does make sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants