RFR: 8189102: All tools should support -?, -h and --help
Hi, please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477 See the webrev for a detailed description of the changes. If required, I'll make break-out changes to be reviewed separately. I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html Best regards, Goetz.
Goetz, I understand why you might want to ensure that a basic set of help options is supported, but I don't understand why that justifies removing older options, like "-help" for many tools. In addition, I notice the CSR says:
*Compatibility Risk Description:* <https://bugs.openjdk.java.net/browse/JDK-8191477#> Only new flags are added, none removed.
But that is not true, since your edits for javac and javadoc remove the option completely. Also, in the CSR, look for these typos: serveral deperecation OpenJdk (should be OpenJDK) Also, I note that you've changed localized resource files. The usual procedure is to never touch those files, since they get updated later by Oracle's localization team. -- Jon On 11/17/2017 03:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html
Best regards, Goetz.
Hi Jon, thanks for your feedback. Sorry for getting the javac/Javadoc wrong, I missed that. The point is that the option implementation there does not have the possibility to accept an option but not document it. javac: I'd like to propose to add -help again. javac else prints: javac: invalid flag: -help Usage: javac <options> <source files> use --help for a list of possible options Which isn't that nice. Javadoc: I'd prefer to remove -help because then it's not documented which is more streamlined with the overall idea of this change. And Javadoc behaves "friendly": Javadoc -help prints the usage but exits with return code '1'. I don't think that's a major problem. I'll update the CSR accordingly once we decide on this. I'm happy not to edit the properties files :) I'll revert that. (Although I would have liked to edit some of the German translations.) I fixed the typos in the CSR. Best regards, Goetz Change to javac: --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java Tue Oct 10 14:39:45 2017 +0200 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java Mon Nov 20 12:19:49 2017 +0100 @@ -360,7 +360,7 @@ }, // Note: -h is already taken for "native header output directory". - HELP("-? --help", "opt.help", STANDARD, INFO) { + HELP("-? --help -help", "opt.help", STANDARD, INFO) { @Override public void process(OptionHelper helper, String option) throws InvalidValueException { Log log = helper.getLog();
-----Original Message----- From: Jonathan Gibbons [mailto:jonathan.gibbons@oracle.com] Sent: Freitag, 17. November 2017 19:30 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; core-libs- dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler- dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Goetz,
I understand why you might want to ensure that a basic set of help options is supported, but I don't understand why that justifies removing older options, like "-help" for many tools.
In addition, I notice the CSR says:
Compatibility Risk Description: <https://bugs.openjdk.java.net/browse/JDK-8191477#> Only new flags are added, none removed.
But that is not true, since your edits for javac and javadoc remove the option completely.
Also, in the CSR, look for these typos: serveral deperecation OpenJdk (should be OpenJDK)
Also, I note that you've changed localized resource files. The usual procedure is to never touch those files, since they get updated later by Oracle's localization team.
-- Jon
On 11/17/2017 03:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
JShell changes — The code change is fine. The help change in the properties file is not consistent with the other items, note: --help-extra, -X Print help on… so this should be: —help, -h, -? Or, if there is a tool-wide standard, then the —help-extra entry should be changed. The tests of “—help” should be updated to include the now documented “-h” and “-?” As Jon noted, the _ja and _zh_CN properties should not be touched. -Robert
On Nov 17, 2017, at 3:23 AM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html
Best regards, Goetz.
Hi Robert, thanks for looking at my change. I'm fine with listing the flags in a different order, but it's ambiguous. Java itslef lists "-? -h --help" therefore I chose this for most of the tools, too. But just tell me how to put it. Below, I list all the option help messages for the tools on linux. --help-extra is quite consistent. Best regards, Goetz. --help-extra, -X Print help on extra options --help-extra, -X --help-extra, -X Print help on non-standard options and exit -X print help on extra options to the output stream --help-extra print help on extra options to the output stream --help-extra Print help on extra options --help-extra Give help on extra options -? -h --help display this help message -? -h --help display this help message -h, --help (Print this help message.) -?, --help Print this help message -? -h --help Print this help message --help, -h, -? Display command line options and exit -? -h --help Print this help message -h -? --help Print this help message -? -h --help -? -h --help : display this help message and exit -? -h --help : display this help message and exit -? -h --help print this help message and exit -? | -h | --help to print this help message jmap -? -h --help to print this help message usage: jps [-? -h --help] -? -h --help to print this help message -? -h --help Prints this help message. -? -h --help print this help message -? -h --help Print this synopsis of standard options and exit Use "keytool -? -h or --help" for all available commands --help print this help message to the output stream -?, -h, --help Print this help message -h, --help, -? Print this help message -?, -h, --help Print this help message -? -h --help Print this help message and exit -?, -h, --help print this help message -?, -h, --help print this help message -? -h --help Print this help message -?, -h, --help[:compat] Give this, or optionally the compatibility, help [-? -h --help] Print this help message jstatd -?|-h|--help
-----Original Message----- From: Robert Field [mailto:robert.field@oracle.com] Sent: Freitag, 17. November 2017 20:07 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
JShell changes —
The code change is fine.
The help change in the properties file is not consistent with the other items, note: --help-extra, -X Print help on… so this should be: —help, -h, -? Or, if there is a tool-wide standard, then the —help-extra entry should be changed.
The tests of “—help” should be updated to include the now documented “- h” and “-?”
As Jon noted, the _ja and _zh_CN properties should not be touched.
-Robert
On Nov 17, 2017, at 3:23 AM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
On 17/11/2017 11:23, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
A question for Jon Gibbons: In JEP 293 the guideline is that tools should support `--help`. Do you think this should be expanded to include `-help`, and `-?` (and maybe `-h` where it make sense)? As regards the webrev then the changes to the localized resources can be dropped. As Jon noted, we don't usually edit these directly as they are bulk updated/replaced by translation drops that usually happen late in each release. It's not clear to me that it's wroth trying to update the JAXB, JAX-WS, and CORBA tools. One reason the corresponding tool modules are deprecated-for-removal and there is a draft JEP to remove them completely [1]. In addition, the JAXB ad JAX-WS tools are problematic to change as they are owned by upstream projects on the Java EE github - any changes to the code in these modules needs to coordinated to avoid having a fork here. -Alan [1] http://openjdk.java.net/jeps/8189188
On 11/18/17 12:41 AM, Alan Bateman wrote:
A question for Jon Gibbons: In JEP 293 the guideline is that tools should support `--help`. Do you think this should be expanded to include `-help`, and `-?` (and maybe `-h` where it make sense)? I think we should standardize on --help, and -h where possible, as the standard options.
I think we should continue to allow -help where it has previously been accepted. I don't think we should add -help for new tools. I wasn't aware that -? is in common usage. If it is, it makes sense to allow it. -- Jon
Hi Alan, thanks for looking at my change. Thanks for pointing to JEP 293. I see my change mostly follows its ideas. As Jon, I don't think asking for support of -help makes sense as it's not according to the gnu style, and it's not that hard to get it right anyways. I'll remove the edits to the properties files. The deprecated tools are orbd, wsgen wsimport schemagen Is that correct? I'll remove them from my change. The test has a list of tools not to test, I'll add them there. Best regards, Goetz.
-----Original Message----- From: Alan Bateman [mailto:Alan.Bateman@oracle.com] Sent: Samstag, 18. November 2017 09:42 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com>; core-libs- dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler- dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
On 17/11/2017 11:23, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
A question for Jon Gibbons: In JEP 293 the guideline is that tools should support `--help`. Do you think this should be expanded to include `-help`, and `-?` (and maybe `-h` where it make sense)?
As regards the webrev then the changes to the localized resources can be dropped. As Jon noted, we don't usually edit these directly as they are bulk updated/replaced by translation drops that usually happen late in each release.
It's not clear to me that it's wroth trying to update the JAXB, JAX-WS, and CORBA tools. One reason the corresponding tool modules are deprecated-for-removal and there is a draft JEP to remove them completely [1]. In addition, the JAXB ad JAX-WS tools are problematic to change as they are owned by upstream projects on the Java EE github - any changes to the code in these modules needs to coordinated to avoid having a fork here.
-Alan
On 20/11/2017 13:41, Lindenmaier, Goetz wrote:
:
The deprecated tools are orbd, wsgen wsimport schemagen Is that correct?
Yes, plus xjc, idlj, servertool and tnameserv (the full list is in the draft JEP that I linked to). -Alan
I am OK with all commands supporting --help, but I am not sure if every tool should show it on the help screen if the tools's other options are still using the old single-"-" style. It looks like the tools are half-converted. Is there a timetable to add "--" support to all tools? Thanks Max
On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html
Best regards, Goetz.
We are already in a hybrid world of old-style and new-style options :-( All the new module-related options added in JDK 9 are "new style", following the guidelines of JEP 293. It does not make sense to add "--" to some of the older tools, that may be going away at some point, and while we may be able to add the ability for other tools to accept "--" options, we have to be careful about removing support for existing options. -- Jon On 11/18/17 4:28 PM, Weijun Wang wrote:
I am OK with all commands supporting --help, but I am not sure if every tool should show it on the help screen if the tools's other options are still using the old single-"-" style. It looks like the tools are half-converted.
Is there a timetable to add "--" support to all tools?
Thanks Max
On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html
Best regards, Goetz.
Hi Max, I think there are so many tools mixing both kinds of options, that it's rather a gain if all at least document the same, up to date help message, than if the documentation is skipped for some. After my change, all the help messages are quite similar. I updated them to use the same wording while trying to keep the sentence structure in accordance with the other documented flags, see below. Best regards, Goetz. -? -h --help display this help message -? -h --help display this help message -h, --help (Print this help message.) -?, --help Print this help message -? -h --help Print this help message --help, -h, -? Display command line options and exit -? -h --help Print this help message -h -? --help Print this help message -? -h --help -? -h --help : display this help message and exit -? -h --help : display this help message and exit -? -h --help print this help message and exit -? | -h | --help to print this help message jmap -? -h --help to print this help message usage: jps [-? -h --help] -? -h --help to print this help message -? -h --help Prints this help message. -? -h --help print this help message -? -h --help Print this synopsis of standard options and exit Use "keytool -? -h or --help" for all available commands --help print this help message to the output stream -?, -h, --help Print this help message -h, --help, -? Print this help message -?, -h, --help Print this help message -? -h --help Print this help message and exit -?, -h, --help print this help message -?, -h, --help print this help message -? -h --help Print this help message -?, -h, --help[:compat] Give this, or optionally the compatibility, help [-? -h --help] Print this help message jstatd -?|-h|--help
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Sonntag, 19. November 2017 01:28 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
I am OK with all commands supporting --help, but I am not sure if every tool should show it on the help screen if the tools's other options are still using the old single-"-" style. It looks like the tools are half-converted.
Is there a timetable to add "--" support to all tools?
Thanks Max
On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Hi Goetz I understand your intention. If the reason is that one day every tool will switch to this new style, please at least do not include kinit, klist and ktab. These Windows-only commands are meant to emulate MIT krb5 tools with the same names and we need to keep the option (and maybe the help screen) as similar as possible. I am OK with supporting the --help option undocumented. Thanks Max
On Nov 20, 2017, at 9:46 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi Max,
I think there are so many tools mixing both kinds of options, that it's rather a gain if all at least document the same, up to date help message, than if the documentation is skipped for some.
After my change, all the help messages are quite similar. I updated them to use the same wording while trying to keep the sentence structure in accordance with the other documented flags, see below.
Best regards, Goetz.
-? -h --help display this help message -? -h --help display this help message -h, --help (Print this help message.) -?, --help Print this help message -? -h --help Print this help message --help, -h, -? Display command line options and exit -? -h --help Print this help message -h -? --help Print this help message -? -h --help -? -h --help : display this help message and exit -? -h --help : display this help message and exit -? -h --help print this help message and exit -? | -h | --help to print this help message jmap -? -h --help to print this help message usage: jps [-? -h --help] -? -h --help to print this help message -? -h --help Prints this help message. -? -h --help print this help message -? -h --help Print this synopsis of standard options and exit Use "keytool -? -h or --help" for all available commands --help print this help message to the output stream -?, -h, --help Print this help message -h, --help, -? Print this help message -?, -h, --help Print this help message -? -h --help Print this help message and exit -?, -h, --help print this help message -?, -h, --help print this help message -? -h --help Print this help message -?, -h, --help[:compat] Give this, or optionally the compatibility, help [-? -h --help] Print this help message jstatd -?|-h|--help
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Sonntag, 19. November 2017 01:28 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
I am OK with all commands supporting --help, but I am not sure if every tool should show it on the help screen if the tools's other options are still using the old single-"-" style. It looks like the tools are half-converted.
Is there a timetable to add "--" support to all tools?
Thanks Max
On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Hi Max, while removing the comments from the k-tools, I saw this: --- a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java Tue Oct 10 14:39:45 2017 +0200 +++ b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java Tue Nov 21 13:09:17 2017 +0100 @@ -356,7 +358,6 @@ System.out.println("\t-t \t shows keytab entry timestamps"); System.out.println("\t-K \t shows keytab entry key value"); System.out.println("\t-e \t shows keytab entry key type"); - System.out.println("\nUsage: java sun.security.krb5.tools.Klist " + - "-help for help."); + System.out.println("\n-? -h --help print this help message and exit"); } } I don't think the old comment is in the original program :) and anyways, -help is not supported by Klist. It prints the usage called with the flag, but just because it prints it on any wrong flag. So should I replace the comment here? Or at least remove it? Best regards, Goetz
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Monday, November 20, 2017 3:49 PM To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi Goetz
I understand your intention.
If the reason is that one day every tool will switch to this new style, please at least do not include kinit, klist and ktab. These Windows-only commands are meant to emulate MIT krb5 tools with the same names and we need to keep the option (and maybe the help screen) as similar as possible.
I am OK with supporting the --help option undocumented.
Thanks Max
On Nov 20, 2017, at 9:46 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi Max,
I think there are so many tools mixing both kinds of options, that it's rather a gain if all at least document the same, up to date help message, than if the documentation is skipped for some.
After my change, all the help messages are quite similar. I updated them to use the same wording while trying to keep the sentence structure in accordance with the other documented flags, see below.
Best regards, Goetz.
-? -h --help display this help message -? -h --help display this help message -h, --help (Print this help message.) -?, --help Print this help message -? -h --help Print this help message --help, -h, -? Display command line options and exit -? -h --help Print this help message -h -? --help Print this help message -? -h --help -? -h --help : display this help message and exit -? -h --help : display this help message and exit -? -h --help print this help message and exit -? | -h | --help to print this help message jmap -? -h --help to print this help message usage: jps [-? -h --help] -? -h --help to print this help message -? -h --help Prints this help message. -? -h --help print this help message -? -h --help Print this synopsis of standard options and exit Use "keytool -? -h or --help" for all available commands --help print this help message to the output stream -?, -h, --help Print this help message -h, --help, -? Print this help message -?, -h, --help Print this help message -? -h --help Print this help message and exit -?, -h, --help print this help message -?, -h, --help print this help message -? -h --help Print this help message -?, -h, --help[:compat] Give this, or optionally the compatibility, help [-? -h --help] Print this help message jstatd -?|-h|--help
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Sonntag, 19. November 2017 01:28 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
I am OK with all commands supporting --help, but I am not sure if every tool should show it on the help screen if the tools's other options are still using the old single-"-" style. It looks like the tools are half-converted.
Is there a timetable to add "--" support to all tools?
Thanks Max
On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Hi Goetz I would just remove it. Thanks Max
On Nov 22, 2017, at 2:53 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi Max,
while removing the comments from the k-tools, I saw this:
--- a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java Tue Oct 10 14:39:45 2017 +0200 +++ b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java Tue Nov 21 13:09:17 2017 +0100 @@ -356,7 +358,6 @@ System.out.println("\t-t \t shows keytab entry timestamps"); System.out.println("\t-K \t shows keytab entry key value"); System.out.println("\t-e \t shows keytab entry key type"); - System.out.println("\nUsage: java sun.security.krb5.tools.Klist " + - "-help for help."); + System.out.println("\n-? -h --help print this help message and exit"); } }
I don't think the old comment is in the original program :) and anyways, -help is not supported by Klist. It prints the usage called with the flag, but just because it prints it on any wrong flag.
So should I replace the comment here? Or at least remove it?
Best regards, Goetz
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Monday, November 20, 2017 3:49 PM To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi Goetz
I understand your intention.
If the reason is that one day every tool will switch to this new style, please at least do not include kinit, klist and ktab. These Windows-only commands are meant to emulate MIT krb5 tools with the same names and we need to keep the option (and maybe the help screen) as similar as possible.
I am OK with supporting the --help option undocumented.
Thanks Max
On Nov 20, 2017, at 9:46 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi Max,
I think there are so many tools mixing both kinds of options, that it's rather a gain if all at least document the same, up to date help message, than if the documentation is skipped for some.
After my change, all the help messages are quite similar. I updated them to use the same wording while trying to keep the sentence structure in accordance with the other documented flags, see below.
Best regards, Goetz.
-? -h --help display this help message -? -h --help display this help message -h, --help (Print this help message.) -?, --help Print this help message -? -h --help Print this help message --help, -h, -? Display command line options and exit -? -h --help Print this help message -h -? --help Print this help message -? -h --help -? -h --help : display this help message and exit -? -h --help : display this help message and exit -? -h --help print this help message and exit -? | -h | --help to print this help message jmap -? -h --help to print this help message usage: jps [-? -h --help] -? -h --help to print this help message -? -h --help Prints this help message. -? -h --help print this help message -? -h --help Print this synopsis of standard options and exit Use "keytool -? -h or --help" for all available commands --help print this help message to the output stream -?, -h, --help Print this help message -h, --help, -? Print this help message -?, -h, --help Print this help message -? -h --help Print this help message and exit -?, -h, --help print this help message -?, -h, --help print this help message -? -h --help Print this help message -?, -h, --help[:compat] Give this, or optionally the compatibility, help [-? -h --help] Print this help message jstatd -?|-h|--help
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Sonntag, 19. November 2017 01:28 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
I am OK with all commands supporting --help, but I am not sure if every tool should show it on the help screen if the tools's other options are still using the old single-"-" style. It looks like the tools are half-converted.
Is there a timetable to add "--" support to all tools?
Thanks Max
On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
OK, thanks! Best regards, Goetz.
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Wednesday, November 22, 2017 8:08 AM To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi Goetz
I would just remove it.
Thanks Max
On Nov 22, 2017, at 2:53 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi Max,
while removing the comments from the k-tools, I saw this:
--- a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kl ist.java Tue Oct 10 14:39:45 2017 +0200 +++ b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kl ist.java Tue Nov 21 13:09:17 2017 +0100 @@ -356,7 +358,6 @@ System.out.println("\t-t \t shows keytab entry timestamps"); System.out.println("\t-K \t shows keytab entry key value"); System.out.println("\t-e \t shows keytab entry key type"); - System.out.println("\nUsage: java sun.security.krb5.tools.Klist " + - "-help for help."); + System.out.println("\n-? -h --help print this help message and exit"); } }
I don't think the old comment is in the original program :) and anyways, - help is not supported by Klist. It prints the usage called with the flag, but just because it prints it on any wrong flag.
So should I replace the comment here? Or at least remove it?
Best regards, Goetz
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Monday, November 20, 2017 3:49 PM To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi Goetz
I understand your intention.
If the reason is that one day every tool will switch to this new style, please at least do not include kinit, klist and ktab. These Windows-only commands are meant to emulate MIT krb5 tools with the same names and we need to keep the option (and maybe the help screen) as similar as possible.
I am OK with supporting the --help option undocumented.
Thanks Max
On Nov 20, 2017, at 9:46 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi Max,
I think there are so many tools mixing both kinds of options, that it's rather a gain if all at least document the same, up to date help message, than if the documentation is skipped for some.
After my change, all the help messages are quite similar. I updated them to use the same wording while trying to keep the sentence structure in accordance with the other documented flags, see below.
Best regards, Goetz.
-? -h --help display this help message -? -h --help display this help message -h, --help (Print this help message.) -?, --help Print this help message -? -h --help Print this help message --help, -h, -? Display command line options and exit -? -h --help Print this help message -h -? --help Print this help message -? -h --help -? -h --help : display this help message and exit -? -h --help : display this help message and exit -? -h --help print this help message and exit -? | -h | --help to print this help message jmap -? -h --help to print this help message usage: jps [-? -h --help] -? -h --help to print this help message -? -h --help Prints this help message. -? -h --help print this help message -? -h --help Print this synopsis of standard options and exit Use "keytool -? -h or --help" for all available commands --help print this help message to the output stream -?, -h, --help Print this help message -h, --help, -? Print this help message -?, -h, --help Print this help message -? -h --help Print this help message and exit -?, -h, --help print this help message -?, -h, --help print this help message -? -h --help Print this help message -?, -h, --help[:compat] Give this, or optionally the compatibility, help [-? -h --help] Print this help message jstatd -?|-h|--help
-----Original Message----- From: Weijun Wang [mailto:weijun.wang@oracle.com] Sent: Sonntag, 19. November 2017 01:28 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; compiler-dev@openjdk.java.net; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
I am OK with all commands supporting --help, but I am not sure if every tool should show it on the help screen if the tools's other options are still using the old single-"-" style. It looks like the tools are half-converted.
Is there a timetable to add "--" support to all tools?
Thanks Max
On Nov 17, 2017, at 7:23 PM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Hi, I prepared a new webrev trying to incorporate all change requests: * I removed changes to non-english property files. * I removed help texts from ktab, kinit, klist * I removed changes to corba tools * I added test cases for new flags to tool specific tests (to the ones I spotted) * I added -help back to javac I left the removal of -help from Javadoc, as Javadoc just prints the help message anyways. New webrev: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.03/index.h... I uploaded the parts of the patch I removed within the webrev: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.03/help_in... http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.03/KinitKt... http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.03/corba_h... Best regards, Goetz.
-----Original Message----- From: serviceability-dev [mailto:serviceability-dev- bounces@openjdk.java.net] On Behalf Of Lindenmaier, Goetz Sent: Friday, November 17, 2017 12:23 PM To: core-libs-dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler-dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: RFR: 8189102: All tools should support -?, -h and --help
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Hi, I looked at some of the components I maintain, and they look good. Wrt. the test: 1. The local variables/fields don't comply with the coding conventions, we have been following in the JDK. 2. Hmm, someone parked a .ini file in bin directory, later removed, perhaps on windows simply check for .exe ? Should a check exist to verify if file has executable permissions ?"File.canExecute". 171 // Returns true if the file is not a tool. 172 static boolean notATool(String file) { 173 file = file.toLowerCase(); 174 if (file.endsWith(".dll") || 175 file.endsWith(".map") || 176 file.endsWith(".pdb") || 177 file.equals("server")) { // server subdir on windows. 178 return true; 179 } 180 return false; 181 } 3. Typo in comment 201 // Check whether 'flag' appears in 'line' as a word of itself. I must not s/I must/It must/ 4. There is a method to check for isEnglishLocale in TestHelper, perhaps use it to have the test bale out in non english locales as early as possible ? 5. Has this been tested on all platforms ? do you need help testing it ? Kumar On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028615.html
Best regards, Goetz.
Hi Kumar, Thanks for looking at my change!
I looked at some of the components I maintain, and they look good. May I ask which these are? So I can account whether all parts have been reviewed?
1. The local variables/fields don't comply with the coding conventions, we have been following in the JDK. Removed all '_' from fields. Anything else? TOOLS_NOT_TO_TEST is formatted as the similar list in VersionCheck.java.
2. Hmm, someone parked a .ini file in bin directory, later removed, perhaps on windows simply check for .exe ? Should a check exist to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool. 172 static boolean notATool(String file) { 173 file = file.toLowerCase(); 174 if (file.endsWith(".dll") || 175 file.endsWith(".map") || 176 file.endsWith(".pdb") || 177 file.equals("server")) { // server subdir on windows. 178 return true; 179 } 180 return false; 181 } I anyways wondered why .dll + friends are in the exe directory and not in the lib directory. But a check for executable file is better than this list. Changed.
3. Typo in comment Fixed.
4. There is a method to check for isEnglishLocale in TestHelper, perhaps use it to have the test bale out in non english locales as early as possible ? I added a bailout at the beginning of the test. Thanks for the advice! But thinking about this a @requires englishLocale would be useful here.
5. Has this been tested on all platforms ? do you need help testing it ? Our testing is on ntamd64, linuxppc64, linuxppc64le, linuxs390x, linuxx86_64, sun_64 and aixppc64. I run the jdk jtreg tests on these platforms. I once ran the change with the jdk-hs repo, where we test test/hotspot/jtreg and most jck tests. All passed. I don't think this is very platform dependent (most differences are between unix/windows) so this platform coverage should suffice I think.
I'll post a new webrev later on. Best regards, Goetz.
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Hi, I uploaded a new webrev: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.04/ This includes the changes - to jshell requested by Robert - to the test as requested by Kumar. See also incremental patch and the test output including all the help messages referenced in the webrev. Best regards, Goetz.
-----Original Message----- From: Kumar Srinivasan [mailto:kumar.x.srinivasan@oracle.com] Sent: Montag, 27. November 2017 17:43 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler-dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions, we have been following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed, perhaps on windows simply check for .exe ? Should a check exist to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool. 172 static boolean notATool(String file) { 173 file = file.toLowerCase(); 174 if (file.endsWith(".dll") || 175 file.endsWith(".map") || 176 file.endsWith(".pdb") || 177 file.equals("server")) { // server subdir on windows. 178 return true; 179 } 180 return false; 181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps use it to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Thumbs up for JShell changes. -Robert
On Nov 28, 2017, at 3:16 AM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
I uploaded a new webrev: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.04/
This includes the changes - to jshell requested by Robert - to the test as requested by Kumar.
See also incremental patch and the test output including all the help messages referenced in the webrev.
Best regards, Goetz.
-----Original Message----- From: Kumar Srinivasan [mailto:kumar.x.srinivasan@oracle.com] Sent: Montag, 27. November 2017 17:43 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler-dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions, we have been following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed, perhaps on windows simply check for .exe ? Should a check exist to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool. 172 static boolean notATool(String file) { 173 file = file.toLowerCase(); 174 if (file.endsWith(".dll") || 175 file.endsWith(".map") || 176 file.endsWith(".pdb") || 177 file.equals("server")) { // server subdir on windows. 178 return true; 179 } 180 return false; 181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps use it to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Thanks, Robert! Best regards, Goetz.
-----Original Message----- From: Robert Field [mailto:robert.field@oracle.com] Sent: Mittwoch, 29. November 2017 19:31 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: Kumar Srinivasan <kumar.x.srinivasan@oracle.com>; serviceability-dev (serviceability-dev@openjdk.java.net) <serviceability- dev@openjdk.java.net>; compiler-dev@openjdk.java.net; core-libs- dev@openjdk.java.net Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Thumbs up for JShell changes.
-Robert
On Nov 28, 2017, at 3:16 AM, Lindenmaier, Goetz <goetz.lindenmaier@sap.com> wrote:
Hi,
I uploaded a new webrev: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.04/
This includes the changes - to jshell requested by Robert - to the test as requested by Kumar.
See also incremental patch and the test output including all the help messages referenced in the webrev.
Best regards, Goetz.
-----Original Message----- From: Kumar Srinivasan [mailto:kumar.x.srinivasan@oracle.com] Sent: Montag, 27. November 2017 17:43 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler-dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions, we have been following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed, perhaps on windows simply check for .exe ? Should a check exist to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool. 172 static boolean notATool(String file) { 173 file = file.toLowerCase(); 174 if (file.endsWith(".dll") || 175 file.endsWith(".map") || 176 file.endsWith(".pdb") || 177 file.equals("server")) { // server subdir on windows. 178 return true; 179 } 180 return false; 181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps use it to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html
Best regards, Goetz.
Hi, Besides my private comments to you, there are 2-3 internal tests which fail. Have you run all the langtools tests ? There are 4 Windows tests that fail with langtools: jdk/javadoc/doclet/testHelpOption/TestHelpOption.java jdk/javadoc/tool/CheckResourceKeys.java jdk/javadoc/tool/ToolProviderTest.java tools/javap/InvalidOptions.java tools/jdeps/MultiReleaseJar.java This changeset needs to be vetted thoroughly using internal build and test systems. Any Oracle engineer willing to sponsor this ? Kumar On 11/28/2017 3:16 AM, Lindenmaier, Goetz wrote:
Hi,
I uploaded a new webrev: http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.04/
This includes the changes - to jshell requested by Robert - to the test as requested by Kumar.
See also incremental patch and the test output including all the help messages referenced in the webrev.
Best regards, Goetz.
-----Original Message----- From: Kumar Srinivasan [mailto:kumar.x.srinivasan@oracle.com] Sent: Montag, 27. November 2017 17:43 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler-dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions, we have been following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed, perhaps on windows simply check for .exe ? Should a check exist to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool. 172 static boolean notATool(String file) { 173 file = file.toLowerCase(); 174 if (file.endsWith(".dll") || 175 file.endsWith(".map") || 176 file.endsWith(".pdb") || 177 file.equals("server")) { // server subdir on windows. 178 return true; 179 } 180 return false; 181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps use it to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8189102 CSR: https://bugs.openjdk.java.net/browse/JDK-8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017- October/028615.html Best regards, Goetz.
Hi Kumar, I'm already looking at the issues from your other mail. Detailed comments will follow. I'll also check and fix the new tests you report. I think we don't run the langtool tests, i.e. I know we didn't run them before the repos were merged. Obviously we should add them to our testing :) Thanks for further testing and best regards, Goetz.
-----Original Message----- From: Kumar Srinivasan [mailto:kumar.x.srinivasan@oracle.com] Sent: Freitag, 1. Dezember 2017 15:42 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net; 'compiler-dev@openjdk.java.net' <compiler-dev@openjdk.java.net>; serviceability-dev (serviceability- dev@openjdk.java.net) <serviceability-dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
Hi,
Besides my private comments to you, there are 2-3 internal tests which fail.
Have you run all the langtools tests ? There are 4 Windows tests that fail with langtools:
jdk/javadoc/doclet/testHelpOption/TestHelpOption.java jdk/javadoc/tool/CheckResourceKeys.java jdk/javadoc/tool/ToolProviderTest.java tools/javap/InvalidOptions.java tools/jdeps/MultiReleaseJar.java
This changeset needs to be vetted thoroughly using internal build and test systems. Any Oracle engineer willing to sponsor this ?
Kumar
On 11/28/2017 3:16 AM, Lindenmaier, Goetz wrote:
Hi,
I uploaded a new webrev: http://cr.openjdk.java.net/~goetz/wr17/8189102- helpMessage/webrev.04/
This includes the changes - to jshell requested by Robert - to the test as requested by Kumar.
See also incremental patch and the test output including all the help messages referenced in the webrev.
Best regards, Goetz.
-----Original Message----- From: Kumar Srinivasan [mailto:kumar.x.srinivasan@oracle.com] Sent: Montag, 27. November 2017 17:43 To: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> <mailto:goetz.lindenmaier@sap.com> Cc: core-libs-dev@openjdk.java.net <mailto:core-libs- dev@openjdk.java.net> ; 'compiler-dev@openjdk.java.net <mailto:compiler-dev@openjdk.java.net> ' <compiler-dev@openjdk.java.net> <mailto:compiler- dev@openjdk.java.net> ; serviceability-dev (serviceability- dev@openjdk.java.net <mailto:dev@openjdk.java.net> ) <serviceability-dev@openjdk.java.net> <mailto:serviceability- dev@openjdk.java.net> Subject: Re: RFR: 8189102: All tools should support -?, -h and - -help
Hi,
I looked at some of the components I maintain, and they look good.
Wrt. the test:
1. The local variables/fields don't comply with the coding conventions, we have been following in the JDK.
2. Hmm, someone parked a .ini file in bin directory, later removed, perhaps on windows simply check for .exe ? Should a check exist to verify if file has executable permissions ?"File.canExecute".
171 // Returns true if the file is not a tool. 172 static boolean notATool(String file) { 173 file = file.toLowerCase(); 174 if (file.endsWith(".dll") || 175 file.endsWith(".map") || 176 file.endsWith(".pdb") || 177 file.equals("server")) { // server subdir on windows. 178 return true; 179 } 180 return false; 181 }
3. Typo in comment
201 // Check whether 'flag' appears in 'line' as a word of itself. I must not
s/I must/It must/
4. There is a method to check for isEnglishLocale in TestHelper, perhaps use it to have the test bale out in non english locales as early as possible ?
5. Has this been tested on all platforms ? do you need help testing it ?
Kumar
On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
Hi,
please review this change. I also filed a CSR for this: http://cr.openjdk.java.net/~goetz/wr17/8189102-
helpMessage/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK- 8189102 CSR: https://bugs.openjdk.java.net/browse/JDK- 8191477
See the webrev for a detailed description of the changes.
If required, I'll make break-out changes to be reviewed separately.
I had posted a RFR before, but improved the change to give a more complete overview of currently supported flags for the CSR: http://mail.openjdk.java.net/pipermail/hotspot- dev/2017-
October/028615.html
Best regards, Goetz.
participants (6)
-
Alan Bateman
-
Jonathan Gibbons
-
Kumar Srinivasan
-
Lindenmaier, Goetz
-
Robert Field
-
Weijun Wang