Skip to content

Fix: multi-jdk regression (due to AEsh migration)#2515

Open
quintesse wants to merge 4 commits into
jbangdev:mainfrom
quintesse:fix_jdk_regression
Open

Fix: multi-jdk regression (due to AEsh migration)#2515
quintesse wants to merge 4 commits into
jbangdev:mainfrom
quintesse:fix_jdk_regression

Conversation

@quintesse

Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • ai-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 88f30aa8-3123-4d9f-8a9f-7307cdd5691e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@quintesse quintesse force-pushed the fix_jdk_regression branch from d3806df to 1e6ee37 Compare June 5, 2026 23:22
@quintesse

Copy link
Copy Markdown
Contributor Author

Still a couple of failing tests, but that's because I changed them a bit because they were actually not testing the right way anymore for multi-jdk. But I won't have time to fix that until sunday at the earliest.
If you want you can merge your fix first, @maxandersen , and I'll rebase mine on it later. They are very similar anyway.

@maxandersen

Copy link
Copy Markdown
Collaborator

@quintesse best you review and merge what makes sense. You know it better than me.

I just offered the machine derived one in hope it could save some time.

Merge and/or close what you find b best. If you want a second eye test before just let me know which pr :)

@stalep

stalep commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

@quintesse there are several fixes with #2516 that might be helpful to get in first?

@quintesse quintesse force-pushed the fix_jdk_regression branch 3 times, most recently from c6be05c to e48b505 Compare June 9, 2026 14:55
@quintesse quintesse force-pushed the fix_jdk_regression branch from e48b505 to 151ac3d Compare June 10, 2026 22:23
maxandersen added a commit to jbangdev/jbang-devkitman that referenced this pull request Jun 11, 2026
…dows

On macOS, temp directories resolve through /private/var (e.g.
/var/folders/... -> /private/var/folders/...) causing
jdkFolder.startsWith(jdksRoot) to return false when the jdkFolder
path has been resolved via toRealPath() but jdksRoot has not.

This caused LinkedJdk.linked() to fall through to ExternalJdkProvider,
producing external-<hash> ids instead of the correct provider id.

The fix adds an isUnderRoot() helper that falls back to comparing
real (canonical) paths when the simple startsWith check fails.
The resolved real root path is cached for performance.

Fixes jbangdev/jbang#2515 test failures on macOS and Windows.
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.

3 participants