On Fri, 15 Oct 2021 13:34:48 GMT, Weijun Wang <[email protected]> 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
