Repository navigation
fix(lockfile): merge platform data when a tool's options newly diverge from an empty on-disk entry - #10710
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates lockfile entry reconciliation in ChangesLockfile Empty-Options Adoption
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Greptile SummaryThis PR fixes a lockfile data-loss bug (#10564) where a backend starting to emit deterministic options (e.g.
Confidence Score: 5/5The fix is narrowly scoped to the empty→non-empty options transition and does not touch any other merge paths; three regression tests cover the three mutually-dependent code sites. All three changes work together correctly: the adoption logic in No files require special attention; Important Files Changed
Reviews (2): Last reviewed commit: "fix(lockfile): merge platform data when ..." | Re-trigger Greptile |
…e from an empty on-disk entry `set_platform_info` (used by `mise lock`) and `merge_tool_entries` (used by the general `mise install`/`mise use` lockfile-update path) both key/match lockfile entries by exact `(version, options)` equality. When a backend starts reporting deterministic lockfile options for a version that previously round-tripped with none (e.g. java's `shorthand_vendor`, which is always attached for a shorthand version request), the freshly-resolved entry no longer matches the existing on-disk entry's key: - `set_platform_info` falls into its "no match" branch and pushes a brand new sibling entry, leaving the old one's checksums/URLs unreferenced. - `merge_tool_entries` is worse: an existing entry with no matching key is silently dropped from the merged result entirely. Either way, `mise lock`/`mise install` ends up fragmenting a single tool version across sibling lockfile entries, and `mise install --locked` then fails on any platform whose checksum only lives in the orphaned entry. Fix: when no exact `(version, options)` match exists and the freshly resolved `options` are non-empty, adopt an existing entry for the same version that still has empty options — merge its platforms in and update its options in place, instead of creating (or losing) a sibling. The adoption target is chosen by smallest key so the output stays deterministic, and `preserve_absent_tool_entries` (monorepo root lockfiles) skips resurrecting an adopted empty-options entry as a duplicate sibling while still preserving fragmented entries for tools not resolved in the current run. Scoped narrowly to the empty-options -> non-empty-options transition reported; options that differ in other ways (e.g. a real `release_type` change) continue to produce separate entries, since those genuinely describe different downloadable artifacts. + 3 regression tests reproducing the reported java shorthand_vendor scenario across `set_platform_info`, `merge_tool_entries`, and the monorepo preserve pass. discussion jdx#10564 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
45bc102 to
33369d1
Compare
Summary
Fixes the lockfile data loss reported in discussion #10564: when a backend starts attaching deterministic lockfile options to a version that previously round-tripped with no options (e.g. java's
shorthand_vendor, always attached for a shorthand request likejava = "26"),mise lockfragments the tool into duplicate[[tools.java]]siblings and a subsequent run drops the entry holding all the platform checksums/URLs — leavingcore:javapinned with zero platform info, somise install --lockedfails on every platform.Root cause
Both lockfile writers key/match entries by exact
(version, options)equality:set_platform_info(themise lockpath): the freshly-resolved(26.0.1, {shorthand_vendor})no longer matches the on-disk(26.0.1, {})entry, so a brand-new sibling entry is pushed and the old entry's checksums/URLs become unreferenced.merge_tool_entries(themise install/mise uselockfile-update path): worse — an existing entry whose key no longer matches anything freshly resolved is silently dropped from the merged result.Fix
When there is no exact
(version, options)match and the freshly-resolved options are non-empty, adopt the existing entry for the same version that still has empty options: merge its platforms in and update its options in place, instead of creating (or losing) a sibling.preserve_absent_tool_entries(monorepo root lockfiles) skips resurrecting an adopted empty-options entry as a duplicate sibling. The skip is snapshot-based so fragmented entries belonging to tools not resolved in the current run are still preserved verbatim (no new data-loss path).release_typechange) continue to produce separate entries — those genuinely describe different downloadable artifacts. The reverse transition (an entry losing its options, e.g. after a mise downgrade) is intentionally not handled: resurrecting options the current mise no longer reports would be wrong.This matches suggested direction 2 from the report ("merge platforms from the existing entry into the new one rather than spawning a sibling").
Tests
Three regression tests reproducing the reported java
shorthand_vendorscenario:test_set_platform_info_adopts_empty_options_entry_on_new_options— an entry with existing platform checksums gains options on a latermise lock; all platforms must end up on a single entry.test_merge_tool_entries_adopts_empty_options_entry_on_new_options— same via the install-path merge; the existing entry's platforms must be adopted, not dropped.test_preserve_absent_does_not_resurrect_adopted_empty_options_entry— monorepo preserve pass must not duplicate an adopted entry, while still preserving fragmented entries for tools not resolved in the current run.cargo fmt --all -- --checkpasses locally;cargo check/ test run deferred to CI.Fixes the issue reported in discussion #10564.
🤖 Generated with Claude Code
Summary by CodeRabbit