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

[Vladimir Ivanov]

Updated webrev:
   http://cr.openjdk.java.net/~vlivanov/8217404/webrev.01

Verified that it works as expected on Linux, Windows, MacOS, and Solaris.

Best regards,
Vladimir Ivanov

On 18/01/2019 16: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.
> 
> 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
>>