RFR: JDK-8085822 JEP 223: New Version-String Scheme (initial integration)

Daniel D. Daugherty daniel.daugherty at oracle.com
Mon Jun 8 23:31:20 UTC 2015 

 > 
http://cr.openjdk.java.net/~ihse/JDK-8085822-JEP-223-initial-patch/webrev.01 


General comment: Not all copyright years were updated.
General comment: It looks like support for the 'patch' value is not 
completely
     implemented through all the Makefiles. I didn't audit for this, but 
it's
     just my impression.

common/autoconf/configure.ac
     No comments.

common/autoconf/flags.m4
     No comments.

common/autoconf/generated-configure.sh
     There are multiple Copyright notices in this file. Why?

     L4076: # Verify that a given string represent a valid version 
number, and assing it to
     L4077: # a variable.
         Fixed two typos and reformat a bit:
           # Verify that a given string represents a valid version 
number, and
           # assigning it to a variable.

     L20186-20189: indent for the block is off

     L20256-20259: indent for the block is off

     L20262:     as_fn_error $? "--with--version-string must have a 
value" "$LINENO" 5
         The '--with--version...' part doesn't match previous usages where
         '--with-version...' is used

     L20275:       # Unspecified numerical fields is interpreted as 0.
         Grammar: 'is interpreted' -> 'are interpreted'

     L20286:         as_fn_error $? "Version string contains + but both 
'BUILD' and 'OPT' is missing" "$LINENO" 5
         Grammar: 'is missing' -> 'are missing'

     L20292:       as_fn_error $? "--with--version-string fails to parse
         The '--with--version...' part doesn't match previous usages where
         '--with-version...' is used

     L20297-L20302: indent for the block is off

     L20307:       as_fn_error $? "--with--version-pre-base must have a 
