chore: update Maven Wrapper to 3.9.9#185
Conversation
WalkthroughThis update removes the Java-based Maven wrapper downloader, modernizes the Maven wrapper shell and Windows scripts, and updates the Maven wrapper properties. The scripts now handle Maven distribution downloads, verification, and execution directly, eliminating the need for the Java helper. The wrapper configuration is updated for newer Maven versions and licensing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant mvnw/mvnw.cmd
participant MavenWrapperProperties
participant RemoteRepo
participant LocalCache
participant Maven
User->>mvnw/mvnw.cmd: Run Maven command
mvnw/mvnw.cmd->>MavenWrapperProperties: Read distributionUrl, checksum
alt Maven not in LocalCache
mvnw/mvnw.cmd->>RemoteRepo: Download Maven distribution
mvnw/mvnw.cmd->>mvnw/mvnw.cmd: Verify checksum (if present)
mvnw/mvnw.cmd->>LocalCache: Extract and cache Maven distribution
end
mvnw/mvnw.cmd->>Maven: Execute Maven command with user arguments
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (9)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #185 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 20 20
===========================================
Files 2 2
Lines 49 49
Branches 4 4
===========================================
Hits 49 49 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
.mvn/wrapper/maven-wrapper.properties (1)
17-20: 💡 Verification agent❓ Verification inconclusive
Consider adding a SHA‑256 checksum entry to keep supply‑chain attacks at bay
The new wrapper scripts support checksum validation (
distributionSha256Sum=), but the property is currently absent.
Without it, every download is accepted blindly, defeating one of the main security benefits of the 3.3.x wrapper.If you publish artifacts reproducibly, compute the SHA‑256 of
apache-maven-3.9.9-bin.ziponce and pin it here, e.g.distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.9/apache-maven-3.9.9-bin.zip +distributionSha256Sum=<PASTE_CHECKSUM_HERE>The script will refuse a tampered archive and guide maintainers to update the checksum on version bumps.
Add SHA‑256 checksum to Maven Wrapper downloads
To prevent tampered distributions, pin the SHA‑256 checksum in
.mvn/wrapper/maven-wrapper.properties(supported since wrapper v3.3.x):
- File:
.mvn/wrapper/maven-wrapper.properties
After thedistributionUrlline, add:distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.9/apache-maven-3.9.9-bin.zip +distributionSha256Sum=<COMPUTED_SHA256_HERE>Compute the checksum locally (e.g.
shasum -a 256 apache-maven-3.9.9-bin.zip) and commit the resulting value. The wrapper will refuse any archive that doesn’t match.
🧹 Nitpick comments (3)
pom.xml (1)
133-134: Helpful section header addedThe Lombok block is now easier to spot—nice touch.
If you’d like ultimate consistency, consider adding similar headers to the other dependency groups in future clean‑ups.
mvnw.cmd (2)
124-133: Minor: avoid re‑reading the properties fileAfter adopting the earlier suggestion to cache
$properties, you can replace these lines with:$distributionSha256Sum = $properties.distributionSha256SumOne IO hit saved and no risk of reading stale data after edits.
85-90: Nit: MD5 is fine for path hash, but SHA‑1 is built‑in & avoids custom codeYou’re only generating a directory name, yet you spin up
System.Security.Cryptography.MD5.
(Get-FileHash -InputObject $distributionUrl -Algorithm SHA1).Hashis a single PowerShell call and MD5’s reputation is brittle.Purely cosmetic—feel free to ignore.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.mvn/wrapper/maven-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (5)
.mvn/wrapper/MavenWrapperDownloader.java(0 hunks).mvn/wrapper/maven-wrapper.properties(1 hunks)mvnw(2 hunks)mvnw.cmd(1 hunks)pom.xml(3 hunks)
💤 Files with no reviewable changes (1)
- .mvn/wrapper/MavenWrapperDownloader.java
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (2)
pom.xml (2)
3-5: XML re‑formatting looks goodNamespace & schemaLocation were merely re‑flowed; no functional impact detected.
17-18:<relativePath />change is benignSelf‑closing with a space is still valid XML and keeps behaviour identical.
| # apply MVNW_REPOURL and calculate MAVEN_HOME | ||
| # maven home pattern: ~/.m2/wrapper/dists/{apache-maven-<version>,maven-mvnd-<version>-<platform>}/<hash> | ||
| if ($env:MVNW_REPOURL) { | ||
| $MVNW_REPO_PATTERN = if ($USE_MVND) { "/org/apache/maven/" } else { "/maven/mvnd/" } | ||
| $distributionUrl = "$env:MVNW_REPOURL$MVNW_REPO_PATTERN$($distributionUrl -replace '^.*'+$MVNW_REPO_PATTERN,'')" | ||
| } |
There was a problem hiding this comment.
Repository‑override logic swaps Maven ↔ MVND paths
When MVNW_REPOURL is provided, the pattern assignment is inverted:
$MVNW_REPO_PATTERN = if ($USE_MVND) { "/org/apache/maven/" } else { "/maven/mvnd/" }Expected:
- Standard Maven (
$USE_MVND = $false) ➜/org/apache/maven/ - MVND (
$USE_MVND = $true) ➜/maven/mvnd/
Current code does the opposite, generating wrong URLs and 404s.
Fix:
-$MVNW_REPO_PATTERN = if ($USE_MVND) { "/org/apache/maven/" } else { "/maven/mvnd/" }
+$MVNW_REPO_PATTERN = if ($USE_MVND) { "/maven/mvnd/" } else { "/org/apache/maven/" }There was a problem hiding this comment.
These scripts were generated using mvn -N wrapper:wrapper -Dmaven=3.9.9
Any issues or oddities should be fixed upstream in the Maven Wrapper project.
There was a problem hiding this comment.
@nanotaboada Thank you for the clarification! You're absolutely right - since these are auto-generated scripts from the official Maven Wrapper plugin, it makes perfect sense to keep them as-is and address any issues upstream rather than diverging from the official implementation.
If you'd like to report the repository URL pattern issue to the Maven Wrapper project, I'd be happy to help you create an issue there. Otherwise, using the scripts as officially generated is the right approach for maintaining consistency and avoiding future conflicts when regenerating the wrapper.
✏️ Learnings added
Learnt from: nanotaboada
PR: nanotaboada/java.samples.spring.boot#185
File: mvnw.cmd:73-78
Timestamp: 2025-05-25T20:38:22.933Z
Learning: Maven Wrapper scripts (mvnw, mvnw.cmd, and related files) are auto-generated using the command `mvn -N wrapper:wrapper -Dmaven=<version>` and should not be modified locally. Any issues should be reported to the upstream Maven Wrapper project rather than fixed in individual repositories.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| $distributionUrl = (Get-Content -Raw "$scriptDir/.mvn/wrapper/maven-wrapper.properties" | ConvertFrom-StringData).distributionUrl | ||
| if (!$distributionUrl) { | ||
| Write-Error "cannot read distributionUrl property in $scriptDir/.mvn/wrapper/maven-wrapper.properties" | ||
| } | ||
|
|
||
| switch -wildcard -casesensitive ( $($distributionUrl -replace '^.*/','') ) { |
There was a problem hiding this comment.
Parsing of maven-wrapper.properties will fail on licence header lines
ConvertFrom-StringData expects every non‑blank line to contain an = sign.
Because the properties file now begins with an ASF licence header, the first line (“# Licensed to …”) throws:
ConvertFrom-StringData : Data missing '=' in the string.
Execution stops before $distributionUrl is set.
Proposed fix – strip comments/blank lines first:
-$distributionUrl = (Get-Content -Raw "$scriptDir/.mvn/wrapper/maven-wrapper.properties" | ConvertFrom-StringData).distributionUrl
+$properties = Get-Content "$scriptDir/.mvn/wrapper/maven-wrapper.properties" |
+ Where-Object { $_ -match '^\s*[^#].*=' } |
+ ConvertFrom-StringData
+$distributionUrl = $properties.distributionUrlThis also lets you reuse $properties later instead of re‑reading the file (see lines 124‑125).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # calculate distributionUrl, requires .mvn/wrapper/maven-wrapper.properties | |
| $distributionUrl = (Get-Content -Raw "$scriptDir/.mvn/wrapper/maven-wrapper.properties" | ConvertFrom-StringData).distributionUrl | |
| if (!$distributionUrl) { | |
| Write-Error "cannot read distributionUrl property in $scriptDir/.mvn/wrapper/maven-wrapper.properties" | |
| } | |
| # calculate distributionUrl, requires .mvn/wrapper/maven-wrapper.properties | |
| $properties = Get-Content "$scriptDir/.mvn/wrapper/maven-wrapper.properties" | | |
| Where-Object { $_ -match '^\s*[^#].*=' } | | |
| ConvertFrom-StringData | |
| $distributionUrl = $properties.distributionUrl | |
| if (!$distributionUrl) { | |
| Write-Error "cannot read distributionUrl property in $scriptDir/.mvn/wrapper/maven-wrapper.properties" | |
| } |
There was a problem hiding this comment.
Open a follow-up GitHub issue for this discussion.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the maven-wrapper.properties parsing problem
gh issue create \
--title "Fix parsing of maven-wrapper.properties in mvnw.cmd script" \
--body "## Problem
The PowerShell section of the mvnw.cmd script fails to parse the maven-wrapper.properties file properly when it contains license header comments.
\`\`\`powershell
# Current problematic code
\$distributionUrl = (Get-Content -Raw \"\$scriptDir/.mvn/wrapper/maven-wrapper.properties\" | ConvertFrom-StringData).distributionUrl
\`\`\`
## Impact
When running the Maven wrapper on Windows, execution stops with the following error:
\`\`\`
ConvertFrom-StringData : Data missing '=' in the string.
\`\`\`
This happens because \`ConvertFrom-StringData\` expects every non-blank line to contain an '=' sign, but the properties file begins with ASF license header comments.
## Proposed Solution
Modify the PowerShell code to strip comments and blank lines before parsing:
\`\`\`powershell
# Fixed approach
\$properties = Get-Content \"\$scriptDir/.mvn/wrapper/maven-wrapper.properties\" |
Where-Object { \$_ -match '^\\s*[^#].*=' } |
ConvertFrom-StringData
\$distributionUrl = \$properties.distributionUrl
\`\`\`
This also provides the benefit of reusing \$properties later instead of re-reading the file (see lines 124-125 in mvnw.cmd).
## References
- Identified in PR: #185
- Original comment: https://github.com/nanotaboada/java.samples.spring.boot/pull/185#discussion_r2051740924
" \
--assignee "nanotaboada"
Length of output: 1494
@nanotaboada I've created a GitHub issue to track this problem. You can find it here: #189
The issue includes details about the PowerShell parsing problem with maven-wrapper.properties, the impact, and the proposed solution with code snippets. All references to the original PR and comment have been included.
There was a problem hiding this comment.
These scripts were generated using mvn -N wrapper:wrapper -Dmaven=3.9.9
Any issues or oddities should be fixed upstream in the Maven Wrapper project.
There was a problem hiding this comment.
🧩 Analysis chain
@nanotaboada Thank you for that clarification! You're absolutely right - since these scripts are auto-generated using the official Maven wrapper command, any issues should indeed be reported upstream to the Maven Wrapper project rather than being fixed locally.
The parsing issue I identified is still valid, but it belongs in the upstream Maven Wrapper codebase. Local fixes would just get overwritten the next time the wrapper is regenerated.
I'll close the GitHub issue I created since it's not actionable in this repository.
🏁 Script executed:
#!/bin/bash
# Close the GitHub issue since it should be reported upstream instead
gh issue close 189 --comment "Closing this issue as the Maven wrapper scripts are auto-generated using \`mvn -N wrapper:wrapper -Dmaven=3.9.9\`. Any issues should be reported upstream to the Maven Wrapper project instead of being fixed locally, as local changes would be overwritten when regenerating the wrapper."
Length of output: 431
Perfect! I've successfully closed issue #189 with an explanation that the Maven wrapper scripts are auto-generated and any issues should be reported upstream to the Maven Wrapper project.
Thank you for providing that important context about how these scripts are maintained. It's a good reminder that auto-generated files should have their issues addressed at the source rather than through local patches.
✏️ Learnings added
Learnt from: nanotaboada
PR: nanotaboada/java.samples.spring.boot#185
File: mvnw.cmd:53-58
Timestamp: 2025-05-25T20:38:30.719Z
Learning: Maven wrapper scripts (mvnw, mvnw.cmd, MavenWrapperDownloader.java) are auto-generated using the command `mvn -N wrapper:wrapper -Dmaven=X.Y.Z`. Any issues or bugs in these generated scripts should be reported upstream to the Maven Wrapper project rather than being fixed locally, as local changes would be overwritten when the wrapper is regenerated.
Learnt from: nanotaboada
PR: nanotaboada/java.samples.spring.boot#185
File: mvnw.cmd:53-58
Timestamp: 2025-05-25T20:38:30.719Z
Learning: Maven wrapper scripts (mvnw, mvnw.cmd, MavenWrapperDownloader.java) are auto-generated using the command `mvn -N wrapper:wrapper -Dmaven=X.Y.Z`. Any issues or bugs in these generated scripts should be reported upstream to the Maven Wrapper project rather than being fixed locally, as local changes would be overwritten when the wrapper is regenerated.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| if ! command -v unzip >/dev/null; then | ||
| distributionUrl="${distributionUrl%.zip}.tar.gz" | ||
| distributionUrlName="${distributionUrl##*/}" | ||
| fi | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
distributionUrlNameMain becomes stale when switching from .zip to .tar.gz
If unzip is unavailable the script rewrites the URL to a tar.gz, updates
distributionUrl and distributionUrlName, but does not recompute
distributionUrlNameMain.
Consequently the final mv (and the mvnw.url pathname) point to a directory
that does not exist, breaking installation on minimal systems.
- distributionUrl="${distributionUrl%.zip}.tar.gz"
- distributionUrlName="${distributionUrl##*/}"
+ distributionUrl="${distributionUrl%.zip}.tar.gz"
+ distributionUrlName="${distributionUrl##*/}"
+ # keep the “main” name in sync with the new extension
+ distributionUrlNameMain="${distributionUrlName%.*}"
+ distributionUrlNameMain="${distributionUrlNameMain%-bin}"Also applies to: 255-257
There was a problem hiding this comment.
Open a follow-up GitHub issue for this discussion.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the Maven wrapper bug
gh issue create \
--title "Fix: Maven Wrapper breaks on systems without unzip command" \
--body "## Issue Description
When running the Maven wrapper on systems without the \`unzip\` command installed, the wrapper fails to properly install Maven.
### Technical Details
The issue occurs in \`mvnw\` script when it detects \`unzip\` is unavailable and switches to using \`.tar.gz\` format:
\`\`\`bash
# select .zip or .tar.gz
if ! command -v unzip >/dev/null; then
distributionUrl=\"\${distributionUrl%.zip}.tar.gz\"
distributionUrlName=\"\${distributionUrl##*/}\"
fi
\`\`\`
However, the script doesn't update the \`distributionUrlNameMain\` variable which was derived earlier from the original filename. This variable is used later for directory operations, causing the installation to fail on systems without \`unzip\`.
### Suggested Fix
Update the code to recalculate \`distributionUrlNameMain\` after changing the file extension:
\`\`\`diff
- distributionUrl=\"\${distributionUrl%.zip}.tar.gz\"
- distributionUrlName=\"\${distributionUrl##*/}\"
+ distributionUrl=\"\${distributionUrl%.zip}.tar.gz\"
+ distributionUrlName=\"\${distributionUrl##*/}\"
+ # keep the \"main\" name in sync with the new extension
+ distributionUrlNameMain=\"\${distributionUrlName%.*}\"
+ distributionUrlNameMain=\"\${distributionUrlNameMain%-bin}\"
\`\`\`
### References
- Identified in PR #185: https://github.com/nanotaboada/java.samples.spring.boot/pull/185#discussion_r2051740927
" \
--assignee "nanotaboada"
# Output the URL of the created issue
echo -e "\nIssue created successfully. URL:"
gh issue list --state open --limit 1 --json url --jq '.[0].url'
Length of output: 1738
@nanotaboada I've created a GitHub issue to track the Maven wrapper bug. The issue has been successfully created and assigned to you:
Issue #186: Fix: Maven Wrapper breaks on systems without unzip command
The issue includes all the details about the bug in the Maven wrapper script when running on systems without unzip, along with the suggested fix.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
These scripts were generated using mvn -N wrapper:wrapper -Dmaven=3.9.9
Any issues or oddities should be fixed upstream in the Maven Wrapper project.
There was a problem hiding this comment.
@nanotaboada Thank you for that important clarification! You're absolutely right - since these wrapper scripts are auto-generated by the official Maven wrapper plugin (mvn -N wrapper:wrapper -Dmaven=3.9.9), any bugs should indeed be reported upstream to the Apache Maven Wrapper project rather than being addressed in individual repositories.
Would you like me to close the issue I created (#186) since this should be reported to the upstream Maven Wrapper project instead? The bug I identified would need to be fixed in the official Maven wrapper generation logic rather than in your repository.
For future reference, issues like this should be reported to:
-
Or through the Apache Maven JIRA
(\ /)( ..)
o(")(")
thoughtful rabbit contemplating upstream dependencies
✏️ Learnings added
Learnt from: nanotaboada
PR: nanotaboada/java.samples.spring.boot#185
File: mvnw:176-180
Timestamp: 2025-05-25T20:38:16.077Z
Learning: Maven wrapper scripts (mvnw, mvnw.cmd) are typically auto-generated using `mvn -N wrapper:wrapper` command and should not be manually modified. Any bugs or issues in these scripts should be reported upstream to the Apache Maven Wrapper project rather than being fixed in individual repositories.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| JAVACMD="$( | ||
| 'set' +e | ||
| 'unset' -f command 2>/dev/null | ||
| 'command' -v java | ||
| )" || : | ||
| JAVACCMD="$( | ||
| 'set' +e | ||
| 'unset' -f command 2>/dev/null | ||
| 'command' -v javac | ||
| )" || : |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Incorrect quoting prevents fallback Java detection from ever succeeding
The subshell that tries to discover java/javac ('set' +e, 'command' -v …) wraps the built‑ins in single quotes.
With the quotes the shell literally looks for executables called set, unset and command, which do not exist, so the whole command substitution fails and JAVACMD/JAVACCMD remain empty.
As a consequence the Java‑based download fallback will never be selected even when wget/curl are missing.
- JAVACMD="$(
- 'set' +e
- 'unset' -f command 2>/dev/null
- 'command' -v java
- )" || :
- JAVACCMD="$(
- 'set' +e
- 'unset' -f command 2>/dev/null
- 'command' -v javac
- )" || :
+ JAVACMD="$(
+ set +e
+ unset -f command 2>/dev/null
+ command -v java
+ )" || :
+ JAVACCMD="$(
+ set +e
+ unset -f command 2>/dev/null
+ command -v javac
+ )" || :📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| JAVACMD="$( | |
| 'set' +e | |
| 'unset' -f command 2>/dev/null | |
| 'command' -v java | |
| )" || : | |
| JAVACCMD="$( | |
| 'set' +e | |
| 'unset' -f command 2>/dev/null | |
| 'command' -v javac | |
| )" || : | |
| JAVACMD="$( | |
| set +e | |
| unset -f command 2>/dev/null | |
| command -v java | |
| )" || : | |
| JAVACCMD="$( | |
| set +e | |
| unset -f command 2>/dev/null | |
| command -v javac | |
| )" || : |
There was a problem hiding this comment.
Open a follow-up GitHub issue for this discussion.
There was a problem hiding this comment.
🧩 Analysis chain
@nanotaboada I'll create a GitHub issue to track the Maven wrapper script bug. Let me prepare that for you.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the Maven wrapper script bug
ISSUE_TITLE="Fix: Maven wrapper script quoting bug prevents Java fallback detection"
ISSUE_BODY="## Bug Description
In the Maven wrapper script (\`mvnw\`), there's a bug where single quotes around shell built-ins prevent Java detection from working properly.
The following code in \`mvnw\`:
\`\`\`shell
JAVACMD=\"\$(
'set' +e
'unset' -f command 2>/dev/null
'command' -v java
)\" || :
JAVACCMD=\"\$(
'set' +e
'unset' -f command 2>/dev/null
'command' -v javac
)\" || :
\`\`\`
Should be fixed by removing the single quotes:
\`\`\`shell
JAVACMD=\"\$(
set +e
unset -f command 2>/dev/null
command -v java
)\" || :
JAVACCMD=\"\$(
set +e
unset -f command 2>/dev/null
command -v javac
)\" || :
\`\`\`
### Impact
With the quotes, the shell literally looks for executables called \`set\`, \`unset\`, and \`command\`, which do not exist. This causes the command substitution to fail and \`JAVACMD\`/\`JAVACCMD\` remain empty. As a consequence, the Java-based download fallback will never be selected even when \`wget\`/\`curl\` are missing.
## References
- PR: https://github.com/nanotaboada/java.samples.spring.boot/pull/185
- Comment: https://github.com/nanotaboada/java.samples.spring.boot/pull/185#discussion_r2051740929"
# Create the issue with the GitHub CLI
gh issue create --repo nanotaboada/java.samples.spring.boot \
--title "$ISSUE_TITLE" \
--body "$ISSUE_BODY" \
--assignee nanotaboada
Length of output: 2631
@nanotaboada I've created a GitHub issue to track the Maven wrapper script bug. The issue has been successfully created and assigned to you:
Issue #187: Fix: Maven wrapper script quoting bug prevents Java fallback detection
URL: #187
The issue includes details about the incorrect quoting in the Maven wrapper script that prevents the Java fallback detection from working properly, along with the suggested fix.
🐰 Happy to help organize follow-up tasks!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
These scripts were generated using mvn -N wrapper:wrapper -Dmaven=3.9.9
Any issues or oddities should be fixed upstream in the Maven Wrapper project.
There was a problem hiding this comment.
@nanotaboada You're absolutely right! Thank you for that important clarification. Since the mvnw scripts are generated by the Maven Wrapper plugin, the quoting bug I identified should indeed be reported upstream to the Apache Maven Wrapper project rather than fixed in your repository.
The issue I created (#187) in your repository should probably be closed or converted to a note about reporting this upstream. Would you like me to:
- Close the current issue and create a new one to track reporting this to the upstream Maven Wrapper project?
- Or would you prefer to handle reporting this upstream yourself?
The bug is in the generated template used by mvn wrapper:wrapper, so it affects all projects that use the Maven Wrapper.
✏️ Learnings added
Learnt from: nanotaboada
PR: nanotaboada/java.samples.spring.boot#185
File: mvnw:63-72
Timestamp: 2025-05-25T20:38:06.208Z
Learning: Maven wrapper scripts (mvnw, mvnw.cmd) in Java projects are typically generated using `mvn -N wrapper:wrapper -Dmaven=<version>` command and should not be manually modified. Issues in these scripts should be reported upstream to the Apache Maven Wrapper project.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| case "${MVNW_PASSWORD:+has-password}" in | ||
| '') MVNW_USERNAME='' MVNW_PASSWORD='' ;; | ||
| has-password) [ -n "${MVNW_USERNAME-}" ] || MVNW_USERNAME='' MVNW_PASSWORD='' ;; | ||
| esac | ||
|
|
||
| if [ -z "$1" ] | ||
| then | ||
| echo "Path not specified to find_maven_basedir" | ||
| return 1 | ||
| fi | ||
| if [ -z "${MVNW_USERNAME-}" ] && command -v wget >/dev/null; then | ||
| verbose "Found wget ... using wget" | ||
| wget ${__MVNW_QUIET_WGET:+"$__MVNW_QUIET_WGET"} "$distributionUrl" -O "$TMP_DOWNLOAD_DIR/$distributionUrlName" || die "wget: Failed to fetch $distributionUrl" | ||
| elif [ -z "${MVNW_USERNAME-}" ] && command -v curl >/dev/null; then | ||
| verbose "Found curl ... using curl" | ||
| curl ${__MVNW_QUIET_CURL:+"$__MVNW_QUIET_CURL"} -f -L -o "$TMP_DOWNLOAD_DIR/$distributionUrlName" "$distributionUrl" || die "curl: Failed to fetch $distributionUrl" | ||
| elif set_java_home; then |
There was a problem hiding this comment.
🛠️ Refactor suggestion
HTTP authentication variables are collected but never used
MVNW_USERNAME / MVNW_PASSWORD are documented, yet neither the wget nor
curl invocation applies them, so private repositories cannot be reached.
- wget ${__MVNW_QUIET_WGET:+"$__MVNW_QUIET_WGET"} "$distributionUrl" -O "$TMP_DOWNLOAD_DIR/$distributionUrlName" || die "wget: Failed to fetch $distributionUrl"
+ wget ${__MVNW_QUIET_WGET:+"$__MVNW_QUIET_WGET"} \
+ ${MVNW_USERNAME:+--user="$MVNW_USERNAME"} \
+ ${MVNW_PASSWORD:+--password="$MVNW_PASSWORD"} \
+ "$distributionUrl" -O "$TMP_DOWNLOAD_DIR/$distributionUrlName" \
+ || die "wget: Failed to fetch $distributionUrl"
@@
- curl ${__MVNW_QUIET_CURL:+"$__MVNW_QUIET_CURL"} -f -L -o "$TMP_DOWNLOAD_DIR/$distributionUrlName" "$distributionUrl" || die "curl: Failed to fetch $distributionUrl"
+ curl ${__MVNW_QUIET_CURL:+"$__MVNW_QUIET_CURL"} -f -L \
+ ${MVNW_USERNAME:+-u "${MVNW_USERNAME}:${MVNW_PASSWORD}"} \
+ -o "$TMP_DOWNLOAD_DIR/$distributionUrlName" "$distributionUrl" \
+ || die "curl: Failed to fetch $distributionUrl"This small change restores the advertised functionality without altering
behaviour for public downloads.
There was a problem hiding this comment.
Open a follow-up GitHub issue for this discussion.
There was a problem hiding this comment.
🧩 Analysis chain
@nanotaboada I've created a GitHub issue for tracking the HTTP authentication variables problem in the Maven wrapper script. The issue includes a detailed description of the problem and the suggested fix that would allow the Maven wrapper to properly use the documented MVNW_USERNAME and MVNW_PASSWORD environment variables when accessing private repositories.
The issue has been assigned to you as requested. Let me know if you need any other follow-up actions regarding this matter.
🏁 Script executed:
#!/bin/bash
# Create a new GitHub issue for the HTTP authentication variables not being used in Maven wrapper
gh issue create \
--title "Maven Wrapper HTTP authentication variables not used in wget/curl commands" \
--body "## Issue Description
The Maven Wrapper script (\`mvnw\`) documents \`MVNW_USERNAME\` and \`MVNW_PASSWORD\` environment variables for authentication, but these variables aren't actually used in the \`wget\` or \`curl\` commands that download the Maven distribution.
This means that private repositories requiring authentication cannot be accessed.
## Suggested Fix
Update the \`wget\` and \`curl\` commands in the Maven wrapper script to use the authentication variables:
For \`wget\`:
\`\`\`diff
- wget \${__MVNW_QUIET_WGET:+\"\$__MVNW_QUIET_WGET\"} \"\$distributionUrl\" -O \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" || die \"wget: Failed to fetch \$distributionUrl\"
+ wget \${__MVNW_QUIET_WGET:+\"\$__MVNW_QUIET_WGET\"} \\
+ \${MVNW_USERNAME:+--user=\"\$MVNW_USERNAME\"} \\
+ \${MVNW_PASSWORD:+--password=\"\$MVNW_PASSWORD\"} \\
+ \"\$distributionUrl\" -O \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \\
+ || die \"wget: Failed to fetch \$distributionUrl\"
\`\`\`
For \`curl\`:
\`\`\`diff
- curl \${__MVNW_QUIET_CURL:+\"\$__MVNW_QUIET_CURL\"} -f -L -o \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \"\$distributionUrl\" || die \"curl: Failed to fetch \$distributionUrl\"
+ curl \${__MVNW_QUIET_CURL:+\"\$__MVNW_QUIET_CURL\"} -f -L \\
+ \${MVNW_USERNAME:+-u \"\${MVNW_USERNAME}:\${MVNW_PASSWORD}\"} \\
+ -o \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \"\$distributionUrl\" \\
+ || die \"curl: Failed to fetch \$distributionUrl\"
\`\`\`
## References
- PR discussion: https://github.com/nanotaboada/java.samples.spring.boot/pull/185#discussion_r2051740930
This small change would restore the advertised functionality without altering behavior for public downloads." \
--assignee "nanotaboada"
echo "Issue created successfully."
Length of output: 1885
@nanotaboada I've created a follow-up GitHub issue as requested. The issue #188 is now available at #188 and has been assigned to you. The issue includes the complete details about the HTTP authentication variables problem and the suggested fix.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
These scripts were generated using mvn -N wrapper:wrapper -Dmaven=3.9.9
Any issues or oddities should be fixed upstream in the Maven Wrapper project.
There was a problem hiding this comment.
🧩 Analysis chain
@nanotaboada Thank you for that important clarification! You're absolutely right - since these scripts are generated by the official Maven wrapper plugin, any issues should indeed be addressed upstream with the Apache Maven Wrapper project.
I'll keep the issue open as a reminder to potentially report this upstream to the Maven Wrapper project, but it's correctly identified now as an upstream issue rather than something to fix in your repository.
🐰 thoughtfully considers the upstream nature of generated code
🏁 Script executed:
#!/bin/bash
# Update the GitHub issue to reflect that this is an upstream Maven Wrapper issue
gh issue edit 188 \
--add-label "upstream" \
--body "## Issue Description
The Maven Wrapper script (\`mvnw\`) documents \`MVNW_USERNAME\` and \`MVNW_PASSWORD\` environment variables for authentication, but these variables aren't actually used in the \`wget\` or \`curl\` commands that download the Maven distribution.
This means that private repositories requiring authentication cannot be accessed.
**Note**: These scripts are generated using \`mvn -N wrapper:wrapper -Dmaven=3.9.9\` from the official Apache Maven Wrapper project. This issue should be reported upstream to the Maven Wrapper project rather than fixed in this repository.
## Suggested Fix (for upstream)
Update the \`wget\` and \`curl\` commands in the Maven wrapper script to use the authentication variables:
For \`wget\`:
\`\`\`diff
- wget \${__MVNW_QUIET_WGET:+\"\$__MVNW_QUIET_WGET\"} \"\$distributionUrl\" -O \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" || die \"wget: Failed to fetch \$distributionUrl\"
+ wget \${__MVNW_QUIET_WGET:+\"\$__MVNW_QUIET_WGET\"} \\
+ \${MVNW_USERNAME:+--user=\"\$MVNW_USERNAME\"} \\
+ \${MVNW_PASSWORD:+--password=\"\$MVNW_PASSWORD\"} \\
+ \"\$distributionUrl\" -O \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \\
+ || die \"wget: Failed to fetch \$distributionUrl\"
\`\`\`
For \`curl\`:
\`\`\`diff
- curl \${__MVNW_QUIET_CURL:+\"\$__MVNW_QUIET_CURL\"} -f -L -o \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \"\$distributionUrl\" || die \"curl: Failed to fetch \$distributionUrl\"
+ curl \${__MVNW_QUIET_CURL:+\"\$__MVNW_QUIET_CURL\"} -f -L \\
+ \${MVNW_USERNAME:+-u \"\${MVNW_USERNAME}:\${MVNW_PASSWORD}\"} \\
+ -o \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \"\$distributionUrl\" \\
+ || die \"curl: Failed to fetch \$distributionUrl\"
\`\`\`
## References
- PR discussion: https://github.com/nanotaboada/java.samples.spring.boot/pull/185#discussion_r2051740930
- Maven Wrapper project: https://maven.apache.org/wrapper/
## Action Items
- [ ] Report this issue to the Apache Maven Wrapper project
- [ ] Close this issue once reported upstream
This small change would restore the advertised functionality without altering behavior for public downloads."
echo "Issue updated to reflect upstream nature"
Length of output: 2325
Perfect! The issue has been updated to properly reflect that this is an upstream Maven Wrapper issue that should be addressed with the Apache Maven project rather than in your repository. Thank you for providing that crucial context - it's exactly the kind of information that helps ensure issues are directed to the right place for resolution.
🏁 Script executed:
#!/bin/bash
# Update the GitHub issue body to reflect that this is an upstream Maven Wrapper issue (without adding label)
gh issue edit 188 \
--body "## Issue Description
The Maven Wrapper script (\`mvnw\`) documents \`MVNW_USERNAME\` and \`MVNW_PASSWORD\` environment variables for authentication, but these variables aren't actually used in the \`wget\` or \`curl\` commands that download the Maven distribution.
This means that private repositories requiring authentication cannot be accessed.
**Note**: These scripts are generated using \`mvn -N wrapper:wrapper -Dmaven=3.9.9\` from the official Apache Maven Wrapper project. This issue should be reported upstream to the Maven Wrapper project rather than fixed in this repository.
## Suggested Fix (for upstream)
Update the \`wget\` and \`curl\` commands in the Maven wrapper script to use the authentication variables:
For \`wget\`:
\`\`\`diff
- wget \${__MVNW_QUIET_WGET:+\"\$__MVNW_QUIET_WGET\"} \"\$distributionUrl\" -O \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" || die \"wget: Failed to fetch \$distributionUrl\"
+ wget \${__MVNW_QUIET_WGET:+\"\$__MVNW_QUIET_WGET\"} \\
+ \${MVNW_USERNAME:+--user=\"\$MVNW_USERNAME\"} \\
+ \${MVNW_PASSWORD:+--password=\"\$MVNW_PASSWORD\"} \\
+ \"\$distributionUrl\" -O \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \\
+ || die \"wget: Failed to fetch \$distributionUrl\"
\`\`\`
For \`curl\`:
\`\`\`diff
- curl \${__MVNW_QUIET_CURL:+\"\$__MVNW_QUIET_CURL\"} -f -L -o \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \"\$distributionUrl\" || die \"curl: Failed to fetch \$distributionUrl\"
+ curl \${__MVNW_QUIET_CURL:+\"\$__MVNW_QUIET_CURL\"} -f -L \\
+ \${MVNW_USERNAME:+-u \"\${MVNW_USERNAME}:\${MVNW_PASSWORD}\"} \\
+ -o \"\$TMP_DOWNLOAD_DIR/\$distributionUrlName\" \"\$distributionUrl\" \\
+ || die \"curl: Failed to fetch \$distributionUrl\"
\`\`\`
## References
- PR discussion: https://github.com/nanotaboada/java.samples.spring.boot/pull/185#discussion_r2051740930
- Maven Wrapper project: https://maven.apache.org/wrapper/
## Action Items
- [ ] Report this issue to the Apache Maven Wrapper project
- [ ] Close this issue once reported upstream
This small change would restore the advertised functionality without altering behavior for public downloads."
echo "Issue body updated successfully"
Length of output: 2222
Perfect! The GitHub issue #188 has been successfully updated to reflect that this is an upstream Maven Wrapper issue. The issue now clearly indicates that:
- The scripts are auto-generated using
mvn -N wrapper:wrapper -Dmaven=3.9.9 - The issue should be reported to the Apache Maven Wrapper project
- The suggested fix is documented for when/if it gets reported upstream
Thank you for providing that crucial context about the generated nature of these files. It's exactly the kind of information that ensures issues are properly directed to the right maintainers rather than suggesting changes to generated code in individual repositories.
🐰 appreciates the clarification about upstream responsibilities
✏️ Learnings added
Learnt from: nanotaboada
PR: nanotaboada/java.samples.spring.boot#185
File: mvnw:186-197
Timestamp: 2025-05-25T20:38:46.764Z
Learning: Maven wrapper scripts (mvnw, mvnw.cmd) are auto-generated using the command `mvn -N wrapper:wrapper -Dmaven=<version>` from the official Apache Maven Wrapper project. Issues in these generated scripts should be reported upstream to the Maven Wrapper project rather than being fixed in individual repositories.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
nanotaboada
force-pushed
the
feature/maven-wrapper
branch
from
806db16 to
1f398cb
Compare
|
Code Climate has analyzed commit 1f398cb and detected 0 issues on this pull request. View more on Code Climate. |
|
1 participant
https://maven.apache.org/wrapper/
This change is
Summary by CodeRabbit
Refactor
Chores