RFR: 8189102: All tools should support -?, -h and --help

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Tue Nov 28 08:50:22 UTC 2017


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.
> >



More information about the compiler-dev mailing list