RFR: 8352184: Jtreg tests using CommandLineOptionTest.getVMTypeOption() and optionsvalidation.JVMOptionsUtils fail on static JDK [v3]

[Aleksey Shipilev]

On Tue, 25 Mar 2025 15:25:56 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

>> Please review following changes, thanks.
>> 
>> - Add `static` to the vm_info for static JDK. The `-version` output now contains `static` on static JDK, e.g.:
>> 
>> 
>> $ static-jdk/bin/java -version
>> openjdk version "25-internal" 2025-09-16
>> OpenJDK Runtime Environment (fastdebug build 25-internal-adhoc.jianglizhou.jdk)
>> OpenJDK 64-Bit Server VM (fastdebug build 25-internal-adhoc.jianglizhou.jdk, mixed mode, static, sharing)
>> 
>> $ jdk/bin/java -version
>> openjdk version "25-internal" 2025-09-16
>> OpenJDK Runtime Environment (fastdebug build 25-internal-adhoc.jianglizhou.jdk)
>> OpenJDK 64-Bit Server VM (fastdebug build 25-internal-adhoc.jianglizhou.jdk, mixed mode, sharing)
>> 
>> 
>> Following changes resolve jtreg test failures on static JDK due to '-server|-client|-minimal|-zero' flag added by `CommandLineOptionTest.getVMTypeOption()` or `optionsvalidation.JVMOptionsUtils`. '-server|-client|-minimal|-zero' flags are unrecognized on static JDK (please see https://bugs.openjdk.org/browse/JDK-8350982).
>> 
>> - Add `jdk.test.lib.Platform.isStatic()`, which checks for `static` in `java.vm.info` system property to determine if current test is running on static JDK. 
>> - Change `CommandLineOptionTest` to only call `getVMTypeOption()` on regular JDK (`!Platform.isStatic()`).
>> - Change `optionsvalidation.JVMOptionsUtils` to only set VMType to '-server|-client|-minimal' on regular JDK ( `!Platform.isStatic()`.
>
> Jiangli Zhou has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address @dholmes-ora's comment: Use `strlen` to get the length for ", static" and ", sharing".

Looks basically fine, just nits:

src/hotspot/share/runtime/abstract_vm_version.cpp line 167:

> 165:   const char* sharing_info = ", sharing";
> 166:   size_t len = strlen(mode) + (is_vm_statically_linked() ? strlen(static_info) : 0) +
> 167:                (CDSConfig::is_using_archive() ? strlen(sharing_info) : 0) + 1;

Make it easier to add new cases:

Suggestion:

  size_t len = strlen(mode) +
               (is_vm_statically_linked() ? strlen(static_info) : 0) +
               (CDSConfig::is_using_archive() ? strlen(sharing_info) : 0) + 
               1;

test/hotspot/jtreg/runtime/CommandLine/OptionsValidation/common/optionsvalidation/JVMOptionsUtils.java line 56:

> 54: 
> 55:     /* Used to start the JVM with the same type as current */
> 56:     static String VMType = null;

No need, `static`-s are implicitly initialized to `null`. Otherwise there is a style question why any other fields are not initialized! Maybe initialize it straight in `clinit`, like `GCType` is initialized.

test/hotspot/jtreg/runtime/CommandLine/OptionsValidation/common/optionsvalidation/JVMOptionsUtils.java line 71:

> 69:             } else if (Platform.isMinimal()) {
> 70:                 VMType = "-minimal";
> 71:             }

OK, so `isStatic` supercedes everything? Then just add a preceding block, like:


if (Platform.isStatic()) {
    // In static JDK, no launcher options are accepted.
    VMType = null;
} else if (Platform.isServer()) {
    VMType = "-server";
} else if (Platform.isClient()) {
    VMType = "-client";
} else if (Platform.isMinimal()) {
    VMType = "-minimal";
} else {
    VMType = null;
}


?

-------------

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24171#pullrequestreview-2718108323
PR Review Comment: https://git.openjdk.org/jdk/pull/24171#discussion_r2014724228
PR Review Comment: https://git.openjdk.org/jdk/pull/24171#discussion_r2014690413
PR Review Comment: https://git.openjdk.org/jdk/pull/24171#discussion_r2014719650