Skip to content

fix(go): improve version fetching logic to support deeply nested sub-modules#8823

Merged
jdx merged 3 commits intojdx:mainfrom
roele:issues/8444
Mar 30, 2026
Merged

fix(go): improve version fetching logic to support deeply nested sub-modules#8823
jdx merged 3 commits intojdx:mainfrom
roele:issues/8444

Conversation

@roele
Copy link
Copy Markdown
Contributor

@roele roele commented Mar 30, 2026

Currently go backend version resolution has issues with deeply nested sub-modules. For example a ls-remote go:github.com/go-kratos/kratos/cmd/kratos/v2 shows 1.x versions even though go list -versions returns no versions. This is due to the fact that mise does a root-first lookup at github.com/go-kratos/kratos while also dropping the /v2 suffix. Installation of such sub-modules will then fail trying to install the resolved latest 1.x version where instead they should pass latest.

This PR is changing the version lookup behaviour to following:

Before:
mise-debug ls-remote go:github.com/go-kratos/kratos -> 1.x
mise-debug ls-remote go:github.com/go-kratos/kratos/v2 -> 2.x
mise-debug ls-remote go:github.com/go-kratos/kratos/cmd/kratos/v2 -> 1.x

After:
mise-debug ls-remote go:github.com/go-kratos/kratos -> 1.x
mise-debug ls-remote go:github.com/go-kratos/kratos/v2 -> 2.x
mise-debug ls-remote go:github.com/go-kratos/kratos/cmd/kratos/v2 -> No versions

Copilot AI review requested due to automatic review settings March 30, 2026 17:52
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

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 improves Go module version resolution, specifically for deep sub-modules that do not return versions via go list -versions. It ensures that when latest is requested for such modules, the installation correctly uses the @latest tag rather than resolving to a parent module's version. The changes include a refactored fetch_go_module_versions helper, updated installation logic, and new E2E and unit tests. Feedback was provided regarding potential performance optimizations through caching and a suggestion to use is_some_and for more idiomatic Rust code.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves Go module version resolution for deeply nested sub-modules so that modules with no tagged versions don’t incorrectly inherit parent module versions (allowing installs to proceed using @latest).

Changes:

  • Add an “exact module path first” lookup for go list -m -versions and treat a successful response with no versions as authoritative.
  • Adjust install behavior to preserve @latest for deep modules that report zero versions, avoiding installing an unrelated parent module’s resolved version.
  • Add parsing tests for go list -json output without a Versions field and extend e2e coverage for a deep sub-module case.

Reviewed changes

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

File Description
src/backend/go.rs Adds a reusable version-fetch helper and updates lookup/install logic to correctly handle deep sub-modules with no versions.
e2e/backend/test_go_install_slow Updates Go toolchain version used by the test and adds an e2e assertion covering deep sub-module @latest install behavior.

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

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR fixes version fetching for deeply nested Go sub-modules (e.g. github.com/go-kratos/kratos/cmd/kratos/v2) where go list -versions returns an empty list but the old code fell through to a parent-path lookup and returned misleading 1.x versions, causing installs to fail.

Key changes:

  • _list_remote_versions now probes the exact module path first via a new fetch_go_module_versions helper. If that call succeeds but returns an empty version list, it returns immediately — treating the empty result as authoritative — instead of falling back to a parent path that may have unrelated versions.
  • fetch_go_module_versions caches go list -versions results via a DashMap-backed CacheManager (disk-persisted, keyed on a hash of the module path), avoiding duplicate network calls within a session.
  • A belt-and-suspenders guard in install_version_ resets the install target to @latest if the resolved version conflicts with an empty version list for the exact module path.
  • GoModInfo.versions gains #[serde(default)] so that responses without a Versions field (the real-world case for deep sub-modules) deserialize correctly instead of erroring.
  • Two unit tests and one slow e2e test cover the new behaviour.

Confidence Score: 4/5

Safe to merge after addressing the DashMap lock-across-await concern; the core logic fix is correct and well-tested.

The version-fetching logic fix is correct and the new e2e test validates the primary scenario. Two P2 findings remain: a DashMap RefMut held across an async await point (holding a shard lock for the full go list subprocess duration, which can stall parallel installations) and a redundant .to_string() allocation in the hash call. The await-point lock-hold is the more material concern given mise's parallel tool-installation capability.

src/backend/go.rs — specifically the fetch_go_module_versions method where the DashMap shard lock is held across the async get_or_try_init_async call.

Important Files Changed

Filename Overview
src/backend/go.rs Core fix: adds an exact-path go list -versions check in _list_remote_versions before falling back to parent-path heuristics, preventing deep submodule paths from inheriting parent module versions. Introduces a per-instance DashMap<String, CacheManager> to cache results; however, the RefMut from DashMap::entry().or_insert_with() is held across an async await point, holding the shard lock for the duration of the subprocess and reducing parallelism.
e2e/backend/test_go_install_slow Updates Go version pin from 1.22 to 1.24 and adds an end-to-end test asserting that a deep submodule (github.com/go-kratos/kratos/cmd/kratos/v2@latest) installs and runs correctly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_list_remote_versions(tool_name)"] --> B["fetch_go_module_versions(exact path)"]
    B --> C{Result?}
    C -- "Ok(Some(versions))" --> D["return Ok(versions)\n(empty or non-empty)"]
    C -- "Ok(None) — go list failed" --> E["Build parent-path indices\n(root-first heuristic)"]
    E --> F["for each parent path\n(skip exact tool_name)"]
    F --> G["fetch_go_module_versions(parent path)"]
    G --> H{Result?}
    H -- "Ok(Some(versions))" --> I["return Ok(versions)"]
    H -- "Ok(None)" --> F
    F -- "exhausted" --> J["return Ok([])"]

    subgraph fetch_go_module_versions
        K["DashMap entry\nor_insert_with CacheManager"] --> L["get_or_try_init_async"]
        L --> M["go list -mod=readonly -m -versions -json"]
        M -- "Error" --> N["Ok(None)"]
        M -- "Success" --> O["parse GoModInfo"]
        O --> P["Ok(Some(versions))"]
    end
Loading

Reviews (3): Last reviewed commit: "fix(go): improve version fetching logic ..." | Re-trigger Greptile

…modules

- add caching for go module versions
@jdx jdx enabled auto-merge (squash) March 30, 2026 19:02
@jdx jdx disabled auto-merge March 30, 2026 19:48
@jdx jdx merged commit ea7818e into jdx:main Mar 30, 2026
32 of 34 checks passed
@roele roele deleted the issues/8444 branch March 30, 2026 19:55
mise-en-dev added a commit that referenced this pull request Mar 31, 2026
### 🚀 Features

- **(python)** add GitHub provenance verification for prebuilt binaries
by @malept in [#8820](#8820)

### 🐛 Bug Fixes

- **(ci)** use rustls-native-roots for Windows CI build by @jdx in
[#8822](#8822)
- **(go)** improve version fetching logic to support deeply nested
sub-modules by @roele in [#8823](#8823)
- **(shim)** prevent infinite recursion when system shims dir is on PATH
by @andrewthauer in [#8816](#8816)
- go backend missing supports_lockfile_url() override by
@palootcenas-outreach in [#8790](#8790)
- strip shims from PATH in credential and template subprocesses by
@antonioacg in [#8802](#8802)

### 📚 Documentation

- fix typo in shims documentation for fish by @roele in
[#8798](#8798)

### 📦️ Dependency Updates

- update ghcr.io/jdx/mise:alpine docker digest to 3e6d001 by
@renovate[bot] in [#8794](#8794)
- pin dependencies by @renovate[bot] in
[#8793](#8793)

### 📦 Registry

- fix flutter version sorting by @roele in
[#8818](#8818)
- add svgo (npm:svgo) by @3w36zj6 in
[#8817](#8817)

### New Contributors

- @antonioacg made their first contribution in
[#8802](#8802)
- @palootcenas-outreach made their first contribution in
[#8790](#8790)

## 📦 Aqua Registry Updates

#### New Packages (3)

- [`RasKrebs/sonar`](https://github.com/RasKrebs/sonar)
- [`emacs-eask/cli`](https://github.com/emacs-eask/cli)
-
[`superradcompany/microsandbox`](https://github.com/superradcompany/microsandbox)

#### Updated Packages (4)

- [`dimo414/bkt`](https://github.com/dimo414/bkt)
- [`lxc/incus`](https://github.com/lxc/incus)
-
[`shinagawa-web/gomarklint`](https://github.com/shinagawa-web/gomarklint)
- [`updatecli/updatecli`](https://github.com/updatecli/updatecli)
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.

4 participants