Fix Java feature installMaven failure when version="none" (missing var/platform) - #1721
Conversation
Co-authored-by: Kaniska244 <186041440+Kaniska244@users.noreply.github.com>
Co-authored-by: Kaniska244 <186041440+Kaniska244@users.noreply.github.com>
|
Copilot Change the configuration of the newly added test by replacing the base image with |
Co-authored-by: Kaniska244 <186041440+Kaniska244@users.noreply.github.com>
Updated the |
There was a problem hiding this comment.
🟡 Changes recommended
The new uname-based platform detection misses common Linux architecture aliases (e.g. i386/amd64), which can cause incorrect SDKMAN platform selection on some systems.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes a regression in the Java feature’s SDKMAN version resolution path when version: "none" is used, ensuring Maven installation can still proceed even if ${SDKMAN_DIR}/var/platform was never created in the current run.
Changes:
- Added
get_sdkman_platform()to safely obtain the SDKMAN platform identifier with auname-based fallback. - Updated
find_version_list()to use the new helper instead of unconditionally reading${SDKMAN_DIR}/var/platform. - Added a regression test scenario covering
version: "none"withinstallMaven: trueand a specific Maven version.
File summaries
| File | Description |
|---|---|
| src/java/install.sh | Adds a platform-detection fallback so SDKMAN API calls don’t depend on var/platform being present. |
| test/java/scenarios.json | Adds a scenario reproducing the “no Java install, but install Maven 3.8.8” case. |
| test/java/install_maven_specific_version_no_java.sh | Adds a test asserting Maven 3.8.8 is installed successfully in the new scenario. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Copilot The patch version bump required for the java feature. |
Co-authored-by: Kaniska244 <186041440+Kaniska244@users.noreply.github.com>
Bumped the Java feature patch version to 1.8.3 in 3cde164. |
When
version: "none"is combined withinstallMaven: trueand a plain numericmavenVersion(e.g.3.8.8), the feature aborts with:This is a regression from #1714, which made
find_version_list()read${SDKMAN_DIR}/var/platformunconditionally. That file is only written by SDKMAN's own bootstrap. Withversion: "none", the JDK install returns early without bootstrapping SDKMAN, so whenSDKMAN_DIRalready exists (cached layer/base image) the file is absent and the subsequent Maven install fails.Changes
src/java/install.sh: Added aget_sdkman_platform()helper that readsvar/platformwhen present, and otherwise derives the platform identifier fromuname(mirroring get.sdkman.io'sinfer_platform). Replaced the unconditionalcatinfind_version_list()with a call to this helper.test/java: Added theinstall_maven_specific_version_no_javascenario (version: "none",installMaven: true,mavenVersion: "3.8.8",installGradle: false) and a test asserting Maven 3.8.8 installs successfully.