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.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
