fix(zig): resolve master channel to the concrete nightly version#10423
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR adds rolling release channel support for ChangesZig Master Rolling Channel Support
Sequence Diagram(s)sequenceDiagram
participant User
participant ToolVersion
participant ZigPlugin
participant ziglang.org
User->>ToolVersion: resolve_version("master")
ToolVersion->>ZigPlugin: is_rolling_channel("master")
ZigPlugin-->>ToolVersion: true
alt already installed & no latest_versions lookup
ToolVersion->>ZigPlugin: latest_installed_channel_version("master")
ZigPlugin-->>ToolVersion: "0.15.0-dev.XXXX" (cached install)
ToolVersion-->>User: resolved to installed dev version
else online resolution needed
ToolVersion->>ZigPlugin: resolve_channel_version(config, "master")
ZigPlugin->>ziglang.org: GET /download/index.json
ziglang.org-->>ZigPlugin: JSON { master: { version: "0.15.0-dev.YYYY" } }
ZigPlugin-->>ToolVersion: "0.15.0-dev.YYYY"
ToolVersion->>ZigPlugin: get_tarball_url(tv with -dev. version)
ZigPlugin->>ZigPlugin: nightly_tarball_url()
ZigPlugin-->>ToolVersion: https://ziglang.org/builds/zig-{arch}-{os}-{version}.tar.xz
ToolVersion-->>User: installs under versioned directory
else offline
ToolVersion-->>User: returns rolling spec unchanged
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/zig.rs`:
- Around line 324-330: The HTTP_FETCH.json() call and subsequent pointer/parsing
operations use the `?` operator, which propagates transient network errors as
failures instead of allowing fallback to normal resolution. Modify this block to
catch any errors from the HTTP fetch or JSON parsing and return `Ok(None)` in
those cases, while still returning `Ok(Some(version_string))` when the channel
is successfully resolved. This allows the system to gracefully degrade when the
remote index is temporarily unavailable.
- Around line 297-308: The latest_installed_channel_version method is manually
sorting versions using Versioning::new which violates repo policy for version
ordering. Replace the manual sort using sorted_by_cached_key(|s|
(Versioning::new(s), s.clone())) with a call to the Backend API method
list_installed_versions_matching, which already provides versions in the correct
order according to backend semantics. Filter the results returned by this API
for -dev. versions and take the last one, removing the manual Versioning::new
sorting entirely.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 02bf78fc-93db-4ad4-adee-e6602cc8eae8
📒 Files selected for processing (5)
docs/lang/zig.mde2e/core/test_zig_slowsrc/backend/mod.rssrc/plugins/core/zig.rssrc/toolset/tool_version.rs
Greptile SummaryThis PR fixes a long-standing issue where
Confidence Score: 5/5Safe to merge. Channel-resolution logic is correctly scoped, the URL format matches what ziglang.org actually serves, minisign verification is unchanged, and the e2e test validates the core fix. The three-level fast-path (installed nightly → network resolve → literal fallthrough) correctly mirrors the existing latest pattern and handles offline/prefer-offline modes consistently. The nightly_tarball_url helper produces the correct zig-{arch}-{os}-{version} format confirmed against the live index. No logic, safety, or correctness defects were found. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix(zig): resolve master channel to the ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
JamBalaya56562
force-pushed
the
fix-zig-master-channel
branch
from
aa7d475 to
875cff8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/toolset/tool_version.rs (1)
414-443: 💤 Low valueClarify the fall-through comment to cover the global-offline case.
The comment at lines 438–443 accurately describes the case where online resolution fails (
resolve_channel_versionreturnsOk(None)), but it doesn't mention that we also reach this fall-through whensettings.offline()istrueandopts.offlineisfalse(globalMISE_OFFLINEwithout prune-style offline). In that case the code skips theresolve_channel_versioncall (line 430) and theopts.offlineearly return (line 435), falling through to normal resolution.Consider updating the comment to clarify both scenarios, for example:
- // Online but the channel did not resolve to a concrete version -- - // either the index lacked the channel key, or the fetch failed - // transiently (resolve_channel_version maps both to Ok(None)). Fall - // through to normal resolution, which still matches the literal - // channel name in the backend's version list as before. + // Fall through to normal resolution when the channel cannot be + // concretized: either we're offline (global MISE_OFFLINE without + // opts.offline), or online resolution failed (the index lacked the + // channel key / fetch error; resolve_channel_version maps both to + // Ok(None)). Normal resolution can still match the literal channel + // name in the backend's version list as before. (`#10251`)🤖 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/toolset/tool_version.rs` around lines 414 - 443, The comment at lines 438–443 in the rolling release channel handling block needs clarification. Currently it only describes the case where online resolution fails (resolve_channel_version returns Ok(None)), but it doesn't account for the case where global offline mode is enabled (settings.offline() is true) while opts.offline is false, which also causes the code to skip the resolve_channel_version call and fall through to normal resolution. Update the comment to cover both scenarios: the online resolution failure case and the global-offline-without-prune-style-offline case, so it clearly explains all paths that lead to the fall-through behavior.
🤖 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.
Nitpick comments:
In `@src/toolset/tool_version.rs`:
- Around line 414-443: The comment at lines 438–443 in the rolling release
channel handling block needs clarification. Currently it only describes the case
where online resolution fails (resolve_channel_version returns Ok(None)), but it
doesn't account for the case where global offline mode is enabled
(settings.offline() is true) while opts.offline is false, which also causes the
code to skip the resolve_channel_version call and fall through to normal
resolution. Update the comment to cover both scenarios: the online resolution
failure case and the global-offline-without-prune-style-offline case, so it
clearly explains all paths that lead to the fall-through behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a949887b-abc6-4997-89db-151d3c2c4c6d
📒 Files selected for processing (5)
docs/lang/zig.mde2e/core/test_zig_slowsrc/backend/mod.rssrc/plugins/core/zig.rssrc/toolset/tool_version.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/core/test_zig_slow
- docs/lang/zig.md
- src/plugins/core/zig.rs
- src/backend/mod.rs
|
I think we should share the logic with existing rolling release support in the http backend, maybe other backends support it but I don't remember. |
`zig@master` was pinned to the literal "master": mise installed it under installs/zig/master and never re-resolved it, so `mise upgrade`/`mise outdated` compared "master" == "master" and never picked up new nightlies (a master install could sit weeks out of date). Zig publishes "master" as a rolling channel in its download index (ziglang.org/download/index.json) whose `version` field points at the current nightly (e.g. 0.17.0-dev.836+...), with the build hosted under ziglang.org/builds/. Resolve such rolling channels to their concrete version at resolution time, like "latest", so installs land in versioned dirs and upgrade/outdated track new builds: - Add Backend::is_rolling_channel, Backend::latest_installed_channel_version and Backend::resolve_channel_version hooks (default no-op). - In ToolVersion::resolve_version, re-resolve a channel like "latest": reuse an installed build of THAT channel (never an unrelated installed release, so zig@master is not short-circuited to a stable that happens to be installed), which keeps hook-env/exec network-free; otherwise resolve via the backend; offline falls back to the literal channel. - Implement for zig "master": read /master/version from the index, reuse the latest installed -dev nightly, and build the /builds/ tarball URL for the resolved dev version (not a top-level index key) in get_tarball_url / resolve_lock_info. minisign verification is unchanged. mach-latest is left as a follow-up (machengine index format unverified). Addresses discussion jdx#10251. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JamBalaya56562
force-pushed
the
fix-zig-master-channel
branch
from
875cff8 to
a64ff0d
Compare
3 participants
What
zig@masterwas effectively frozen: mise resolved it to the literal"master", installed it underinstalls/zig/master, and never re-resolved it. Somise upgrade/mise outdatedcompared"master" == "master"and never picked up newer nightlies — amasterinstall could sit weeks out of date (reported in #10251, stuck on a 23-day-old nightly).Zig publishes
masteras a rolling channel in its download index (ziglang.org/download/index.json), whoseversionfield points at the current nightly (e.g.0.17.0-dev.836+...), with the build hosted underziglang.org/builds/(not on the source forge).Fix
Resolve rolling channels to their concrete version at resolution time — like
latest— so installs land in versioned dirs andmise upgrade/outdatedtrack new builds:Backend::is_rolling_channel,Backend::latest_installed_channel_version, andBackend::resolve_channel_versionhooks (default no-op).ToolVersion::resolve_version, handle a channel likelatest: reuse an installed build of that channel (never an unrelated installed release, sozig@masteris not short-circuited to an installed stable) to keep hook-env/exec network-free; otherwise re-resolve via the backend; offline falls back to the literal channel.master: read/master/versionfrom the index, reuse the latest installed-devnightly, and build the/builds/tarball URL for the resolved dev version (not a top-level index key) inget_tarball_url/resolve_lock_info. minisign verification is unchanged.mach-latestis left as a follow-up (the machengine index format is unverified).Tests
e2e/core/test_zig_slow: with only a stable Zig installed,zig@mastermust resolve to a-devnightly (not short-circuit to the stable), install under a versioned dir, and not create a frozeninstalls/zig/master.Addresses discussion #10251.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
masteras a rolling release channel that resolves to concrete nightly/dev builds, so installs and upgrades automatically follow newer nightly versions.Documentation
zig@masterresolution and how upgrades keep moving forward.Bug Fixes
Tests
zig@masternightly/dev behavior and naming.