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

build: Introduce gradle version catalog #5683

Merged
merged 34 commits into from
Jul 3, 2024

Conversation

devinrsmith
Copy link
Member

@devinrsmith devinrsmith commented Jun 26, 2024

This is the first step in modernizing our gradle dependency management by using a gradle version catalog; this PR migrates all of our bespoke Classpaths.groovy versions into a standardized libs.versions.toml.

The biggest benefit is that dependabot will be able to submit gradle dependency version bump PRs. As developers, we should also benefit from greater standardization (IntelliJ works as you would expect with autocomplete, click-through, and find usages support).

The learning curve should be relatively small.

The intention is to continue the migration to the version catalog. Additional dependencies we declare outside of Classpaths.groovy should be relatively straightforward to migrate.

Fixes #5177

@devinrsmith devinrsmith added build NoDocumentationNeeded dependencies Pull requests that update a dependency file NoReleaseNotesNeeded No release notes are needed. labels Jun 26, 2024
@devinrsmith devinrsmith added this to the 3. May 2024 milestone Jun 26, 2024
@devinrsmith devinrsmith self-assigned this Jun 26, 2024
@devinrsmith devinrsmith changed the title Introduce gradle version catalog build: Introduce gradle version catalog Jun 27, 2024
@devinrsmith devinrsmith marked this pull request as ready for review June 27, 2024 16:17
@devinrsmith
Copy link
Member Author

I tried to compare the difference between ./gradlew -q server-jetty-app:dependencies, but unfortunately there are sorting differences in the output that makes it very hard to compare between branches (main vs this branch).

I audited the difference in runtime classpath using ./gradlew server-jetty-app:installDist between branches (main vs this branch), and did verify that the dependency versions (and transitively) are exactly the same.

buildSrc/src/main/groovy/Classpaths.groovy Outdated Show resolved Hide resolved
buildSrc/src/main/groovy/Classpaths.groovy Outdated Show resolved Hide resolved
extensions/iceberg/build.gradle Show resolved Hide resolved
extensions/protobuf/build.gradle Outdated Show resolved Hide resolved
buildSrc/src/main/groovy/Classpaths.groovy Outdated Show resolved Hide resolved
buildSrc/src/main/groovy/Classpaths.groovy Outdated Show resolved Hide resolved
@@ -14,8 +14,8 @@ dependencies {
testImplementation project(path: ':Base', configuration: 'tests')

testRuntimeOnly project(':log-to-slf4j')
Copy link
Member

Choose a reason for hiding this comment

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

libs.XYZ is pretty nice, in that it's based on a symbol rather a string. I wonder if we could do similarly for project dependencies.

Copy link
Member

Choose a reason for hiding this comment

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

I mean its groovy, you can do just about anything. I believe I've seen this done on some other projects, I'll see if I can find where I saw it.

Copy link
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

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

A few remarks, but nothing I think should hold up the merge (assuming classpaths are verified to be the same).

@@ -14,8 +14,8 @@ dependencies {
testImplementation project(path: ':Base', configuration: 'tests')

testRuntimeOnly project(':log-to-slf4j')
Copy link
Member

Choose a reason for hiding this comment

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

I mean its groovy, you can do just about anything. I believe I've seen this done on some other projects, I'll see if I can find where I saw it.

}

dependencies {
implementation(libs.hadoop.common) {
Copy link
Member

Choose a reason for hiding this comment

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

I didn't realize this had moved to Classpaths - I don't love that it's a plugin, but better than repeating it in two places.

@devinrsmith devinrsmith enabled auto-merge (squash) July 3, 2024 18:58
@devinrsmith devinrsmith requested a review from rcaudy July 3, 2024 19:00
@devinrsmith devinrsmith merged commit 0241ba7 into deephaven:main Jul 3, 2024
17 checks passed
@devinrsmith devinrsmith deleted the add-libs-versions-toml branch July 3, 2024 20:24
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build dependencies Pull requests that update a dependency file NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve gradle version management
4 participants