8321139: jlink's compression plugin doesn't handle -c option correctly

Reviewed-by: jpai, alanb

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

@@ -362,10 +362,13 @@ public final class TaskHelper {
 
                 if (plugin instanceof DefaultCompressPlugin) {
                     plugOption
-                        = new PluginOption(false,
+                        = new PluginOption(true,
                             (task, opt, arg) -> {
                                 Map<String, String> m = addArgumentMap(plugin);
-                                m.put(plugin.getName(), DefaultCompressPlugin.LEVEL_2);
+                                String level = (arg != null && !arg.isEmpty())
+                                        ? arg
+                                        :"zip-6";
+                                m.put(plugin.getName(), level);
                             }, false, "--compress", "-c");
                     mainOptions.add(plugOption);
                 } else if (plugin instanceof DefaultStripDebugPlugin) {

src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties

@@ -61,14 +61,14 @@ Class optimization: convert Class.forName calls to constant loads.
 class-for-name.usage=\
 \  --class-for-name          Class optimization: convert Class.forName calls to constant loads.
 
-compress.argument=<zip-[0-9]>[:filter=<pattern-list>]
+compress.argument=<zip-'{0-9}'>[:filter=<pattern-list>]
 
 compress.description= Compression to use in compressing resources.
 
 compress.usage=\
 \  --compress <compress>     Compression to use in compressing resources:\n\
 \                            Accepted values are:\n\
-\                            zip-[0-9], where zip-0 provides no compression,\n\
+\                            zip-'{0-9}', where zip-0 provides no compression,\n\
 \                            and zip-9 provides the best compression.\n\
 \                            Default is zip-6.
 
@@ -307,15 +307,15 @@ plugin.opt.disable-plugin=\
 \      --disable-plugin <pluginname>     Disable the plugin mentioned
 
 plugin.opt.compress=\
-\      --compress <compress>             Compression to use in compressing resources:\n\
+\      --compress <compress>             Compress all resources in the output image:\n\
 \                                        Accepted values are:\n\
-\                                        zip-[0-9], where zip-0 provides no compression,\n\
+\                                        zip-'{0-9}', where zip-0 provides no compression,\n\
 \                                        and zip-9 provides the best compression.\n\
 \                                        Default is zip-6.\n\
 \                                        Deprecated values to be removed in a future release:\n\
-\                                        0:  No compression. Equivalent to zip-0.\n\
+\                                        0:  No compression. Use zip-0 instead.\n\
 \                                        1:  Constant String Sharing\n\
-\                                        2:  Equivalent to zip-6.
+\                                        2:  ZIP. Use zip-6 instead.
 
 plugin.opt.strip-debug=\
 \  -G, --strip-debug                     Strip debug information

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

@@ -64,12 +64,15 @@ Developers are responsible for updating their custom runtime images.
 `--bind-services`
 :   Link service provider modules and their dependencies.
 
-`-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:
+    zip-{0-9}, where zip-0 provides no compression,
+    and zip-9 provides the best compression. Default is zip-6.
 
-    -   `0`: No compression
+:   Deprecated values to be removed in a future release:
+    -   `0`: No compression. Use zip-0 instead.
     -   `1`: Constant string sharing
-    -   `2`: ZIP
+    -   `2`: ZIP. Use zip-6 instead.
 
 `--disable-plugin` *pluginname*
 :   Disables the specified plug-in. See [jlink Plug-ins] for the list of
@@ -170,14 +173,18 @@ For a complete list of all available plug-ins, run the command
 ### Plugin `compress`
 
 Options
-:   `--compress=`{`0`\|`1`\|`2`}\[`:filter=`*pattern-list*\]
+:   `--compress=zip-`{`0`-`9`}\[`:filter=`*pattern-list*\]
 
 Description
 :   Compresses all resources in the output image.
+    Accepted values are:
+    zip-{0-9}, where zip-0 provides no compression,
+    and zip-9 provides the best compression. Default is zip-6.
 
-    -   Level 0: No compression
+:   Deprecated values to be removed in a future release:
+    -   Level 0: No compression. Use zip-0 instead.
     -   Level 1: Constant string sharing
-    -   Level 2: ZIP
+    -   Level 2: ZIP. Use zip-6 instead.
 
     An optional *pattern-list* filter can be specified to list the pattern of
     files to include.

test/jdk/tools/jlink/JLinkTest.java

@@ -42,10 +42,7 @@ import tests.JImageGenerator;
 /*
  * @test
  * @summary Test image creation
- * @bug 8189777
- * @bug 8194922
- * @bug 8206962
- * @bug 8240349
+ * @bug 8189777 8194922 8206962 8240349 8163382 8165735 8166810 8173717 8321139
  * @author Jean-Francois Denise
  * @requires (vm.compMode != "Xcomp" & os.maxMemory >= 2g)
  * @library ../lib
@@ -358,6 +355,39 @@ public class JLinkTest {
             helper.generateDefaultImage(userOptions, moduleName).assertFailure("Error: Invalid compression level invalid");
         }
 
+        // short command  without argument
+        {
+            String[] userOptions = {"-c"};
+            String moduleName = "invalidCompressLevelEmpty";
+            helper.generateDefaultJModule(moduleName, "composite2");
+            helper.generateDefaultImage(userOptions, moduleName).assertFailure("Error: no value given for -c");
+        }
+
+        // invalid short command
+        {
+            String[] userOptions = {"-c", "3", "--output", "image"};
+            String moduleName = "invalidCompressLevel3";
+            helper.generateDefaultJModule(moduleName, "composite2");
+            helper.generateDefaultImage(userOptions, moduleName).assertFailure("Error: Invalid compression level 3");
+        }
+
+
+        // invalid argument value
+        {
+            String[] userOptions = {"--compress", "42", "--output", "image"};
+            String moduleName = "invalidCompressLevel42";
+            helper.generateDefaultJModule(moduleName, "composite2");
+            helper.generateDefaultImage(userOptions, moduleName).assertFailure("Error: Invalid compression level 42");
+        }
+
+        // invalid argument value
+        {
+            String[] userOptions = {"--compress", "zip-", "--output", "image"};
+            String moduleName = "invalidCompressLevelZip";
+            helper.generateDefaultJModule(moduleName, "composite2");
+            helper.generateDefaultImage(userOptions, moduleName).assertFailure("Error: Invalid compression level zip-");
+        }
+
         // orphan argument - JDK-8166810
         {
             String[] userOptions = {"--compress", "2", "foo" };

test/jdk/tools/jlink/TaskHelperTest.java

@@ -48,7 +48,7 @@ import jdk.tools.jlink.internal.TaskHelper.BadArgs;
 /*
  * @test
  * @summary Test TaskHelper option parsing
- * @bug 8303884
+ * @bug 8303884 8321139
  * @modules jdk.jlink/jdk.tools.jlink.internal
  *          jdk.jlink/jdk.tools.jlink.plugin
  * @run junit TaskHelperTest
@@ -59,19 +59,22 @@ public class TaskHelperTest {
 
     private static final List<Option<TaskHelperTest>> OPTIONS = List.of(
         new Option<>(true, (task, opt, arg) -> {
-            System.out.println(arg);
             mainArgValue = arg;
         }, true, "--main-expecting"),
         new Option<>(false, (task, opt, arg) -> {
             mainFlag = true;
-        }, true, "--main-no-arg")
+        }, true, "--main-no-arg"),
+        new Option<>(true, (task, opt, arg) -> {
+            compressArgValue = (arg != null && !arg.isEmpty()) ? arg : "zip-6";
+        }, "--compress", "-c")
     );
 
     private static String argValue;
     private static String mainArgValue;
     private static boolean mainFlag = false;
+    private static String compressArgValue;
 
-    public record ArgTestCase(String cmdLine, String[] tokens, String pluginArgValue, String mainArgValue, boolean mainFlagSet) {};
+    public record ArgTestCase(String cmdLine, String[] tokens, String pluginArgValue, String mainArgValue, boolean mainFlagSet) {}
 
     public static class TestPluginWithRawOption implements Plugin {
         @Override
@@ -118,6 +121,7 @@ public class TaskHelperTest {
         argValue = null;
         mainArgValue = null;
         mainFlag = false;
+        compressArgValue= null;
     }
 
     public static Stream<ArgTestCase> gnuStyleUsages() {
@@ -217,4 +221,41 @@ public class TaskHelperTest {
         var remaining = optionsHelper.handleOptions(this, args);
         assertEquals(2, remaining.size());
     }
-}
+
+    record CompressTestCase(String[] tokens, String expectedCompressValue) {}
+
+    public static Stream<CompressTestCase> compressUsages() {
+        return Stream.of(
+
+                new CompressTestCase(new String[] {"-c", "0"}, "0"),
+                new CompressTestCase(new String[] {"--compress=zip-0"}, "zip-0"),
+
+                new CompressTestCase(new String[] {"-c", "1"}, "1"),
+                new CompressTestCase(new String[] {"--compress=zip-1"}, "zip-1"),
+
+                new CompressTestCase(new String[] {"-c", "2"}, "2"),
+                new CompressTestCase(new String[] {"--compress=zip-2"}, "zip-2"),
+
+                new CompressTestCase(new String[] {"--compress=zip-3"}, "zip-3"),
+                new CompressTestCase(new String[] {"--compress=zip-4"}, "zip-4"),
+                new CompressTestCase(new String[] {"--compress=zip-5"}, "zip-5"),
+                new CompressTestCase(new String[] {"--compress=zip-6"}, "zip-6"),
+                new CompressTestCase(new String[] {"--compress=zip-7"}, "zip-7"),
+                new CompressTestCase(new String[] {"--compress=zip-8"}, "zip-8"),
+                new CompressTestCase(new String[] {"--compress=zip-9"}, "zip-9")
+                );
+    }
+
+    @ParameterizedTest
+    @MethodSource("compressUsages")
+    public void testCompressOptionArg(CompressTestCase testCase) throws TaskHelper.BadArgs, IOException {
+        var remaining = optionsHelper.handleOptions(this, testCase.tokens);
+
+        // trigger Plugin::configure
+        taskHelper.getPluginsConfig(null, null, null);
+
+        assertTrue(remaining.isEmpty());
+        assertEquals(testCase.expectedCompressValue, compressArgValue);
+    }
+}
+

test/setup_aot/TestSetupAOT.java

@@ -137,7 +137,7 @@ public class TestSetupAOT {
         String jlinkOutput = tmpDir + File.separator + "jlinkOutput";
 
         execTool("jlink", "--help")
-            .shouldContain("Compression to use in compressing resources");
+            .shouldContain("Compress all resources in the output image");
         execTool("jlink", "--list-plugins")
             .shouldContain("List of available plugins",
                            "--generate-cds-archive ");