RFR: 8374219: Fix issues in jpackage's Executor class [v2]

[Alexander Matveev]

On Tue, 6 Jan 2026 22:32:33 GMT, Alexey Semenyuk <asemenyuk at openjdk.org> wrote:

>> - Move code shared between jpackage's Executor and jpackage's test lib Executor into `jdk.jpackage.internal.util.CommandOutputControl` class using the latter one as the baseline for the new class; [CommandOutputControl class javadoc](https://alexeysemenyukoracle.github.io/jpackage-javadoc/jdk.jpackage/jdk/jpackage/internal/util/CommandOutputControl.html).
>> - Place "execute with retries" logic into `jdk.jpackage.internal.util.RetryExecutor` class and use it from both jpackage and jpackage's test lib. Use `jdk.jpackage.internal.RetryExecutor` class as the baseline.
>> - Add `ObjectFactory`, `ExecutorFactory`, and `RetryExecutorFactory` interfaces to the "jdk.jpackage.internal" package. They enable the use of command mocks with jpackage.
>> - Add `jdk.jpackage.test.mock` package. It facilitates creating command mocks. Use it to test LibProvidersLookup, LinuxSystemEnvironment, LinuxPackageArch, MacDmgSystemEnvironment, and MacDmgPackager classes.
>> 
>> Supplementary changes:
>>  - Make `jdk.jpackage.internal.SystemEnvironment` and all implementing classes package private
>
> Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision:
> 
>   test/Executor: minor fix

Looks good with some comments.

src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacDmgPackager.java line 274:

> 272:                 .setMaxAttemptsCount(10)
> 273:                 .setAttemptTimeoutMillis(3000)
> 274:                 .setWriteOutputToFile(true)

Do we always write output to file with new implementation? I see that you call `storeOutputInFiles()` for `hdiutil`.

src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/CommandOutputControl.java line 7:

> 5:  * This code is free software; you can redistribute it and/or modify it
> 6:  * under the terms of the GNU General Public License version 2 only, as
> 7:  * published by the Free Software Foundation.

Missing "Classpath" exception.

test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CommandOutputControlTest.java line 129:

> 127:         var desc = spec.create().description();
> 128:         assertFalse(desc.isBlank());
> 129: //        System.out.println(desc);

Remove if not needed.

test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CommandOutputControlTest.java line 698:

> 696:     private static List<OutputTestSpec> testSomeSavedOutput() {
> 697:         var testIds = List.<Integer>of(/* 10, 67, 456 */);
> 698:         if (testIds.isEmpty()) {

Can you explain what this method do? `testIds` is always empty, so not sure what it tries to do.

test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CommandOutputControlTest.java line 1003:

> 1001:                             }
> 1002:                             case CatCommandAction _ -> {
> 1003:                                 // Not used, no pint to implement.

`pint` -> `point`

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

PR Review: https://git.openjdk.org/jdk/pull/28733#pullrequestreview-3629203357
PR Review Comment: https://git.openjdk.org/jdk/pull/28733#discussion_r2663456818
PR Review Comment: https://git.openjdk.org/jdk/pull/28733#discussion_r2666734358
PR Review Comment: https://git.openjdk.org/jdk/pull/28733#discussion_r2666938204
PR Review Comment: https://git.openjdk.org/jdk/pull/28733#discussion_r2666955055
PR Review Comment: https://git.openjdk.org/jdk/pull/28733#discussion_r2666960201