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

trip over unknown configure options #157

Closed
boegel opened this issue Mar 27, 2013 · 12 comments · Fixed by #3025
Closed

trip over unknown configure options #157

boegel opened this issue Mar 27, 2013 · 12 comments · Fixed by #3025
Labels
Milestone

Comments

@boegel
Copy link
Member

boegel commented Mar 27, 2013

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.

@pforai
Copy link
Contributor

pforai commented May 28, 2013

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?

@boegel
Copy link
Member Author

boegel commented May 29, 2013

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!

@fgeorgatos
Copy link
Collaborator

correction: Autoconf is the relevant one here and exists as easyconfig

@Flamefire
Copy link
Contributor

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:

  1. Fail on WARNING: unrecognized options:
  2. Warn on "error" occurence
  3. 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"]

@boegel
Copy link
Member Author

boegel commented Dec 10, 2019

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).

@boegel boegel modified the milestones: 3.x, 4.x Dec 10, 2019
@Flamefire
Copy link
Contributor

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?

@boegel
Copy link
Member Author

boegel commented Jan 14, 2020

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...

@Flamefire
Copy link
Contributor

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?

@Flamefire
Copy link
Contributor

Flamefire commented Oct 24, 2023

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)])

@boegel
Copy link
Member Author

boegel commented Oct 25, 2023

@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...

@Flamefire
Copy link
Contributor

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

@boegel
Copy link
Member Author

boegel commented Dec 18, 2024

fixed with #3025

@boegel boegel closed this as completed Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants