Skip to content

fix(aqua): support override env selectors#10200

Merged
jdx merged 6 commits into
jdx:mainfrom
risu729:codex-20260602-184158-bd41ff
Jun 12, 2026
Merged

fix(aqua): support override env selectors#10200
jdx merged 6 commits into
jdx:mainfrom
risu729:codex-20260602-184158-bd41ff

Conversation

@risu729

@risu729 risu729 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • parse overrides[].envs in Aqua registry package overrides
  • apply envs as an additional override selector alongside goos, goarch, and supported variants
  • bump the compiled Aqua registry cache version because the archived override layout changed

What is overrides[].envs?

Aqua registry packages can define overrides to replace parts of a package definition for specific runtime targets. The envs field is one of Aqua's override selectors: each entry can match a GOOS value such as windows, a GOARCH value such as arm64, a combined GOOS/GOARCH value such as windows/arm64, or all.

Aqua treats envs as an optional selector alongside goos, goarch, and variants: if it is present, at least one listed environment must match the target runtime; if other selectors are also present, they must match too.

Relevant Aqua references:

Packages using this field

Two live aqua-registry entries show why this matters:

Why mise needs support

Before this change, mise parsed and applied Aqua overrides for goos, goarch, and supported variants, but ignored overrides[].envs. Env-only overrides therefore never applied. That could make mise choose the wrong file or package configuration for valid Aqua registry packages, especially on Windows and Darwin targets.

Supporting envs makes mise's native Aqua backend match Aqua's selector semantics for these registry entries and prevents platform-specific override data from being silently skipped.

Tests

  • rustfmt --edition 2024 --check crates/aqua-registry/src/types.rs crates/aqua-registry/src/cache.rs
  • git diff --check
  • cargo test -p aqua-registry override_envs --no-default-features

Summary by CodeRabbit

  • New Features

    • Added environment selectors for overrides, allowing rules to target specific OS, architecture, or combined "os/arch" identifiers (including an "all" option) for more precise selection of files, URLs, and variants.
  • Chores

    • Bumped the compiled registry cache layout version; previously compiled caches will be ignored and rebuilt automatically on next use.

@coderabbitai

coderabbitai Bot commented Jun 2, 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e158595b-4f6b-4096-9bb3-e180610c46ca

📥 Commits

Reviewing files that changed from the base of the PR and between a9b0147 and 9d9df3b.

📒 Files selected for processing (1)
  • crates/aqua-registry/src/types.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/aqua-registry/src/types.rs

📝 Walkthrough

Walkthrough

Bumps compiled registry cache layout version from "v2" to "v3". Adds envs: Vec<String> to AquaOverride and refactors AquaOverride::matches to include envs_match (empty or "all" matches any; entries match OS, arch, or "os/arch"). Adds unit tests for envs behavior.

Changes

Registry Environment Overrides

Layer / File(s) Summary
Cache version invalidation
crates/aqua-registry/src/cache.rs
Compiled registry cache directory version constant incremented from "v2" to "v3", changing where compiled registries are read/written.
Environment-specific override matching
crates/aqua-registry/src/types.rs
AquaOverride gains an optional envs: Vec<String> selector (serde-defaulted). AquaOverride::matches refactored to use envs_match which treats an override as applicable when envs is empty or when any selector equals "all", the target OS, the target arch, or the combined os/arch. Unit tests added covering OS-only, OS/arch, "all", and interaction with goarch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • jdx/mise#10167: Both PRs modify the compiled registry cache version constant in crates/aqua-registry/src/cache.rs to change/invalidate the compiled registry cache directory.

Poem

