Skip to content

fix(java): prefer base vendor for latest command and ensure proper sorting - #11109

Merged
jdx merged 2 commits into
jdx:mainfrom
roele:issues/11096
Jul 20, 2026
Merged

jdx merged 2 commits into
jdx:mainfrom
roele:issues/11096

Conversation

@roele

@roele roele commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Fixes sorting for multi-feature vendor prefixes and adds preference for base vendor in context of latest command as mentioned in #11096.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Java version selection by preferring base vendor variants over more specialized dashed variants.
    • Corrected Java release sorting to produce more consistent version results.
  • Tests
    • Added end-to-end coverage to verify latest java@liberica resolves to the base Liberica version format.

Copilot AI review requested due to automatic review settings July 18, 2026 11:54
@coderabbitai

coderabbitai Bot commented Jul 18, 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: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: fbcb9a16-14e9-49f4-8f49-47aa26b17bc7

📥 Commits

Reviewing files that changed from the base of the PR and between 264a7bc and 712310b.

📒 Files selected for processing (1)
  • src/plugins/core/java.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/plugins/core/java.rs

📝 Walkthrough

Walkthrough

Java remote-version sorting now parses metadata versions and prefers base vendor variants. The CLI end-to-end test verifies that java@liberica resolves to the expected base Liberica format.

Changes

Java version ordering

Layer / File(s) Summary
Remote version sorting and validation
src/plugins/core/java.rs, e2e/cli/test_latest
Sort keys derive version and build information from metadata, account for vendor dash counts, and verify base Liberica resolution through the CLI test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit hops through versions bright,
Base Liberica wins the flight.
Dashes sort with careful grace,
Tests confirm the proper place.
“Binky!” says the rabbit, “clean and right!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: preferring the base vendor for latest lookups and adjusting sorting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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.

Copilot AI 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.

Pull request overview

This PR adjusts Java version ordering in the core Java plugin so mise latest selects the expected release when multiple vendor “variants” exist (e.g., liberica vs liberica-nik), and improves sorting when the displayed version string includes extra vendor/features segments.

Changes:

  • Sort Java entries using JavaMetadata.version (the numeric version) rather than the full rendered string key, fixing ordering for vendor strings with extra -feature- segments.
  • Add a secondary sort key that prefers “base” vendors (fewer - in the vendor name) so latest resolves to the canonical vendor when multiple match.
  • Extend the mise latest E2E test to assert base-vendor preference for java@liberica.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/plugins/core/java.rs Updates remote-version sorting to use numeric metadata version and prefer base vendors for latest resolution.
e2e/cli/test_latest Adds an assertion ensuring mise latest java@liberica resolves to the base vendor (not a variant like liberica-nik).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps

greptile-apps Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes two related bugs in the Java plugin's version sorting for the latest command. Previously, version comparisons used the full map key (v, e.g. liberica-24.0.1) where the raw numeric version (m.version) is correct, causing Versioning::new() to receive a vendor-prefixed string that couldn't be semantically compared. A new vendor_dashes sort key is also introduced to deprioritise specialised vendor variants (e.g. liberica-nik) over their base counterparts (e.g. liberica).

  • Version sort key fixed: Versioning::new(&m.version) and m.version.rsplit_once(…) now use the raw numeric version string instead of the full version-key string, enabling correct semantic ordering across vendors.
  • Base vendor preference: vendor_dashes = -(dash count in vendor name) makes entries with hyphen-free vendor names sort later (i.e. rank higher for latest), so liberica beats liberica-nik.
  • E2E coverage added: a new assertion confirms mise latest java@liberica returns a liberica-<N.N.N> string (not a specialised variant).

Confidence Score: 5/5

Safe to merge — changes are narrowly scoped to the sort key in one method and add a new e2e test for the fixed behaviour.

Both changes are straightforward and well-motivated: using the raw numeric version for Versioning parsing is clearly more correct than feeding it a vendor-prefixed string, and the vendor_dashes heuristic is simple, non-destructive, and backed by a new end-to-end test.

No files require special attention.

Important Files Changed

Filename Overview
src/plugins/core/java.rs Two bug fixes in the version sort key: version/build_num now use m.version (raw numeric) instead of v (full key), enabling correct semantic version comparison; new vendor_dashes field deprioritizes hyphenated vendor variants like liberica-nik in favour of the base vendor.
e2e/cli/test_latest New e2e assertion verifies that mise latest java@liberica resolves to a base liberica version (not liberica-nik or other specialised variants).

Reviews (2): Last reviewed commit: "Merge branch 'main' into issues/11096" | Re-trigger Greptile

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/plugins/core/java.rs`:
- Around line 418-423: Update build_num extraction in the Versioning flow to use
only the '+' suffix; remove the fallback that splits on '.'. Preserve the
default build number behavior when no '+' build metadata exists, while allowing
Versioning to handle dot-separated version segments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 09633500-2db5-4618-aba6-8cc14415932d

📥 Commits

Reviewing files that changed from the base of the PR and between b46ddef and 264a7bc.

📒 Files selected for processing (2)
  • e2e/cli/test_latest
  • src/plugins/core/java.rs

Comment thread src/plugins/core/java.rs
Comment on lines +418 to +423
let version = Versioning::new(&m.version);
// Extract build suffix after a '+', '.' if present. If not present, treat as 0.
let build_num = v
let build_num = m
.version
.rsplit_once('+')
.or_else(|| v.rsplit_once('.'))
.or_else(|| m.version.rsplit_once('.'))

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the fallback that splits on . for build_num extraction.

The .or_else(|| m.version.rsplit_once('.')) fallback can mistakenly extract the patch version as the build number for releases lacking build metadata (e.g., for 17.0.2, it extracts 2).

If Versioning ignores build metadata in precedence (per SemVer), this creates a sorting inversion where an older base release like 17.0.2 (incorrectly assigned build_num = 2) sorts higher than its own subsequent build like 17.0.2+1 (correctly assigned build_num = 1). Since Versioning natively handles dot-separated version segments (like 17.0.2.8), build_num is strictly only needed to break ties for + suffixes.

💡 Proposed fix
-                // Extract build suffix after a '+', '.' if present. If not present, treat as 0.
+                // Extract build suffix after a '+' if present. If not present, treat as 0.
                 let build_num = m
                     .version
                     .rsplit_once('+')
-                    .or_else(|| m.version.rsplit_once('.'))
                     .and_then(|(_, tail)| {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let version = Versioning::new(&m.version);
// Extract build suffix after a '+', '.' if present. If not present, treat as 0.
let build_num = v
let build_num = m
.version
.rsplit_once('+')
.or_else(|| v.rsplit_once('.'))
.or_else(|| m.version.rsplit_once('.'))
let version = Versioning::new(&m.version);
// Extract build suffix after a '+' if present. If not present, treat as 0.
let build_num = m
.version
.rsplit_once('+')
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/plugins/core/java.rs` around lines 418 - 423, Update build_num extraction
in the Versioning flow to use only the '+' suffix; remove the fallback that
splits on '.'. Preserve the default build number behavior when no '+' build
metadata exists, while allowing Versioning to handle dot-separated version
segments.

@jdx
jdx merged commit 60fafe9 into jdx:main Jul 20, 2026
33 checks passed
@roele
roele deleted the issues/11096 branch July 20, 2026 20:53
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