On Sun, 22 Sep 2024 15:00:04 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> treat -version: -jre-restrict-search and -jre-no-restrict-search like any
>> other unknown option
>
> src/java.base/share/native/libjli/java.c line 1401:
>
>> 1399: SetupSplashScreenEnvVars(const char *splash_file_path, // "-splash:"
>> option value (may be NULL)
>> 1400: char *jar_path // the jar file being launched
>> (may be NULL)
>> 1401: ) {
>
> I think this would be a bit cleaner if you put comment on the function to
> document the two args. That would remove the // comments that make this hard
> to read right now.
Done, I've now moved those comments as function documentation.
> src/java.base/share/native/libjli/java.h line 58:
>
>> 56: * java runtime (older, newer or same version JRE installed at a
>> different location) than
>> 57: * the one the launcher belongs to.
>> 58: * That support was discontinued starting Java 9. However, java launcher
>> in JDK 1.8 can still
>
> I assume this should say "JDK 9" rather than "Java 9".
>
> I read this paragraph a few times and one thing that I don't think is made
> clear is that it's the JDK 8 launcher that is exec'ing the JDK N launcher
> with the env variable set. Instead it speaks of launching the "higher version
> java runtimes". At some point we are going to have to fix an exit route. In
> the mean-time, anyone touching this code will read this and it may not be
> clear how the JDK N launcher gets exec'ed with this env variable set.
Hello Alan, I've updated the PR to clarify this part of the comment. Let me
know if the latest version reads better.
> At some point we are going to have to fix an exit route.
I suspect that exit route would have to come as a CSR in Java 8 to discontinue
support for launching Java runtimes greater than Java 23 (for example),
through the mJRE selection mechanism.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20997#discussion_r1770761203
PR Review Comment: https://git.openjdk.org/jdk/pull/20997#discussion_r1770760965
