[13] RFR (S): 8217404: --with-jvm-features doesn't work when multiple features are explicitly disabled

[Magnus Ihse Bursie]

On 2019-01-19 01:39, Vladimir Ivanov wrote:
> Thanks, Igor.
>
>> overall your fix looks reasonable, but w/ it we can get 
>> unintentionally disabled features (b/c grep doesn't do full word 
>> match). although this problem wasn't really introduced by your fix, I 
>> think it's be better to fix it as a part of your patch. I see two 
>> possible solutions:
>
> I was aware of such drawback, but decided to leave it as is, since it 
> doesn't affect existing features.
>
>>   -  add "-w" to grep, but I am not sure if "-w" is supported by all 
>> grep implementations
>>   - use $XARGS instead of $ECHO when we get DISABLE_X. in this case 
>> you will need to revert your changes in 'if test ...' lines
>
> I'm in favor of using "-w" and I see different grep flags being used 
> already, but would like somebody from Build team confirm they are OK 
> with such solution.

I think an even better solution is to use the pattern of 
HOTSPOT_CHECK_JVM_FEATURE. This should solve all potential problems, and 
is moving the abstraction level up slightly.

I've been working for some time now on better structure for handling the 
JVM feature testing. While we are using the feature testing as I 
intended it, the underlying support for doing this in a good way has 
never been put into place. Unfortunately, this fix has been on low 
priority and been mostly idling on my disk, half done, for several 
months now. So we need to have an interim solution to this problem. But 
I'd like to see that the fix takes at least a step towards a better 
abstraction.

Vladimir, if you're okay with it I'd like to propose this as a patch to 
the problem instead:

http://cr.openjdk.java.net/~ihse/JDK-8217404-fix-multiple-disabled-jvm-features/webrev.01

/Magnus


>
> Best regards,
> Vladimir Ivanov
>
>>> On Jan 18, 2019, at 3:33 PM, Vladimir Ivanov 
>>> <vladimir.x.ivanov at oracle.com> wrote:
>>>
>>> http://cr.openjdk.java.net/~vlivanov/8217404/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8217404
>>>
>>> --with-jvm-features doesn't work properly when multiple features are 
>>> explicitly disabled:
>>>
>>> $ bash configure --with-jvm-features="-aot -jvmci -graal"
>>> ...
>>> checking if jvmci module jdk.internal.vm.ci should be built... yes
>>> checking if graal module jdk.internal.vm.compiler should be built... 
>>> yes
>>> checking if aot should be enabled... yes
>>> ...
>>>
>>> The problem in the following code:
>>>
>>>   DISABLE_AOT=`$ECHO $DISABLED_JVM_FEATURES | $GREP aot`
>>>   if test "x$DISABLE_AOT" = "xaot"; then
>>>     ENABLE_AOT="false"
>>>   fi
>>>
>>> Since DISABLED_JVM_FEATURES ("aot jvmci graal") contains the list of 
>>> explicitly disabled features, grep over it returns the whole list 
>>> when there's a match. The subsequent check fails because there's no 
>>> exact match, though DISABLE_AOT contains "aot" .
>>>
>>> Proposed fix is to check there's no match instead.
>>>
>>> After the fix it works as expected:
>>>
>>> $ bash configure --with-jvm-features="-aot -jvmci -graal"
>>> ...
>>> checking if jvmci module jdk.internal.vm.ci should be built... no, 
>>> forced
>>> checking if graal module jdk.internal.vm.compiler should be built... 
>>> no, forced
>>> checking if aot should be enabled... no, forced
>>> ...
>>>
>>> (The fix doesn't address the case when one feature has a name which 
>>> is a proper substring of another feature, but there are no such 
>>> cases at the moment.)
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>