Repository navigation
build: add the Maven wrapper and use it in CI - #20496
Conversation
Pin Maven 3.10.0 with the Maven wrapper so CI no longer changes Maven version when the runner image updates. CI scripts and workflows call ./mvnw instead of mvn. Register the wrapper scripts from Apache Maven Wrapper in LICENSE, NOTICE and licenses.yaml.
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
Fix the wrapper archive/checksum fallback before merging. On hosts without unzip, the wrapper changes the configured ZIP download to TAR.GZ but still validates it against the ZIP checksum, so it exits before extracting Maven; the new wrapper is therefore not portable to minimal environments even though it includes that fallback.
Reviewed 21 of 21 changed files.
Validation: GitHub reports all listed PR checks passing; local git merge-tree --write-tree origin/master HEAD completed successfully. sh -n mvnw, bash -n for all changed shell scripts, and git diff --check excluding the intentional CRLF Windows wrapper passed. No local Maven build or test suite was run.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
This is an automated review by Codex GPT-5.6 Luna(Max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
|
I'm unsure if this is actually worth it - it could make it more likely that we will never start supporting more recent versions...and eventually lag behind... |
FrankChen021
left a comment
There was a problem hiding this comment.
🟢 Approval recommended
The wrapper still fails to bootstrap Maven on hosts without unzip: it downloads a .tar.gz archive but validates it against the configured ZIP checksum. The author’s reply explains the generated-script and upstream constraint. Because this affects only hosts lacking unzip and is recoverable by installing it, I consider the P2 portability issue non-blocking for the current CI and common developer setup; it remains actionable for broader wrapper portability.
Reviewed all 21 changed files: .github/scripts/analyze_dependencies_script.sh, .github/scripts/create-jacoco-coverage-report.sh, .github/scripts/license_checks_script.sh, .github/scripts/openrewrite.sh, .github/scripts/packaging-check.sh, .github/scripts/run_docker-tests, .github/scripts/run_unit-tests, .github/scripts/static-checks-maven.sh, .github/scripts/strict-compilation.sh, .github/scripts/web-checks.sh, .github/workflows/codeql.yml, .github/workflows/cron-job-its.yml, .github/workflows/docker-tests.yml, .github/workflows/worker.yml, .mvn/wrapper/maven-wrapper.properties, LICENSE, NOTICE, docs/development/build.md, licenses.yaml, mvnw, and mvnw.cmd.
Validation: sh -n mvnw and bash -n for all 10 changed shell scripts passed. git diff --check passed for all changed files except mvnw.cmd; the full check reports that generated Windows wrapper’s intentional CRLF line endings as trailing whitespace. No Maven build or test suite was run.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
The nice thing is that dependabot already has built-in support for maven-wrapper, so we should get updates just like any other dependency. |
kgyrtkirk
left a comment
There was a problem hiding this comment.
Cool - In that case; I shouldn't be an obstacle ;)
For keeping consistency it would be nice to validate that all calls are to ./mvnw under .github ( there are some smaller checks running somewhere at the end of the jacoco job which is active for all prs)
Seems like setup-java will cache that too...so it should be good
Description
CI currently uses whatever Maven the GitHub runner image ships, so a runner image update can change the Maven version (and with it Maven Resolver behaviour) without any change in this repository (This happened to us internally with our own GitHub runners). This PR adds the Maven wrapper and switches CI to it, so the Maven version is pinned in the repository and changes only through a reviewed PR.
ubuntu-latestcurrently provides).mvninvocations in.github/scripts/*and.github/workflows/*now use./mvnw.This PR has: