8321139: jlink's man page does not document the --compress option correctly #28359

ammbra · 2025-11-17T17:54:21Z

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.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8321139: jlink's man page does not document the --compress option correctly (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28359/head:pull/28359
$ git checkout pull/28359

Update a local copy of the PR:
$ git checkout pull/28359
$ git pull https://git.openjdk.org/jdk.git pull/28359/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28359

View PR using the GUI difftool:
$ git pr show -t 28359

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28359.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-11-17T17:56:07Z

👋 Welcome back ammbra! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-11-17T17:56:14Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-11-17T17:56:57Z

@ammbra The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-11-17T18:03:49Z

Webrevs

AlanBateman · 2025-11-17T19:52:46Z

src/jdk.jlink/share/man/jlink.md

-`-c ={0|1|2}` or `--compress={0|1|2}`
-:   Enable compression of resources:
+`-c=zip-[0-9]` or `--compress=zip-[0-9]`
+:   Enable compression of resources. The accepted values are:


All good for --compress. Can you double check that -c actually works?

Thank you so much for spotting this! -c does not work and jlink --help command does not list it either. Will remove it from this part.

I suspect something has got messed up in the CLI parsing. -c should be the short form of --compress. At this time, -c, with no params, is the equivalent of --compress 2 so we get a confusing warning. As we've found, providing parameters to the short form doesn't seem to work. So I think we need to dig into this more.

Also a comment on the "Deprecated values to be removed in a future release" section. It would be easy to read it that zip-0 and zip-6 are being deprecated. Instead of "Equivalent to zip-0", then maybe we should say "Use zip-0 instead".

So maybe I should investigate first what happened to -c argument and then improve the documentation?

That would be great, if you have the cycles.

I looked into the issue and provided a fix. Hence I modified the description of this PR and issue related to it. Please let me know if is ok to change the title of the bug and PR as well, to proper reflect the changes introduced. Thank you 🙏 .

…elp+documentation messages.

jaikiran · 2025-11-20T12:12:12Z

src/jdk.jlink/share/man/jlink.md


-`-c ={0|1|2}` or `--compress={0|1|2}`
-:   Enable compression of resources:
+`-c zip-[0-9]` or `--compress=zip-[0-9]`


Nit - the use of square brackets is usually meant to imply "optional". Here it is mandatory to specify a digit between 0 and 9 following the "zip-" text. Perhaps using angular bracket would be better?

-c zip-<0-9> or --compress=zip-<0-9>

Good point. Thank you.

Nit - the use of square brackets is usually meant to imply "optional". Here it is mandatory to specify a digit between 0 and 9 following the "zip-" text. Perhaps using angular bracket would be better?

Hmmm, square brackets in the shell are a regex indicating a wildcard. I think that [0-9] reads better in the shell/command line context.

No objections from me to let it stay in the current form then.

{ } is use in some man pages pick one from the items from inside the braces. Maybe zip-{0..9} would work here as the jlink man page already uses [ ] for optional items.

The message showed when running jlink --help option mentions zip-[0-9]. In my current approach, I kept that implementation aligned with what help would show when running the jlink --help command (same approach regarding jlink --list-plugins output).

jlink --help Possible options include: --add-modules <mod>[,<mod>...] Root modules to resolve in addition to the initial modules. <mod> can also be ALL-MODULE-PATH. --bind-services Link in service provider modules and their dependencies --compress <compress> Compression to use in compressing resources: Accepted values are: zip-[0-9], where zip-0 provides no compression, and zip-9 provides the best compression. Default is zip-6. Deprecated values to be removed in a future release: 0: No compression. Use zip-0 instead. 1: Constant String Sharing 2: ZIP. Use zip-6 instead.

I did not intervene over the output of jlink -- list-plugins.

jlink --list-plugins List of available plugins: --add-options <options> Prepend the specified <options> string, which may include whitespace before any other options when invoking the virtual machine in the resulting image. --compress <compress> Compression to use in compressing resources: Accepted values are: zip-[0-9], where zip-0 provides no compression, and zip-9 provides the best compression. Default is zip-6.

Given the situations mentioned above, what do you believe to be the best way forward? 🙏

jaikiran · 2025-11-21T09:38:03Z

test/jdk/tools/jlink/TaskHelperTest.java

        assertEquals(2, remaining.size());
    }
+
+    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.

Hi Jaikiran,

Thank you for spotting that! I guided myself from the previous tests done there and forgot this parameter. I actually do not need it and will remove it.

jaikiran · 2025-11-21T09:40:06Z

test/jdk/tools/jlink/TaskHelperTest.java

+                new CompressTestCase(new String[] {"--compress=zip-6"}, "zip-6", false),
+                new CompressTestCase(new String[] {"--compress=zip-7"}, "zip-7", false),
+                new CompressTestCase(new String[] {"--compress=zip-8"}, "zip-8", false),
+                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.

Thank you for pointing out that part. I pushed a new commit to cover suggested values and more (I followed the previous example on testing for invalid values 😁).

jaikiran · 2025-11-21T09:50:20Z

The change overall looks reasonable to me.

Given that we ended up fixing the behaviour of -c option in this PR, I think we should rename the JBS issue to something like "jlink's compression plugin doesn't handle -c option correctly". It should then be OK to address the man page and the help text changes as part of this same fix. I believe a CSR will be needed for this change.

My knowledge of this tool is very limited, so please wait for Alan's input before doing the JBS issue change.

jaikiran · 2025-11-21T10:33:01Z

test/jdk/tools/jlink/TaskHelperTest.java

+                new CompressTestCase(new String[] {"--compress=zip-10"}, "zip-10")
+        );
+
+        for (var args : invalidOptValues) {


Nit - might be better to rename args to testCase.

jaikiran · 2025-11-21T10:35:50Z

test/jdk/tools/jlink/TaskHelperTest.java

+            try {
+                var remaining = optionsHelper.handleOptions(this, args.tokens);
+                assertTrue(remaining.isEmpty());
+                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.

To my understanding, for compression plugin the validation of the actual value is performed in DefaultCompressPlugin (

jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultCompressPlugin.java

Line 118 in d57fc1b

throw new IllegalArgumentException("Invalid compression level " + level);

and

jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultCompressPlugin.java

Line 115 in d57fc1b

throw new IllegalArgumentException("Invalid compression level " + level);

).

At the moment, optionsHelper.handleOptions seems to set up TaskHelper's internal state and it shouldn't throw exceptions for syntactically correct options. In my opinion, this fix is not that much related to the value allowed, but to the syntax allowed for the option. I noticed that there is another test class JLinkTest that tests for actual values. When I tested locally my build of this modified jlink, for invalid values I did get that IllegalArgumentException.

However, I am very new to the testing framework and am welcoming more thoughts/ideas 🙏 .

AlanBateman · 2025-11-21T15:46:41Z

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java

+                                String level = (arg != null && !arg.isEmpty())
+                                        ? arg
+                                        :"zip-6";
+                                m.put(plugin.getName(), level);


Thanks for fixing this.

Update compression level explanation from jlink man page.

b1517c3

ammbra changed the title ~~8321138-jlink's man page does not document the --compress option correctly~~ 8321139-jlink's man page does not document the --compress option correctly Nov 17, 2025

openjdk bot added the core-libs [email protected] label Nov 17, 2025

ammbra changed the title ~~8321139-jlink's man page does not document the --compress option correctly~~ 8321139:jlink's man page does not document the --compress option correctly Nov 17, 2025

ammbra changed the title ~~8321139:jlink's man page does not document the --compress option correctly~~ 8321139: jlink's man page does not document the --compress option correctly Nov 17, 2025

openjdk bot added the rfr Pull request is ready for review label Nov 17, 2025

AlanBateman reviewed Nov 17, 2025

View reviewed changes

ammbra added 3 commits November 17, 2025 21:45

Remove legacy shortcut for --compress.

8749e51

Align behavior of compression levels to the input received and sync h…

c290e28

…elp+documentation messages.

Remove extra line.

2e6a3f1

jaikiran reviewed Nov 20, 2025

View reviewed changes

jaikiran reviewed Nov 21, 2025

View reviewed changes

Add some negative tests for suggested values.

fccce74

jaikiran reviewed Nov 21, 2025

View reviewed changes

AlanBateman reviewed Nov 21, 2025

View reviewed changes

ammbra added 3 commits November 21, 2025 19:44

Refactor variable name.

82f4e8d

Fix typo in dependencies.

6a21afb

Fix typo in man page too.

d068438

8321139: jlink's man page does not document the --compress option correctly #28359

Are you sure you want to change the base?

8321139: jlink's man page does not document the --compress option correctly #28359

Conversation

ammbra commented Nov 17, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Nov 17, 2025

Uh oh!

openjdk bot commented Nov 17, 2025

Uh oh!

openjdk bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaikiran commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

ammbra commented Nov 17, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Nov 17, 2025 •

edited

Loading

mlbridge bot commented Nov 17, 2025 •

edited

Loading

AlanBateman Nov 18, 2025 •

edited

Loading