easybuild-framework
easybuild-framework copied to clipboard
Deprecate failure to resolve a template value
A failure in template resolving is currently hidden in the log although it is likely a bug in the code (either EasyBuild, EasyBlock or EasyConfig) This may lead to cryptic error messages later on or misbehave silently which is even worse. To not break builds completely deprecate continuing for now and promote to a hard error later. The message is now highly visible and existing cases can be fixed until the next release.
I did an extensive run during development of https://github.com/easybuilders/easybuild-framework/pull/3129 with this issue promoted to a hard failure and did not encounter a single instance. So I think there should be no issues with existing ECs (or very few)
@boegel To have enough time to check for existing issues until next release I recommend merging this soon.
Fleshed out some changes into #3287, there are still some logic issues that would need resolving:
- the
cfg['ext_list']['options']should not be resolved bycfgas this would be using the wrong values - More general:
fetch_extension_sourcesshould run after creating the instances to avoid duplicating the logic of updating the template dict values which has the potential of becoming out of sync - One failure is with the python multidep as for some reason the dependencies are flattened but resolved but resolval is partial as pyver isn't set yet. This mirrors in short/long_mod_name. Not sure if this is a problem but it may.
- final failure is due to test yep files differing from the ec files (yep files use some replacement in yep, while ec use
%(name)sreplacements). IMO this is a mistake and the files should be equivalent (before templating in EB)
@Flamefire conflict resolution and merge with develop please.
@akesandgren Done, so this is ready
This one's too complicated for me, someone else that feels confident around that code needs to do the review.
@Flamefire Can you do a sync with develop here too, so the correct set of configurations for the test suite runs is used in CI?
Rebased.
@Flamefire Can this be re-targeted to the 5.0.x branch?
@Flamefire Can this be re-targeted to the
5.0.xbranch?
Sure can do. Do we want the deprecation there or the failure? For me the deprecation would be good to have "yesterday" and failure in 5.x
@Flamefire Can this be re-targeted to the
5.0.xbranch?Sure can do. Do we want the deprecation there or the failure? For me the deprecation would be good to have "yesterday" and failure in 5.x
That works for me, but only if we can ensure that not everyone who uses EasyBuild 4.9.0 will be exposed to deprecation warnings because we didn't do the necessary work to try and avoid them.
For EasyBuild 5.0 we have an opportunity to make it a hard failure, so let's try doing that. Worst case we make it just a warning again later if it turns out that was a bit ambitious...
That works for me, but only if we can ensure that not everyone who uses EasyBuild 4.9.0 will be exposed to deprecation warnings because we didn't do the necessary work to try and avoid them.
I already did some work for that all over the place in this PR.
Yes there might be some fallout but the only ways to find those is to do extensive test reinstalls with this turned to an error or just let it be as-is: A deprecation that this will be an error in 5.0 (as you seem to agree) and users can ignore this or better: report this. So I don't see it as necessarily bad if it bothers someone ;-)
As mentioned it would have been good to have the deprecation in yesterday such that we could have had time to find common remaining issues.
For EasyBuild 5.0 we have an opportunity to make it a hard failure, so let's try doing that. Worst case we make it just a warning again later if it turns out that was a bit ambitious...
Might even need to deprecate it targetting 5.1 instead of 5.0 and/or allow this to pass via a switch. IIRC we have an option to allow e.g. deprecated toolchains which by default would error, don't we? We could use the same here for a version or 2. But the earlier we get this tested a lot the better.
I found the option: --silence-deprecation-warnings=resolve-templates will now silence the warning and even avoid the failure if this gets merged into 5.0 as-is. What do you say @boegel , is this enough now?
PR for 5.x (with hard error): https://github.com/easybuilders/easybuild-framework/pull/4516