RFR: 8333727: Use JOpt in jpackage to parse command line [v6]

[Alexey Semenyuk]

On Thu, 13 Nov 2025 03:17:22 GMT, Alexander Matveev <almatvee at openjdk.org> wrote:

>> Alexey Semenyuk has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   MainResources.properties: remove unreferenced L10N key
>
> src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/HelpFormatter.java line 46:
> 
>> 44:         this.optionGroups = Objects.requireNonNull(optionGroups);
>> 45:         this.formatter = Objects.requireNonNull(formatter);
>> 46: 
> 
> Extra new line.

Fixed

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/JOptSimpleOptionsBuilder.java line 184:
> 
>> 182:             try {
>> 183:                 initializer.run();
>> 184:                 throw new AssertionError();
> 
> Can you put a comment on why we need `throw new AssertionError()`? I assume that `initializer.run()` never exits.

Added a comment

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/OptionSpec.java line 126:
> 
>> 124:      * <p>
>> 125:      * If the option has three names "a", "b", and "c", the stream will have three
>> 126:      * option spec objects each with a single name. The firt will have name "a", the
> 
> `firt` -> `first`

Fixed

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/StandardAppImageFileOption.java line 227:
> 
>> 225:         }).map(option -> {
>> 226:             var spec = context.mapOptionSpec(option.spec());
>> 227:             var strValue = Optional.ofNullable(properties.get(spec.name().name()));
> 
> `name().name()` why we need name of the name and what it means?

`OptionSpec.name()` method returns a reference to an object of type `OptionName` that also has a `name()` method. Option name can be "option", or "--option", or "-o" string. So I decided to introduce the `OptionName` class to encapsulate these variants.

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/StandardOption.java line 492:
> 
>> 490:                 context.asFileSource().ifPresent(propertyFile -> {
>> 491:                     b.converterExceptionFactory(forMessageWithOptionValueAndName(propertyFile));
>> 492:                     b.converterExceptionFormatString("error.properties-paramater-not-path");
> 
> `paramater` -> `parameter`. Below as well.

Fixed

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/StandardOption.java line 544:
> 
>> 542:             }))
>> 543:             .converterExceptionFactory(ERROR_WITH_VALUE_AND_OPTION_NAME)
>> 544: //            .converterExceptionFormatString("error.paramater-not-launcher-shortcut-dir")
> 
> Remove if not needed.

Fixed

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/cli/StandardOption.java line 644:
> 
>> 642:                     final var theCause = cause.orElseThrow();
>> 643:                     if (theCause instanceof AddLauncherSyntaxException) {
>> 644: //                        return ERROR_WITH_VALUE_AND_OPTION_NAME.create(optionName,
> 
> Remove if not needed.

Fixed

> test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/cli/StandardOptionTest.java line 403:
> 
>> 401:             }
>> 402: 
>> 403: //            Builder expectMalformedError(String v) {
> 
> Remove if not needed.

Fixed

> test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/cli/StandardOptionTest.java line 544:
> 
>> 542:                 buildLauncherShortcutTest().optionValue("false"),
>> 543:                 buildLauncherShortcutTest().optionValue(""),
>> 544: //                buildLauncherShortcutTest().optionValue("").propertyFile(true),
> 
> Remove and line 547.

Fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528828988
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528820042
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528837414
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528844728
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528827278
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528856437
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528856223
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528827729
PR Review Comment: https://git.openjdk.org/jdk/pull/28163#discussion_r2528828212