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