On Fri, 15 Oct 2021 13:34:48 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix -version in jarsigner and update tests > > 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). Fixed so `-version` can be used with other options in jarsigner. > 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. Removed this method after the change for the comment as above. > 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. Method was removed. > 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. Updated the test as suggested. > test/jdk/sun/security/tools/keytool/VersionTest.java line 40: > >> 38: .shouldHaveExitValue(0); >> 39: >> 40: SecurityTools.keytool("-version -extraopt") > > Same comment as above. Done. ------------- PR: https://git.openjdk.java.net/jdk/pull/5954