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.

Sign up for GitHub

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

Jump to bottom

Upgrade to Gradle 8.10 and JDK 21 #877

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Upgrade to Gradle 8.10 and JDK 21 #877

wants to merge 3 commits into from

Conversation

alyssahursh
Copy link
Contributor

Motivation

Commits

This PR contains two atomic commits, each of which build and run successfully

  • Upgrade to Gradle 8.10 (automated result of running gradle wrapper --gradle-version 8.10)
  • Upgrade to JDK 21

Testing

  • ./gradlew build and ./gradlew run both succeed

Note

After accepting this commit, you will need to install JDK 21 and update JAVA_HOME to point to the file system location where JDK 21 is installed

@nurse-the-code
Copy link
Collaborator

This may address issue #669

@nurse-the-code
Copy link
Collaborator

nurse-the-code commented Aug 15, 2024 •
edited
Loading

We will want to make sure that all code and documentation references point to the most recent version of Temurin 21 LTS. This it will be important in order to make sure the golden hashing procedure works correctly. Should also probably change Oracle references to Temurin

@alyssahursh
Copy link
Contributor Author

@nurse-the-code I come from corporate closed source where I don't have a choice in which JDK distro I use :) Is this the version you want linked to from the README? https://adoptium.net/temurin/releases/?version=21

Do these two lines need to be updated as well?

artoonie
artoonie approved these changes Aug 15, 2024
View reviewed changes
Copy link
Collaborator

@artoonie artoonie left a comment

Choose a reason for hiding this comment

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

I suspect we will indeed need to update the github workflows as well. This approval should trigger the workflow. Thank you again!

(Approval is pending additional review from @nurse-the-code to make sure the wiki and all documentation matches. Some of that lives outside this repo. Only approving to get the workflows to run.)

artoonie
artoonie requested changes Aug 15, 2024
View reviewed changes
Copy link
Collaborator

@artoonie artoonie left a comment

Choose a reason for hiding this comment

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

Workflow did not run, trying again...

artoonie and others added 2 commits August 15, 2024 20:21
@artoonie @alyssahursh
run test on PR, not on push
cd397c7
@alyssahursh
Upgrade to Gradle 8.10
d52a7e3 
This commit is the automated result of running `gradle wrapper --gradle-version 8.10`
follwing the instructions at https://docs.gradle.org/current/userguide/upgrading_version_8.html

Gradle 8.4+ is requried for JDK 21

Testing:
* ./gradlew run and ./gradlew build both succeed
@alyssahursh
Copy link
Contributor Author

alyssahursh commented Aug 16, 2024 •
edited
Loading

Alright. I have zero experience with GitHub, and none of my normal ways of working seem to apply out here, so you'll have to forgive me if I'm doing things in ways that don't make sense to you! I'm open to feedback and/or instruction :)

I force-pushed a revision with the following changes:

  • Updated Upgrade to JDK 21 to:
    • Point the JDK 21 README link to Temurin instead of Oracle
    • Update ./github/workflows/test.yml and ./github/workflows/release.yml to use Temurin 21.0.4
  • Rebased on top of @artoonie's commit to run github workflows on PR instead of push to prove that the changes to ./github/workflows/test.yml had the intended effect

Testing:

  • Installed Temurin 21.0.4 locally
  • Updated $JAVA_HOME to /Library/Java/JavaVirtualMachines/temurin-21.jdk/Contents/Home
  • ./gradlew build and ./gradlew run both succeed
  • Build, Lint, and Test / build github workflow step succeeded

@artoonie Let me know if you want to merge all of this work together from this PR or if you want me to pull your commit back off of my branch to merge them separately.

@yezr yezr linked an issue Aug 16, 2024 that may be closed by this pull request
Update JDK and all other deps for v2.0.0 #669
Open
@nurse-the-code
Copy link
Collaborator

nurse-the-code commented Aug 16, 2024 •
edited
Loading

@nurse-the-code I come from corporate closed source where I don't have a choice in which JDK distro I use :) Is this the version you want linked to from the README? https://adoptium.net/temurin/releases/?version=21

Correct. As long as it is the most recent LTS version of 21.

I had meant to put that link in but it looks like I forgot to do that.

