8168256: Plugin alias options in jlink --help output seems to be in an arbitrary order
authorjlaskey
Mon, 21 Nov 2016 10:08:20 -0400
changeset 42174 fa763b059501
parent 42173 e7945d9c50df
child 42175 f188c9c9f2de
8168256: Plugin alias options in jlink --help output seems to be in an arbitrary order Reviewed-by: sundar, mchung
jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/JImageTask.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/JImageTask.java	Sun Nov 20 23:19:54 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/JImageTask.java	Mon Nov 21 10:08:20 2016 -0400
@@ -32,6 +32,7 @@
 import java.nio.file.Files;
 import java.nio.file.PathMatcher;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.MissingResourceException;
@@ -190,19 +191,12 @@
             if (options.help) {
                 if (unhandled.isEmpty()) {
                     log.println(TASK_HELPER.getMessage("main.usage", PROGNAME));
-
-                    for (Option<?> o : RECOGNIZED_OPTIONS) {
-                        String name = o.aliases()[0];
-
-                        if (name.startsWith("--")) {
-                            name = name.substring(2);
-                        } else if (name.startsWith("-")) {
-                            name = name.substring(1);
-                        }
-
-                        log.println(TASK_HELPER.getMessage("main.opt." + name));
-                    }
-
+                    Arrays.asList(RECOGNIZED_OPTIONS).stream()
+                        .filter(option -> !option.isHidden())
+                        .sorted()
+                        .forEach(option -> {
+                             log.println(TASK_HELPER.getMessage(option.resourceName()));
+                        });
                     log.println(TASK_HELPER.getMessage("main.opt.footer"));
                 } else {
                     try {
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties	Sun Nov 20 23:19:54 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/resources/jimage.properties	Mon Nov 21 10:08:20 2016 -0400
@@ -60,34 +60,36 @@
 warn.prefix=Warning:
 
 main.opt.dir=\
-\  --dir                             Target directory for extract directive
+\       --dir                        Target directory for extract directive
 
 main.opt.include=\
-\  --include <pattern-list>          Pattern list for filtering entries.
+\       --include <pattern-list>     Pattern list for filtering entries.
+
+main.opt.full-version=\
+\       --full-version               Print full version information
+
+main.opt.help=\
+\  -h,  --help                       Print usage message
+
+main.opt.verbose=\
+\       --verbose                    Listing prints entry size and offset\n\
+\                                    attributes
+
+main.opt.version=\
+\       --version                    Print version information
+
+main.command.files=\
+\       @<filename>                  Read options from file
 
 main.opt.footer=\
 \n\
-\For options requiring a <pattern-list>, the value will be a comma\
-\ separated list of elements each using one the following forms:\n\
+\For options requiring a <pattern-list>, the value will be a comma separated\n\
+\list of elements each using one the following forms:\n\
 \  <glob-pattern>\n\
 \  glob:<glob-pattern>\n\
 \  regex:<regex-pattern>
 
 
-main.opt.full-version=\
-\  --full-version                    Print full version information
-
-main.opt.help=\
-\  -h,  --help                       Print usage message
-
-main.opt.verbose=\
-\  --verbose                         Listing prints entry size and offset attributes
-
-main.opt.version=\
-\  --version                         Print version information
-
-main.command.files=\
-\  @<filename>                       Read options from file
 
 err.not.a.task=task must be one of <extract | info | list | verify>: {0}
 err.missing.arg=no value given for {0}
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java	Sun Nov 20 23:19:54 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java	Mon Nov 21 10:08:20 2016 -0400
@@ -134,7 +134,7 @@
         }, true, "--full-version"),
         new Option<JlinkTask>(false, (task, opt, arg) -> {
             task.options.ignoreSigning = true;
-        }, true, "--ignore-signing-information"),};
+        }, "--ignore-signing-information"),};
 
     private static final String PROGNAME = "jlink";
     private final OptionsValues options = new OptionsValues();
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java	Sun Nov 20 23:19:54 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java	Mon Nov 21 10:08:20 2016 -0400
@@ -46,8 +46,8 @@
 import java.util.MissingResourceException;
 import java.util.ResourceBundle;
 import java.util.Set;
