Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-7292

Change build system to use "--release 8" instead of "-source/-target" when invoking javac

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 6.0
    • Fix Version/s: 6.1, 6.x, 7.0, 6.3
    • Component/s: general/build
    • Labels:
    • Lucene Fields:
      New

      Description

      Currently we pass -source 1.8 -target 1.8 to javac and javadoc when compiling our source code. We all know that this brings problems, because cross-compiling does not really work. We create class files that are able to run on Java 8, but when it is compiled with java 9, it is not sure that some code may use Java 9 APIs that are not available in Java 8. Javac prints a warning about this (it complains about the bootclasspath not pointing to JDK 8 when used with source/target 1.8).

      Java 8 is the last version of Java that has this trap. From Java 9 on, instead of passing source and target, the recommended way is to pass a single -release 8 parameter to javac (see http://openjdk.java.net/jeps/247). This solves the bootsclasspath problem, because it has all the previous java versions as "signatures" (like forbiddenapis), including deprecated APIs,... everything included. You can find this in the $JAVA_HOME/lib/ct.sym file (which is a ZIP file, so you can open it with a ZIP tool of your choice). In Java 9+, this file also contains all old APIs from Java 6+.

      When invoking the compiler with -release 8, there is no risk of accidentally using API from newer versions.

      The migration here is quite simple: As we require Java 8 already, there is (theoretically) no need to pass source and target anymore. It is enough to just pass -release 8 if we detect Java 9 as compiling JVM. Nevertheless I plan to do the following:

      • remove properties javac.source and javac.target from Ant build
      • add javac.release property and define it to be "8" (not "1.8", this is new version styling that also works with Java 8+ already)
      • remove attributes in the <javac source="..." target="..."/> calls
      • add a new Ant property javac.release.args that is dynamically evaluated inside our compile macro: On Java 9 it evaluates to -release ${javac.release}, for java 8 it uses -source ${javac.release} -target ${javac.release} for backwards compatibility
      • pass this new arg to javac as <arg line="..."/>

      By this we could theoretically remove the check from smoketester about the compiling JDK (the MANIFEST check), because although compiled with Java 9, the class files were actually compiled against the old Java API from ct.sym file.

      I will also align the warnings to reenable -Xlint:options.

      1. LUCENE-7292.patch
        15 kB
        Uwe Schindler
      2. LUCENE-7292.patch
        11 kB
        Uwe Schindler
      3. LUCENE-7292.patch
        10 kB
        Uwe Schindler
      4. LUCENE-7292.patch
        10 kB
        Uwe Schindler

        Activity

        Hide
        thetaphi Uwe Schindler added a comment -

        Preliminary patch.

        Unfortunately neither forbiddenapis nor javacc currently support "8" as a valid release version. I have to hack around this. Forbiddenapis 2.1 will support a new syntax for jdk releases, but not the currently used 2.0.

        I think I will add preliminary extra argument that can be passed to those legacy tools.

        Show
        thetaphi Uwe Schindler added a comment - Preliminary patch. Unfortunately neither forbiddenapis nor javacc currently support "8" as a valid release version. I have to hack around this. Forbiddenapis 2.1 will support a new syntax for jdk releases, but not the currently used 2.0. I think I will add preliminary extra argument that can be passed to those legacy tools.
        Hide
        thetaphi Uwe Schindler added a comment - - edited

        Patch with workaround for forbiddenapis and javacc. It also removes the debug output that I added.

        Not yet solved/discussed is smoke tester. This includes how to handle manifest's <attribute name="X-Compile-Source-JDK" value="${javac.release}"/>.

        Show
        thetaphi Uwe Schindler added a comment - - edited Patch with workaround for forbiddenapis and javacc. It also removes the debug output that I added. Not yet solved/discussed is smoke tester. This includes how to handle manifest's <attribute name="X-Compile-Source-JDK" value="${javac.release}"/> .
        Hide
        thetaphi Uwe Schindler added a comment -

        Currently the manifest looks like this:

        Manifest-Version: 1.0
        Ant-Version: Apache Ant 1.8.3
        Created-By: 9-ea+118 (Oracle Corporation)
        Extension-Name: org.apache.lucene
        Specification-Title: Lucene Search Engine: core
        Specification-Version: 7.0.0
        Specification-Vendor: The Apache Software Foundation
        Implementation-Title: org.apache.lucene
        Implementation-Version: 7.0.0-SNAPSHOT 6113e1f2fabf6668b4bdbd7640af45b
         ebcc2e505 - Uwe Schindler - 2016-05-19 16:30:11
        Implementation-Vendor: The Apache Software Foundation
        X-Compile-Source-JDK: 8
        X-Compile-Target-JDK: 8
        

        I think this is fine for now, smoketester still fails with Java 9, but thats a different issue to decide. Michael McCandless?

        Show
        thetaphi Uwe Schindler added a comment - Currently the manifest looks like this: Manifest-Version: 1.0 Ant-Version: Apache Ant 1.8.3 Created-By: 9-ea+118 (Oracle Corporation) Extension-Name: org.apache.lucene Specification-Title: Lucene Search Engine: core Specification-Version: 7.0.0 Specification-Vendor: The Apache Software Foundation Implementation-Title: org.apache.lucene Implementation-Version: 7.0.0-SNAPSHOT 6113e1f2fabf6668b4bdbd7640af45b ebcc2e505 - Uwe Schindler - 2016-05-19 16:30:11 Implementation-Vendor: The Apache Software Foundation X-Compile-Source-JDK: 8 X-Compile-Target-JDK: 8 I think this is fine for now, smoketester still fails with Java 9, but thats a different issue to decide. Michael McCandless ?
        Hide
        thetaphi Uwe Schindler added a comment -

        This patch also makes the "compact" profile options work correct with Java 9.

        Show
        thetaphi Uwe Schindler added a comment - This patch also makes the "compact" profile options work correct with Java 9.
        Hide
        mikemccand Michael McCandless added a comment -

        What check is smoke tester angry about with java 9?

        Show
        mikemccand Michael McCandless added a comment - What check is smoke tester angry about with java 9?
        Hide
        thetaphi Uwe Schindler added a comment - - edited

        Mike I mean the check that validates if all JAR files were compiled against the "official" JDK version (in our case 8). We have to fix this to accept the right things or remove completely, because now we could also compile against Java 9 and the class files are guaranteed to work with Java 8, too (as javac checks the signatures against Java 8 signatures with -release 8.

        Show
        thetaphi Uwe Schindler added a comment - - edited Mike I mean the check that validates if all JAR files were compiled against the "official" JDK version (in our case 8). We have to fix this to accept the right things or remove completely, because now we could also compile against Java 9 and the class files are guaranteed to work with Java 8, too (as javac checks the signatures against Java 8 signatures with -release 8 .
        Hide
        mikemccand Michael McCandless added a comment -

        OK should we just remove the check?

        Show
        mikemccand Michael McCandless added a comment - OK should we just remove the check?
        Hide
        thetaphi Uwe Schindler added a comment - - edited

        OK should we just remove the check?

        That is the open question that I wanted to start a discussion on. In my opinion it is obsolete for the reasns posted before:

        • Java 8 is the last version that have the trap of not correctly cross-compiling. As we require Java 8 already, you can only compile agans Java 8 or the prereleases of Java 9
        • Java 9 allows cross-compiling, because it useds the old method/classsignatures while compiling

        The only thing that we cannot check is that it really compiles against old Java 8 when release manager only excutes against Java 9 (at some point in future). A compile bug could make code compile in Java 9, but fail on Java 8's compiler.

        Because of this I am unsure if we should still require the release manager to compile against the "offcial version".

        Show
        thetaphi Uwe Schindler added a comment - - edited OK should we just remove the check? That is the open question that I wanted to start a discussion on. In my opinion it is obsolete for the reasns posted before: Java 8 is the last version that have the trap of not correctly cross-compiling. As we require Java 8 already, you can only compile agans Java 8 or the prereleases of Java 9 Java 9 allows cross-compiling, because it useds the old method/classsignatures while compiling The only thing that we cannot check is that it really compiles against old Java 8 when release manager only excutes against Java 9 (at some point in future). A compile bug could make code compile in Java 9, but fail on Java 8's compiler. Because of this I am unsure if we should still require the release manager to compile against the "offcial version".
        Hide
        thetaphi Uwe Schindler added a comment -

        In addition, Javadocs does not yet support -release. But this should change hopefully.

        Show
        thetaphi Uwe Schindler added a comment - In addition, Javadocs does not yet support -release . But this should change hopefully.
        Hide
        thetaphi Uwe Schindler added a comment -

        Steve Rowe: Do you know if the Maven build could be fixed to do the same? Maybe by updating the compiler plugin version to a Java 9 compatible one?

        Show
        thetaphi Uwe Schindler added a comment - Steve Rowe : Do you know if the Maven build could be fixed to do the same? Maybe by updating the compiler plugin version to a Java 9 compatible one?
        Hide
        steve_rowe Steve Rowe added a comment -

        Uwe, it looks like there are several Java9-related maven problems: https://cwiki.apache.org/confluence/display/MAVEN/Java+9+-+Jigsaw, and none of the affected plugins has released fixes yet, though it appears that SNAPSHOT versions address (some of?) the problems.

        I can see that the above-linked maven-compiler-plugin SNAPSHOT source includes support for -release: from https://svn.apache.org/viewvc/maven/plugins/branches/maven-compiler-plugin_jigsaw-ea/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java?view=markup#l146:

        146:    /**
        147:	     * The -release argument for the Java compiler, supported since Java9
        148:	     */
        149:	    @Parameter( property = "maven.compiler.release" )
        150:	    protected String release;
        
        Show
        steve_rowe Steve Rowe added a comment - Uwe, it looks like there are several Java9-related maven problems: https://cwiki.apache.org/confluence/display/MAVEN/Java+9+-+Jigsaw , and none of the affected plugins has released fixes yet, though it appears that SNAPSHOT versions address (some of?) the problems. I can see that the above-linked maven-compiler-plugin SNAPSHOT source includes support for -release : from https://svn.apache.org/viewvc/maven/plugins/branches/maven-compiler-plugin_jigsaw-ea/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java?view=markup#l146 : 146: /** 147: * The -release argument for the Java compiler, supported since Java9 148: */ 149: @Parameter( property = "maven.compiler.release" ) 150: protected String release;
        Hide
        thetaphi Uwe Schindler added a comment -

        Updated patch: The Javadocs command also supports "-release", so I was able to apply the same trick.

        Unfortunately, the javadoc command fails for other reasons (and is disabled on Jenkins with Java 9), but it generally works.

        Show
        thetaphi Uwe Schindler added a comment - Updated patch: The Javadocs command also supports "-release", so I was able to apply the same trick. Unfortunately, the javadoc command fails for other reasons (and is disabled on Jenkins with Java 9), but it generally works.
        Hide
        thetaphi Uwe Schindler added a comment - - edited

        Hi,
        I keep the Maven build unmodified for now, I will just update some version numbers. I will commit the Ant changes soon. I will upload the final patch in a minute.
        Uwe

        Show
        thetaphi Uwe Schindler added a comment - - edited Hi, I keep the Maven build unmodified for now, I will just update some version numbers. I will commit the Ant changes soon. I will upload the final patch in a minute. Uwe
        Hide
        thetaphi Uwe Schindler added a comment -

        Updated patch. Will commit soon.

        Show
        thetaphi Uwe Schindler added a comment - Updated patch. Will commit soon.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit b9caf83bfaf14d4571c1e309ac30b3cc0b7a223a in lucene-solr's branch refs/heads/master from Uwe Schindler
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b9caf83 ]

        LUCENE-7292: Use '-release' instead of '-source/-target' during compilation on Java 9+ to ensure real cross-compilation

        Show
        jira-bot ASF subversion and git services added a comment - Commit b9caf83bfaf14d4571c1e309ac30b3cc0b7a223a in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b9caf83 ] LUCENE-7292 : Use '-release' instead of '-source/-target' during compilation on Java 9+ to ensure real cross-compilation
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 7bd6d94952cb0710415c73b038963bf92275d2fb in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7bd6d94 ]

        LUCENE-7292: Use '-release' instead of '-source/-target' during compilation on Java 9+ to ensure real cross-compilation

        Show
        jira-bot ASF subversion and git services added a comment - Commit 7bd6d94952cb0710415c73b038963bf92275d2fb in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7bd6d94 ] LUCENE-7292 : Use '-release' instead of '-source/-target' during compilation on Java 9+ to ensure real cross-compilation
        Hide
        thetaphi Uwe Schindler added a comment -

        I also updated smoke tester to accept new JDK versioning. But I did not remove the extra check for the Java version that was used to compile.

        Show
        thetaphi Uwe Schindler added a comment - I also updated smoke tester to accept new JDK versioning. But I did not remove the extra check for the Java version that was used to compile.
        Hide
        thetaphi Uwe Schindler added a comment -

        Build 135 of Java 9 changed to use "long GNU style options", so -release changed to --release. Reopnening to fix build.xml for Java 9. This also requires to update Jenkins to at least this build.

        See http://mail.openjdk.java.net/pipermail/compiler-dev/2016-September/010358.html
        And: http://mail.openjdk.java.net/pipermail/compiler-dev/2016-September/010357.html

        Show
        thetaphi Uwe Schindler added a comment - Build 135 of Java 9 changed to use "long GNU style options", so -release changed to --release . Reopnening to fix build.xml for Java 9. This also requires to update Jenkins to at least this build. See http://mail.openjdk.java.net/pipermail/compiler-dev/2016-September/010358.html And: http://mail.openjdk.java.net/pipermail/compiler-dev/2016-September/010357.html
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 3712bf58196cd0bd56fad213547dee12029e7cbf in lucene-solr's branch refs/heads/master from Uwe Schindler
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3712bf5 ]

        LUCENE-7292: Fix build to use "--release 8" instead of "-release 8" on Java 9 (this changed with recent EA build b135)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 3712bf58196cd0bd56fad213547dee12029e7cbf in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3712bf5 ] LUCENE-7292 : Fix build to use "--release 8" instead of "-release 8" on Java 9 (this changed with recent EA build b135)
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit b67a062f9db6372cf654a4366233e953c89f2722 in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b67a062 ]

        LUCENE-7292: Fix build to use "--release 8" instead of "-release 8" on Java 9 (this changed with recent EA build b135)

        Show
        jira-bot ASF subversion and git services added a comment - Commit b67a062f9db6372cf654a4366233e953c89f2722 in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b67a062 ] LUCENE-7292 : Fix build to use "--release 8" instead of "-release 8" on Java 9 (this changed with recent EA build b135)
        Hide
        shalinmangar Shalin Shekhar Mangar added a comment -

        Closing after 6.3.0 release.

        Show
        shalinmangar Shalin Shekhar Mangar added a comment - Closing after 6.3.0 release.

          People

          • Assignee:
            thetaphi Uwe Schindler
            Reporter:
            thetaphi Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development