On Fri, 21 Nov 2025 10:25:24 GMT, Ana Maria Mihalceanu <[email protected]> wrote:
>> This PR looks into aligning the behavior and documentation for `--compress`
>> option and plugin of `jlink`:
>>
>> - When an user provides `-c {0|1|2}` to `jlink`, then the tool should
>> process it as when receiving `--compress={0|1|2}`. As these values are now
>> deprecated, a warning should be issued to the end user.
>> - When an user provides `-c zip-[0-9]` to `jlink`, then the tool should
>> process it as when receiving `--compress=zip-[0-9]`.
>> - When no compression level is given, meaning the `jlink` command does not
>> contain either `-c` or `--compress` with a value, the default level selected
>> is `zip-6`.
>> - The `--compress` option description reflects above behavior and warns that
>> previous compression levels are deprecated to be removed in a future release.
>> - The `--plugin` option description reflects the implementation behavior and
>> warns that previous compression levels are deprecated to be removed in a
>> future release.
>>
>> Some implementation details and choices:
>> - While current `jlink` man page states that the tool supports `-c={0|1|2}`,
>> I inspired myself on how `javac` supports the shortened options
>> https://docs.oracle.com/en/java/javase/25/docs/specs/man/javac.html#options.
>> - While `-c 0` and `--compress=0` produce the same compression level as of
>> `zip-0`, I preferred not to tie the new compression level to the old value
>> for the option. I believe that this approach would make it easier/cleaner to
>> remove the code for the deprecated values (when their time comes).
>> - While `-c 2` and `--compress=2` produce the same compression level as of
>> `zip-6`, I preferred not to tie the new compression level to the old value
>> for the option. I believe that this approach would make it easier/cleaner to
>> remove the code for the deprecated values (when their time comes).
>> - As I didn't affect the actual compression implementation, only the
>> options, I tested only how the options are mapped. The actual set and
>> validation of the options was not affected, hence I didn't change those
>> tests.
>
> Ana Maria Mihalceanu has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Add some negative tests for suggested values.
test/jdk/tools/jlink/TaskHelperTest.java line 279:
> 277: var remaining = optionsHelper.handleOptions(this,
> args.tokens);
> 278: assertTrue(remaining.isEmpty());
> 279: assertEquals(args.expectedCompressValue,
> compressArgValue);
I think these 2 asserts are an oversight. Since we expect
`optionsHelper.handleOptions(...)` to always throw a `BadArgs` for these
invalid option values, we should replace these 2 asserts with a:
fail("processing of " + Arrays.toString(testCase.tokens) + " was expected to
fail, but didn't");
This will then rightly fail the test if the exception isn't thrown.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28359#discussion_r2549308889