A rabbit hops through registry trees,
Bumping caches with nimble ease,
Env selectors whisper "all" or "win/arm",
Matches choose files, urls, and charm,
A tiny hop, a cleaner breeze. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(aqua): support override env selectors' directly and clearly describes the primary change: adding support for environment selectors in Aqua registry package overrides.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the Aqua registry override matching logic to support the envs selector field, which was previously silently ignored, causing platform-specific overrides (e.g. Windows .bat file selection, Darwin/cargo routing) to never apply.

  • Adds envs: Vec<String> to AquaOverride and a standalone envs_match helper that accepts GOOS, GOARCH, GOOS/GOARCH, or the all wildcard per Aqua's spec.
  • Refactors AquaOverride::matches from an imperative if/else if/else chain that required at least one of goos/goarch to be set, to a composable chain of is_none_or guards — each selector is now independently optional, which matches Aqua's semantics and also correctly enables variants-only overrides that the old code would always reject.
  • Bumps COMPILED_REGISTRY_CACHE_VERSION to v3 to invalidate stale caches that lack the new field in the archived layout.

Confidence Score: 5/5

Safe to merge; the change is a targeted, well-tested bug fix with no regressions on existing matching paths.

The refactored matches logic correctly models Aqua's selector semantics: each of goos, goarch, envs, and variants is independently optional, matching only when present and satisfied. The envs_match helper covers all documented selector forms. Four new unit tests directly exercise the added paths including combined-selector cases, and the cache version bump ensures no stale archived data will be read with the new layout.

No files require special attention.

Important Files Changed

Filename Overview
crates/aqua-registry/src/types.rs Adds envs field to AquaOverride, refactors matches to use per-selector is_none_or guards, and introduces envs_match helper; four new focused unit tests cover OS, GOOS/GOARCH combo, "all" wildcard, and combined goarch+envs cases.
crates/aqua-registry/src/cache.rs Single-line bump of the compiled registry cache version from v2 to v3 to invalidate stale caches that lack the new envs field in the archived override layout.

Reviews (7): Last reviewed commit: "style(aqua): format env wildcard test" | Re-trigger Greptile

Comment thread crates/aqua-registry/src/types.rs Outdated
@risu729 risu729 marked this pull request as ready for review June 3, 2026 23:45
@risu729 risu729 marked this pull request as draft June 4, 2026 20:29
@risu729 risu729 force-pushed the codex-20260602-184158-bd41ff branch from 27747ab to f67c7a5 Compare June 4, 2026 20:50
@risu729

risu729 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

This comment was generated by an AI coding assistant.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat crates/aqua-registry/src/types.rs
echo "---"
cat crates/aqua-registry/src/cache.rs

Length of output: 78464



@risu729 — reviewed. The implementation is clean and correct. Here are my findings:


Review of PR #10200fix(aqua): support override env selectors

crates/aqua-registry/src/cache.rs

The version bump from "v2" to "v3" is appropriate: adding envs: Vec<String> to AquaOverride changes its rkyv archive layout, so old compiled caches must be invalidated.


crates/aqua-registry/src/types.rs

AquaOverride struct — The new field is correctly annotated:

#[serde(default)]
envs: Vec<String>,

Defaulting to an empty Vec means all existing overrides without envs remain unaffected. The rkyv derives on the struct automatically handle serialisation of the new field.

AquaOverride::matches() — Logic is sound. envs acts as an OR selector, combined with goos/goarch/variants via AND, which matches the Aqua spec:

&& (self.envs.is_empty() || envs_match(&self.envs, os, arch))

envs_match() — Handles all four valid entry forms (GOOS, GOARCH, GOOS/GOARCH, all) correctly:

fn envs_match(envs: &[String], os: &str, arch: &str) -> bool {
    let os_arch = format!("{os}/{arch}");
    envs.iter()
        .any(|env| env == "all" || env == os || env == arch || env == &os_arch)
}

One minor observation: env == arch allows bare GOARCH entries (e.g., "arm64") to match any OS, which is consistent with the PR description. In theory this could be ambiguous if a GOOS name collided with a GOARCH name, but Go's actual GOOS/GOARCH values have no overlap, so this is safe in practice. It might be worth a brief doc comment on envs_match to clarify the matching precedence.

Test coverage — Three tests cover the main scenarios well (OS-only, OS/arch combined, envs + goarch interaction). One small gap: there is no test for envs: [all]. While the behaviour is trivial to reason about from the code, a short test would give explicit documentation and regression coverage for that special value.