Do these two lines need to be updated as well?

That looks like what I was thinking of. Whoever does the code review on this (probably either myself or @yezr) should search to make sure any references to the Java version throughout the code base point to the new version. And if we are referencing specific JDK or JRE packages, we need to make sure that we are pointing to the Temurin LTS builds.

And as @artoonie said, documentation will have to be updated. Included in that should be directing users to use the correct Temurin builds (in case they want to check the golden hash).

As a bonus (doesn't have to be in this PR and is not critical for the RCTab 2.0 work), in the documentation we could direct users to using something like JEnv to manage different Java versions.

@nurse-the-code nurse-the-code self-assigned this Aug 16, 2024
@artoonie
Copy link
Collaborator

@artoonie Let me know if you want to merge all of this work together from this PR or if you want me to pull your commit back off of my branch to merge them separately.

I'm good with the change in this PR -- I'll close my other PR! @alyssahursh

Whoever does the code review on this (probably either myself or @yezr) should search to make sure any references to the Java version throughout the code base point to the new version

Done, and I don't see any missed references @nurse-the-code

And as @artoonie said, documentation will have to be updated. Included in that should be directing users to use the correct Temurin builds (in case they want to check the golden hash).

Are you okay merging this first then updating the docs @nurse-the-code ?

--

This PR looks good to me, and will await @nurse-the-code's approval re: documentation before merging.

artoonie
artoonie previously approved these changes Aug 16, 2024
View reviewed changes
@yezr
Copy link
Collaborator

yezr commented Aug 16, 2024

I've added a task in our RCTab internal tracker to update all documentation references to Java 21 and Gradle 8.10. LGTM!

@alyssahursh alyssahursh mentioned this pull request Aug 17, 2024
Merged
@alyssahursh
Copy link
Contributor Author

alyssahursh commented Aug 17, 2024 •
edited
Loading

This upgrade fails the release scripts because of incompatibilities with org.beryx.jlink 2.x. I've made some progress on additional upgrades but I'm still failing the Mac releases: https://github.com/alyssahursh/rcv/actions/runs/10429848480/job/28887874919

Mac releases fail at Prepare keychain:

Run export TEMP_PWD=temporary-password-to-avoid-GUI-prompt
  export TEMP_PWD=temporary-password-to-avoid-GUI-prompt
  echo "Decode Base64 certificates"
  echo $MACOS_CERTIFICATE | base64 --decode > certificate.p12
  echo "Create and unlock keychain"
  security create-keychain -p $TEMP_PWD build.keychain
  security unlock-keychain -p $TEMP_PWD build.keychain
  echo "Import certificates into keychain"
  # Note: in the next command, the -A should not be used outside of GitHub actions.
  # It allows any application to read the keychain, which is fine in an ephemeral environment,
  # but not fine if you run this on your own machine.
  security import certificate.p12 -k build.keychain -P $MACOS_CERTIFICATE_PWD -A -T /usr/bin/codesign -T /usr/bin/productbuild -T /usr/bin/security
  security set-key-partition-list -S apple-tool:,apple:,codesign: -s -k $TEMP_PWD build.keychain
  shell: /bin/bash -e {0}
  env:
    JAVA_HOME: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/21.0.4-7.0/arm64/Contents/Home
    JAVA_HOME_21_ARM64: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/21.0.4-7.0/arm64/Contents/Home
    MACOS_CERTIFICATE: 
    MACOS_CERTIFICATE_PWD: 
Decode Base64 certificates
Create and unlock keychain
Import certificates into keychain
security: SecKeychainItemImport: One or more parameters passed to a function were not valid.
Error: Process completed with exit code 1.

Compared with a recent successful run:

Run export TEMP_PWD=temporary-password-to-avoid-GUI-prompt
  export TEMP_PWD=temporary-password-to-avoid-GUI-prompt
  echo "Decode Base64 certificates"
  echo $MACOS_CERTIFICATE | base64 --decode > certificate.p12
  echo "Create and unlock keychain"
  security create-keychain -p $TEMP_PWD build.keychain
  security unlock-keychain -p $TEMP_PWD build.keychain
  echo "Import certificates into keychain"
  # Note: in the next command, the -A should not be used outside of GitHub actions.
  # It allows any application to read the keychain, which is fine in an ephemeral environment,
  # but not fine if you run this on your own machine.
  security import certificate.p12 -k build.keychain -P $MACOS_CERTIFICATE_PWD -A -T /usr/bin/codesign -T /usr/bin/productbuild -T /usr/bin/security
  security set-key-partition-list -S apple-tool:,apple:,codesign: -s -k $TEMP_PWD build.keychain
  shell: /bin/bash -e {0}
  env:
    JAVA_HOME: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/20.0.1-9/arm64/Contents/Home
    JAVA_HOME_20_ARM64: /Users/runner/hostedtoolcache/Java_Temurin-Hotspot_jdk/20.0.1-9/arm64/Contents/Home
    MACOS_CERTIFICATE: ***
    MACOS_CERTIFICATE_PWD: ***
Decode Base64 certificates
Create and unlock keychain
Import certificates into keychain
1 identity imported.
1 certificate imported.
keychain: "/Users/runner/Library/Keychains/build.keychain-db"
version: 512
class: 0x00000010 
attributes:
    0x00000000 <uint32>=0x00000010 
    0x00000001 <blob>="RCV Resources"
    0x00000002 <blob>=<NULL>
    0x00000003 <uint32>=0x00000001 
    0x00000004 <uint32>=0x00000000 
    0x00000005 <uint32>=0x00000000 
    0x00000006 <blob>=0xB0FBC1CB80D35FF358804FB844A24A1880FF47C5  "\260\373\301\313\200\323_\363X\200O\270D\242J\030\200\377G\305"
    0x00000007 <blob>=<NULL>
    0x00000008 <blob>=0x7B38373139316361322D306663392D313164342D383439612D3030303530326235323132327D00  "{87191ca2-0fc9-11d4-849a-000502b52122}\000"
    0x00000009 <uint32>=0x0000002A  "\000\000\000*"
    0x0000000A <uint32>=0x00000800 
    0x0000000B <uint32>=0x00000800 
    0x0000000C <blob>=0x0000000000000000 
    0x0000000D <blob>=0x0000000000000000 
    0x0000000E <uint32>=0x00000001 
    0x0000000F <uint32>=0x00000001 
    0x00000010 <uint32>=0x00000001 
    0x00000011 <uint32>=0x00000000 
    0x00000012 <uint32>=0x00000001 
    0x00000013 <uint32>=0x00000001 
    0x00000014 <uint32>=0x00000001 
    0x00000015 <uint32>=0x00000001 
    0x00000016 <uint32>=0x00000001 
    0x00000017 <uint32>=0x00000001 
    0x00000018 <uint32>=0x00000001 
    0x00000019 <uint32>=0x00000001 
    0x0000001A <uint32>=0x00000001

It looks like $MACOS_CERTIFICATE is unset. If anyone's got hints, I'm interested!

It looks like this is due to the fact that I don't have access to the repo secrets. I'll amend the org.beryx.jlink upgrade to the the Upgrade to JDK 21 commit in the morning :)

@alyssahursh
Upgrade to JDK 21
6dfe958 
JDK 20 is a feature release and is not recommended for use in production:
https://www.oracle.com/java/technologies/javase/jdk20-archive-downloads.html

JDK 21 is the latest long-term support (LTS) release of the JDK:
https://www.oracle.com/java/technologies/downloads/#java21

After accepting this commit, you will need to install JDK 21 and update
JAVA_HOME to point to the file system location where JDK 21 is installed

Testing:
* ./gradlew run and ./gradle build both succeed
@alyssahursh
Copy link
Contributor Author

I force-pushed a revision with the following changes:

Testing:

@artoonie
Copy link
Collaborator

Thank you! That testing procedure is fine; we have a scheduled release build every 2 weeks that will let us know if the mac build is somehow failing. (It's unlikely if the other two pass, unless we need to accept a new apple agreement.)

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artoonie artoonie artoonie left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@nurse-the-code nurse-the-code

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update JDK and all other deps for v2.0.0
4 participants
@alyssahursh @nurse-the-code @artoonie @yezr