Repository navigation
Migrate to Java 25 and Gradle 9.5.1 (without Java source changes) - #1894
Thevakumar-Luheerathan wants to merge 24 commits into
Conversation
…27-050400-74f7e6bf - Upgrade Gradle wrapper to 9.5.1 - Replace com.gradle.enterprise with com.gradle.develocity plugin - Upgrade net.researchgate.release to 3.1.0 for Gradle 9 compatibility - Replace all deprecated buildDir references with layout.buildDirectory - Replace Project.exec() calls with ExecOperations injection pattern - Remove deprecated sourceCompatibility/targetCompatibility (replaced by toolchain) - Add Java 25 toolchain to all Java subprojects - Upgrade Ballerina Gradle plugin to 4.0.0 (Java 25 + Gradle 9.5.1 compatible) - Update ballerinaLangVersion to 2201.14.0-20260527-050400-74f7e6bf - Update all platform.java21 TOML sections to platform.java25 - Add patchBallerinaScripts task to fix bal binary permissions and remove --sun-misc-unsafe-memory-access=allow flag (removed in Java 25) - Upgrade SpotBugs Gradle plugin to 6.5.1 for Java 25 class file support - Add SpotBugs exclusions for THROWS/AT/USELESS_STRING detectors introduced in SpotBugs 4.9.x (bundled in plugin >= 6.2.0) - Update CI workflow to use pull-request-build-template.yml@java-25-migration
- openapi-extension-tests: replace eager layout.buildDirectory.get().asFile with lazy Provider<Directory> to fix null basedir on Windows (Gradle 9.5.1) - openapi-tool: add PatchBallerinaToolScriptsTask to set execute permissions and remove --sun-misc-unsafe-memory-access=allow flag from bal scripts unpacked by the Ballerina plugin (mirrors module-ballerina-openapi approach) - module-ballerina-openapi/Dependencies.toml: update distribution version to 2201.14.0-20260527-050400-74f7e6bf and openapi package to 2.4.2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Nested copy{} blocks inside a copy{} spec called project.copy{} with
a relative into() path, whose basedir resolved to null under Gradle 9.5.1.
Restructure into flat, sequential copy calls with absolute destination
paths and guard checks for optional source directories.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Applied fixes: A4, B3, B5+C1 - Make patchBallerinaScripts idempotent and add task @input properties (A1, B1) - Use token-level JVM flag removal instead of line filter (B4) - Remove mavenLocal() from plugin repositories (A4) - Scope SpotBugs suppressions to module packages (A5) - Add Gradle wrapper integrity checksum (B3) - Wire jacocoVersion property to JaCoCo toolVersion (B5) - Bump JaCoCo to 0.8.14 for stable Java 25 support (C1)
# Conflicts: # openapi-tool/Dependencies.toml
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PatchBallerinaToolScriptsTask was chmodding a directory (target/extracted-distributions/jballerina-tools-zip/...) that is never created in the openapi-tool project, leaving bal in the plugin's actual jballerina-tools-<version>/bin non-executable and causing :openapi-tool:test to fail with exit 126. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe PR upgrades Gradle to 9.5.1, updates plugins and CI toolchains, migrates build paths to ChangesGradle and CI modernization
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This updates the Gradle, Java, and related build tooling configuration without Java source changes. No unresolved merge-readiness risk is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description accurately states the migration purpose and the Java-source exclusion, but it does not provide most required template sections, including goals, approach, testing, security checks, release note, documentation, migration details, and related project information. Resolution Complete the required pull request template. Add goals, approach, user stories, release note, documentation impact, training, certification, marketing, automation test results, security checks, samples, related PRs, migration steps, tested environments, and learning details. Mark non-applicable sections as N/A with a brief reason.
✨ Finishing Touches🧪 Generate unit tests (beta)
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. I’m a rabbit with a Gradle hare, Comment |
…ipts These forced all builds onto JDK25 regardless of the JAVA_HOME the build was invoked with, breaking genuine JDK21 builds. The scripts now only fix executable permissions on the unpacked bal binaries and otherwise stay JDK-independent. Also reverted an accidental platform identifier change (java21 -> java25) in module-ballerina-openapi.
openapi-cli/src/test/resources/expected_gen/ballerina_project/service_type.bal was accidentally committed (does not exist on master) despite being a runtime-generated artifact of testServiceTypeGeneration. Since it lived in the shared ballerina_project resource directory, it was present from a clean checkout before any test ran, causing testServiceGenerationWithout DataBinding's semantic model compilation to see an orphaned service-object type reference and fail with a spurious compile diagnostic. Verified: reproduces identically on JDK21 and JDK25 (not a JDK version difference); removing it fixes the test on both.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ballerina-to-openapi/build.gradle (1)
60-93: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAvoid eager evaluation of
buildDirectoryat configuration time.Calling
project.layout.buildDirectory.get().asFileduring the configuration phase eagerly resolves the directory property. This violates modern Gradle best practices and breaks both Configuration Caching and Project Isolation. Instead, construct paths lazily usinglayout.buildDirectory.dir(...)providers and pass them directly to task properties.
ballerina-to-openapi/build.gradle#L60-L93: replace.get().asFileconcatenations with lazily evaluatedlayout.buildDirectory.dir(...)providers forbDistribution,ballerinaDist, andartifactExtractedPath, and pass these providers directly tooutputs.dir,into, andfromto avoid eager resolution.openapi-cli/build.gradle#L60-L93: replace.get().asFileconcatenations with lazily evaluatedlayout.buildDirectory.dir(...)providers forbDistribution,ballerinaDist, andartifactExtractedPath, and pass these providers directly tooutputs.dir,into, andfrom.openapi-core/build.gradle#L60-L93: replace.get().asFileconcatenations with lazily evaluatedlayout.buildDirectory.dir(...)providers forbDistribution,ballerinaDist, andartifactExtractedPath, and pass these providers directly tooutputs.dir,into, andfrom.openapi-integration-tests/build.gradle#L44-L50: replace.get().asFilewith a lazily evaluatedlayout.buildDirectory.dir(...)provider forbDistribution, and pass it tooutputs.dirandinto.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ballerina-to-openapi/build.gradle` around lines 60 - 93, Make the build-directory path handling lazy in ballerina-to-openapi/build.gradle:60-93, openapi-cli/build.gradle:60-93, and openapi-core/build.gradle:60-93 by defining bDistribution, ballerinaDist, and artifactExtractedPath with layout.buildDirectory.dir(...) providers, then passing those providers directly to outputs.dir, into, and from in jBallerinaPack, unpackStdLibs, and copyStdlibs. Apply the same change in openapi-integration-tests/build.gradle:44-50 for bDistribution, passing its provider directly to outputs.dir and into; remove all configuration-time buildDirectory.get().asFile resolution at these sites.
🧹 Nitpick comments (1)
module-ballerina-openapi/build.gradle (1)
25-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider skipping this task on Windows for consistency.
In
openapi-tool/build.gradle, the equivalent script patching task safely exits early on Windows (if (Os.isFamily(Os.FAMILY_WINDOWS)) { return }). Consider adding the same check here to avoid unnecessary operations and ensure consistent behavior across modules.♻️ Proposed refactor
`@TaskAction` void patch() { + if (Os.isFamily(Os.FAMILY_WINDOWS)) { return } def binDirs = [ project.layout.buildDirectory.dir("target/extracted-distributions/jballerina-tools-zip/jballerina-tools-${project.ballerinaLangVersion}/bin").get().asFile, new File("${project.rootDir}/target/ballerina-runtime/bin")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@module-ballerina-openapi/build.gradle` around lines 25 - 41, Update the PatchBallerinaOpenAPIScriptsTask.patch task to return immediately on Windows, using the project’s existing Apache Ant Os platform check pattern from openapi-tool/build.gradle. Keep the current binDirs construction and executable-bit patching unchanged for non-Windows systems.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-release.yml:
- Line 13: Disable persisted checkout credentials on every listed
actions/checkout step by setting persist-credentials to false:
.github/workflows/publish-release.yml:13,
.github/workflows/build-ballerina-to-openapi.yml:10,
.github/workflows/build-timestamped-master.yml:13, and
.github/workflows/pull-request.yml:12 and :34. In
.github/workflows/publish-release.yml:13, add scoped authentication for the
final PR operation; in .github/workflows/publish-ballerina-to-openapi.yml:13,
disable persistence and explicitly authenticate the later branch push.
In `@build.gradle`:
- Around line 120-128: Update the createProperties task to write both generated
properties files under the respective subprojects’ layout.buildDirectory rather
than src/main/resources, then configure each subproject’s
sourceSets.main.resources.srcDir(...) to include its generated directory while
preserving the existing property content.
In `@openapi-integration-tests/build.gradle`:
- Line 130: The jBallerinaPack dependency points to patchBallerinaScripts in
another module, leaving this module’s extracted distribution scripts unpatched
and non-executable. Update the openapi-integration-tests packaging flow so its
own jBallerinaPack output is patched, either by invoking patching within
jBallerinaPack or by adding and depending on a module-local patch task.
---
Outside diff comments:
In `@ballerina-to-openapi/build.gradle`:
- Around line 60-93: Make the build-directory path handling lazy in
ballerina-to-openapi/build.gradle:60-93, openapi-cli/build.gradle:60-93, and
openapi-core/build.gradle:60-93 by defining bDistribution, ballerinaDist, and
artifactExtractedPath with layout.buildDirectory.dir(...) providers, then
passing those providers directly to outputs.dir, into, and from in
jBallerinaPack, unpackStdLibs, and copyStdlibs. Apply the same change in
openapi-integration-tests/build.gradle:44-50 for bDistribution, passing its
provider directly to outputs.dir and into; remove all configuration-time
buildDirectory.get().asFile resolution at these sites.
---
Nitpick comments:
In `@module-ballerina-openapi/build.gradle`:
- Around line 25-41: Update the PatchBallerinaOpenAPIScriptsTask.patch task to
return immediately on Windows, using the project’s existing Apache Ant Os
platform check pattern from openapi-tool/build.gradle. Keep the current binDirs
construction and executable-bit patching unchanged for non-Windows systems.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02f3cf0e-ac4f-49db-84e9-cd804955de84
📒 Files selected for processing (22)
.github/workflows/build-ballerina-to-openapi.yml.github/workflows/build-timestamped-master.yml.github/workflows/publish-ballerina-to-openapi.yml.github/workflows/publish-release.yml.github/workflows/pull-request.ymlballerina-to-openapi/build.gradlebuild.gradleconfig/checkstyle/build.gradlegradle.propertiesgradle/javaProject.gradlegradle/wrapper/gradle-wrapper.propertiesmodule-ballerina-openapi/build.gradleopenapi-bal-task-plugin/build.gradleopenapi-cli/build.gradleopenapi-client-native/build.gradleopenapi-core/build.gradleopenapi-extension-tests/build.gradleopenapi-integration-tests/build.gradleopenapi-tool/build.gradleopenapi-validator/build.gradlesettings.gradlespotbugs-exclude.xml
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable persisted checkout credentials across all updated workflows.
actions/checkout stores its token in .git/config by default, exposing it to subsequent build scripts and downloaded commands. Set persist-credentials: false everywhere, then configure scoped authentication only where later Git operations require it.
.github/workflows/publish-release.yml#L13-L13: disable persistence before executing downloadedhubcode; authenticate the final PR operation explicitly..github/workflows/build-ballerina-to-openapi.yml#L10-L10: disable persistence; no later Git authentication is required..github/workflows/build-timestamped-master.yml#L13-L13: disable persistence; publishing credentials are already passed explicitly..github/workflows/publish-ballerina-to-openapi.yml#L13-L13: disable persistence and explicitly authenticate the later branch push..github/workflows/pull-request.yml#L12-L12: disable persistence in the Ubuntu job..github/workflows/pull-request.yml#L34-L34: disable persistence in the Windows job.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 13-13: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 5 files
.github/workflows/publish-release.yml#L13-L13(this comment).github/workflows/build-ballerina-to-openapi.yml#L10-L10.github/workflows/build-timestamped-master.yml#L13-L13.github/workflows/publish-ballerina-to-openapi.yml#L13-L13.github/workflows/pull-request.yml#L12-L12.github/workflows/pull-request.yml#L34-L34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/publish-release.yml at line 13, Disable persisted checkout
credentials on every listed actions/checkout step by setting persist-credentials
to false: .github/workflows/publish-release.yml:13,
.github/workflows/build-ballerina-to-openapi.yml:10,
.github/workflows/build-timestamped-master.yml:13, and
.github/workflows/pull-request.yml:12 and :34. In
.github/workflows/publish-release.yml:13, add scoped authentication for the
final PR operation; in .github/workflows/publish-ballerina-to-openapi.yml:13,
disable persistence and explicitly authenticate the later branch push.
Source: Linters/SAST tools
| task createProperties { | ||
| doLast { | ||
| def clientNativeVersionFile = | ||
| new File("$project.rootDir/openapi-cli/src/main/resources/openapi-client-native-version.properties") | ||
| clientNativeVersionFile.text = "version=${clientNativeVersion}" | ||
| def propContent = "version=${clientNativeVersion}" | ||
| new File("$project.rootDir/openapi-cli/src/main/resources/openapi-client-native-version.properties") | ||
| .text = propContent | ||
| new File("$project.rootDir/openapi-bal-task-plugin/src/main/resources/openapi-client-native-version.properties") | ||
| .text = propContent | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Avoid mutating source directories during the build.
The createProperties task writes generated properties directly into src/main/resources. This modifies the tracked source tree during the build, which breaks Gradle's task up-to-date checks, disrupts Configuration Caching, and pollutes the version control workspace.
Consider generating these properties into the respective subprojects' layout.buildDirectory instead, and include those generated paths in their sourceSets.main.resources.srcDir(...).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.gradle` around lines 120 - 128, Update the createProperties task to
write both generated properties files under the respective subprojects’
layout.buildDirectory rather than src/main/resources, then configure each
subproject’s sourceSets.main.resources.srcDir(...) to include its generated
directory while preserving the existing property content.
| classpath = files() | ||
| } | ||
| } | ||
| jBallerinaPack.dependsOn ':module-ballerina-openapi:patchBallerinaScripts' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Mismatched script patching task dependency.
The task :module-ballerina-openapi:patchBallerinaScripts patches scripts in module-ballerina-openapi's build directory, not the extracted-distribution directory of this module. If the integration tests rely on executing the Ballerina binaries extracted by this module's jBallerinaPack, they will fail with permission denied errors because the extracted scripts are never made executable.
Consider either patching the scripts directly within this module's jBallerinaPack task or creating a dedicated patch task for this module's extracted distribution.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openapi-integration-tests/build.gradle` at line 130, The jBallerinaPack
dependency points to patchBallerinaScripts in another module, leaving this
module’s extracted distribution scripts unpatched and non-executable. Update the
openapi-integration-tests packaging flow so its own jBallerinaPack output is
patched, either by invoking patching within jBallerinaPack or by adding and
depending on a module-local patch task.
|
1 participant
Purpose
Migrates the module to Java 25 and Gradle 9.5.1, excluding any Java source (
*.java) changes. This branch contains only build/config updates (Gradle wrapper, build scripts,Ballerina.toml/Dependencies.toml, workflows, spotbugs/checkstyle config).Note:
java-25-migrationalso includes a YAML schema-ordering fix (YamlSchemaOrderPatch.javaand related changes inOASResult.java/SubCmdBase.java) plus a test-isolation fix inOpenAPICmdTest.java, made necessary by the dependency version bump in this migration. Those are intentionally excluded here to keep this PR's diff Java-free; seejava-25-migrationfor the full migration including those fixes.Verified that the diff against
mastercontains no*.javafiles.