value" "$LINENO" 5
     L20315:         { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: 
--with--version-pre-base value...
     L20316: $as_echo "$as_me: WARNING: --with--version-pre-base value...
         The '--with--version...' part doesn't match previous usages where
         '--with-version...' is used

     L20327-20332: indent for the block is off

     L20337:       as_fn_error $? "--with--version-pre-debuglevel must 
have...
     L20345:         { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: 
--with--version-pre-debuglevel value...
     L20346: $as_echo "$as_me: WARNING: --with--version-pre-debuglevel value
         The '--with--version...' part doesn't match previous usages where
         '--with-version...' is used

     L20361-20366: indent for the block is off

     L20371:       as_fn_error $? "--with--version-opt must have...
     L20379:         { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: 
--with--version-opt value
     L20380: $as_echo "$as_me: WARNING: --with--version-opt value has been
         The '--with--version...' part doesn't match previous usages where
         '--with-version...' is used

         At this point, I'm going to stop pointing out the 
'--with-version...'
         and '--with--version...' differences; don't know which usage is 
right.

     L20388-L20388: indent is off by one

     L20388:        username=`$ECHO "$USER" | $TR -d -c '[a-z][A-Z][0-9]'`
         This assumes that the "USER" variable is set. Should there
         be a check for "" and perhaps use "no_user_specified" or
         something like that? Perhaps the build setup always makes
         sure that "USER" is set to something. Don't know.

     L20395-L20400: indent for the block is off

     L20413-L20431: indent of all blocks in this range are off

     L20444-L20449: indent for the block is off

     L20457-L20475: indent of all blocks in this range are off

     L20486-L20491: indent for the block is off

     L20504-L20522: indent of all blocks in this range are off

     L20533-L20538: indent for the block is off

     L20551-L20569: indent of all blocks in this range are off

     L20580-L20585: indent for the block is off

     L20598-L20616: indent of all blocks in this range are off

common/autoconf/help.m4
     No comments.

common/autoconf/jdk-options.m4
     Don't know why the 'elliptic curve crypto implementation' support
     is relocated as part of this changeset, but ...

     No comments.

common/autoconf/spec.gmk.in
     No comments.

common/autoconf/version-numbers
     No comments.

common/nb_native/nbproject/configurations.xml
     No comments.

make/Images.gmk
     No comments.

make/Install.gmk
     No comments.

make/Javadoc.gmk
     Did you mean to remove the 'clean' target?

make/JrtfsJar.gmk
     No comments.

make/MacBundles.gmk
     No comments.

make/jprt.properties
     No comments.

hotspot/make/Makefile
     No comments.

hotspot/make/aix/Makefile
     No comments.

hotspot/make/aix/makefiles/buildtree.make
     No comments.

hotspot/make/aix/makefiles/defs.make
     No comments.

hotspot/make/aix/makefiles/vm.make
     No comments.

hotspot/make/bsd/Makefile
     No comments.

hotspot/make/bsd/makefiles/buildtree.make
     No comments.

hotspot/make/bsd/makefiles/defs.make
     No comments.

hotspot/make/bsd/makefiles/vm.make
     No comments.

hotspot/make/defs.make
     No comments.

hotspot/make/jdk_version
     No comments.

hotspot/make/linux/Makefile
     No comments.

hotspot/make/linux/makefiles/buildtree.make
     No comments.

hotspot/make/linux/makefiles/defs.make
     No comments.

hotspot/make/linux/makefiles/vm.make
     No comments.

hotspot/make/solaris/Makefile
     No comments.

hotspot/make/solaris/makefiles/buildtree.make
     No comments.

hotspot/make/solaris/makefiles/defs.make
     No comments.

hotspot/make/solaris/makefiles/sparcWorks.make
     No comments.

hotspot/make/solaris/makefiles/vm.make
     No comments.

hotspot/make/windows/build.make
     No comments.

hotspot/make/windows/makefiles/compile.make
     No changes in the frames view.
     Update: udiff view shows a blank line deleted at the end of the file.

hotspot/make/windows/makefiles/debug.make
     No comments.

hotspot/make/windows/makefiles/defs.make
     No comments.

hotspot/make/windows/makefiles/fastdebug.make
     No comments.

hotspot/make/windows/makefiles/product.make
     No comments.

hotspot/make/windows/makefiles/vm.make
     No comments.

hotspot/make/windows/projectfiles/common/Makefile
     No comments.

hotspot/src/share/vm/prims/jvm.h
     No comments.

hotspot/src/share/vm/runtime/arguments.cpp
     No comments.

hotspot/src/share/vm/runtime/java.cpp
     L661: void JDK_Version::fully_initialize(
     L662:     uint8_t major, uint8_t minor, uint8_t security, uint8_t 
update) {
     L663:   // This is only called when current is less than 1.6 and 
we've gotten

         Since you're ripping out vestigial version support, I think this
         function can go away since this is version 9 and newer. Don't 
really
         know for sure, but based on that comment...

hotspot/src/share/vm/runtime/java.hpp
     No comments.

hotspot/src/share/vm/runtime/vmStructs.cpp
     L1240: please make the 'int' parameter align like the rest.

hotspot/src/share/vm/runtime/vm_version.cpp
     L84: void Abstract_VM_Version::initialize() {
     L85:   // FIXME: Initialization can probably be removed now.
         I agree, but that would entail also removing the
         _initialized and the uses of it... Follow on bug fix?

hotspot/src/share/vm/runtime/vm_version.hpp
     No comments.

hotspot/test/runtime/6981737/Test6981737.java
     No comments.

jdk/make/CompileDemos.gmk
     No comments.

jdk/make/data/mainmanifest/manifest.mf
     No comments.

jdk/make/gensrc/GensrcMisc.gmk
     No comments.

jdk/make/launcher/Launcher-jdk.accessibility.gmk
     No comments.

jdk/make/launcher/Launcher-jdk.pack200.gmk
     No comments.

jdk/make/launcher/LauncherCommon.gmk
     No comments.

jdk/make/lib/CoreLibraries.gmk
     No comments.

jdk/src/java.base/share/classes/sun/misc/Version.java.template
     L149:      * Returns the security version of the running JVM if 
it's 1.6 or newer
         This JavaDoc update is wrong, but it might not be important
         if sun.misc.Version class is going away.

jdk/src/java.base/share/native/include/jvm.h
     No comments.

jdk/src/java.base/share/native/launcher/defines.h
     No comments.

jdk/src/java.base/share/native/launcher/main.c
     No comments.

jdk/src/java.base/share/native/libjava/System.c
     No comments.

jdk/src/java.base/share/native/libjava/Version.c
     No comments.

jdk/src/java.base/share/native/libjava/jdk_util.c
     No comments.

jdk/src/java.base/windows/native/common/version.rc
     No comments.

jdk/src/java.desktop/windows/native/libawt/windows/awt.rc
     No comments.

jdk/src/jdk.accessibility/windows/native/common/AccessBridgeStatusWindow.RC
     No comments.

jdk/test/sun/misc/Version/Version.java
     No comments.

langtools/make/gensrc/GensrcCommon.gmk
     No comments.

langtools/src/java.compiler/share/classes/javax/lang/model/SourceVersion.java
     old L171:                 case "1.9":
     new L171:                 case "9":
         Should this logic support both versions? Will dropping
         "1.9" here prevent a pre-version changeset JVM from
         being dropped into a JDK for triage purposes?

         Granted we don't often triage 'javac' with different JVMs, but...

langtools/test/tools/javac/options/modes/InfoOptsTest.java
     No comments.

langtools/test/tools/javac/options/modes/SourceTargetTest.java
     No comments.

nashorn/make/BuildNashorn.gmk
     No comments.

nashorn/make/build.xml
     No comments.

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Version.java
     No comments.

common/autoconf/jdk-version.m4
     No comments.

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/version.properties.template 
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/version.properties-template
     No comments.

common/bin/test_builds.sh
hotspot/make/jdk6_hotspot_distro
     No comments.

Dan


On 6/5/15 8:07 AM, Magnus Ihse Bursie wrote:
> This review request covers the main part of the work for JEP-223, the 
> new version string format [1]. Basically, we'll call this release Java 
> "9", instead of Java "1.9.0".
>
> This patch is a folding of all work that has been done so far in the 
> branch JEP-223-branch in jdk9/sandbox. As you can see, it mostly 
> covers build changes, with some code changes in hotspot, jdk, nashorn 
> and langtools that either are corresponding changes in the product 
> code due to the compiler define flags changing from the build, or 
> follow-up changes to handle the new format.
>
> The JEP-223 work is not finished by this patch. In fact, there are 
> known issues remaining even after this patch, typically by code that 
> reads the "java.version" property and tries to parse it. However, this 
> patch is not directly destined for jdk9/dev, but will go into the 
> special verona/stage forest. As for all patches destined for 
> verona/stage it will be code reviewed as if going to jdk9/dev. Once in 
> verona/stage it will bide its time, and it will be complemented with 
> follow-up patches to address remaining issues. When all such issues 
> are resolved and JEP-223 is fully implemented, all changes will be 
> pushed at once (without further code reviews) into jdk9/dev.
>
> This patch has been contributed by Magnus Ihse Bursie, Kumar 
> Srinivasan and Alejandro Murillo.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8085822
> WebRev: 
> http://cr.openjdk.java.net/~ihse/JDK-8085822-JEP-223-initial-patch/webrev.01
>
> [1] http://openjdk.java.net/jeps/223
>

More information about the compiler-dev mailing list