fix(aqua): resolve bin paths for prefixed v tags#9759
Conversation
Greptile SummaryThis PR fixes binary-path resolution for Aqua packages whose git tags include an extra
Confidence Score: 5/5Safe to merge — the change is local-only, never triggers remote I/O, and the existing existence-filter in list_bin_paths naturally discards whichever candidate paths don't match the installed layout. The logic is narrow and well-tested: version_candidates covers all reachable cases (no prefix, prefix ending in v, prefix not ending in v, version already including the prefix), the unit tests exercise both the canonical and pre-resolved-tag call paths, and deduplication guards against redundant links. No existing invariants are broken. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/aqua-bin-pa..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review
This pull request refactors version handling in the AquaBackend by introducing a version_candidates helper to generate potential version strings, including those with 'v' prefixes. Feedback identifies a logic flaw where already-prefixed versions might skip the 'v' variant, potentially causing installation failures, and suggests optimizing performance by moving the candidate generation outside the file iteration loop.
This comment was marked as outdated.
This comment was marked as outdated.
### 🚀 Features - add --inactive option to outdated and upgrade commands for inactive tools by @roele in [#9640](#9640) ### 🐛 Bug Fixes - **(aqua)** resolve bin paths for prefixed v tags by @risu729 in [#9759](#9759) - **(bun)** create bunx alongside bun.exe on Windows install by @JamBalaya56562 in [#9732](#9732) - **(dotnet)** use shared prerelease tool option by @risu729 in [#9720](#9720) - **(node)** use matching node in npm shim by @jdx in [#9749](#9749) - **(task)** resolve bash deterministically on Windows to avoid WSL launcher by @JamBalaya56562 in [#9750](#9750) ### 📚 Documentation - **(secrets)** clarify age strict mode default by @risu729 in [#9737](#9737) - **(tasks)** add bash shebang to conditional-dependencies example by @JamBalaya56562 in [#9747](#9747) - update backend tool option docs by @risu729 in [#9738](#9738) ### 📦 Registry - remove tools with zero users by @jdx in [#9725](#9725) - add scalafmt ([github:scalameta/scalafmt](https://github.com/scalameta/scalafmt)) by @pokir in [#9757](#9757) - remove flarectl by @risu729 in [#9756](#9756) ### Chore - **(release)** strip pre-existing sponsor block before appending canonical one by @jdx in [#9745](#9745) ### New Contributors - @pokir made their first contribution in [#9757](#9757)
Summary
list_bin_paths()local-only while allowing file path templates to try both canonical and prefixed-vtag-shaped versions.vafter the prefix, such astool-v1.2.3for canonical1.2.3.Why
PR #5562 aligned
list_bin_paths()with install-time version resolution by sharing the full resolver, but it was reverted in #5574 after the hyperfine check showed a serious slowdown.The slowdown came from putting remote resolution on the PATH/bin-path path: the shared resolver could fetch GitHub tags/releases, probe URLs/HEADs, and build fallback errors even when
list_bin_paths()only needed local installed file paths.This keeps the fix local-only.
list_bin_paths()still reads registry metadata and installed files through the existing cache, but does not callget_version_tags(),get_url(),github_release_*, orHTTP.head(). Install already has the resolved tag/version, so it passes that directly into file-link creation instead of recomputing from the canonical version.Tests
/home/risu/.cargo/bin/cargo test -p mise test_version_candidates/home/risu/.cargo/bin/cargo test -p mise test_srcs_/home/risu/.cargo/bin/cargo test -p mise backend::aqua::testsgit diff --check