-
Notifications
You must be signed in to change notification settings - Fork 283
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
trip over unknown configure options #157
Comments
What about creating configure from configure.ac, run the autoconf program with |
That's a very good suggestion, but my experiences with recreating I'll give this a try first when I get to this, thanks! |
correction: Autoconf is the relevant one here and exists as easyconfig |
This came up again. There already is Some use cases:
So my (backward compatible) suggestion would be to accept a list of regexes with an optional mode which can be "ignore", "warn" or "fail". Every line of output is matched against the regexes in-order and the first matching one wins and the corresponding action is executed. This list should be a property of the EB so descendants can change, overwrite or insert entries. Example:
|
Error reporting is something I would like to use making improvements on, so I'm very happy to see contributors working on this. I feeel your proposal makes a lot of sense @Flamefire, and could help a great deal is catching errors that otherwise go ignored. Of course we'll need to be careful with suddenly treated the use of unrecognized Perhaps the new method could also try and do a good job of trying to report the (first) error message that made a build fail. For example, the first |
I disagree here. I'd rather have a false-positive requiring me to change/correct the EC/EB than a false-negative where the installed module behaves differently than expected. IMO (even a "very verbose") warning will usually just be ignored or not even read. The regular output of eb is already quite a lot (~22 lines per regular install) so when installing a large dependency chain you just won't see that warning. Additionally it adds complexity into the code and redoes the mistake (IMO) with the strictness level (There you either get wrong failures or don't see errors) The log is also not good and has to be improved severly. Installing Python 3.7.4 w/o any deps triggers 688(!) warnings. Having warnings there will also be ignored.
I don't fully understand. The new method looks for warnings/error messages that did not trigger a non-zero return value. For those cases there already are reporting mechanisms in place. And while context is great, you usually (always?) have the full output in the log anyway and can simply search for the mentioned lines. Isn't that enough? |
Maybe people find it challenging to pinpoint the exact reason why a That's easier said than done in general though... |
I guess the new method can be used by means of this: https://github.com/easybuilders/easybuild-framework/pull/3118/files#diff-ffbfe3a62fa741e88ecd3f774ebc0f63R656-R658 But bringing that into EB would be hard as one would need to touch the "fail and show output when non-zero exit was detected" to something specific to that command. EB already shows the first few chars from the output on non-zero exits, maybe just keep that? |
No solution after all that time? Can we maybe just modify the configuremake easyblock to error on "configure: WARNING: unrecognized options:" in the output? We have e.g. used in freefem already |
@Flamefire That's worth considering for EasyBuild 5.0, but that would also be quite strict, it may trigger the need for a lot of cleanup on the easyconfigs side... |
I added #3025 and would argue that this required cleanup is a good thing. Who knows what subtile errors exist in those easyconfigs and the fix is trivial if it is a simple left-over |
fixed with #3025 |
We need to make sure we trip when unknown configure options are being used.
The autoconf
configure
script will only issue a warning, but will happily continue, e.g. when using--enable-mpi-threads
with OpenMPI v1.6.4:This particular one was only figured out when OpenMPI was being used to build and test larger programs depending on its functionality, i.e. CP2K, so way too late in the build cycle.
The text was updated successfully, but these errors were encountered: