RFR: JDK-8240820 Replace AC_ARG_ENABLE with UTIL_ARG_ENABLE
Erik Joelsson
erik.joelsson at oracle.com
Tue Mar 10 17:46:46 UTC 2020
Hello Magnus,
This looks very good and will certainly make it easier to write good
--enable- flags. I would only find one mistake:
jdk-options.m4:533: double space
/Erik
On 2020-03-10 07:22, Magnus Ihse Bursie wrote:
> The function AC_ARG_ENABLE provided by autoconf is a very bare-bones
> function, which needs a lot of boilerplate code to do the right thing.
> We have duplicated this code all over the place, often with bugs or
> omissions of functionality.
>
> This patch introduces a new UTIL_ARG_ENABLE which sets up a
> best-practices framework for handling AC_ARG_ENABLE, and allows users
> to just call this and be sure we do the right thing. The code in
> UTIL_ARG_ENABLE is definitely non-trivial, but I've tried hard to make
> it as documented and clear as possible. In any case, I think the
> drawbacks from having a complex implementation like this is more than
> compensated for by the added simplicity for all the places it can be
> trivially used.
>
> I have also tried to keep the difference introduced in the JVM
> features patch between "available" and "enabled", so it's clear if a
> feature is not enabled because it is not possible to enable, or just
> because it is not the default.
>
> I have also made some usability fixes to UTIL_DEFUN_NAMED. The testing
> I added could certainly have been more comprehensive, but at least
> it's a start. The real good testing would be to be able to run
> configure with different arguments, but that needs another kind of
> testing framework than what we got so far. I also have expanded the
> set of assert statements available for testing.
>
> This cleanup highlights the major differences in how we handle this
> kind of flags, for instance if we set the flag value to "true" or
> "yes" in spec.gmk. I have opted not to touch any of that in this
> patch, but I feel a follow-up cleaning might be warranted.
>
> This change should not have any impact on normal use cases, but the
> added checks that command line options are correct might catch
> incorrect syntax that was previously ignored or handled incorrectly.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8240820
> WebRev:
> http://cr.openjdk.java.net/~ihse/JDK-8240820-introduce-UTIL_ARG_ENABLE/webrev.01
>
> /Magnus
More information about the build-dev
mailing list