RFR: 8340114: Remove outdated SelectVersion() function from the launcher and update the code comments explaining the code flow [v7]
Jaikiran Pai
jpai at openjdk.org
Mon Sep 23 04:33:51 UTC 2024
On Sun, 22 Sep 2024 15:00:04 GMT, Alan Bateman <alanb at 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
More information about the core-libs-dev
mailing list