Build Spring with snapshot build as a CI job #1251
Conversation
WalkthroughGeneralizes the CI workflow from a single Caffeine build job to a matrix-driven job that builds multiple external projects (caffeine, spring-framework) using parameterized paths and a shared Gradle init script; the init script comment was generalized and dynamic-version caching was added. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant M as Matrix Runner (per project)
participant Repo as External Repo
participant Gradle as Gradle (init script)
GH->>M: Start job "Build ${{ matrix.project }} with snapshot"
M->>M: Create /tmp/${project}
M->>Repo: git clone ${url} into /tmp/${project}
M->>Gradle: Run build with --init-script .github/workflows/use-snapshot.gradle.kts
Gradle-->>M: Build results
M-->>GH: Report job status
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1251 +/- ##
=========================================
Coverage 88.50% 88.50%
Complexity 2373 2373
=========================================
Files 90 90
Lines 7802 7802
Branches 1555 1555
=========================================
Hits 6905 6905
Misses 452 452
Partials 445 445 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
FYI @sdeleuze |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/use-snapshot.gradle.kts (1)
12-21: Disable caching for dynamic versions, not just changing modules.You’re using useVersion("+"), which is a dynamic version. cacheChangingModulesFor(0, "seconds") does not affect dynamic version caching. Without this, Gradle may cache the resolved version for 24h, defeating “latest snapshot” intent.
resolutionStrategy { eachDependency { if (requested.group == "com.uber.nullaway") { useVersion("+") } } - cacheChangingModulesFor(0, "seconds") + cacheChangingModulesFor(0, "seconds") + cacheDynamicVersionsFor(0, "seconds") }
🧹 Nitpick comments (5)
.github/workflows/use-snapshot.gradle.kts (1)
3-7: Prefer mavenLocal() before mavenCentral() to ensure the locally published snapshot wins.With a dynamic selector ("+"), Gradle may select a higher remote release from mavenCentral over your locally published version. Putting mavenLocal first increases the chance we actually consume the just-published snapshot.
allprojects { repositories { - mavenCentral() - mavenLocal() + mavenLocal() + mavenCentral() gradlePluginPortal() } }.github/workflows/continuous-integration.yml (4)
79-89: Mark the external builds as non-blocking and lock down permissions.Given this job is intended to be non-blocking and runs untrusted code from external repos, consider:
- continue-on-error: true so overall CI stays green.
- Minimal token permissions for defense-in-depth (contents: read).
compile-other-projects: name: "Build ${{ matrix.project }} with snapshot" + continue-on-error: true strategy: fail-fast: false matrix: include: - project: caffeine url: https://github.com/ben-manes/caffeine.git - project: spring-framework url: https://github.com/spring-projects/spring-framework.git + permissions: + contents: read runs-on: ubuntu-latest
94-99: Consider adding Java 17 to the toolchain for external builds.Spring currently targets Java 17+; while 21 typically works, adding 17 reduces risk of unexpected toolchain mismatches.
- - name: 'Set up JDKs' + - name: 'Set up JDKs' uses: actions/setup-java@v4 with: - java-version: 21 + java-version: | + 21 + 17 distribution: 'temurin'
110-113: Use assemble instead of build -x test -x javadoc to speed up and focus on compilation.assemble exercises compilation (and thus NullAway) without running verification tasks some builds may wire into build.
- ./gradlew --init-script use-snapshot.gradle.kts build -x test -x javadoc + ./gradlew --init-script use-snapshot.gradle.kts assemble
100-104: Optional: Pin a short local snapshot version that sorts higher than any release.If your locally published version isn’t greater than the highest release on Maven Central, “+” may still pick the remote. One tactic is setting ORG_GRADLE_PROJECT_VERSION_NAME to a very high pre-release (e.g., 9999.0.0-dev-SNAPSHOT) for the publish step.
If you want, I can propose a concrete env var value consistent with your versioning plugin.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/continuous-integration.yml(2 hunks).github/workflows/use-snapshot.gradle.kts(1 hunks)
🔇 Additional comments (2)
.github/workflows/use-snapshot.gradle.kts (1)
1-1: Good generalization of the init script comment.The wording now matches the new multi-project usage in CI.
.github/workflows/continuous-integration.yml (1)
79-113: Verify NullAway integration in Spring-Framework
It looks like the spring-framework repo has no com.uber.nullaway or Error Prone plugin configured, so the current init-script won’t enable NullAway checks when building Spring. Without that, this CI job won’t actually exercise NullAway on Spring.Next steps:
- Confirm whether you want to inject the Error Prone + NullAway Gradle plugins via the init script.
- Or switch this matrix entry to a Spring module/repo that already applies those plugins.
Let me know which approach you’d prefer.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/use-snapshot.gradle.kts (1)
15-21: Disabling dynamic-version caching is the right call for '+' resolution; minor Kotlin DSL nitSetting cacheDynamicVersionsFor(0, "seconds") ensures the latest NullAway snapshot/release is always pulled — aligns with the CI goal.
Two optional Kotlin-DSL tweaks:
- Prefer Duration overloads for type safety.
- Prefer configureEach over all to avoid eager configuration.
Apply this diff to modernize the calls:
- configurations.all { + configurations.configureEach { resolutionStrategy { eachDependency { if (requested.group == "com.uber.nullaway") { useVersion("+") } } - cacheChangingModulesFor(0, "seconds") - cacheDynamicVersionsFor(0, "seconds") + cacheChangingModulesFor(java.time.Duration.ZERO) + cacheDynamicVersionsFor(java.time.Duration.ZERO) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/continuous-integration.yml(2 hunks).github/workflows/use-snapshot.gradle.kts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/continuous-integration.yml
🔇 Additional comments (1)
.github/workflows/use-snapshot.gradle.kts (1)
1-1: Generalized init-script comment — LGTMAccurately reflects the broader, multi-project usage.
|
@msridhar I have upgraded to NullAway 0.12.9 via spring-projects/spring-framework#35340, so Not sure if you need that information, but be aware Spring Framework build is using https://github.com/spring-gradle-plugins/nullability-plugin which set NullAway version automatically. Can be customized as documented here in the nullability {
nullAwayVersion = "0.12.10-SNAPSHOT"
} |
|
Thanks @sdeleuze! For now the same Gradle init script we used for Caffeine to build with the snapshot seems also to work for Spring, by intercepting the NullAway version dependence at a low level. But if that ever breaks we can look into setting the property on the nullability plugin. |
Just like we build Caffeine on CI, this PR adds a job to build the Spring Framework with the latest NullAway snapshot. Fortunately a lot of build configuration can be shared between the two projects.
This is a non-blocking job (it fact it's failing right now) but hopefully it will give us some early signal on new crashes and other disruptive changes we may be making.
Summary by CodeRabbit