-
Notifications
You must be signed in to change notification settings - Fork 285
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
deprecate unknown configure args in ConfigureMake
easyblock
#3026
base: develop
Are you sure you want to change the base?
Conversation
This makes sense to include in EasyBuild 4.x, but we should still try and do a sweep across all easyconfigs using That's a huge list of easyconfigs, so it's a pretty painful exercise, but a necessary one imho, unless we're willing to accept exposing people to these warnings (which may help in getting the problems fixed, I guess). |
I'd opt for the latter. I actually run the EasyBlock which fails hard instead on our cluster such that those things actually make me fix it. A scary warning to convince more people to cleanup the ECs sounds like a good compromise to get this big task done. Doing a bulk fix is going to be hard as even though you can use |
I did a quick scan for " Most of these were done as a part of the regression test for EasyBuild v4.8.2, so recent installations, but some are older (marked with
Although not a blocker for merging this PR, it would be good to have these easyconfigs updated so the warning won't present itself when installing these... |
I checked my recent tree and also found the Tcl issue. However it looks like it is some configure called by configure passing those flags as we only do |
I did a scan for "
|
That's quite a few and we may want to fix them (for the non-ancient and at least easy ones) for EB 5. But: Do we still need to consider/handle EB 4.x PRs after 4.9.0 was released? |
@boegel I compiled as list of EC filenames from your 2 comments and mentioned in #3025 and am running tests building those using develop. The resulting PRs should be all findable by https://github.com/easybuilders/easybuild-easyconfigs/pulls?q=is%3Apr+author%3AFlamefire++configure |
Test report by @Flamefire Overview of tested easyconfigs (in order)
Build succeeded for 75 out of 75 (75 easyconfigs in total) |
@boegel The above test report was done with a temporary change to issue an error instead of a warning. As you can see all ECs you identified are now fixed. So I'd say this is ready now. |
4294ae0
to
70a582e
Compare
More compatible version of #3025 for 4.x to only deprecate and verbosely warn about unknown configure args