Re: JDK 9 RFR of JDK-8039102: Add raw and unchecked lint warnings to build of jdk repository
It's always nice to see more of these getting enabled. Looks good! /Erik On 2014-04-08 19:54, Joe Darcy wrote: Hello, With the effort to clear the jdk repo of rawtypes and unchecked warnings (see JDK-8039096), I'd like to get an early review for the happy day when those warnings can be turned on in the build: JDK-8039102: Add raw and unchecked lint warnings to build of jdk repository The patch is as expected: diff -r f1cc18a769e5 make/Setup.gmk --- a/make/Setup.gmkTue Apr 08 17:36:13 2014 +0200 +++ b/make/Setup.gmkTue Apr 08 10:45:40 2014 -0700 @@ -27,7 +27,7 @@ # To build with all warnings enabled, do the following: # make JAVAC_WARNINGS=-Xlint:all -Xmaxwarns 1 -JAVAC_WARNINGS := -Xlint:-unchecked,-deprecation,-overrides,auxiliaryclass,cast,classfile,dep-ann,divzero,empty,overloads,serial,static,try,varargs -Werror +JAVAC_WARNINGS := -Xlint:-deprecation,-overrides,auxiliaryclass,cast,classfile,dep-ann,divzero,empty,overloads,rawtypes,serial,static,try,unchecked,varargs -Werror # Any java code executed during a JDK build to build other parts of the JDK must be # executed by the bootstrap JDK (probably with -Xbootclasspath/p: ) and for this Full webrev: http://cr.openjdk.java.net/~darcy/8039102.0/ There are several source code related lint warnings not currently enabled in the jdk build. fallthrough Warn about falling through from one case of a switch statement to the next. finally Warn about finally clauses that do not terminate normally. overridesWarn about issues regarding method overrides. rawtypesWarn about use of raw types. (see JDK-8039096) unchecked Warn about unchecked operations (see JDK-8039096) Once a few more of these are resolved, it will probably be time to switch JAVAC_WARNINGS more fully to an opt-out syntax for any remaining problem areas: -Xlint:all,-deprecation,-processing, ... Thanks, -Joe
Re: RFR: 8039411 : Add environment variable support to fixpath
Indentation look weird around line 460 and 470 (tab vs space?), otherwise it's ok to me. /Erik On 2014-04-08 21:31, Mike Duigou wrote: I have made the changes Tim suggested along with other cleanups, additional diagnostics and refinements. http://cr.openjdk.java.net/~mduigou/JDK-8039411/1/webrev/ I have not tested this change on mingw/msys and probably won't attempt to unless it is required. Mike On Apr 8 2014, at 08:10 , Tim Bell tim.b...@oracle.com wrote: Hi Mike Looks good - one thing to pick on is line 402 - I'd like to see what was in var if the setting fails. Tim On 04/08/14 08:35, Erik Joelsson wrote: Hello Mike, My C is a bit rusty, but I think it looks good in general. If you are able to test it on mingw/msys I think that would be good since it's a pretty big change. /Erik On 2014-04-07 23:53, Mike Duigou wrote: Hello all; Currently the fixpath utility used in windows builds expects that the first parameter it is passed will be the path of the executable. In some cases it's desirable to define environment variables which will apply during the execution of that executable. This change adds support for defining environment variables. The variables appear before the executable. Currently the command line parsing assumes that all arguments containing = before the command path are environment variables. (This precludes the executable having '=' in it's path, which is unlikely anyway). The remainder of the changes were lint warnings suggested by Visual C. (mostly const) https://bugs.openjdk.java.net/browse/JDK-8039411 http://cr.openjdk.java.net/~mduigou/JDK-8039411/0/webrev/ Mike
Re: RFR: 8039411 : Add environment variable support to fixpath
On Apr 8 2014, at 03:43 , Dave Pointon dpoin...@linux.vnet.ibm.com wrote: On Mon, 2014-04-07 at 14:53 -0700, Mike Duigou wrote: Hello all; http://cr.openjdk.java.net/~mduigou/JDK-8039411/0/webrev/ Mike Hiya Mike , My, you _were_ busy :-) Given that I'm not formally a reviewer, nor indeed, author, I thought you might be interested in the following observations ... I prefer an expert over a title any day of the week. :-) * I wonder if it would not be better (more readable) to utilise one of the POSIX standard string comparison library functions e.g. strcmp() or strncmp(), within the function body - matching the prototype update that you made. * The replacement of strcpy() by memmove() isn't clear to me in replace_cygdrive_cygwin() Both of these changes are to silence warnings from Visual Studio about use of string functions without explicit sizing. Since this code is Windows specific I've preferred the Windows library functions over the similar C Standard Library functions. I probably should have gone all the way and converted malloc to LocalAlloc, etc. Mike
RFR 8030011: Update Hotspot version string output
Please review this change to make the hotspot related output produced by java -version match the corresponding JDK output: webrev: http://cr.openjdk.java.net/~amurillo/9/8030011/ Bug: https://bugs.openjdk.java.net/browse/JDK-8030011 Note that we initially wanted to obtain more information from the repo being built and add it to the hotspot version output, but that will require changes in the JPRT, so we have decided to split the change in 2 bugs. One to make the version match (above webrev) and another one, an RFE, to add additional information extracted from the repo. Note that in the current version of vm_version.cpp, there is no error checking in product mode, I was suggested to make that explicit. Release Engineering did some testing and I also ran several cases with full and just hotspot JPRT builds. Here is a summary of how the new output compares to the old one: (1) Release Engineering builds (9-dev): from jdk9-dev build: java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00) Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00, mixed mode) compared with what we currently have java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h247-20140326-b06-b00) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (2) Release Engineering builds (jdk9): java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) Server VM (build 1.9.0-ea-b07, mixed mode) compared with what we currently have java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (3) JPRT Full builds: java version 1.9.0-internal # Java(TM) SE Runtime Environment (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00, mixed mode) (4) JPRT hotspot only builds: java version 1.9.0-ea-fastdebug # Java(TM) SE Runtime Environment (build 1.9.0-ea-fastdebug-b06) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404031820.amurillo.jdk9-hs-ver-str-HS-fastdebug, mixed mode) in this one the built VM is inserted into a promoted build bundle, since we do not have the JDK build number info in the hotspot repo, we can't match the build number in the JDK portion. With the RFE mentioned above, we can extract the build info from the repo and add it to the hotspot portion. I want to mention, that this may change once the new JDK version change is implemented but we don't know when that will be implemented yet, so we need to go ahead with this to get rid of the old hotspot express output. And most of these changes will still have to be done anyways BTW, john Coomes and Dan Daugherty provided feedback in some pieces of this webrev, but I need full reviews now. Thanks -- Alejandro
JDK 9 RFR of JDK-8039865: Add fallthrough lint warning to build of jdk repository
Hello, There are a relatively small number of fallthrough lint warnings in the sources of the JDK 9 jdk repo. Once those warnings are cleared (JDK-8039501), the fallthough lint option should be added to the build: diff -r f1cc18a769e5 make/Setup.gmk --- a/make/Setup.gmkTue Apr 08 17:36:13 2014 +0200 +++ b/make/Setup.gmkWed Apr 09 18:41:21 2014 -0700 @@ -27,7 +27,7 @@ # To build with all warnings enabled, do the following: # make JAVAC_WARNINGS=-Xlint:all -Xmaxwarns 1 -JAVAC_WARNINGS := -Xlint:-unchecked,-deprecation,-overrides,auxiliaryclass,cast,classfile,dep-ann,divzero,empty,overloads,serial,static,try,varargs -Werror +JAVAC_WARNINGS := -Xlint:-unchecked,-deprecation,-overrides,auxiliaryclass,cast,classfile,dep-ann,divzero,empty,fallthrough,overloads,serial,static,try,varargs -Werror # Any java code executed during a JDK build to build other parts of the JDK must be g # executed by the bootstrap JDK (probably with -Xbootclasspath/p: ) and for this Thanks, -Joe
Re: RFR 8030011: Update Hotspot version string output
Hi Alejandro, Given we have to maintain the JDK version information in two places (top level repo and hotspot repo) wouldn't it have been simpler to keep hotspot_version file and HOTSPOT_RELEASE_VERSION and simply set the major/minor/build values to match those of the JDK version? That aside, in jdk_version file hotspot copyright should now be 2014 */vm.make: This line is way too long. ! VM_VER_DEFS = -DHOTSPOT_RELEASE_VERSION=\$(HS_BUILD_VER)\ -DJRE_RELEASE_VERSION=\$(JRE_RELEASE_VER)\ -DJDK_MAJOR_VERSION=\$(JDK_MAJOR_VERSION)\ -DJDK_MINOR_VERSION=\$(JDK_MINOR_VERSION)\ -DJDK_MICRO_VERSION=\$(JDK_MICRO_VERSION)\ -DJDK_BUILD_NUMBER=\$(JDK_BUILD_NUMBER)\ Not clear why we suddenly need defines for all the component pieces either. You could have kept the HS_XXX variables, just adding the micro part. David On 10/04/2014 10:15 AM, Alejandro E Murillo wrote: Please review this change to make the hotspot related output produced by java -version match the corresponding JDK output: webrev: http://cr.openjdk.java.net/~amurillo/9/8030011/ Bug: https://bugs.openjdk.java.net/browse/JDK-8030011 Note that we initially wanted to obtain more information from the repo being built and add it to the hotspot version output, but that will require changes in the JPRT, so we have decided to split the change in 2 bugs. One to make the version match (above webrev) and another one, an RFE, to add additional information extracted from the repo. Note that in the current version of vm_version.cpp, there is no error checking in product mode, I was suggested to make that explicit. Release Engineering did some testing and I also ran several cases with full and just hotspot JPRT builds. Here is a summary of how the new output compares to the old one: (1) Release Engineering builds (9-dev): from jdk9-dev build: java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00) Java HotSpot(TM) 64-Bit Server VM (build 1.9.0-ea-langtools-nightly-h257-20140328-b07-b00, mixed mode) compared with what we currently have java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-langtools-nightly-h247-20140326-b06-b00) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (2) Release Engineering builds (jdk9): java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) Server VM (build 1.9.0-ea-b07, mixed mode) compared with what we currently have java version 1.9.0-ea Java(TM) SE Runtime Environment (build 1.9.0-ea-b07) Java HotSpot(TM) 64-Bit Server VM (build 25.0-b62, mixed mode) (3) JPRT Full builds: java version 1.9.0-internal # Java(TM) SE Runtime Environment (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404091627.amurillo.jdk9-hs-ver-str-co-b00, mixed mode) (4) JPRT hotspot only builds: java version 1.9.0-ea-fastdebug # Java(TM) SE Runtime Environment (build 1.9.0-ea-fastdebug-b06) # Java HotSpot(TM) Server VM (build 1.9.0-internal-201404031820.amurillo.jdk9-hs-ver-str-HS-fastdebug, mixed mode) in this one the built VM is inserted into a promoted build bundle, since we do not have the JDK build number info in the hotspot repo, we can't match the build number in the JDK portion. With the RFE mentioned above, we can extract the build info from the repo and add it to the hotspot portion. I want to mention, that this may change once the new JDK version change is implemented but we don't know when that will be implemented yet, so we need to go ahead with this to get rid of the old hotspot express output. And most of these changes will still have to be done anyways BTW, john Coomes and Dan Daugherty provided feedback in some pieces of this webrev, but I need full reviews now. Thanks
