On Mon, 2 Mar 2026 21:53:17 GMT, Alexey Semenyuk <[email protected]> wrote:
>> - Decouple jpackage implementation and test helpers from the current OS: >> replace OperatingSystem.current() calls with parameters and/or fields of >> type jdk.internal.util.OperatingSystem where feasible. >> - Support running jpackage without making it run actual packaging. jpackage >> will exit after it parses the command line, validates the options, and >> builds a model. > > Alexey Semenyuk has updated the pull request incrementally with one > additional commit since the last revision: > > Update test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AppImageFile.java > > Co-authored-by: Andrey Turbanov <[email protected]> src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacDmgSystemEnvironment.java line 46: > 44: List<? extends Exception> errors; > 45: > 46: if (OperatingSystem.isMacOS()) { Is there better design for this? Doing platform specific check in platform specific code does not look right to me. Maybe created new class `PlatformTools` in shared location and return tool path only for specific platform or Optional.empty() for other platforms. src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacDmgSystemEnvironment.java line 54: > 52: .toList(); > 53: } else { > 54: // The code runs on an OS other than macOS. Presume this is > mock testing. I might be wrong, but code under "jdk.jpackage/macos" is not included during build time on other platforms. Do we plan to add all platform specific code to other platforms during build time? Will we move all code under "share"? test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java line 54: > 52: private Path relativePathInJavaHome() { > 53: Path path = Path.of("bin", toolName()); > 54: if (OperatingSystem.isWindows()) { Should we use `TKit` for OS check in test code? Above you changed from `OperatingSystem` to `TKit` and here is opposite. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29908#discussion_r2875661981 PR Review Comment: https://git.openjdk.org/jdk/pull/29908#discussion_r2875655530 PR Review Comment: https://git.openjdk.org/jdk/pull/29908#discussion_r2875730968
