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

Update gradle wrapper to 8.8 #2288

Merged
merged 1 commit into from
Jul 6, 2024
Merged

Update gradle wrapper to 8.8 #2288

merged 1 commit into from
Jul 6, 2024

Conversation

8Keep
Copy link
Contributor

@8Keep 8Keep commented Jun 23, 2024

Fix #2287

@8Keep 8Keep requested a review from stephengold June 23, 2024 21:04
@8Keep 8Keep force-pushed the master branch 3 times, most recently from 43edae5 to 19df0e2 Compare June 23, 2024 21:54
@stephengold
Copy link
Member

Did you follow the documented update procedure?

@8Keep
Copy link
Contributor Author

8Keep commented Jun 24, 2024

I didn't read the whole doc, most of it is irrelevant to us, but yeah that's pretty much what I did.

@stephengold
Copy link
Member

Running gradlew wrapper --gradle-version=8.8 modifies the following files:

  • gradle/wrapper/gradle-wrapper.jar
  • gradle/wrapper/gradle-wrapper.properties
  • gradlew
  • gradlew.bat

Please include those change in this PR.

@stephengold
Copy link
Member

I've reviewed the changes and don't see any problems. However, a change like this needs thorough testing more than it needs human review. I'm curious how much testing has been done so far:

  • platforms (Android, Linux, macOS, and Windows)?
  • tasks ("build", "clean", "dist", "install", "release", "mergedJavadoc", "mergedSource", "createZipDistribution", "copyLibs")?
  • command-line options ("-PuseCommitHashAsVersionName", "-PbuildNativeProjects", "-PskipPrebuildLibraries")?

@stephengold
Copy link
Member

@8Keep I remain curious.

@8Keep
Copy link
Contributor Author

8Keep commented Jul 3, 2024

Let me try these later today. We should also run these in CI, I'm surprised that we don't already.

@8Keep
Copy link
Contributor Author

8Keep commented Jul 6, 2024

@stephengold I've ran all the tasks you've sent, except release, because that doesn't exist. Unless you meant publish, which I can't finish due to not having a key. publishToMavenLocal does succeed.

copyLibs needed an update on line 117 of build.gradle. The thing is that according to some online sources, this was changed with gradle 7, so it may not have worked ever since the gradle 7 upgrade was done.

@stephengold
Copy link
Member

Good progress, thank you.

I've ran all the tasks you've sent, except release, because that doesn't exist. Unless you meant publish, which I can't finish due to not having a key. publishToMavenLocal does succeed.

You're right: I was thinking of the publish tasks. Since "publishToMavenLocal" succeeded, there's a high probability that "publishMavenPublicationToSNAPSHOTRepository" and "publishMavenPublicationToOSSRHRepository" will also work.

I think this PR is ready for integration into the "master" branch. Do you agree?

@8Keep
Copy link
Contributor Author

8Keep commented Jul 6, 2024

Yes, I agree. I want to do some followups, but I'll start these in separate prs:

  1. Start running a java 21 build in CI
  2. Start running more in CI to test all the tasks
  3. Start getting rid of deprecated gradle features in the build, so the upgrade to gradle 9 goes smoothly

@8Keep 8Keep merged commit a3a9011 into jMonkeyEngine:master Jul 6, 2024
11 checks passed
@8Keep
Copy link
Contributor Author

8Keep commented Jul 6, 2024

It looks like publishing natives did fail once it was committed. Stephen, do you know what the implications of this error are:

https://github.com/jMonkeyEngine/jmonkeyengine/actions/runs/9820622481/job/27115496415#step:5:549

@stephengold
Copy link
Member

I think it's related to a change at Sonatype. On 11 June, I got a cryptic e-mail from them about updating our plugin settings. Maybe those changes took effect sometime between 3 July and now?

I've generated Sonatype user tokens before, for my personal repos. I don't want to do it tonight when I'm tired. I'll investigate further tomorrow when I'm fresh.

@stephengold
Copy link
Member

And now I see the CI for 5248fb0 succeeded, so perhaps it was just a transient network error. Let's wait and see whether it recurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gradle Upgrade
2 participants