On Sun, 1 Feb 2026 23:39:13 GMT, Alexey Semenyuk <[email protected]> wrote:
> Provide finer control over the jpackage console output. > > Add an optional value to the `--verbose` option. The description of the > option has changed from: > > --verbose > Enables verbose output > > to > > --verbose [<[-]category(,[-]category)*>] > Configures verbose output. Where "category" is one of > "all" > "console" > "log" > "errors" > "progress" > "resources" > "summary" > "tools" > "trace" > "warnings" > > Suppress all console output, enable logging via System.Logger API: > --verbose log > Enable all message categories in the console: > --verbose console > Enable all message categories, but "trace" and "tools" in the > console: > --verbose console,-trace,-tools > Enable "trace" and "tools" message categories in the console: > --verbose trace,tools > Enable "trace" and "tools" message categories in the console and > enable logging via System.Logger API: > --verbose log,trace,tools > > If the option is specified without the value, it is equivalent to > --verbose console,-trace > If the option is not specified it is equivalent to > --verbose errors,warnings > > > Message categories explained in > [JDK-8377120](https://bugs.openjdk.org/browse/JDK-8377120) CSR. > > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Looks good overall with some comments. It is a big change and I will do second round of review. src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxDebPackager.java line 153: > 151: List<String> cmdline = new ArrayList<>(); > 152: Stream.of(sysEnv.fakeroot(), > sysEnv.dpkgdeb()).map(Path::toString).forEach(cmdline::add); > 153: cmdline.add("--verbose"); Why we adding it always vs only if verbose was enabled? src/jdk.jpackage/linux/classes/jdk/jpackage/internal/LinuxPackager.java line 166: > 164: try { > 165: errors = findErrorsInOutputPackage(); > 166: } catch (IOException ex) { Do we really want to ignore only `IOException`? It used to be just `Exception`. Can you document why `IOException` is not critical, but any other potential exceptions are critical. src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/AppImageSigner.java line 150: > 148: try (var content = Files.list(frameworkPath)) { > 149: content.forEach(toConsumer(path -> { > 150: codesigners.codesignMacBundle().accept(path); Why this change? macOS Frameworks are not application bundles and our `MacBundle.isValid()` will fail on Frameworks which means they will not be signed. src/jdk.jpackage/share/classes/jdk/jpackage/internal/IOUtils.java line 68: > 66: Files.deleteIfExists(path); > 67: } catch (IOException ex) { > 68: Log.trace(ex, "Faile to delete [%s]", path); `Faile to delete [%s]` -> `Failed to delete [%s]` src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/LogConfigParser.java line 53: > 51: > 52: Supplier<IllegalArgumentException> ex = () -> { > 53: return new IllegalArgumentException(String.format("Inavlid > value: [%s]", str)); `Inavlid value: [%s]` -> `Invalid value: [%s]` test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/cli/MainTest.java line 200: > 198: > I18N.format("error.parameter-not-empty-directory", invalidTempValue, > "--temp")), > 199: // Silent failure. > 200: build().args("--verbose", "", "--temp", > invalidTempValue).expectErrorExitCode(), Confused about these two test cases. I tried both manually via command line and I am getting error printed that `--temp` directory not empty. Why "Silent failure" does not cover it? test/jdk/tools/jpackage/share/AppContentTest.java line 101: > 99: @Test(ifOS = MACOS) > 100: @Parameter("NOT_DIRECTORY") > 101: @Parameter("NON_STANDARD_DIRECTOTY_NAME") `NON_STANDARD_DIRECTOTY_NAME` -> `NON_STANDARD_DIRECTORY_NAME ` ------------- Changes requested by almatvee (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/29515#pullrequestreview-4185069860 PR Review Comment: https://git.openjdk.org/jdk/pull/29515#discussion_r3151082418 PR Review Comment: https://git.openjdk.org/jdk/pull/29515#discussion_r3150994599 PR Review Comment: https://git.openjdk.org/jdk/pull/29515#discussion_r3150971198 PR Review Comment: https://git.openjdk.org/jdk/pull/29515#discussion_r3151132580 PR Review Comment: https://git.openjdk.org/jdk/pull/29515#discussion_r3151131265 PR Review Comment: https://git.openjdk.org/jdk/pull/29515#discussion_r3151049056 PR Review Comment: https://git.openjdk.org/jdk/pull/29515#discussion_r3151134463
