Hi Vladimir, 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: - 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
Thanks, -- Igor > On Jan 18, 2019, at 3:33 PM, Vladimir Ivanov <vladimir.x.iva...@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