#[test]
fn test_override_envs_all_matches_any_platform() {
    let yml = r#"
packages:
  - url: https://example.com/tool-default
    format: raw
    overrides:
      - envs:
          - all
        url: https://example.com/tool-all
"#;
    let pkg = first_registry_package(yml);
    for (os, arch) in [("linux", "amd64"), ("darwin", "arm64"), ("windows", "amd64")] {
        let resolved = pkg.clone().with_version(&["1.0.0"], os, arch);
        assert_eq!(
            resolved.url("1.0.0", os, arch).unwrap(),
            "https://example.com/tool-all",
            "expected 'all' env to match {os}/{arch}"
        );
    }
}

Summary

Area Assessment
Correctness of matching logic
Cache version bump
Backward compatibility (empty envs default)
rkyv serialisation
Test coverage ✅ (minor gap: "all" value not tested)

The PR is in good shape. The only actionable suggestion is the optional "all" test above — feel free to include it or skip it as you see fit.

This comment was generated by an AI coding assistant.


🧠 Learnings used
Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:12.203Z
Learning: When posting comments on GitHub PRs or discussions, include a note that the comment was AI-generated (e.g., '*This comment was generated by an AI coding assistant.*').

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:18.847Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-03T17:35:47.414Z
Learning: Prefer `aqua:` backend for registry entries; use `github:` or `gitlab:` when the tool isn't in aqua but ships releases. Avoid tier 3 backends (npm, pipx, gem, cargo, go, dotnet) for registry PRs unless explicitly approved.

@risu729

risu729 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

This comment was generated by an AI coding assistant.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@risu729, I'll re-run the review on the latest changes now.

This comment was generated by an AI coding assistant.


🧠 Learnings used
Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:12.203Z
Learning: When posting comments on GitHub PRs or discussions, include a note that the comment was AI-generated (e.g., '*This comment was generated by an AI coding assistant.*').

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-03T17:35:47.414Z
Learning: Prefer `aqua:` backend for registry entries; use `github:` or `gitlab:` when the tool isn't in aqua but ships releases. Avoid tier 3 backends (npm, pipx, gem, cargo, go, dotnet) for registry PRs unless explicitly approved.

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:18.847Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@risu729

risu729 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

This comment was generated by an AI coding assistant.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat crates/aqua-registry/src/types.rs
echo "---CACHE---"
cat crates/aqua-registry/src/cache.rs

Length of output: 79235



@risu729 — re-reviewed. The implementation is complete and all previous gaps have been addressed.


Review of PR #10200 (latest) — fix(aqua): support override env selectors

crates/aqua-registry/src/cache.rs

✅ Cache version bumped to "v3" — correct and necessary.

crates/aqua-registry/src/types.rs

Item Status
AquaOverride.envs: Vec<String> with #[serde(default)]
envs_match() handles all, GOOS, GOARCH, GOOS/GOARCH forms
AquaOverride::matches() AND-combines envs with goos/goarch/variants
test_override_envs_match_os
test_override_envs_match_os_arch
test_override_envs_all_matches_any_platform ✅ (added since last review)
test_override_envs_combine_with_goarch

No issues found. The envs: [all] test I flagged in the earlier review has been incorporated. The PR is in good shape and ready for merge.

This comment was generated by an AI coding assistant.


🧠 Learnings used
Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:12.203Z
Learning: When posting comments on GitHub PRs or discussions, include a note that the comment was AI-generated (e.g., '*This comment was generated by an AI coding assistant.*').

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:18.847Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-03T17:35:47.414Z
Learning: Prefer `aqua:` backend for registry entries; use `github:` or `gitlab:` when the tool isn't in aqua but ships releases. Avoid tier 3 backends (npm, pipx, gem, cargo, go, dotnet) for registry PRs unless explicitly approved.

@risu729 risu729 marked this pull request as ready for review June 5, 2026 17:43
@jdx jdx merged commit 35a8bb0 into jdx:main Jun 12, 2026
33 checks passed
@risu729 risu729 deleted the codex-20260602-184158-bd41ff branch June 12, 2026 09:55
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.

2 participants