Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.0
    • Component/s: None
    • Labels:
    • Lucene Fields:
      New

      Description

      The dev list thread "[VOTE] Move trunk to Java 8" passed.
      http://markmail.org/thread/zcddxioz2yvsdqkc

      This issue is to actually move trunk to java 8.

      1. LUCENE-5950.patch
        1.36 MB
        Ryan Ernst
      2. LUCENE-5950.patch
        52 kB
        Ryan Ernst
      3. LUCENE-5950.patch
        36 kB
        Ryan Ernst
      4. LUCENE-5950.patch
        36 kB
        Ryan Ernst
      5. LUCENE-5950.patch
        54 kB
        Ryan Ernst
      6. LUCENE-5950.patch
        35 kB
        Ryan Ernst
      7. LUCENE-5950-javadocpatcher.patch
        3 kB
        Uwe Schindler

        Activity

        Hide
        rjernst Ryan Ernst added a comment -

        This patch moves trunk to java 8. However, it doesn't yet work! Java 8u20 appears to have a bug when trying to compile with source as 1.8:
        https://bugs.openjdk.java.net/browse/JDK-8056984?page=com.atlassian.streams.streams-jira-plugin:activity-stream-issue-tab

        Still, I am putting up this patch so that when u40 is released, we can be ready. This patch was against git hash 611fa4956377d0448f7038f70d3be36ec882c1e6 and svn revision 1624882.

        Show
        rjernst Ryan Ernst added a comment - This patch moves trunk to java 8. However, it doesn't yet work! Java 8u20 appears to have a bug when trying to compile with source as 1.8: https://bugs.openjdk.java.net/browse/JDK-8056984?page=com.atlassian.streams.streams-jira-plugin:activity-stream-issue-tab Still, I am putting up this patch so that when u40 is released, we can be ready. This patch was against git hash 611fa4956377d0448f7038f70d3be36ec882c1e6 and svn revision 1624882 .
        Hide
        thetaphi Uwe Schindler added a comment -

        I will contact Rory from Oracle about that. We might get the fix into the next bugfix release.

        Also the issue mentions a workaround to split some statement into two. Maybe we can fix this.

        Show
        thetaphi Uwe Schindler added a comment - I will contact Rory from Oracle about that. We might get the fix into the next bugfix release. Also the issue mentions a workaround to split some statement into two. Maybe we can fix this.
        Hide
        rjernst Ryan Ernst added a comment -

        New patch which avoids the javac issue (see SpanMultiTermQueryWrapper). Lucene tests pass, solr tests have security manager failures in CachingDirectoryFactoryTest. I also left the JRE_IS_MINIMUM_JAVA8 constant because the morphline tests have an assumeFalse on this...which means the tests simply should not be run on trunk? It was unclear if the issue had been addressed (looks like it was from almost a year ago, months before the GA release).

        Show
        rjernst Ryan Ernst added a comment - New patch which avoids the javac issue (see SpanMultiTermQueryWrapper ). Lucene tests pass, solr tests have security manager failures in CachingDirectoryFactoryTest. I also left the JRE_IS_MINIMUM_JAVA8 constant because the morphline tests have an assumeFalse on this...which means the tests simply should not be run on trunk? It was unclear if the issue had been addressed (looks like it was from almost a year ago, months before the GA release).
        Hide
        rjernst Ryan Ernst added a comment -

        One more patch, that is now up to date with trunk (CachingDirectoryFactoryTest appears to succeed now).

        Show
        rjernst Ryan Ernst added a comment - One more patch, that is now up to date with trunk (CachingDirectoryFactoryTest appears to succeed now).
        Hide
        rjernst Ryan Ernst added a comment -

        Note the patch has a stupid mistake on the changes entry. I have it fixed locally now..

        Show
        rjernst Ryan Ernst added a comment - Note the patch has a stupid mistake on the changes entry. I have it fixed locally now..
        Hide
        thetaphi Uwe Schindler added a comment - - edited

        There are some changes in common-build that are wrong: The workaround for the Ant bug is exactly like that, that we pass "build.compiler=javac1.7" for every version >= 1.8, because Ant 1.8.3 and 1.8.4 have a bug, so we must pass javac1.7 (see link supplied in comment). So don't change that condition! Because we are now on minimum Java 8, you can set build.compiler always to javac1.7 as workaround:

          <!-- workaround for https://issues.apache.org/bugzilla/show_bug.cgi?id=53347 -->
          <condition property="build.compiler" value="javac1.7">
            <or>
              <antversion exactly="1.8.3" />
              <antversion exactly="1.8.4" />
            </or>
          </condition>
        

        Also some conditions/fails are commented out (also in root build.xml regarding smoke tester). The "needed minimum Java 8" <fail/> was commented out, why?

        Show
        thetaphi Uwe Schindler added a comment - - edited There are some changes in common-build that are wrong: The workaround for the Ant bug is exactly like that, that we pass "build.compiler=javac1.7" for every version >= 1.8, because Ant 1.8.3 and 1.8.4 have a bug, so we must pass javac1.7 (see link supplied in comment). So don't change that condition! Because we are now on minimum Java 8, you can set build.compiler always to javac1.7 as workaround: <!-- workaround for https://issues.apache.org/bugzilla/show_bug.cgi?id=53347 --> <condition property= "build.compiler" value= "javac1.7" > <or> <antversion exactly= "1.8.3" /> <antversion exactly= "1.8.4" /> </or> </condition> Also some conditions/fails are commented out (also in root build.xml regarding smoke tester). The "needed minimum Java 8" <fail/> was commented out, why?
        Hide
        thetaphi Uwe Schindler added a comment -

        This condition is also wrong:

           <condition property="javadoc.args" value="-Xdoclint:none" else="">
             <not>
        -      <equals arg1="${build.java.runtime}" arg2="1.7"/>
        +      <equals arg1="${build.java.runtime}" arg2="1.8"/>
             </not>
           </condition>
        

        It can stay as is, but ideally needs to be converted to no longer be conditionally. With that above condition, the javadoc's won't build:
        If the Java version is Java 8+, we must pass "-Xdoclint:none", so correct is and unconditional:

        <property name="javadoc.args" value="-Xdoclint:none"/>
        
        Show
        thetaphi Uwe Schindler added a comment - This condition is also wrong: <condition property= "javadoc.args" value= "-Xdoclint:none" else=""> <not> - <equals arg1= "${build.java.runtime}" arg2= "1.7" /> + <equals arg1= "${build.java.runtime}" arg2= "1.8" /> </not> </condition> It can stay as is, but ideally needs to be converted to no longer be conditionally. With that above condition, the javadoc's won't build: If the Java version is Java 8+, we must pass "-Xdoclint:none", so correct is and unconditional: <property name= "javadoc.args" value= "-Xdoclint:none" />
        Hide
        rjernst Ryan Ernst added a comment -

        Ok, I changed build.compiler back to javac1.7. And the commented out min version was unintentional. Both are fixed with this new patch.

        Show
        rjernst Ryan Ernst added a comment - Ok, I changed build.compiler back to javac1.7 . And the commented out min version was unintentional. Both are fixed with this new patch.
        Hide
        thetaphi Uwe Schindler added a comment -

        Please go through the build.xml changes a second time and check all them. A simple search/replace in build.xml is likely to break.

        Show
        thetaphi Uwe Schindler added a comment - Please go through the build.xml changes a second time and check all them. A simple search/replace in build.xml is likely to break.
        Hide
        thetaphi Uwe Schindler added a comment -

        In nightly-smoke the condition that checks for Java 8 is still "1.7".

        Show
        thetaphi Uwe Schindler added a comment - In nightly-smoke the condition that checks for Java 8 is still "1.7".
        Hide
        thetaphi Uwe Schindler added a comment -
        +Oracle Java 8 or OpenJDK 8, be sure to not use the GA build 147 or
        

        GA build 147 was the broken Java 7 initial release. So the whole sentence can go away. We may need to change it to make sure that 8u20 cannot be used to compile.

        Show
        thetaphi Uwe Schindler added a comment - +Oracle Java 8 or OpenJDK 8, be sure to not use the GA build 147 or GA build 147 was the broken Java 7 initial release. So the whole sentence can go away. We may need to change it to make sure that 8u20 cannot be used to compile.
        Hide
        rjernst Ryan Ernst added a comment -

        Ok, I addressed the comments, did another search for "7" in ant files, ran ant documentation-lint and ant nightly-smoke.

        Show
        rjernst Ryan Ernst added a comment - Ok, I addressed the comments, did another search for "7" in ant files, ran ant documentation-lint and ant nightly-smoke .
        Hide
        thetaphi Uwe Schindler added a comment -

        Patch looks fine to me. I will test it tomorrow with Java 8 on Windows with my huge number of JDK installations...

        Thanks for already "hacking" around bugs in javac and ecj-compiler (for those who wonder why the code changes were needed like removal of diamond operator or splitting chained method invocations into two lines).

        Show
        thetaphi Uwe Schindler added a comment - Patch looks fine to me. I will test it tomorrow with Java 8 on Windows with my huge number of JDK installations... Thanks for already "hacking" around bugs in javac and ecj-compiler (for those who wonder why the code changes were needed like removal of diamond operator or splitting chained method invocations into two lines).
        Hide
        rcmuir Robert Muir added a comment -

        These hacks all need comments indicating what they are doing. Otherwise they will get inadvertently reverted.

        Show
        rcmuir Robert Muir added a comment - These hacks all need comments indicating what they are doing. Otherwise they will get inadvertently reverted.
        Hide
        rjernst Ryan Ernst added a comment -

        Good point Robert. New patch adds comments for each place that needed a change to work with javac/ecj. While adding the comment for MergedIterator, I noticed that eclipse also has a similar error with another instance of MergedIterator above that. So there are probably some more places needing changes to work completely in IDEs (i haven't got a full compile to work in intellij b/c of some very odd error with ResourceLoader inside SolrVelocityResourceLoader...)

        Show
        rjernst Ryan Ernst added a comment - Good point Robert. New patch adds comments for each place that needed a change to work with javac/ecj. While adding the comment for MergedIterator, I noticed that eclipse also has a similar error with another instance of MergedIterator above that. So there are probably some more places needing changes to work completely in IDEs (i haven't got a full compile to work in intellij b/c of some very odd error with ResourceLoader inside SolrVelocityResourceLoader...)
        Hide
        thetaphi Uwe Schindler added a comment -

        Hi,
        In preparation to this move I changed Jenkins servers as following:

        • For now disabled trunk builds on ASF Jenkins. The current Java 8 installed there crushes in the network layer on solr tests. I will update it tomorrow.
        • Policeman Jenkins was changed that trunk builds use Java 8+ in the JVM randomization script (easy change)
        • Flonkings Jenkisn seems dead (HTTP times out, SSH closes connection). Simon Willnauer: Can you check this server?
        Show
        thetaphi Uwe Schindler added a comment - Hi, In preparation to this move I changed Jenkins servers as following: For now disabled trunk builds on ASF Jenkins. The current Java 8 installed there crushes in the network layer on solr tests. I will update it tomorrow. Policeman Jenkins was changed that trunk builds use Java 8+ in the JVM randomization script (easy change) Flonkings Jenkisn seems dead (HTTP times out, SSH closes connection). Simon Willnauer : Can you check this server?
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1640833 from Ryan Ernst in branch 'dev/trunk'
        [ https://svn.apache.org/r1640833 ]

        LUCENE-5950: Move to Java 8 as minimum Java version

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1640833 from Ryan Ernst in branch 'dev/trunk' [ https://svn.apache.org/r1640833 ] LUCENE-5950 : Move to Java 8 as minimum Java version
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1640837 from Uwe Schindler in branch 'dev/trunk'
        [ https://svn.apache.org/r1640837 ]

        LUCENE-5950: Disable Eclipse null analysis on Java 8 (requires @Null stuff Robert and I hate)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1640837 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1640837 ] LUCENE-5950 : Disable Eclipse null analysis on Java 8 (requires @Null stuff Robert and I hate)
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1640843 from Ryan Ernst in branch 'dev/trunk'
        [ https://svn.apache.org/r1640843 ]

        LUCENE-5950: Remove ecj hacks no longer necessary with current ecj settings

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1640843 from Ryan Ernst in branch 'dev/trunk' [ https://svn.apache.org/r1640843 ] LUCENE-5950 : Remove ecj hacks no longer necessary with current ecj settings
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1640874 from Uwe Schindler in branch 'dev/trunk'
        [ https://svn.apache.org/r1640874 ]

        LUCENE-5950: These 2 hacks are still needed to make the whole thing to compile in Eclipse's compiler (IDE)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1640874 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1640874 ] LUCENE-5950 : These 2 hacks are still needed to make the whole thing to compile in Eclipse's compiler (IDE)
        Hide
        thetaphi Uwe Schindler added a comment -

        Attached is a patch that removes the Javadocs Frame Injection patcher (LUCENE-5072, http://www.kb.cert.org/vuls/id/225657), because Java 8 is no longer affected by this bug. So its impossible to generate Javadocs on Lucene that are vulnerable if we use Java 8 minimum.
        I will commit this in a moment.

        Show
        thetaphi Uwe Schindler added a comment - Attached is a patch that removes the Javadocs Frame Injection patcher ( LUCENE-5072 , http://www.kb.cert.org/vuls/id/225657 ), because Java 8 is no longer affected by this bug. So its impossible to generate Javadocs on Lucene that are vulnerable if we use Java 8 minimum. I will commit this in a moment.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1640876 from Uwe Schindler in branch 'dev/trunk'
        [ https://svn.apache.org/r1640876 ]

        LUCENE-5950: Remove Javadocs patcher (no longer needed with Java 8 minimum)

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1640876 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1640876 ] LUCENE-5950 : Remove Javadocs patcher (no longer needed with Java 8 minimum)
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1640958 from Ryan Ernst in branch 'dev/trunk'
        [ https://svn.apache.org/r1640958 ]

        LUCENE-5950: Update changes entries

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1640958 from Ryan Ernst in branch 'dev/trunk' [ https://svn.apache.org/r1640958 ] LUCENE-5950 : Update changes entries

          People

          • Assignee:
            rjernst Ryan Ernst
            Reporter:
            rjernst Ryan Ernst
          • Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development