On Thu, 14 Oct 2021 16:04:08 GMT, Hai-May Chao <hc...@openjdk.org> wrote:
> It'd be useful to have a -version option for keytool and jarsigner. Many > other JDK tools already have a -version option. This is to add -version > option to keytool and jarsigner like jar command does. > > CSR review: > https://bugs.openjdk.java.net/browse/JDK-8275174 For the CSR, is it necessary to list the `-help` output change? src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line 348: > 346: } > 347: doPrintVersion(); > 348: } What happens if `-version` appears elsewhere? I tried some other commands. When both `-version` and other arguments are provided, `java` ignores the others and prints out version info. `javac` prints out the version info and compiles as well. It's possible that user writes a script or alias that always run the tool with some options (Ex: `-keystore`, `-storepass`, `-verbose`). Maybe we should not fail if `-version` is used with other options (no matter at the beginning or the end). src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line 600: > 598: } > 599: > 600: static void usageTooManyArg() { Since the method is only for `-version` and the printed message also mentions "version", maybe it's better to choose a specific name. src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line 602: > 600: static void usageTooManyArg() { > 601: System.out.println(rb.getString( > 602: "Option.should.not.be.specified.for.version")); Neither arguments not options should be specified here. test/jdk/sun/security/tools/jarsigner/VersionTest.java line 40: > 38: .shouldHaveExitValue(0); > 39: > 40: SecurityTools.jarsigner("-version -extraopt") You might want to use some legal arguments here, for example, `-version -help`. Otherwise, the error might be because the extra option is not supported. Or, you can check if an expected error message is printed out. test/jdk/sun/security/tools/keytool/VersionTest.java line 40: > 38: .shouldHaveExitValue(0); > 39: > 40: SecurityTools.keytool("-version -extraopt") Same comment as above. ------------- PR: https://git.openjdk.java.net/jdk/pull/5954