Skip to content

fix: resolve SdkmanJdkProvider folder resolution#96

Merged
quintesse merged 1 commit intojbangdev:mainfrom
ayagmar:fix/sdkman-detection
Feb 23, 2026
Merged

fix: resolve SdkmanJdkProvider folder resolution#96
quintesse merged 1 commit intojbangdev:mainfrom
ayagmar:fix/sdkman-detection

Conversation

@ayagmar
Copy link
Copy Markdown
Contributor

@ayagmar ayagmar commented Feb 23, 2026

Fix SDKMAN JDK detection for normal SDKMAN folder names

--jdk-providers=sdkman could fail to find installed JDKs like 25.0.1-tem / 25.0.2-zulu ect..

Note

If multiple installed SDKMAN JDKs match a requested major version, selection is now deterministic.
For open major requests (for example 21+), it selects the lowest matching major and the newest patch within that major.
For exact major requests (for example 21), it selects the newest patch within that major.

Validation

image

Fixes #95

@ayagmar ayagmar force-pushed the fix/sdkman-detection branch from 3e6961e to be6f28b Compare February 23, 2026 12:04
Copy link
Copy Markdown
Contributor

@quintesse quintesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ayagmar , thanks for your PR!

I haven't had time to take a good look but I'm fairly positive this approach is the wrong one. If the BaseFoldersJdkProvider is rejecting the folders because they don't follow the normal naming it expects then the typical thing to do is override the acceptFolder() method to use an implementation that works for this particular use-case.

I'm actually guessing that an earlier base implementation didn't check for the suffix on the end of folder names so it just accepted all of them. And then later that got changed and nobody noticed because there were no tests and sdkman wasn't part of the standard set of providers.

But you can look at the implementations of Linux- Mise- and ScoopJdkProvider to see how they do it.

If you can make that change I'll happily accept your PR .

@ayagmar
Copy link
Copy Markdown
Contributor Author

ayagmar commented Feb 23, 2026

Hey @quintesse
Thanks for your review, I will have a look

Edit: Done I simplified resolution and also squashed the other commits to reduce noise

@ayagmar ayagmar force-pushed the fix/sdkman-detection branch from cac279e to 3da7c1a Compare February 23, 2026 16:23
@ayagmar ayagmar force-pushed the fix/sdkman-detection branch from 3da7c1a to 18c4cc4 Compare February 23, 2026 16:39
@quintesse quintesse merged commit 9bab786 into jbangdev:main Feb 23, 2026
3 checks passed
@quintesse
Copy link
Copy Markdown
Contributor

Perfect. Thanks @ayagmar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sdkman jdk provider does not resolve and use available jdk versions

2 participants