+import java.util.stream.Stream;
 
-import jdk.tools.jlink.internal.plugins.ExcludeFilesPlugin;
 import jdk.tools.jlink.internal.plugins.ExcludeJmodSectionPlugin;
 import jdk.tools.jlink.plugin.Plugin;
 import jdk.tools.jlink.plugin.Plugin.Category;
@@ -90,8 +90,7 @@
         public boolean showUsage;
     }
 
-    public static class Option<T> {
-
+    public static class Option<T> implements Comparable<T> {
         public interface Processing<T> {
 
             void process(T task, String opt, String arg) throws BadArgs;
@@ -100,17 +99,34 @@
         final boolean hasArg;
         final Processing<T> processing;
         final boolean hidden;
-        final String[] aliases;
+        final String name;
+        final String shortname;
 
-        public Option(boolean hasArg, Processing<T> processing, boolean hidden, String... aliases) {
+        public Option(boolean hasArg, Processing<T> processing, boolean hidden, String name, String shortname) {
+            if (!name.startsWith("--")) {
+                throw new RuntimeException("option name missing --, " + name);
+            }
+            if (!shortname.isEmpty() && !shortname.startsWith("-")) {
+                throw new RuntimeException("short name missing -, " + shortname);
+            }
+
             this.hasArg = hasArg;
             this.processing = processing;
-            this.aliases = aliases;
             this.hidden = hidden;
+            this.name = name;
+            this.shortname = shortname;
         }
 
-        public Option(boolean hasArg, Processing<T> processing, String... aliases) {
-            this(hasArg, processing, false, aliases);
+        public Option(boolean hasArg, Processing<T> processing, String name, String shortname) {
+            this(hasArg, processing, false, name, shortname);
+        }
+
+        public Option(boolean hasArg, Processing<T> processing, boolean hidden, String name) {
+            this(hasArg, processing, hidden, name, "");
+        }
+
+        public Option(boolean hasArg, Processing<T> processing, String name) {
+            this(hasArg, processing, false, name, "");
         }
 
         public boolean isHidden() {
@@ -118,16 +134,10 @@
         }
 
         public boolean matches(String opt) {
-            for (String a : aliases) {
-                if (a.equals(opt)) {
-                    return true;
-                } else if (opt.startsWith("--")
-                        && (hasArg && opt.startsWith(a + "="))) {
-                    return true;
-                }
-            }
-            return false;
-        }
+            return opt.equals(name) ||
+                   opt.equals(shortname) ||
+                   hasArg && opt.startsWith("--") && opt.startsWith(name + "=");
+         }
 
         public boolean ignoreRest() {
             return false;
@@ -137,28 +147,54 @@
             processing.process(task, opt, arg);
         }
 
-        public String[] aliases() {
-            return aliases;
+        public String getName() {
+            return name;
+        }
+
+        public String resourceName() {
+            return resourcePrefix() + name.substring(2);
+        }
+
+        public String getShortname() {
+            return shortname;
+        }
+
+        public String resourcePrefix() {
+            return "main.opt.";
+        }
+
+        @Override
+        public int compareTo(Object object) {
+            if (!(object instanceof Option<?>)) {
+                throw new RuntimeException("comparing non-Option");
+            }
+
+            Option<?> option = (Option<?>)object;
+
+            return name.compareTo(option.name);
+        }
+
+    }
+
+    private static class PluginOption extends Option<PluginsHelper> {
+        public PluginOption(boolean hasArg,
+                Processing<PluginsHelper> processing, boolean hidden, String name, String shortname) {
+            super(hasArg, processing, hidden, name, shortname);
+        }
+
+        public PluginOption(boolean hasArg,
+                Processing<PluginsHelper> processing, boolean hidden, String name) {
+            super(hasArg, processing, hidden, name, "");
+        }
+
+        public String resourcePrefix() {
+            return "plugin.opt.";
         }
     }
 
-    private static class PlugOption extends Option<PluginsOptions> {
-
-        public PlugOption(boolean hasArg,
-                Processing<PluginsOptions> processing, boolean hidden, String... aliases) {
-            super(hasArg, processing, hidden, aliases);
-        }
-
-        public PlugOption(boolean hasArg,
-                Processing<PluginsOptions> processing, String... aliases) {
-            super(hasArg, processing, aliases);
-        }
-    }
-
-    private final class PluginsOptions {
+    private final class PluginsHelper {
 
         private static final String PLUGINS_PATH = "--plugin-module-path";
-        private static final String POST_PROCESS = "--post-process-path";
 
         private Layer pluginsLayer = Layer.boot();
         private final List<Plugin> plugins;
@@ -170,10 +206,10 @@
         // Each such occurrence results in a Map of arguments. So, there could be multiple
         // args maps per plugin instance.
         private final Map<Plugin, List<Map<String, String>>> pluginToMaps = new HashMap<>();
-        private final List<PlugOption> pluginsOptions = new ArrayList<>();
-        private final List<PlugOption> mainOptions = new ArrayList<>();
+        private final List<PluginOption> pluginsOptions = new ArrayList<>();
+        private final List<PluginOption> mainOptions = new ArrayList<>();
 
-        private PluginsOptions(String pp) throws BadArgs {
+        private PluginsHelper(String pp) throws BadArgs {
 
             if (pp != null) {
                 String[] dirs = pp.split(File.pathSeparator);
@@ -193,13 +229,13 @@
                     addOrderedPluginOptions(plugin, optionsSeen);
                 }
             }
-            mainOptions.add(new PlugOption(false,
+            mainOptions.add(new PluginOption(false,
                     (task, opt, arg) -> {
                         // This option is handled prior
                         // to have the options parsed.
                     },
-                    "--plugin-module-path"));
-            mainOptions.add(new PlugOption(true, (task, opt, arg) -> {
+                    false, "--plugin-module-path"));
+            mainOptions.add(new PluginOption(true, (task, opt, arg) -> {
                     for (Plugin plugin : plugins) {
                         if (plugin.getName().equals(arg)) {
                             pluginToMaps.remove(plugin);
@@ -208,24 +244,24 @@
                     }
                     throw newBadArgs("err.no.such.plugin", arg);
                 },
-                "--disable-plugin"));
-            mainOptions.add(new PlugOption(true, (task, opt, arg) -> {
+                false, "--disable-plugin"));
+            mainOptions.add(new PluginOption(true, (task, opt, arg) -> {
                 Path path = Paths.get(arg);
                 if (!Files.exists(path) || !Files.isDirectory(path)) {
                     throw newBadArgs("err.image.must.exist", path);
                 }
                 existingImage = path.toAbsolutePath();
-            }, true, POST_PROCESS));
-            mainOptions.add(new PlugOption(true,
+            }, true, "--post-process-path"));
+            mainOptions.add(new PluginOption(true,
                     (task, opt, arg) -> {
                         lastSorter = arg;
                     },
                     true, "--resources-last-sorter"));
-            mainOptions.add(new PlugOption(false,
+            mainOptions.add(new PluginOption(false,
                     (task, opt, arg) -> {
                         listPlugins = true;
                     },
-                    "--list-plugins"));
+                    false, "--list-plugins"));
         }
 
         private List<Map<String, String>> argListFor(Plugin plugin) {
@@ -261,8 +297,8 @@
             }
             optionsSeen.add(option);
 
-            PlugOption plugOption
-                    = new PlugOption(plugin.hasArguments(),
+            PluginOption plugOption
+                    = new PluginOption(plugin.hasArguments(),
                             (task, opt, arg) -> {
                                 if (!Utils.isFunctional(plugin)) {
                                     throw newBadArgs("err.provider.not.functional",
@@ -312,7 +348,7 @@
                                     }
                                 }
                             },
-                            "--" + option);
+                            false, "--" + option);
             pluginsOptions.add(plugOption);
 
             if (Utils.isFunctional(plugin)) {
@@ -322,44 +358,44 @@
 
                 if (plugin instanceof DefaultCompressPlugin) {
                     plugOption
-                        = new PlugOption(false,
+                        = new PluginOption(false,
                             (task, opt, arg) -> {
                                 Map<String, String> m = addArgumentMap(plugin);
                                 m.put(DefaultCompressPlugin.NAME, DefaultCompressPlugin.LEVEL_2);
-                            }, "-c");
+                            }, false, "--compress", "-c");
                     mainOptions.add(plugOption);
                 } else if (plugin instanceof StripDebugPlugin) {
                     plugOption
-                        = new PlugOption(false,
+                        = new PluginOption(false,
                             (task, opt, arg) -> {
                                 addArgumentMap(plugin);
-                            }, "-G");
+                            }, false, "--strip-debug", "-G");
                     mainOptions.add(plugOption);
                 } else if (plugin instanceof ExcludeJmodSectionPlugin) {
-                    plugOption = new PlugOption(false, (task, opt, arg) -> {
+                    plugOption = new PluginOption(false, (task, opt, arg) -> {
                             Map<String, String> m = addArgumentMap(plugin);
                             m.put(ExcludeJmodSectionPlugin.NAME,
                                   ExcludeJmodSectionPlugin.MAN_PAGES);
-                        }, "--no-man-pages");
+                        }, false, "--no-man-pages");
                     mainOptions.add(plugOption);
 
-                    plugOption = new PlugOption(false, (task, opt, arg) -> {
+                    plugOption = new PluginOption(false, (task, opt, arg) -> {
                         Map<String, String> m = addArgumentMap(plugin);
                         m.put(ExcludeJmodSectionPlugin.NAME,
                               ExcludeJmodSectionPlugin.INCLUDE_HEADER_FILES);
-                    }, "--no-header-files");
+                    }, false, "--no-header-files");
                     mainOptions.add(plugOption);
                 }
             }
         }
 
-        private PlugOption getOption(String name) throws BadArgs {
-            for (PlugOption o : pluginsOptions) {
+        private PluginOption getOption(String name) throws BadArgs {
+            for (PluginOption o : pluginsOptions) {
                 if (o.matches(name)) {
                     return o;
                 }
             }
-            for (PlugOption o : mainOptions) {
+            for (PluginOption o : mainOptions) {
                 if (o.matches(name)) {
                     return o;
                 }
@@ -461,7 +497,7 @@
         private String getPluginsPath(String[] args) throws BadArgs {
             String pp = null;
             for (int i = 0; i < args.length; i++) {
-                if (args[i].equals(PluginsOptions.PLUGINS_PATH)) {
+                if (args[i].equals(PluginsHelper.PLUGINS_PATH)) {
                     if (i == args.length - 1) {
                         throw new BadArgs("err.no.plugins.path").showUsage(true);
                     } else {
@@ -498,13 +534,13 @@
             // Must extract it prior to do any option analysis.
             // Required to interpret custom plugin options.
             // Unit tests can call Task multiple time in same JVM.
-            pluginOptions = new PluginsOptions(getPluginsPath(args));
+            pluginOptions = new PluginsHelper(getPluginsPath(args));
 
             // First extract plugins path if any
             String pp = null;
             List<String> filteredArgs = new ArrayList<>();
             for (int i = 0; i < args.length; i++) {
-                if (args[i].equals(PluginsOptions.PLUGINS_PATH)) {
+                if (args[i].equals(PluginsHelper.PLUGINS_PATH)) {
                     if (i == args.length - 1) {
                         throw new BadArgs("err.no.plugins.path").showUsage(true);
                     } else {
@@ -529,7 +565,7 @@
             for (int i = 0; i < args.length; i++) {
                 if (args[i].charAt(0) == '-') {
                     String name = args[i];
-                    PlugOption pluginOption = null;
+                    PluginOption pluginOption = null;
                     Option<T> option = getOption(name);
                     if (option == null) {
                         pluginOption = pluginOptions.getOption(name);
@@ -585,32 +621,13 @@
         }
 
         public void showHelp(String progName) {
-            showHelp(progName, true);
-        }
-
-        private void showHelp(String progName, boolean showsImageBuilder) {
             log.println(bundleHelper.getMessage("main.usage", progName));
-            for (Option<?> o : options) {
-                String name = o.aliases[0].substring(1); // there must always be at least one name
-                name = name.charAt(0) == '-' ? name.substring(1) : name;
-                if (o.isHidden() || name.equals("h")) {
-                    continue;
-                }
-                log.println(bundleHelper.getMessage("main.opt." + name));
-            }
-
-            for (Option<?> o : pluginOptions.mainOptions) {
-                if (o.aliases[0].equals(PluginsOptions.POST_PROCESS)
-                        && !showsImageBuilder) {
-                    continue;
-                }
-                String name = o.aliases[0].substring(1); // there must always be at least one name
-                name = name.charAt(0) == '-' ? name.substring(1) : name;
-                if (o.isHidden()) {
-                    continue;
-                }
-                log.println(bundleHelper.getMessage("plugin.opt." + name));
-            }
+            Stream.concat(options.stream(), pluginOptions.mainOptions.stream())
+                .filter(option -> !option.isHidden())
+                .sorted()
+                .forEach(option -> {
+                     log.println(bundleHelper.getMessage(option.resourceName()));
+                });
 
             log.println(bundleHelper.getMessage("main.command.files"));
         }
@@ -619,6 +636,7 @@
             log.println("\n" + bundleHelper.getMessage("main.extended.help"));
             List<Plugin> pluginList = PluginRepository.
                     getPlugins(pluginOptions.pluginsLayer);
+
             for (Plugin plugin : Utils.getSortedPlugins(pluginList)) {
                 showPlugin(plugin, log);
             }
@@ -671,7 +689,7 @@
         }
     }
 
-    private PluginsOptions pluginOptions;
+    private PluginsHelper pluginOptions;
     private PrintWriter log;
     private final ResourceBundleHelper bundleHelper;
 
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties	Sun Nov 20 23:19:54 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties	Mon Nov 21 10:08:20 2016 -0400
@@ -24,49 +24,51 @@
 #
 
 main.usage.summary=\
-Usage: {0} <options> --module-path <modulepath> --add-modules <mods> --output <path>\n\
-use --help for a list of possible options
+Usage: {0} <options> --module-path <modulepath> --add-modules <mods> --output\n\
+\<path> use --help for a list of possible options
 
 main.usage=\
-Usage: {0} <options> --module-path <modulepath> --add-modules <mods> --output <path>\n\
-\Possible options include:
+Usage: {0} <options> --module-path <modulepath> --add-modules <mods> --output
+\<path> Possible options include:
 
 error.prefix=Error:
 warn.prefix=Warning:
 
 main.opt.help=\
-\  -h, --help                        Print this help message
+\  -h, --help                            Print this help message
 
 main.opt.version=\
-\  --version                         Version information
+\      --version                         Version information
 
 main.opt.module-path=\
-\  -p <modulepath>\n\
-\  --module-path <modulepath>        Module path
+\  -p, --module-path <path>              Module path
 
 main.opt.add-modules=\
-\  --add-modules <mod>[,<mod>...]    Root modules to resolve
+\      --add-modules <mod>[,<mod>...]    Root modules to resolve
 
 main.opt.limit-modules=\
-\  --limit-modules <mod>[,<mod>...]  Limit the universe of observable modules
+\      --limit-modules <mod>[,<mod>...]  Limit the universe of observable\n\
+\                                        modules
 
 main.opt.output=\
-\  --output <path>                   Location of output path
+\      --output <path>                   Location of output path
 
 main.command.files=\
-\  @<filename>                       Read options from file
+\      @<filename>                       Read options from file
 
 main.opt.endian=\
-\  --endian <little|big>             Byte order of generated jimage (default:native)
+\      --endian <little|big>             Byte order of generated jimage\n\
+\                                        (default:native)
 
 main.opt.save-opts=\
-\  --save-opts <filename>            Save jlink options in the given file
+\      --save-opts <filename>            Save jlink options in the given file
 
 main.opt.ignore-signing-information=\
-\  --ignore-signing-information      Suppress a fatal error when signed modular JARs \
-\                                    are linked in the image. The signature related \
-\                                    files of the signed modular JARs are not copied \
-\                                    to the runtime image.
+\      --ignore-signing-information      Suppress a fatal error when signed\n\
+\                                        modular JARs are linked in the image.\n\
+\                                        The signature related files of the\n\
+\                                        signed modular JARs are not copied to\n\
+\                                        the runtime image.
 
 main.msg.bug=\
 An exception has occurred in jlink. \
@@ -78,13 +80,13 @@
 \List of available plugins:
 
 main.extended.help.footer=\
-\For options requiring a <pattern-list>, the value will be a comma\
-\ separated list of elements each using one the following forms:\n\
+\For options requiring a <pattern-list>, the value will be a comma separated\n\
+\list of elements each using one the following forms:\n\
 \  <glob-pattern>\n\
 \  glob:<glob-pattern>\n\
 \  regex:<regex-pattern>\n\
-\  @<filename> where filename is the name of a file containing patterns to be\
-\ used, one pattern per line\n\
+\  @<filename> where filename is the name of a file containing patterns to be\n\
+\              used, one pattern per line\n\
 \n\
 
 
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties	Sun Nov 20 23:19:54 2016 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties	Mon Nov 21 10:08:20 2016 -0400
@@ -132,32 +132,33 @@
 List of available plugin options:
 
 plugin.opt.list-plugins=\
-\  --list-plugins                    List available plugins
+\      --list-plugins                    List available plugins
 
 plugin.opt.post-process-path=\
-\  --post-process-path <imagefile>   Post process an existing image
+\      --post-process-path <imagefile>   Post process an existing image
 
 plugin.opt.resources-last-sorter=\
-\  --resources-last-sorter <name>    The last plugin allowed to sort resources
+\      --resources-last-sorter <name>    The last plugin allowed to sort\n\
+\                                        resources
 
 plugin.opt.plugin-module-path=\
-\  --plugin-module-path <modulepath> Custom plugin module path
+\      --plugin-module-path <modulepath> Custom plugin module path
 
 plugin.opt.disable-plugin=\
-\  --disable-plugin <pluginname>     Disable the plugin mentioned
+\      --disable-plugin <pluginname>     Disable the plugin mentioned
 
-plugin.opt.c=\
-\  -c, --compress=<0|1|2>            Enable compression of resources\
-\n                                    More details in --list-plugins option
+plugin.opt.compress=\
+\  -c, --compress=<0|1|2>                Enable compression of resources\n\
+\                                        More details in --list-plugins option
 
-plugin.opt.G=\
-\  -G, --strip-debug                 Strip debug information
+plugin.opt.strip-debug=\
+\  -G, --strip-debug                     Strip debug information
 
 plugin.opt.no-man-pages=\
-\  --no-man-pages                    Exclude man pages
+\      --no-man-pages                    Exclude man pages
 
 plugin.opt.no-header-files=\
-\  --no-header-files                 Exclude include header files
+\      --no-header-files                 Exclude include header files
 
 main.plugin.name=\
 \Plugin Name