Skip to content

fix(java): include shorthand vendor in lock identity#9989

Merged
jdx merged 6 commits into
jdx:mainfrom
risu729:fix/java-shorthand-vendor-lock-options
Jun 12, 2026
Merged

fix(java): include shorthand vendor in lock identity#9989
jdx merged 6 commits into
jdx:mainfrom
risu729:fix/java-shorthand-vendor-lock-options

Conversation

@risu729

@risu729 risu729 commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • include shorthand_vendor in Java lockfile options for shorthand Java requests
  • preserve existing behavior for fully-qualified vendor requests such as temurin-17
  • update Java lockfile option tests for default and non-default shorthand vendors

Details

For shorthand Java requests like java@17 or java@lts, mise resolves the vendor through settings.java.shorthand_vendor. This PR records that resolved vendor in the lockfile identity so changing the shorthand vendor changes the lock key and cannot reuse a lock entry created for a different vendor.

Because the setting's default is openjdk, a default shorthand request such as java@17 now includes shorthand_vendor = "openjdk" in the lockfile options. Fully-qualified requests such as java@openjdk-17 or java@temurin-17 do not add shorthand_vendor, because the vendor is already part of the requested version string.

Verification

  • mise run render
  • mise run format
  • cargo fmt --all -- --check
  • git diff --check
  • cargo test shorthand_vendor
  • cargo test java_options_reads_release_type --bin mise

Summary by CodeRabbit

  • Bug Fixes
    • Improved Java configuration option handling for different version request formats and release types.

@greptile-apps

greptile-apps Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes stale lock invalidation for shorthand Java version requests (e.g. java@17, java@lts) by including the active java.shorthand_vendor setting in the computed lock identity. Previously, changing the vendor from openjdk to temurin could silently reuse a stale lock entry because the vendor was never recorded.

  • Introduces is_shorthand_java_request (returns true when the version string contains no -), then writes shorthand_vendor into lock options only for those requests, leaving fully-qualified forms like temurin-17 unchanged.
  • Sources the vendor dynamically from Settings::get().java.shorthand_vendor (default openjdk from settings.toml) instead of a hard-coded literal, and adds unit tests covering numeric shorthands, lts, and vendor-qualified versions.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to lock key generation and does not affect download, install, or resolution logic.

The fix is straightforward: shorthand_vendor is now written into lock options for any version string that lacks a hyphen, which correctly captures numeric shorthands (17, 21) and named aliases (lts) while leaving vendor-qualified strings (temurin-17) untouched. The heuristic is consistent with how tv_to_java_version already treats digit-prefixed versions. The vendor is sourced from the live settings value rather than a hard-coded literal. Unit tests cover all four distinct cases explicitly, including the previously-unguarded lts alias.

No files require special attention.

Important Files Changed

Filename Overview
src/plugins/core/java.rs Adds is_shorthand_java_request heuristic and threads requested_version/shorthand_vendor through lockfile_options; updates resolve_lockfile_options to pass the runtime settings value; updates and extends unit tests including an explicit lts alias case.

Reviews (8): Last reviewed commit: "fix(java): keep shorthand vendor default..." | Re-trigger Greptile

Comment thread src/plugins/core/java.rs Outdated

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces InstallManifest as a new ToolOptionSource, allowing tool options to be persisted and retrieved from the installation manifest. Key changes include updating BackendArg to track option sources, ensuring manifest options are correctly applied during resolution while being excluded from explicit CLI representations. The Java plugin was also updated to include shorthand_vendor in lockfile identities for shorthand versions. Feedback suggests expanding the Java shorthand detection to include "latest" and "lts" and ensuring that version resolution consistently respects tool-specific configurations over global settings.

Comment thread src/plugins/core/java.rs Outdated
Comment thread src/plugins/core/java.rs
@risu729

risu729 commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

CI note: lint/test-ci are failing because cargo deny check now detects RUSTSEC-2026-0145 for astral-tokio-tar 0.6.1 via rattler_package_streaming. This appears unrelated to this PR: the current main branch is failing the same cargo-deny advisory in the test workflow, and test-ci only failed because lint failed.

This comment was generated by an AI coding assistant.

@risu729 risu729 force-pushed the fix/java-shorthand-vendor-lock-options branch from 7396ae1 to 6438d69 Compare May 31, 2026 08:15
@coderabbitai

coderabbitai Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: aa1c6ab8-35b8-48dd-9889-cc502e37b257

📥 Commits

Reviewing files that changed from the base of the PR and between a0dd654 and 908cb8e.

📒 Files selected for processing (1)
  • src/plugins/core/java.rs

📝 Walkthrough

Walkthrough

This change makes Java lockfile options version-aware. The JavaOptions::lockfile_options method now accepts the requested version and shorthand vendor config, conditionally including release_type for non-GA versions and shorthand_vendor only for shorthand version patterns (those without hyphens). Tests validate the new conditional behavior across GA, EA, and vendor-specific request types.

Changes

Java Lockfile Options Version Awareness

Layer / File(s) Summary
Version-aware lockfile options logic
src/plugins/core/java.rs
JavaOptions::lockfile_options transitions from a no-argument method to a parameterized one accepting requested version and shorthand vendor config. Returns a BTreeMap with conditional keys: release_type when not GA, and shorthand_vendor only when version is shorthand (no hyphens). New is_shorthand_java_request() helper identifies shorthand versions.
Integration and test validation
src/plugins/core/java.rs
resolve_lockfile_options call site passes requested version and java.shorthand_vendor config to the refactored method. Unit tests validate conditional behavior: shorthand vendor for plain versions and GA defaults, both fields for EA, and exclusion of shorthand vendor for hyphenated version strings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • jdx/mise#9984: Both PRs modify a backend's resolve_lockfile_options logic to include additional lockfile identity options derived from tool request parameters.

Poem

🐰 A rabbit hops through versions true,
Shorthand and release type too,
Lockfile options now see the way,
Version-aware, hip-hip-hooray! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(java): include shorthand vendor in lock identity' directly and specifically describes the main change: adding shorthand vendor recording to Java lockfile options for shorthand requests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@risu729

This comment was marked as outdated.

@risu729 risu729 marked this pull request as ready for review June 1, 2026 14:51
@jdx jdx merged commit 0caff1f into jdx:main Jun 12, 2026
33 checks passed
@risu729 risu729 deleted the fix/java-shorthand-vendor-lock-options branch June 12, 2026 09:54
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