On Fri, 13 Sep 2024 12:29:26 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which proposes to remove the > (internal) `SelectVersion()` function from the java launcher and also update > the code comments in the launcher to match the current implementation? > > As noted in https://bugs.openjdk.org/browse/JDK-8340114, the > `SelectVersion()` function in the launcher used to be relevant when JRE > selection was a feature. That feature has been removed since Java 9 > https://bugs.openjdk.org/browse/JDK-8058407. The SelectVersion() in its > current form isn't relevant anymore and can be removed. > > While at it, it was noticed that the current "flowchart" code comments in the > launcher which attempts to explain the flow in the launcher code are > outdated. The commit in this PR updates those comments for macosx and unix > implementation. The windows variant doesn't have a "flowchart", but it too > deserves a high level comment explaining this flow. I haven't updated the > windows variant in this PR because that does a few additional things, which I > need to review and understand better. I plan to take that up in a future > change. > > An existing > `test/jdk/tools/launcher/MultipleJRERemoved.java/MultipleJRERemoved` test had > to be updated due to the changes in this PR. That test was launching `java` > (once) with 3 unsupported JRE selection options and was expecting 3 error > messages (one each for the unsupported option) for that single launch. With > the change in this PR, we don't accumulate and throw all those 3 errors and > instead we fail fast for any of these 3 unsupported JRE selection options. > The fail fast implementation matches what we do with other similar > unsupported options. The test had to be updated to not expect all 3 errors > message in a single launch and instead expect to find one of those error > messages. Given what this test is for, and the fact that JRE version > selection options (rightly) continue to raise an error after this change, I > think, an update to that test should be OK. > > No new tests have been introduced in this PR and tier testing is currently in > progress. This pull request has now been integrated. Changeset: 40cde003 Author: Jaikiran Pai <j...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/40cde003e8061a0eb6b0214d5a44325c3d55cdc6 Stats: 744 lines in 8 files changed: 89 ins; 471 del; 184 mod 8340114: Remove outdated SelectVersion() function from the launcher and update the code comments explaining the code flow Reviewed-by: dholmes, alanb ------------- PR: https://git.openjdk.org/jdk/pull/20997
