-
Notifications
You must be signed in to change notification settings - Fork 4.1k
THRIFT-4766: Fix JDK11 build #1723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
b37e4c7 to
8fa33f4
Compare
|
We need to change the build tooling to prove this out. I will do that and push into your branch to kick another build. This PR built with java-8-openjdk, not java-11-openjdk. |
|
Thanks for the quick response @jeking3 The CI is now failing because there is no JDK: I've added the JDK-11 to the build. |
|
I see, bionic only came with the jre11. Thanks. (next time put it in alphabetically!!) |
|
Looks like the file debian/control needs to be updated for this... |
|
Thanks for the pointer @jeking3. I'll get rid of the jdk7 since the target is 1.8 as well: thrift/lib/java/gradle/sourceConfiguration.gradle Lines 48 to 49 in 99f673a
I'll also plug in the headless ones, since we don't use any GUI: I'll omit 9 and 10 since they are already EOL: https://en.wikipedia.org/wiki/Java_version_history |
06fd1e5 to
bf6e171
Compare
bf6e171 to
8aeb37b
Compare
|
I've also listed the dependencies alphabetically :-) |
|
I'm seeing some errors: |
Change from headless to the normal JDK. Maybe the libasound is pulled in transitively.
|
Our tests load chrome, but something isn't loading libasound. This also happens when you install Visual Studio Code on ubuntu. You can add the libasound2 package to the dockerfile. I would prefer if we use java-11-openjdk-headless... |
|
I prefer using the headless as well. I've added libasound2 package to the Dockerfile. |
|
Still some issues with launching Chrome: |
|
Another one: |
|
@jeking3 can you restart the failed builds? The error looks unrelated. |
|
This has broken the CI builds. It looks like the java runs slower than expected... I think we need to adjust the timeouts on the cross test builds. |
|
I just pushed a fix by increasing the ruby client timeout. |
|
Thanks, @jeking3 I accidentally force pushed, and therefore discarded your commit. |
Pull Request Guidance
Otherwise it will fail with:
Review the following items to ensure a smooth pull request experience.
Did you make a breaking change? If so:
Breaking-Changelabel to the Jira ticket.lib/<language>/README.mdfile.CHANGES.mdfile.Is this change significant enough to be in release notes?
This will add JDK9+ compatibility to Thirft
If there is an Apache Jira ticket:
Is the Apache Jira THRIFT ticket identifier in the PR title?
Is the Apache Jira THRIFT ticket identifier and affected languages in the commit message?
Did you squash your changes to a single commit?
For more information about committing, see CONTRIBUTING.md