easybuild-easyblocks
easybuild-easyblocks copied to clipboard
easybuilders
trip over unknown configure options
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:
configure: WARNING: unrecognized options: --enable-mpi-threads
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.
What about creating configure from configure.ac, run the autoconf program with --warnings=error to make the resulting configure script fail on warnings and threat them as errors?
That's a very good suggestion, but my experiences with recreating configure from configure.ac are not good. I've seen all kinds of problems pop up, and I'm not sure configure.ac is always provided (although it should be).
I'll give this a try first when I get to this, thanks!
This came up again. There already is parse_cmd_outputparse_cmd_output in run.py which can be used for that. However it only warns (by default) and is sometimes not fully useful.
Some use cases:
- Fail on
WARNING: unrecognized options: - Warn on "error" occurence
- Maybe fail on "failed" (e.g. for Python there are test failures due to the sitecustomize.py used), so sort out "expected failures"
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:
regexes = ["WARNING: unrecognized options", ("test_site failed", IGNORE), ("checking.*failed", WARN), "failed"]
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 configure options as failures, since that means breaking installations which have actually worked fine (although perhaps not entirely as was originally intended). For that particular one a very verbose warning would make more sense I think...
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 error: ... compiler error + N lines above (for context), or Error 1 + a couple preceding lines (but ignoring Error 2 & such which typically pop up with parallel make runs).
Of course we'll need to be careful with suddenly treated the use of unrecognized configure options as failures, since that means breaking installations which have actually worked fine (although perhaps not entirely as was originally intended). For that particular one a very verbose warning would make more sense I think...
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.
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.
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?
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 make command is failing, so anything we can do to make that easier (by fleshing out the first actual error and showing that in the output of the eb command) would help.
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 check_log_for_errors(out, [('# (ERROR|FAIL): +[1-9]', ERROR)])
@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