On Thu, 20 Nov 2025 10:51:06 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 two 
> additional commits since the last revision:
> 
>  - Remove extra line.
>  - Align behavior of compression levels to the input received and sync 
> help+documentation messages.

test/jdk/tools/jlink/TaskHelperTest.java line 226:

> 224:     }
> 225: 
> 226:     record CompressTestCase(String[] tokens, String 
> expectedCompressValue, boolean expectedMainFlag) {}

Hello Ana, what is the `expectedMainFlag` meant for? I see that every usage of 
it initializes it to `false`.

test/jdk/tools/jlink/TaskHelperTest.java line 246:

> 244:                 new CompressTestCase(new String[] {"--compress=zip-7"}, 
> "zip-7", false),
> 245:                 new CompressTestCase(new String[] {"--compress=zip-8"}, 
> "zip-8", false),
> 246:                 new CompressTestCase(new String[] {"--compress=zip-9"}, 
> "zip-9", false)

I think a few negative tests would also be useful. For example, `-c 3` 
`--compress=42`, `--compress=zip-`  should all fail.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28359#discussion_r2549124431
PR Review Comment: https://git.openjdk.org/jdk/pull/28359#discussion_r2549130094

Reply via email to