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

Upgrade to Gradle 7.4 #7351

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

Conversation

ryangardner
Copy link

Upgrade to Gradle 7.4

  • Updated gradle wrapper
  • Updated usage of an internal TemporaryFileProvider to the correct location for Gradle 7
  • Update the main -> mainClass for the geode assembly to match Gradle 7 syntax
  • Move some compile scope to compileOnly (required for Gradle 7)
  • Add explicit dependsOn relationship between tasks that have dependencies that Gradle 7 complains about
  • Add a duplicatesStrategy to the processIntegrationTestResources to avoid Gradle 7 failing on a duplicate resources in shiro.ini (this duplicate seems to come from how the source set is configured)
  • Upgrade the RepeatTestExecutor to match the DefaultTestExecutor from Gradle 7.4

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • I couldn't find one

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • I have some errors with bind exceptions for some of the tests, but those also happen on the gradle 6.8 build for me and I haven't looked deeply into them.

  • Have you written or updated unit tests to verify your changes?
    N/A

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

Upgrade to Gradle 7.4
- Updated gradle wrapper
- Updated usage of an internal TemporaryFileProvider to the correct location for Gradle 7
- Update the `main` -> `mainClass` for the geode assembly to match Gradle 7 syntax
- Move some `compile` scope to `compileOnly` (required for Gradle 7)
- Add explicit `dependsOn` relationship between tasks that have dependencies that Gradle 7 complains about
- Add a `duplicatesStrategy` to the processIntegrationTestResources to avoid Gradle 7 failing on a duplicate resources in shiro.ini (this duplicate _seems_ to come from how the source set is configured)
- Upgrade the RepeatTestExecutor to match the DefaultTestExecutor from Gradle 7.4
@rhoughton-pivot rhoughton-pivot added the windows add this label to get Windows JDK11 PR checks too label Feb 9, 2022
@rhoughton-pivot rhoughton-pivot added the jdk8 add this label to get Linux JDK8 PR checks too label Feb 9, 2022
Copy link
Contributor

@demery-pivotal demery-pivotal left a comment

Choose a reason for hiding this comment

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

In gradle-test-projects, the compileOnly dependency in the build.gradle file should be implementation. That fixes the failing acceptance test on my Mac, running with either JDK 8 or JDK 11.

The JMX test that's failing in the distributed test run is notoriously fickle. The failure is likely unrelated to this PR.

geode-core/build.gradle Outdated Show resolved Hide resolved
Copy link
Contributor

@demery-pivotal demery-pivotal left a comment

Choose a reason for hiding this comment

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

I'm happy with this… pending Robert's review of the combineReports stuff.

@ryangardner ryangardner marked this pull request as ready for review February 15, 2022 18:08
Copy link
Contributor

@onichols-pivotal onichols-pivotal left a comment

Choose a reason for hiding this comment

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

comment deleted

@ryangardner
Copy link
Author

Sorry, I've been away from my computer this week - I saw your earlier comment about updating the commit message and I can still do that, I've just not had a chance yet.

@onichols-pivotal
Copy link
Contributor

Hi Ryan, I don't know if you're subscribed dev@geode.apache.org, I just posted an announcement there clarifying things.

@ryangardner
Copy link
Author

ryangardner commented Feb 26, 2022 via email

@rhoughton-pivot
Copy link
Member

Hi @ryangardner , I haven't forgotten about this. We have some internal tooling that is not ready to make the jump to Gradle 7 yet. But when it is, then this will be merged. Thanks so much for the help.

@jdeppe-pivotal jdeppe-pivotal removed their request for review January 25, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jdk8 add this label to get Linux JDK8 PR checks too windows add this label to get Windows JDK11 PR checks too
Projects
None yet
5 participants