On Thu, 18 Sep 2025 22:25:02 GMT, Alexey Semenyuk <[email protected]> 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
