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

#4259 support Java 11 #7514

Merged
merged 14 commits into from Jan 25, 2021
Merged

#4259 support Java 11 #7514

merged 14 commits into from Jan 25, 2021

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Jan 19, 2021

What this PR does / why we need it: This PR makes the changes necessary to update the Dataverse application and tests to run under Java 11. That's generally a good idea, allows us to take advantage of newer Java features, and is currently a prerequisite for #7504 and #7414 (these include a library built for Java 11). Note that running payara on Java 11 requires some jvm option changes that are in the default domain but would not be in domains upgraded per our instructions from glassfish. The changes will need to be documented for the release notes.

Which issue(s) this PR closes:

Closes #4259

Special notes for your reviewer: At the time of the original issue, it was less clear which Java version to upgrade to, but today, it looks like Java 11 (open) is widely available (e.g. default on Ubuntu 18). The primary issues seen were:

  • jacoco 0.8.1 isn't J11 compatible - needed an upgrade
  • Some code/tests depended upon the exact phrasing of Exception messages which changed - rewrote to not depend on the message details.
  • Different JVM options needed for payara with Java 9+ - copied changes from the default payara domain (no change needed if starting a new build)
  • dataverse-ansible updates (see that repo).

Suggestions on how to test this: Should be able to run Dataverse, unit and integration tests without problems on Java 11. One way to do that is with dataverse-ansible, setting the Java version to 11 and maven version to 3.6.3.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:
The following changes are needed to run with an upgraded domain.xml. They can be made using the asadmin interface or by editing the domain.xml file. Notes should include whichever method is decided on:
remove (J8 -specific):

 <jvm-options>-Djava.endorsed.dirs=/usr/local/payara5/glassfish/modules/endorsed:/usr/local/payara5/glassfish/lib/endorsed</jvm-options>
459a448
<jvm-options>-Djava.ext.dirs=${com.sun.aas.javaRoot}/lib/ext${path.separator}${com.sun.aas.javaRoot}/jre/lib/ext${path.separator}${com.sun.aas.instanceRoot}/lib/ext</jvm-options>

Add (Java 9+ specific):

 <jvm-options>[9|]--add-opens=java.base/jdk.internal.loader=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-exports=java.base/jdk.internal.ref=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.base/java.lang=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.base/java.net=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.base/java.nio=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.base/java.util=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.base/sun.nio.ch=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.management/sun.management=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.base/sun.net.www.protocol.jrt=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.base/sun.net.www.protocol.jar=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.naming/javax.naming.spi=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.rmi/sun.rmi.transport=ALL-UNNAMED</jvm-options>
<jvm-options>[9|]--add-opens=java.logging/java.util.logging=ALL-UNNAMED</jvm-options>

Additional documentation:

@donsizemore
Copy link
Contributor

"Remove Java 8 profile" commit on Java 11 / Maven 3.6.3:

Screen Shot 2021-01-20 at 10 18 34

The instance is still up if anybody wants to poke around.

Copy link
Contributor

@scolapasta scolapasta left a comment

Choose a reason for hiding this comment

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

Looks good to me (one minor comment removal suggestion).

@qqmyers or @djbrooke could you add a release notes documents before we move this to QA?

pom.xml Outdated
@@ -43,7 +43,7 @@
Jacoco 0.8.2 seems to break Netbeans code coverage integration so we'll use 0.8.1 instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

this comment is no longer needed, no?

@qqmyers
Copy link
Member Author

qqmyers commented Jan 20, 2021

@djbrooke - the only things I think needs to be in the notes are

  • that this version will require Java 11 to compile and/or run (which is good because it gets on a more modern version (with better performance and security) and allows developers to take advantage of new Java features),
  • that people should install the JDK/JRE per their platform's usual methods (could provide an example for Dataverse's preferred platform?),
  • that Dataverse recommends Java 11 as it has long term support (but 11+ should work), and
  • that the domain.xml file in the config dir (usually /usr/local/payara5/glassfish/domains/domain1/config per our instructions) should be edited to delete the two jvm options note above (-Djava.endorsed.dirs and -Djava.ext.dirs, whatever their values) and add the other jvm options listed above, all to the <domain><configs><config name-"server-config"><java-config> element. (There is/may be a <config name=default-config"> element with jvm-options - that whole config does not appear to be used by Dataverse.)

I haven't checked but I assume there's a prerequisite entry for Java that should be updated as well? (W.r.t. guides, since starting from Payara's default domain will get you the jvm-options already, I don't think any mention of those are needed in the guide, just in upgrade instructions, so the guide change should be simple).

I can try to provide final text, but hopefully you can take the above and create release notes> I'll check to see where the guide needs updates at least.

@djbrooke
Copy link
Contributor

djbrooke commented Jan 20, 2021

@qqmyers thanks for the details, I'll give it a shot!

@djbrooke djbrooke self-assigned this Jan 20, 2021
@qqmyers
Copy link
Member Author

qqmyers commented Jan 21, 2021

Awesome - release notes sound so natural when you write them!

@djbrooke
Copy link
Contributor

djbrooke commented Jan 21, 2021

@qqmyers thanks!

@scolapasta if the release notes look OK to you, can you approve so we can move to QA?

@djbrooke djbrooke assigned scolapasta and unassigned djbrooke Jan 21, 2021
…ava_11

updating Java 1.8 mentions to Java 11
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Should we also update Vagrant and docker-aio to Java 11?

Copy link
Contributor

@scolapasta scolapasta left a comment

Choose a reason for hiding this comment

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

Looks great. Java 11, here we come!

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ Jan 21, 2021
@scolapasta scolapasta removed their assignment Jan 21, 2021
@kcondon kcondon self-assigned this Jan 22, 2021
@kcondon kcondon merged commit a63b32f into IQSS:develop Jan 25, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Jan 25, 2021
@djbrooke djbrooke added this to the 5.4 milestone Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Java 11 Upgrade (no free security fixes for Oracle Java 8 after January 2019)
6 participants