RFR: 8357404: jpackage should attempt to get a package version from the JDK's release file if the --version option is not specified [v2]

Fri Jan 16 04:05:03 UTC 2026

On Fri, 16 Jan 2026 01:43:00 GMT, Alexander Matveev <almatvee at openjdk.org> wrote:

>> - Version will be read from JDK's release file if not provided via `--version` for runtime installer.
>> - Added test to cover added functionality.
>> - On macOS and Windows version from JDK's release file will be normalized if it does not fit platform requirements.
>
> Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8357404: jpackage should attempt to get a package version from the JDK's release file if the --version option is not specified [v3]

Changes requested by asemenyuk (Reviewer).

src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacFromOptions.java line 365:

> 363:             components = version.split("\\.", 4);
> 364:             return String.join(".", Arrays.copyOf(components, components.length - 1));
> 365:         }

Use jdk.jpackage.internal.model.DottedVersion class to break the version string into components.

This will stop at the first invalid character:

DottedVersion ver = DottedVersion.lazy(str);

This will throw `IllegalArgumentException` if it hits the first non-parsable character:

DottedVersion ver = DottedVersion.greedy(str);

src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/RuntimeVersionReader.java line 1:

> 1: /*

The class should include a single function that takes a path to the "release" file and returns a non-null String version extracted from it. It should throw an exception if it can not find a version in the file. A custom or a standard one, e.g. `java.util.NoSuchElementException`. 

Alternatively, it may return an `Optional<String>` and not throw exceptions if it fails to extract a version string from the "release" file.

It should not swallow I/O errors. If it can't handle them on the spot, let them out.

It should not detect where the "release" file is located in the runtime image. 

It should not have platform-specific branching, because there is nothing platform-specific in extracting the version from the "release" file.

"jdk.jpackage.internal.Log" class should not be referenced from other jpackage packages. The only exception is configuring the logger in the "cli" package (this one should eventually be cleaned up).

Adding a reference to the "Log" class here will leak it into the test code, where it will be used outside the jpackage tool provider context. This is undefined behavior. Which, I guess, could have been caught early with a unit test :)

The class should be covered with unit tests:
- Read a version from the valid "release" file
- Read a version from an invalid "release" file
- Read a version from any "release" file, but fail because of I/O error. Easy to simulate this condition by passing a directory into the `readVersion()` method.

src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/RuntimeVersionReader.java line 2:

> 1: /*
> 2:  * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.

Copyright year should be 2026

src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WinFromOptions.java line 157:

> 155:             components = version.split("\\.", 5);
> 156:             return String.join(".", Arrays.copyOf(components, components.length - 1));
> 157:         }

Use the DottedVersion class to break the version string into components.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java line 270:

> 268:         }
> 269: 
> 270:         return getArgumentValue("--app-version", () -> "1.0");

The idea of the Optional class is to reduce branching and null checks in the code.
The entire body of the method can be simplified down to:

    public String version() {
        return Optional.ofNullable(getArgumentValue("--app-version")).or(() -> {
            if (isRuntime()) {
                return RuntimeVersionReader.readVersion(Path.of(getArgumentValue("--runtime-image"))).map(releaseVersion -> {
                    if (TKit.isWindows()) {
                        return WindowsHelper.getNormalizedVersion(releaseVersion);
                    } else if (TKit.isOSX()) {
                        return MacHelper.getNormalizedVersion(releaseVersion);
                    } else {
                        return releaseVersion;
                    }
                });
            } else {
                return Optional.empty();
            }
        }).orElse("1.0");
    }

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java line 362:

> 360:                     }
> 361:             });
> 362: 

The fake runtime's version configuration doesn't belong here. It is specific to a single test and should be part of the test.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java line 765:

> 763: 
> 764:     static String getNormalizedVersion(JPackageCommand cmd, String version) {
> 765:         cmd.verifyIsOfType(PackageType.MAC);

The JPackageCommand instance is unrelated to the version string and is used only to check that it is configured for Mac. It doesn't supply any information required by the method to operate. The method should take a single parameter - a version string.

Why does the MacHelper class have this method, though jpackage doesn't do version normalization on macOS?

test/jdk/tools/jpackage/share/RuntimePackageTest.java line 119:

> 117:     @Parameter("27.1.2.3")
> 118:     @Parameter("27.1.2.3.4")
> 119:     public static void testReleaseFileVersion(String version) {

The test is missing coverage for the new "message.release-version" and "message.version-normalized" messages

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

PR Review: https://git.openjdk.org/jdk/pull/29260#pullrequestreview-3668524711
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696675804
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696760330
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696676762
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696688751
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696726813
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696684510
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696708342
PR Review Comment: https://git.openjdk.org/jdk/pull/29260#discussion_r2696729972