-
Notifications
You must be signed in to change notification settings - Fork 818
feat: Add support for .sdkmanrc file in java-version-file parameter
#736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add support for .sdkmanrc file in java-version-file parameter
#736
Conversation
|
@aparnajyothi-y @HarithaVattikuti sorry for the direct ping. Would it be possible for the team to take a look at the PR or at least provide some feedback any time soon? |
|
Thank you, @guicamest, for your work on this feature and for the detailed documentation and test updates! I noticed that the branch currently has conflicts with Please also ensure that all relevant documentation and test coverage boxes are checked, as per the checklist. |
…m-file-major-minor-patch-with-dist` for `.sdkmanrc`
…-patch-with-dist` test to include the file name of the java-version-file that is used
77c7e7f to
dd4a6bd
Compare
Thanks for taking a look @mahabaleshwars :) I've solved the conflicts 👍 |
|
Hi @guicamest, Thank you for this contribution! This is a valuable addition that will help teams using SDKMAN maintain consistency between local and CI environments. I have a few suggestions to make this PR even better: 📚 Documentation Request File: docs/advanced-usage.md Please add a complete example showing how to use .sdkmanrc with the java-version-file parameter. While we haven't added examples for other tool-version-files previously, this would be a good opportunity to improve our documentation. Example workflow: Also include: Expected .sdkmanrc file format (e.g., java=21.0.1-tem) 🧪 Test Coverage Improvements Invalid file formats 💡 Suggestions Consider adding: Debug logging for .sdkmanrc parsing to help troubleshoot issues ✅ Overall Once the test coverage and documentation are enhanced, this will be ready to merge. Great work on this feature! |
Add example step/file for `.sdkmanrc`
|
@mahabaleshwars Are the changes to the documentation looking good now? 54b5143
|
|
Hi @guicamest, Thank you for this valuable contribution to support .sdkmanrc files! A suggestion for the documentation: Please remove the '(NEW)' marker from the documentation. Since this will become part of the permanent documentation once merged, temporal markers like '(NEW)' can become confusing for future readers who won't know when it was considered ""new"". The feature's novelty is already captured in the PR history and release notes. |
I agree, I'll remove it. It was previously like that, but you suggested to (add) "A note for existing users about this new capability" 🤷 |
|
@mahabaleshwars The
There is already a debug log which outputs the version read from the file: core.debug(`Version from file '${fileContent}'`);I've changed it to: core.debug(`Parsed version '${capturedVersion}' from file '${versionFileName}'`);So as to have information about the filename as well. What do you think? |
|
Hi @guicamest, The dist/ directory is out of sync with your source code changes. After making your code changes, you need to rebuild the distribution files by running: Then commit the updated dist/cleanup/index.js and dist/setup/index.js files. |
|
@mahabaleshwars Thanks for the instructions ❤️ I've pushed the files after I run the commands 👍 |
|
Hi @guicamest, Unfortunately, the same checks are still failing. This issue is most likely due to line endings getting changed from LF to CRLF automatically in your code editor or local Git configuration. Please try running the following commands in your terminal to fix the formatting and validate the changes locally: Format the code: Check the formatting (this will validate the fix immediately): Run the build to ensure everything is correct: After running these, please commit and push the changes. If this doesn't resolve the issue, would it be okay if we commit the fix directly to your branch to help move this forward? |
|
Let' try it one more time: a985bcf :) If it doesn't work then your contribution will be more than welcomed! 😊 |
|
Nice, thanks a lot @mahabaleshwars ! 😌 |
[//]: # (dependabot-start)⚠️ \*\*Dependabot is rebasing this PR\*\*⚠️ Rebasing might not happen immediately, so don't worry if this takes some time. Note: if you make any changes to this PR yourself, they will take precedence over the rebase. --- [//]: # (dependabot-end) Bumps [actions/setup-java](https://github.com/actions/setup-java) from 5.0.0 to 5.1.0. Release notes *Sourced from [actions/setup-java's releases](https://github.com/actions/setup-java/releases).* > v5.1.0 > ------ > > What's Changed > -------------- > > ### New Features > > * Add support for `.sdkmanrc` file in `java-version-file` parameter by [`@guicamest`](https://github.com/guicamest) in [actions/setup-java#736](https://redirect.github.com/actions/setup-java/pull/736) > * Add support for Microsoft OpenJDK 25 builds by [`@the-mod`](https://github.com/the-mod) in [actions/setup-java#927](https://redirect.github.com/actions/setup-java/pull/927) > > ### Bug Fixes & Improvements > > * Update Regex to Support All ASDF Versions for the supported distributions in tool-versions File by [`@aparnajyothi-y`](https://github.com/aparnajyothi-y) in [actions/setup-java#767](https://redirect.github.com/actions/setup-java/pull/767) > * Enhance error logging for network failures to include endpoint/IP details, add retry mechanism and update workflows to use macos-15-intel by [`@priya-kinthali`](https://github.com/priya-kinthali) in [actions/setup-java#946](https://redirect.github.com/actions/setup-java/pull/946) > * Update SapMachine URLs by [`@RealCLanger`](https://github.com/RealCLanger) in [actions/setup-java#955](https://redirect.github.com/actions/setup-java/pull/955) > * Add GitHub Token Support for GraalVM and Refactor Code by [`@mahabaleshwars`](https://github.com/mahabaleshwars) in [actions/setup-java#849](https://redirect.github.com/actions/setup-java/pull/849) > > ### Documentation changes > > * Update documentation to use checkout and Java v5 by [`@lmvysakh`](https://github.com/lmvysakh) in [actions/setup-java#903](https://redirect.github.com/actions/setup-java/pull/903) > * Clarify JAVA\_HOME and PATH setup in README by [`@chiranjib-swain`](https://github.com/chiranjib-swain) in [actions/setup-java#841](https://redirect.github.com/actions/setup-java/pull/841) > > ### Dependency updates > > * Upgrade prettier from 2.8.8 to 3.6.2 and document breaking changes in v5 by [`@dependabot`](https://github.com/dependabot) in [actions/setup-java#873](https://redirect.github.com/actions/setup-java/pull/873) > * Upgrade actions/publish-action from 0.3.0 to 0.4.0 by [`@dependabot`](https://github.com/dependabot) in [actions/setup-java#912](https://redirect.github.com/actions/setup-java/pull/912) > > New Contributors > ---------------- > > * [`@lmvysakh`](https://github.com/lmvysakh) made their first contribution in [actions/setup-java#903](https://redirect.github.com/actions/setup-java/pull/903) > * [`@chiranjib-swain`](https://github.com/chiranjib-swain) made their first contribution in [actions/setup-java#841](https://redirect.github.com/actions/setup-java/pull/841) > * [`@the-mod`](https://github.com/the-mod) made their first contribution in [actions/setup-java#927](https://redirect.github.com/actions/setup-java/pull/927) > * [`@priya-kinthali`](https://github.com/priya-kinthali) made their first contribution in [actions/setup-java#946](https://redirect.github.com/actions/setup-java/pull/946) > * [`@guicamest`](https://github.com/guicamest) made their first contribution in [actions/setup-java#736](https://redirect.github.com/actions/setup-java/pull/736) > > **Full Changelog**: <actions/setup-java@v5...v5.1.0> Commits * [`f2beeb2`](actions/setup-java@f2beeb2) Bump actions/publish-action from 0.3.0 to 0.4.0 ([#912](https://redirect.github.com/actions/setup-java/issues/912)) * [`4e7e684`](actions/setup-java@4e7e684) feat: Add support for `.sdkmanrc` file in `java-version-file` parameter ([#736](https://redirect.github.com/actions/setup-java/issues/736)) * [`46c56d6`](actions/setup-java@46c56d6) Add GitHub Token Support for GraalVM and Refactor Code ([#849](https://redirect.github.com/actions/setup-java/issues/849)) * [`66b9457`](actions/setup-java@66b9457) Update SapMachine URLs ([#955](https://redirect.github.com/actions/setup-java/issues/955)) * [`6ba5449`](actions/setup-java@6ba5449) Enhance error logging for network failures to include endpoint/IP details, ad... * [`de5a937`](actions/setup-java@de5a937) adds microsoft openjdk25 builds ([#927](https://redirect.github.com/actions/setup-java/issues/927)) * [`ead9eaa`](actions/setup-java@ead9eaa) Update Regex to Support All ASDF Versions for the supported distributions in ... * [`8c57fa3`](actions/setup-java@8c57fa3) Clarify JAVA\_HOME and PATH setup in README ([#841](https://redirect.github.com/actions/setup-java/issues/841)) * [`a7ab372`](actions/setup-java@a7ab372) Bump prettier from 2.8.8 to 3.6.2 ([#873](https://redirect.github.com/actions/setup-java/issues/873)) * [`d0351b4`](actions/setup-java@d0351b4) Update documentation to use checkout and Java v5 ([#903](https://redirect.github.com/actions/setup-java/issues/903)) * See full diff in [compare view](actions/setup-java@dded088...f2beeb2) [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
|
Please also adjust the README file. Thanks! |

Description:
Add support for
.sdkmanrcfile injava-version-fileparameter, as well as tests and updated documentation related to the new feature.I've renamed one e2e test (setup-java-version-from-file-major-minor-patch-with-dist) to include the actual name of the file under test. Without it, there were many job runs with the same name (i.e. ${{ matrix.distribution }} version from file 'openjdk64-17.0.10' - ${{ matrix.os }}), making it quite difficult to find the appropriate ones to verify - and losing a key element of the test: the name of the
java-version-file.Related issue: #669
Check list: