Skip to content
This repository has been archived by the owner on Jun 24, 2021. It is now read-only.

JDK-8200446: Update minimum boot JDK to 10 #61

Merged
merged 1 commit into from
Jun 14, 2018
Merged

JDK-8200446: Update minimum boot JDK to 10 #61

merged 1 commit into from
Jun 14, 2018

Conversation

kevinrushforth
Copy link
Collaborator

Fixes JDK-8200446

I'm doing it here first to make sure that the CI system for this sandbox is already using JDK 10 and thus won't be affected.

@kevinrushforth kevinrushforth added the enhancement New feature or request label Apr 5, 2018
@kevinrushforth
Copy link
Collaborator Author

I see the following from the travis-ci build (and similar from appveyor) build:

$ java -Xmx32m -version
Picked up _JAVA_OPTIONS: -Xmx2048m -Xms512m
java version "9.0.4"
Java(TM) SE Runtime Environment (build 9.0.4+11)
Java HotSpot(TM) 64-Bit Server VM (build 9.0.4+11, mixed mode)
...
:verifyJava FAILED
:verifyJava (Thread[main,5,main]) completed. Took 0.019 secs.
FAILURE: Build failed with an exception.

  • Where:
    Build file '/home/travis/build/javafxports/openjdk-jfx/build.gradle' line: 250
  • What went wrong:
    Execution failed for task ':verifyJava'.
    FAIL: java version mismatch: JDK version (9.0.4) < minimum version (10)

And that's why I did the PR here first!

@kevinrushforth
Copy link
Collaborator Author

I filed issue #62 to track updating the boot JDK used by the CI builds. That is a prerequisite for merging this PR.

@kevinrushforth
Copy link
Collaborator Author

Once issue #62 is resolved and we get a successful CI build, I'll squash the commits into one.

@kevinrushforth
Copy link
Collaborator Author

As mentioned in this message on the openjfx-dev list, I would like to hold off on this request for a couple days, say, until Monday to see if there are other comments.

@kevinrushforth
Copy link
Collaborator Author

I mis-diagnosed the problem with the CI builds. They are already doing a clean build, so that was never the issue.

The root cause of the problem is that this proposed fix bumps the generated class file format to that of JDK 10, and gradle 4.3 does not support the JDK 10 classfile format. This causes 'gradle test' to generate more than 7000 warnings of the form:

Malformed class file [XXX.class] found on compile classpath, which means that this class will cause a compile error if referenced in a source file. Gradle 5.0 will no longer allow malformed classes on compile classpath.

Running a test build with '--info --stacktrace' generates an 85 Mbyte log file. This causes the Travis CI build to fail (once it exceeds 4 Mbytes it will kill the build). The Appveyor build passes, but truncates the logfile, so that only about the last 3Mbytes are stored.

The short answer is that we need to update the version of gradle used by the CI builds. Since we will likely need to update to 4.8 once it is release (so we can build with JDK 11), it might be best to hold off on this PR until then. Or we could do it in two steps -- update to gradle 4.6 now and update to 4.8 when it released (after ensuring it will work with JDK 11) -- but it would be a bit more bookkeeping.

I will file a JBS issue for this, but will need to put this on hold for a couple of weeks due to other priorities.

@kevinrushforth
Copy link
Collaborator Author

I filed JDK-8201553 to track updating gradle.

@kevinrushforth kevinrushforth changed the title 8200446: Update minimum boot JDK to 10 [ON HOLD] 8200446: Update minimum boot JDK to 10 Jun 4, 2018
@kevinrushforth
Copy link
Collaborator Author

Note that this this will be unblocked after #92 is merged, which fixes JDK-8201553. Once it is, I'll rebase this on top of the develop branch and then we can resume reviewing this.

@johanvos
Copy link
Collaborator

johanvos commented Jun 7, 2018

Applying these changes after I applied the PR from #92 make the test work (locally on linux), so once #92 is merged, I'm +1 for merging this too. Travis/Appveyor are important checks for making sure it works on all platforms.

@kevinrushforth kevinrushforth changed the title [ON HOLD] 8200446: Update minimum boot JDK to 10 JDK-8200446: Update minimum boot JDK to 10 Jun 8, 2018
@kevinrushforth kevinrushforth self-assigned this Jun 8, 2018
@nlisker
Copy link

nlisker commented Jun 14, 2018

#92 has been merged a week ago. Is there any reason for not merging this PR yet?

@kevinrushforth
Copy link
Collaborator Author

It should be ready to go in soon. I'll rebase the patch again and send a note to openjfx-dev today as a heads up that this is back on review.

Summary: 1) JDK 10 is now the minimum; 2) set sourceCompatibility=10 so we can use JDK 10 features
@kevinrushforth kevinrushforth merged commit 845f9b0 into javafxports:develop Jun 14, 2018
@kevinrushforth kevinrushforth deleted the JDK-8200446 branch June 14, 2018 22:56
javafxports-github-bot pushed a commit that referenced this pull request Jun 15, 2018
Summary: 1) JDK 10 is now the minimum; 2) set sourceCompatibility=10 so we can use JDK 10 features
Reviewed-by: prr, jvos
scientificware pushed a commit to scientificware/openjdk-jfx that referenced this pull request Jun 15, 2018
Summary: 1) JDK 10 is now the minimum; 2) set sourceCompatibility=10 so we can use JDK 10 features
Reviewed-by: prr, jvos
scientificware added a commit to scientificware/openjdk-jfx that referenced this pull request Jun 15, 2018
* 8200285: TabDragPolicy.REORDER prevents ContextMenu from showing
Reviewed-by: kcr, aghaisas

* 8196031: FX Robot mouseMove fails on Windows 10 1709 with HiDPI
Reviewed-by: kcr, mbilla

* 8204956: Cleanup whitespace after fix for JDK-8200285
Reviewed-by: kcr

* 8200446: Update minimum boot JDK to 10 (javafxports#61)
Summary: 1) JDK 10 is now the minimum; 2) set sourceCompatibility=10 so we can use JDK 10 features
Reviewed-by: prr, jvos

* Travis-CI: use integrated jdk constants (javafxports#97)

Reviewed by: @brcolow @johanvos
scientificware added a commit to scientificware/openjdk-jfx that referenced this pull request Jun 15, 2018
* 8200285: TabDragPolicy.REORDER prevents ContextMenu from showing
Reviewed-by: kcr, aghaisas

* 8196031: FX Robot mouseMove fails on Windows 10 1709 with HiDPI
Reviewed-by: kcr, mbilla

* 8204956: Cleanup whitespace after fix for JDK-8200285
Reviewed-by: kcr

* 8200446: Update minimum boot JDK to 10 (javafxports#61)
Summary: 1) JDK 10 is now the minimum; 2) set sourceCompatibility=10 so we can use JDK 10 features
Reviewed-by: prr, jvos

* Travis-CI: use integrated jdk constants (javafxports#97)

Reviewed by: @brcolow @johanvos
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants