8156994: jimage --help is not helpful

Reviewed-by: sundar, alanb

jdk/src/jdk.jlink/share/classes/jdk/tools/jimage/JImageTask.java

@@ -28,19 +28,12 @@ package jdk.tools.jimage;
 import java.io.File;
 import java.io.IOException;
 import java.io.PrintWriter;
-import java.nio.ByteBuffer;
-import java.nio.ByteOrder;
-import java.nio.channels.FileChannel;
 import java.nio.file.Files;
-import static java.nio.file.StandardOpenOption.READ;
-import static java.nio.file.StandardOpenOption.WRITE;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.MissingResourceException;
 import jdk.internal.jimage.BasicImageReader;
 import jdk.internal.jimage.ImageHeader;
-import static jdk.internal.jimage.ImageHeader.MAGIC;
-import static jdk.internal.jimage.ImageHeader.MAJOR_VERSION;
-import static jdk.internal.jimage.ImageHeader.MINOR_VERSION;
 import jdk.internal.jimage.ImageLocation;
 import jdk.tools.jlink.internal.ImageResourcesTree;
 import jdk.tools.jlink.internal.TaskHelper;
@@ -52,22 +45,19 @@ import jdk.tools.jlink.internal.TaskHelper.OptionsHelper;
 class JImageTask {
 
     static final Option<?>[] recognizedOptions = {
-        new Option<JImageTask>(true, (task, opt, arg) -> {
+        new Option<JImageTask>(true, (task, option, arg) -> {
             task.options.directory = arg;
         }, "--dir"),
-        new Option<JImageTask>(false, (task, opt, arg) -> {
+        new Option<JImageTask>(false, (task, option, arg) -> {
             task.options.fullVersion = true;
         }, true, "--fullversion"),
-        new Option<JImageTask>(false, (task, opt, arg) -> {
+        new Option<JImageTask>(false, (task, option, arg) -> {
             task.options.help = true;
         }, "--help"),
-        new Option<JImageTask>(true, (task, opt, arg) -> {
-            task.options.flags = arg;
-        }, "--flags"),
-        new Option<JImageTask>(false, (task, opt, arg) -> {
+        new Option<JImageTask>(false, (task, option, arg) -> {
             task.options.verbose = true;
         }, "--verbose"),
-        new Option<JImageTask>(false, (task, opt, arg) -> {
+        new Option<JImageTask>(false, (task, option, arg) -> {
             task.options.version = true;
         }, "--version")
     };
@@ -81,7 +71,6 @@ class JImageTask {
         String directory = ".";
         boolean fullVersion;
         boolean help;
-        String flags;
         boolean verbose;
         boolean version;
         List<File> jimages = new LinkedList<>();
@@ -94,7 +83,6 @@ class JImageTask {
         EXTRACT,
         INFO,
         LIST,
-        SET,
         VERIFY
     };
 
@@ -145,14 +133,12 @@ class JImageTask {
 
     int run(String[] args) {
         if (log == null) {
-            setLog(new PrintWriter(System.out));
+            setLog(new PrintWriter(System.out, true));
         }
-
         if (args.length == 0) {
             log.println(taskHelper.getMessage("main.usage.summary", PROGNAME));
             return EXIT_ABNORMAL;
         }
-
         try {
             List<String> unhandled = optionsHelper.handleOptions(this, args);
             if(!unhandled.isEmpty()) {
@@ -164,17 +150,38 @@ class JImageTask {
                 for(int i = 1; i < unhandled.size(); i++) {
                     options.jimages.add(new File(unhandled.get(i)));
                 }
-            } else {
-                throw taskHelper.newBadArgs("err.not.a.task", "<unspecified>");
+            } else if (!options.help && !options.version && !options.fullVersion) {
+                throw taskHelper.newBadArgs("err.invalid.task", "<unspecified>");
             }
             if (options.help) {
-                optionsHelper.showHelp(PROGNAME);
+                if (unhandled.isEmpty()) {
+                    log.println(taskHelper.getMessage("main.usage", PROGNAME));
+                    for (Option<?> o : recognizedOptions) {
+                        String name = o.aliases()[0];
+                        if (name.startsWith("--")) {
+                            name = name.substring(2);
+                        } else if (name.startsWith("-")) {
+                            name = name.substring(1);
+                        }
+                        log.println(taskHelper.getMessage("main.opt." + name));
+                    }
+                } else {
+                    try {
+                        log.println(taskHelper.getMessage("main.usage." + options.task.toString().toLowerCase()));
+                    } catch (MissingResourceException ex) {
+                        throw taskHelper.newBadArgs("err.not.a.task", unhandled.get(0));
+                    }
+                }
+                return EXIT_OK;
             }
             if (options.version || options.fullVersion) {
                 taskHelper.showVersion(options.fullVersion);
+
+                if (unhandled.isEmpty()) {
+                    return EXIT_OK;
+                }
             }
-            boolean ok = run();
-            return ok ? EXIT_OK : EXIT_ERROR;
+            return run() ? EXIT_OK : EXIT_ERROR;
         } catch (BadArgs e) {
             taskHelper.reportError(e.key, e.args);
             if (e.showUsage) {
@@ -261,7 +268,7 @@ class JImageTask {
 
         log.println(" Major Version:  " + header.getMajorVersion());
         log.println(" Minor Version:  " + header.getMinorVersion());
-        log.println(" Flags:          " + Integer.toHexString(header.getMinorVersion()));
+        log.println(" Flags:          " + Integer.toHexString(header.getFlags()));
         log.println(" Resource Count: " + header.getResourceCount());
         log.println(" Table Length:   " + header.getTableLength());
         log.println(" Offsets Size:   " + header.getOffsetsSize());
@@ -287,36 +294,7 @@ class JImageTask {
         print(reader, name);
     }
 
-    void set(File file, BasicImageReader reader) throws BadArgs {
-        try {
-            ImageHeader oldHeader = reader.getHeader();
-
-            int value = 0;
-            try {
-                value = Integer.valueOf(options.flags);
-            } catch (NumberFormatException ex) {
-                throw taskHelper.newBadArgs("err.flags.not.int", options.flags);
-            }
-
-            ImageHeader newHeader = new ImageHeader(MAGIC, MAJOR_VERSION, MINOR_VERSION,
-                    value,
-                    oldHeader.getResourceCount(), oldHeader.getTableLength(),
-                    oldHeader.getLocationsSize(), oldHeader.getStringsSize());
-
-            ByteBuffer buffer = ByteBuffer.allocate(ImageHeader.getHeaderSize());
-            buffer.order(ByteOrder.nativeOrder());
-            newHeader.writeTo(buffer);
-            buffer.rewind();
-
-            try (FileChannel channel = FileChannel.open(file.toPath(), READ, WRITE)) {
-                channel.write(buffer, 0);
-            }
-        } catch (IOException ex) {
-            throw taskHelper.newBadArgs("err.cannot.update.file", file.getName());
-        }
-    }
-
-     void verify(BasicImageReader reader, String name, ImageLocation location) {
+      void verify(BasicImageReader reader, String name, ImageLocation location) {
         if (name.endsWith(".class")) {
             byte[] bytes = reader.getResource(location);
 
@@ -387,9 +365,6 @@ class JImageTask {
             case LIST:
                 iterate(this::listTitle, this::listModule, this::list);
                 break;
-            case SET:
-                iterate(this::set, null, null);
-                break;
             case VERIFY:
                 iterate(this::listTitle, null, this::verify);
                 break;
@@ -399,7 +374,8 @@ class JImageTask {
         return true;
     }
 
-    private PrintWriter log;
+    private PrintWriter log = null;
+
     void setLog(PrintWriter out) {
         log = out;
         taskHelper.setLog(log);

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

@@ -24,54 +24,63 @@
 #
 
 main.usage.summary=\
-Usage: {0} <extract|info|list|set|verify> <options> jimage...\n\
-use --help for a list of possible options
+Usage: {0} <extract | info | list | verify> <options> jimage...\n\
+use --help for a list of possible options.
 
 main.usage=\
-Usage: {0} <extract|info|list|set|verify> <options> jimage...\n\
+Usage: {0} <extract | info | list | verify> <options> jimage...\n\
+\n\
+\  extract  - Extract all jimage entries and place in a directory specified\n\
+\             by the --dir=<directory> (default='.') option.\n\
+\n\
+\  info     - Prints detailed information contained in the jimage header.\n\
 \n\
-\  extract  - Extract all jimage entries into separate files into the directory\n\
-\             specified by --dir=<directory> (default='.')\n\
-\  info     - Prints information specified in the jimage header.\n\
 \  list     - Prints the names of all the entries in the jimage.  When used with\n\
-\             --verbose will also print entry attributes ex. size and offset.\n\
-\  set      - sets the value of specific jimage header entries\n\
-\  verify   - Reports errors on any .class entries that don't verify as classes.\n\
+\             --verbose, list will also print entry size and offset attributes.\n\
+\n\
+\  verify   - Reports on any .class entries that don't verify as classes.\n\
 \n\
 Possible options include:
 
+main.usage.extract=\
+\  extract  - Extract all jimage entries and place in a directory specified\n\
+\             by the --dir=<directory> (default='.') option.
+
+main.usage.info=\
+\  info     - Prints detailed information contained in the jimage header.
+
+main.usage.list=\
+\  list     - Prints the names of all the entries in the jimage.  When used with\n\
+\             --verbose, list will also print entry size and offset attributes.
+
+main.usage.verify=\
+\  verify   - Reports errors on any .class entries that don't verify as classes.
+
 error.prefix=Error:
 warn.prefix=Warning:
 
 main.opt.dir=\
-\  --dir                                Target directory for extract
+\  --dir                             Target directory for extract directive
 
-main.opt.flags=\
-\  --flags=value                        Set the jimage flags to value
+main.opt.fullversion=\
+\  --fullversion                     Print full version information
 
 main.opt.help=\
-\  --help                               Print this usage message
+\  --help                            Print usage message
 
 main.opt.verbose=\
-\  --verbose                            Verbose listing
+\  --verbose                         Listing prints entry size and offset attributes
 
 main.opt.version=\
-\  --version                            Version information
+\  --version                         Print version information
 
 main.command.files=\
-\  @<filename>                          Read options from file
-err.cannot.read.file=cannot read file: {0}
-err.cannot.update.file=cannot update file: {0}
-err.file.not.found=cannot find file: {0}
-err.file.error=cannot access file: {0}
-err.flags.not.int=--flags value not integer: {0}
-err.internal.error=internal error: {0} {1} {2}
-err.invalid.arg.for.option=invalid argument for option: {0}
-err.invalid.task=task must be extract|info|list|verify: {0}
+\  @<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}
 err.not.a.dir=not a directory: {0}
 err.not.a.jimage=not a jimage file: {0}
 err.no.jimage=no jimage provided
-err.not.a.task=not a valid task: {0}
 err.option.unsupported={0} not supported: {1}
 err.unknown.option=unknown option: {0}

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

@@ -186,7 +186,7 @@ public class JlinkTask {
 
     int run(String[] args) {
         if (log == null) {
-            setLog(new PrintWriter(System.err));
+            setLog(new PrintWriter(System.err, true));
         }
         try {
             optionsHelper.handleOptions(this, args);

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

@@ -136,6 +136,10 @@ public final class TaskHelper {
         void process(T task, String opt, String arg) throws BadArgs {
             processing.process(task, opt, arg);
         }
+
+        public String[] aliases() {
+            return aliases;
+        }
     }
 
     private static class PlugOption extends Option<PluginsOptions> {