Skip to content

fix: auto-generate non-numeric id when linking JDK with numeric version - #2567

Merged
maxandersen merged 1 commit into
jbangdev:mainfrom
maxandersen:2557
Jul 1, 2026
Merged

fix: auto-generate non-numeric id when linking JDK with numeric version#2567
maxandersen merged 1 commit into
jbangdev:mainfrom
maxandersen:2557

Conversation

@maxandersen

Copy link
Copy Markdown
Collaborator

Problem

When a user tries to link an existing JDK using a numeric version, they hit a catch-22:

Attempt Result
jbang jdk install 21 $JDK_PATH ❌ "id must be non-integer"
jbang jdk install java-21 $JDK_PATH then jbang --java java-21 ❌ "--java only supports numeric"

The correct workaround (jdk install java-21 + --java 21) was not discoverable.

Fix

Instead of rejecting numeric IDs, auto-generate a non-numeric id using the pattern <version>-user:

$ jbang jdk install 21 /path/to/jdk
[jbang] Numeric id detected, using '21-user' as id
[jbang] JDK 21-user-linked has been linked to: /path/to/jdk

If 21-user-linked already exists, it increments: 21-user-1, 21-user-2, etc.

The linked JDK is then found by --java 21 via the version-based lookup in LinkedJdkProvider.

Tests

  • Updated testJdkInstallWithLinkingAndIntegerId → verifies numeric id generates 11-user-linked
  • Added testJdkInstallWithLinkingAndIntegerIdGeneratesIncrementedUserId → verifies counter suffix
  • Updated testJdkInstallWithLinkingToExistingJdkPathWithDifferentVersion → verifies 13-user-linked

Fixes #2557

@coderabbitai

coderabbitai Bot commented Jun 27, 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: 39f3c25e-06a0-4161-8c7f-65a7f467553c

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.

@maxandersen
maxandersen requested a review from quintesse June 27, 2026 07:32
quintesse
quintesse previously approved these changes Jun 27, 2026

@quintesse quintesse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's okay, although personally I wouldn't do the counter thing.
Because now, instead of being backwards compatible, we've introduced a new behaviour.
If before you would have run jbang jdk install 21 /foo/bar twice you would have gotten the message that 21 was already installed and to use --force if you really want to do that. With a counter you might end up installing more and more versions without really noticing. If you really want to install multiple 21s I think it should be an explicit act. (eg. the user explicitly giving it a unique name)

@maxandersen

Copy link
Copy Markdown
Collaborator Author

Good point. what prefix would you use for the one default ?

@quintesse

Copy link
Copy Markdown
Contributor

Good point. what prefix would you use for the one default ?

I'm okay with the one you chose in the PR: "user" 👍

When a user runs 'jbang jdk install 21 /path/to/jdk', instead of
rejecting the numeric id, automatically generate a non-numeric id
using the pattern '<version>-user' (e.g. '21-user'). If that id
already exists, append an incrementing suffix ('21-user-1', etc.).

This fixes the catch-22 where 'jdk install' required a non-numeric id
but '--java' only accepted numeric versions, making it impossible to
link an existing JDK and then use it.

Fixes jbangdev#2557

@quintesse quintesse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@maxandersen
maxandersen merged commit af08660 into jbangdev:main Jul 1, 2026
30 checks passed
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.

regression?: jbang jdk install of specific jdk

2 participants