RFR: 8368030: Make package bundlers stateless

Alexander Matveev almatvee at openjdk.org
Thu Sep 25 02:17:22 UTC 2025


On Thu, 18 Sep 2025 22:25:02 GMT, Alexey Semenyuk <asemenyuk at openjdk.org> wrote:

> Introduce `jdk.jpackage.internal.SystemEnvironment` interface to describe system tools needed for building specific package bundles with three immediate subinterfaces: `WinSystemEnvironment`, `LinuxSystemEnvironment`, and `MacDmgSystemEnvironment`.
> 
> `LinuxSystemEnvironment` has two subinterfaces: `LinuxDebSystemEnvironment` and `LinuxRpmSystemEnvironment`.
> 
> There is no `MacSystemEnvironment` interface as pkg and dmg bundlers are unrelated, unlike rpm and deb bundlers, which share a fair amount of code.
> 
> There is no `MacPkgSystemEnvironment` interface because the pkg bundler doesn't validate tools.
> 
> Instances of these interfaces are created as member fields of the corresponding bundler classes, i.e., bundling system tools are validated only once when a specific bundler is instantiated.
> 
> Move the bundling code away from *Bundler classes into *Packager classes and completely isolate it from the "params". This is a follow-up for the effort to isolate the "params" in [JDK-8333664](https://bugs.openjdk.org/browse/JDK-8333664).

Looks good with minor comments.

src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxDebPackager.java line 82:

> 80:             } catch (IOException ex) {
> 81:                 // Try the default path if differ
> 82:                 if (!realPath.toString().equals(file.toString())) {

Why converting to `String`? `realPath` and `file` are both `Path`. `Path` has `equals` method.

src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxRpmSystemEnvironmentMixin.java line 45:

> 43: 
> 44:         final var errors = Stream.of(
> 45:                 Internal.createRpmbuildToolValidator(),

`createRpmbuildToolValidator` -> `createRpmBuildToolValidator`

src/jdk.jpackage/share/classes/jdk/jpackage/internal/ToolValidator.java line 136:

> 134:                 if (versionParser != null && minimalVersion != null) {
> 135:                     version[0] = versionParser.apply(lines);
> 136:                     if (version[0] != null && minimalVersion.compareTo(version[0]) <= 0) {

Do you know why `<` was changed to `<=`?

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

PR Review: https://git.openjdk.org/jdk/pull/27377#pullrequestreview-3264991311
PR Review Comment: https://git.openjdk.org/jdk/pull/27377#discussion_r2377274639
PR Review Comment: https://git.openjdk.org/jdk/pull/27377#discussion_r2377332414
PR Review Comment: https://git.openjdk.org/jdk/pull/27377#discussion_r2377455791


More information about the core-libs-dev mailing list