RFR: JDK-8239450 Overhaul JVM feature handling in configure

[Magnus Ihse Bursie]

On 2020-02-19 16:00, Erik Joelsson wrote:
> Hello Magnus,
>
> This is certainly a nice improvement. 
Thanks! It's been long overdue...

> It looks good to me. I have some comments on implementation details, 
> but nothing serious enough to require a new webrev.
>
> Instead of using "echo $foo | sed 's/,/ /g'", since we know we run in 
> bash, could just use ${foo//,/ }. (jvm-features.m4:82)
Good point. I just copied this from the old code.
>
> The $AWK expressions are just copied from the previous implementation, 
> so they are probably working ok. I would still recommend using $NAWK 
> to increase likelihood of them really working the same across platforms.
Hm, well, here's the thing. (I had to really check this up.) We set up 
the following definitions:
BASIC_REQUIRE_PROGS(NAWK, [nawk gawk awk])
BASIC_REQUIRE_SPECIAL(AWK, [AC_PROG_AWK])
and AC_PROG_AWK, according to the documentation, "[c]heck for gawk, 
mawk, nawk, and awk, in that order".

So, if you have nawk and awk (but no other) installed, both NAWK and AWK 
will be set to nawk. If you have only awk, both will be set to awk. The 
difference is if you have gawk installed, then NAWK will be nawk and AWK 
will be gawk. On my mac, I only have awk, so both AWK and NAWK will be 
awk. Which works fine with these expressions.

On my ubuntu box, things are even more confused. I have:
$ ls -l /usr/bin/*awk
lrwxrwxrwx 1 root root     21 Feb  6 10:36 awk -> /etc/alternatives/awk*
-rwxr-xr-x 1 root root 658072 Feb 11  2018 gawk*
-rwxr-xr-x 1 root root   3189 Feb 11  2018 igawk*
-rwxr-xr-x 1 root root 125416 Apr  3  2018 mawk*
lrwxrwxrwx 1 root root     22 Feb  6 10:37 nawk -> /etc/alternatives/nawk*

$ ls -l /etc/alternatives/*awk
lrwxrwxrwx 1 root root 13 Feb 10 10:56 /etc/alternatives/awk -> 
/usr/bin/gawk*
lrwxrwxrwx 1 root root 13 Feb 10 10:56 /etc/alternatives/nawk -> 
/usr/bin/gawk*

So awk, nawk and gawk all executes the same binary, i.e. gawk. Only mawk 
is different. So on that machine, AWK would be gawk and NAWK would be 
nawk, but both will execute gawk.

I propose that we remove NAWK, and only use AWK, but we should stop 
using AC_PROG_AWK and define it in an order that is transparent to us. I 
recommend [gawk nawk awk], since on Linux systems nawk (as we've seen) 
is likely to be gawk under disguise anyway, so it's better to be clear 
about that.

Also, if we truly believe the flavor of awk we're using is having any 
significance, we should test what version we've actually found, and then 
either require a specific flavor, and/or test our awk scripts on 
different awks. But that is better left for another time. For now, I'll 
keep the AWK. :-)

> jvm-features.m4:370,372-373: wrong indentation
Good catch!

/Magnus
>
> /Erik
>
> On 2020-02-19 04:05, Magnus Ihse Bursie wrote:
>> The JVM feature handling in configure needs a complete overhaul. The 
>> current logic is hard to understand and to make changes to. The 
>> method of enabling features with --with-jvm-features="feature list" 
>> means that it is not possible to incrementally add features without 
>> throwing away settings done earlier on the command line.
>>
>> With this patch, the most noticeable effect for normal users is the 
>> addition of a group of configure arguments, on the pattern 
>> --enable-jvm-feature-<FEATURE>. So, instead of doing e.g. 
>> --with-jvm-features="zgc,-dtrace", you can now do 
>> --enable-jvm-feature-zgc --disable-jvm-feature-dtrace. The major 
>> benefit from this is that it is possible to build up a command line 
>> in steps, where a later step enables or disables a feature, without 
>> throwing away the settings made earlier (which was what happened if 
>> two --with-jvm-features= options were given).
>>
>> Arguably, this is the way that JVM features should have been 
>> implemented all along. There were ever only two reasons for the 
>> --with-jvm-features argument list, neither of them particularly good: 
>> It allows for simple selection of multiple features (e.g. for the 
>> custom variant), and it avoided the complexity in programmatically 
>> generating autoconf options in m4.
>>
>> I have now bit the bullet, and wrangled m4 into doing this. The old 
>> way of --with-jvm-features="<list>" is of course still supported, but 
>> I think for the most part, the new style is recommended. Some 
>> features, e.g. cds, had their own options (--enable-cds) which were 
>> weirdly translated into features. These options are now defined as 
>> aliases (of e.g. --enable-jvm-feature-cds), and I intend to keep them 
>> as such.
>>
>> Under the hood, much more has changed. There is no longer the 
>> schizophrenic split between handling stuff the "old" way or the "new" 
>> way using features. All features are handled the same, period. 
>> Furthermore, the logic has been cleared up considerably.
>>
>> First of all, I check if a feature is possible to build on your 
>> platform. For instance, CDS is not available on AIX, or dtrace 
>> requires proper tooling. If that is the case, it is considered 
>> unavailable, and it is an error to try to enable it.
>>
>> This check is also done on a per-variant basis. Some features are not 
>> available on all variants. For instance, the "zero" feature is only 
>> available on the "zero" variant.
>>
>> Secondly, not all available features should be enabled by default -- 
>> even if most should be. Therefore, I keep lists of "filtered" 
>> features (one for the platform and one per variant). The default for 
>> a variant is then calculated as all available features, minus all 
>> that are filtered out.
>>
>> If a user has explicitly enabled a feature, it is added to the list 
>> (as long as it is available). If a user has disabled a features, it 
>> is removed from the list.
>>
>> This separation makes it clear if a feature is not built by default 
>> because it is not supported, or because it is not recommended (like 
>> link-time-opt). This distinction was unclear, to say the least, in 
>> the old code.
>>
>> For platforms that are outside my expertise (like zero and aix), I've 
>> done my best to convert the old behavior to this model, but I'd 
>> appreciate if someone knowledgeable about the code can verify. Zero, 
>> for instance, has a long list of features classified as unavailable, 
>> but I'm not sure that this is correct.
>>
>> Hopefully, the new code base should make it much easier to understand 
>> what requirements a certain feature has, and why (if not) it is not 
>> built. Also, I hope that adding a new feature in the future would be 
>> far easier, by just following the model.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8239450
>> WebRev: 
>> http://cr.openjdk.java.net/~ihse/JDK-8239450-overhaul-JVM-features/webrev.01
>>
>> /Magnus