Skip to content

fix: restore multi-JDK features lost in aesh migration#2513

Closed
maxandersen wants to merge 1 commit into
jbangdev:mainfrom
maxandersen:fix/restore-multi-jdk-features
Closed

fix: restore multi-JDK features lost in aesh migration#2513
maxandersen wants to merge 1 commit into
jbangdev:mainfrom
maxandersen:fix/restore-multi-jdk-features

Conversation

@maxandersen

Copy link
Copy Markdown
Collaborator

Problem

PR #2271 (feat: install multiple JDKs with same major version) was merged on May 14, but several of its features were lost when PR #2453 (aesh migration) was squash-merged on June 3. The aesh migration rewrote Jdk.java from picocli to aesh annotations but didn't preserve all the behavioral changes from #2271.

What was lost and is now restored

Jdk.java — 7 fixes

Feature What happened
jdk list --providers/-P Entire option + handler deleted
jdk list --distros/-D Entire option + handler deleted, JdkDistroQuery import dropped
Full version comparison (defVersion string) Reverted to defMajorVersion int — breaks default marker with multiple JDKs of same major version
Separate available/installed mapping Collapsed to single path; linkedId always null, no LinkedJdk detection
Tag display in list output Dropped from text output
jdk uninstall two-pass (canInstallcanUpdate) Replaced with single canUpdate; info message uses raw input instead of jdk.id()
jdk java-env uses canInstall Reverted to canUpdate

TestJdk.java — 4 restored tests

Test What it covers
testJdkInstallWithLinkingAndIntegerId Integer ID rejection when linking
testJdkInstallWithLinkingToExistingJBangJdkPath JBang-managed path rejection
testJdkInstallSameVersion "Already installed" message
testJdkInstallSameVersionForced --force creates versioned dir + symlink (core multi-JDK feature)

Verification

All 30 TestJdk tests pass (including the 4 restored ones), plus TestRun (124) and TestProject (9) — zero failures.

PR jbangdev#2271 (feat: install multiple JDKs with same major version) was
merged on May 14, but its features were partially lost when PR jbangdev#2453
(aesh migration) was squash-merged on June 3.

Restored features in Jdk.java:
- jdk list --providers/-P to list available JDK providers
- jdk list --distros/-D to list available JDK distributions
- Full version comparison (defVersion string) instead of majorVersion
  int, so default marker works with multiple JDKs of same major version
- Separate available/installed JDK mapping with LinkedJdk detection
- Tag display in detailed text output
- Available JDKs show jdk.id in non-detail mode
- jdk uninstall two-pass: tries canInstall first, falls back to
  canUpdate, prints jdk.id() in info message
- jdk java-env uses canInstall predicate

Restored tests in TestJdk.java:
- testJdkInstallWithLinkingAndIntegerId
- testJdkInstallWithLinkingToExistingJBangJdkPath
- testJdkInstallSameVersion
- testJdkInstallSameVersionForced
@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: 19dcfb2b-46b9-4e49-9cc5-3a6c483966bf

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.

@maxandersen maxandersen requested a review from quintesse June 5, 2026 20:56
@maxandersen

Copy link
Copy Markdown
Collaborator Author

@quintesse this is result after asking opus to investigate what features/fixes from #2271 got lost in aesh port.

does it look right or I can also go back rebasing ash pr but that would be much bigger diff I fear.

@quintesse

Copy link
Copy Markdown
Contributor

I already wnet through it by hand but I'll check it against this to see if either of us missed anything

@maxandersen

Copy link
Copy Markdown
Collaborator Author

related to #2515

@maxandersen

Copy link
Copy Markdown
Collaborator Author

on my own quick look and LLM deeper look #2515 does the same but more so closing this.

#2515 covers everything plus additional fixes (the JdkProvidersMixin visibility change, linkedId display, and more thorough test infrastructure updates that make mock JDKs match the real directory layout).

@maxandersen maxandersen closed this Jun 6, 2026
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.

2 participants