On Fri, 16 Jan 2026 01:43:00 GMT, Alexander Matveev <[email protected]>
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
