RFR: JDK-8240820 Replace AC_ARG_ENABLE with UTIL_ARG_ENABLE

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Wed Mar 11 07:35:23 UTC 2020



On 2020-03-10 18:46, Erik Joelsson wrote:
> 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

Eagle eye! :-D

I fixed it before pushing. Thanks for the review.

/Magnus
>
> /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