Skip to content
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

[WIP] Java 10 Support #1007

Conversation

ChristianMurphy
Copy link
Member

@ChristianMurphy ChristianMurphy commented Sep 17, 2017

🏗️ Work in Progress

Checklist
Description of change

Proof of concept investigating what changes will be needed to support Java 9.

🔲 TODO

  • Upgrade resource server to work with Java 9 and 10
  • Upgrade embedded CAS to a version that supports Java 9 and 10

☑️ Done

  • Restore Error Prone (with version that supports Java 9)
  • Set source compatibility version in Gradle
  • Setup Continuous Integration with Java 9
  • Resolve
    Execution failed for task ':uPortal-utils:uPortal-utils-core:compileJava'.
    > java.lang.NoClassDefFoundError: javax/annotation/Generated
    

@ChristianMurphy ChristianMurphy added this to the 5.0.0 milestone Sep 18, 2017
@asgrant
Copy link
Contributor

asgrant commented Sep 26, 2017

@ChristianMurphy thanks for taking a look at this. We are testing at OU with OpenJDK 9.

@ChristianMurphy
Copy link
Member Author

No problem, 😄
I have limited bandwidth at the moment so I may not be able to get back to this for a while. 🕐
If Oakland has any available bandwidth, assistance would be much appreciated. 🙇

@asgrant
Copy link
Contributor

asgrant commented Sep 29, 2017

@ChristianMurphy @Josh31415 is looking at this issue. Looks like we have a few JAXB issues. Josh will fill in the details as he moves along.

@ChristianMurphy
Copy link
Member Author

Rebased changes off of latest master, Gradle 4.2.1 resolved some conflicts between Gradle and openJDK 9.

@JoshBrudnak
Copy link
Contributor

@ChristianMurphy I am working on the javax.annotation.Generated class not found error. The problem seems to be that the javax.annotation package was moved to the java.xml.ws.annotation module.

@ChristianMurphy
Copy link
Member Author

Thanks @JoshBrudnak! 🙇

@ChristianMurphy ChristianMurphy changed the title [POC] [WIP] Java 9 Support [WIP] Java 9 Support Oct 18, 2017
@ChristianMurphy
Copy link
Member Author

rebased on current master

@ChristianMurphy
Copy link
Member Author

Rebased on master, re-adds error prone, which now supports Java 9.

@ChristianMurphy
Copy link
Member Author

@JoshBrudnak any luck looking into the generated class not found error?

@JoshBrudnak
Copy link
Contributor

@ChristianMurphy Yes I was able to fix the generated class error by adding its new path to the dependencies list. There is also an problem with the jaxb dependencies in uPortal-api/uPortal-api-search that I am currently working on.

@ChristianMurphy
Copy link
Member Author

Awesome, thanks @JoshBrudnak!
Any chance that the current changes could be committed and a PR opened targeting Jasig:master or ChristianMurphy:feature/support-java-9?

@JoshBrudnak
Copy link
Contributor

@ChristianMurphy Yes, will do 👍. I was also able to get the jaxb dependencies problem resolved.

Copy link
Member Author

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JoshBrudnak!

@@ -38,6 +38,7 @@ servletApiDependency=javax.servlet:javax.servlet-api:3.0.1
portletApiDependency=org.apache.portals:portlet-api_2.1.0_spec:1.0

# Dependency Versions
activationVersion:1.0.2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ is activationVersion:1.0.2 intended here or should this be activationVersion=1.0.2?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that was a mistake sorry.

@@ -78,6 +80,9 @@ dependencies {
compileOnly "${servletApiDependency}"
}

sourceCompatibility = '9'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 its interesting that this needed to be set here.
I thought allprojects would have covered this 🤷‍♂️

build.gradle Outdated
@@ -75,7 +75,7 @@ allprojects {
}

// Version has to be set directly here, using a variable does not set Java compatability version
sourceCompatibility = 1.8
sourceCompatibility = 1.9
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Open Question ❓
How do we want to handle Java versions with Semantic Versioning?
Upgrading from Java 8 to Java 9 could be considered breaking change (major), however Java 9 is something that would be valuable to get into the 5.x release line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought is would be to set a lower target compatibility allowing this to be added in a feature (minor) release.

sourceCompatibility = 1.9
targetCompatibility = 1.8

Then removing the backwards target in uPortal 6.

@ChristianMurphy
Copy link
Member Author

🚧 👷‍♂️ 🐧 ⛔ CI is running into issues with JpaClusterLockDao

/home/travis/build/Jasig/uPortal/uPortal-utils/uPortal-utils-core/src/main/java/org/apereo/portal/concurrency/locking/JpaClusterLockDao.java:232: error: cannot find symbol
                        query.using(ClusterMutex_.name, mutexName);
                                    ^
  symbol: variable ClusterMutex_

https://travis-ci.org/Jasig/uPortal/jobs/301570883#L1008-L1009

drop Java 8 from test matrix
@tbroyer
Copy link

tbroyer commented Jun 10, 2018

Fwiw, if you want Java 10 support for Error Prone, you need to use net.ltgt.errorprone-javacplugin; it requires Java 9+ and Gradle 4.6+. If you need to keep Java 8 support, then you'd have to conditionally apply (and configure!) either net.ltgt.errorprone (supports Java 8 and 9) or net.ltgt.errorprone-javacplugin (requires Java 9+)

I'm working on bringing the configuration of net.ltgt.errorprone-javacplugin to net.ltgt.errorprone so it'll be easier to conditionally apply the plugins (and use the same task configuration whichever the plugin); and will eventually deprecate, and then replace net.ltgt.errorprone next year when Java 8 is EOL.

@ChristianMurphy
Copy link
Member Author

Fwiw, if you want Java 10 support for Error Prone, you need to use net.ltgt.errorprone-javacplugin; it requires Java 9+ and Gradle 4.6+.

Thanks @tbroyer! 🙇

If you need to keep Java 8 support

We are moving toward using Java 10+ and dropping Java <= 9 support.

@ChristianMurphy ChristianMurphy modified the milestones: 5.2.0, 5.3.0 Aug 3, 2018
@ChristianMurphy ChristianMurphy modified the milestones: 5.3.0, 5.4.0 Oct 10, 2018
@ChristianMurphy ChristianMurphy modified the milestones: 5.4.0, 5.4.1, 5.5.0 Dec 21, 2018
@ChristianMurphy ChristianMurphy modified the milestones: 5.5.0, 5.6.0 Apr 10, 2019
@ChristianMurphy ChristianMurphy mentioned this pull request May 20, 2019
4 tasks
@ChristianMurphy
Copy link
Member Author

ChristianMurphy commented May 20, 2019

The merge conflicts are becoming untenable, closing this.
Continuing language upgrade at #1706.

@ChristianMurphy ChristianMurphy deleted the feature/support-java-9 branch May 20, 2019 17:50
@ChristianMurphy ChristianMurphy mentioned this pull request May 20, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants