> 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


Reply via email to