Skip to content

crates/minimal: cmd_dump accepts --arch to override target - #146

Merged
bryan-minimal merged 3 commits into
mainfrom
cmd-dump-target-arch
Apr 21, 2026
Merged

bryan-minimal merged 3 commits into
mainfrom
cmd-dump-target-arch

Conversation

@bryan-minimal

Copy link
Copy Markdown
Member

Summary

Closes #125. Adds a --arch flag to minimal dump so downstream tools can evaluate arch-matched packages for both architectures.

minimal dump --packages --arch amd64
minimal dump --packages --arch arm64

When --arch is absent, defaults to Target::host() (preserves current behavior).

Why

mctx::graph_from_all_packages_with_target(target) has existed for a while but cmd_dump was hard-coded to Target::host(). For packages that use match { arch = 'Amd64 } => ..., { arch = 'Arm64 } => ... } target to select between per-arch source URLs (claude-code, go, deno, gcloud, etc.), this meant only the host arm's data surfaced in the dump — the other arm was erased at evaluation time.

That blind spot cascades:

  • pkgmgr silently corrupts arch-matched packages on update (skipping them via a guardrail in its current form, once it can see both it can update both)
  • supply-chain's scanner only sees one arch's resolved source URL per package

Exposing the target at the CLI lets consumers run the dump twice and aggregate.

Design notes

  • Follows cmd_materialize's existing --arch pattern verbatim: accepts amd64/x86_64 → Arch::Amd64, arm64/aarch64 → Arch::Arm64. Rejects anything else with an error message naming the bad input and listing valid options.
  • OS is pinned to OS::Linux. Dump consumers want per-arch evaluation of the same pkgs repo, not cross-OS targets. Easy to extend later if a cross-OS case emerges.
  • resolve_target is factored out so it's unit-testable without spinning up a Context. Extracted into a private helper matching the codebase's existing style of small helpers colocated with their command.

Test plan

  • New unit tests in cmd_dump::tests cover the four paths: default (host), amd64 aliases, arm64 aliases, unknown-arch error
  • minimal test — full suite passes, including the new cmd_dump tests
  • minimal run fmt-check — clean
  • minimal run clippy — clean (no new warnings)

Follow-ups (not this PR)

  • pkgmgr's guardrail on multi-arch packages can now be lifted: supply-chain reads the dump for each arch, aggregates the per-arch sha256/url data, and pkgmgr rewrites all arms in one pass.
  • supply-chain's scanner can use the per-arch output to catch arch-specific advisories.

Closes #125. Previously `minimal dump --packages` always evaluated
Nickel against the host system's target, so packages whose
`build_deps` use `match { arch = 'Amd64 } => ..., { arch = 'Arm64 } =>
... } target` only surfaced one arm's data in the dump output — the
other arm was erased at evaluation time with no breadcrumb.

`mctx::graph_from_all_packages_with_target(target)` already exists;
cmd_dump simply wasn't plumbed to it. This commit adds a clap arg:

    minimal dump --packages --arch amd64
    minimal dump --packages --arch arm64

Parsing follows `cmd_materialize`'s existing `--arch` convention
(arm64/aarch64 → Arm64, amd64/x86_64 → Amd64). OS is always Linux;
dump consumers want per-arch evaluation of the same pkgs repo, not
cross-OS targets. When `--arch` is absent, defaults to `Target::host()`
(preserves current behavior).

Unblocks the downstream consumers listed in the issue — pkgmgr's
updater can now see per-arch source data for arch-matched packages
(deno, go, gcloud, etc.) by running the dump once per arch, and
supply-chain's scanner gets the same visibility.

Tests: resolve_target helper has unit tests covering the default path,
both arch aliases (amd64/x86_64, arm64/aarch64), and the error case
for unsupported values.
/// parsing (arm64/aarch64 → Arm64, amd64/x86_64 → Amd64). When the flag is
/// absent, defaults to [`Target::host`]. OS is always Linux — dump consumers
/// want per-arch evaluation of the same pkgs repo, not cross-OS.
fn resolve_target(arch: Option<&str>) -> Result<Target, Error> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Make or use an existing method on Target or Arch: the generic problem of going from an &str into the specific type is something we are bound to use half a dozen times in various places.

Addresses review feedback: the str-to-Arch parsing (arm64/aarch64,
amd64/x86_64) was open-coded separately in cmd_dump and cmd_materialize
— moving it to a shared `impl FromStr for Arch` in common means:

- Single source of truth for which aliases we accept. uname -m output
  works verbatim on both macOS (arm64/x86_64) and Linux (aarch64/amd64).
- New consumers can use `"amd64".parse::<Arch>()` instead of
  re-implementing the match.
- Error formatting ("unsupported architecture: X. Use 'amd64' or
  'arm64'.") lives in one place alongside the parse logic.

Both cmd_dump::resolve_target and cmd_materialize's arch resolution
now delegate to this via `str.parse::<Arch>()`. cmd_materialize loses
the anyhow import shadow (only uses anyhow! from below now). Tests
for the parser move to common/target.rs; cmd_dump tests retain their
integration coverage (resolve_target defaults, Linux-OS wrapping,
error propagation).
@bryan-minimal

Copy link
Copy Markdown
Member Author

Good call on the shared helper — pushed 5aca2a3: added FromStr for Arch in common with full alias set and tests, migrated cmd_dump and cmd_materialize to str.parse. The arch parsing duplication in cmd_materialize is cleaned up as a drive-by since the same point applied there.

@twitchyliquid64 twitchyliquid64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lastly, update the other place where we go from a str to a Target, in the nickel-parsing code.

Idk why i thought looping over all representations was a good idea at the time lol

match Target::all().iter().find(|t| t.as_ref() == target_str) {
None => Err(Error::InvalidTarget { files: program.files(), got: target_str, pos: rt.pos(program.pos_table()) }),
Some(t) => {
target = Some(t.clone());
Ok(())
}
}

Addresses the second review point on #146: decode/src/builds.rs was
looping over Target::all() to find a target by its as_ref() string
("amd64/linux", etc.). Now that Arch has FromStr, Target can too —
consumers outside cmd_dump should use the shared impl instead.

Adds:
- FromStr for OS (linux/macos) with OsParseError
- FromStr for Target, delegates arch/os segment parsing to the
  component FromStrs. Round-trips with Target::as_ref().
- TargetParseError surfaces which segment failed (shape / arch / os)
  so users get a specific message.

decode/src/builds.rs's "target" field parser now does
`target_str.parse::<Target>()` and folds into the existing
Error::InvalidTarget variant on parse failure. No behavior change for
valid inputs.

Also fixes CI fmt-check failure: rustfmt on CI formats the
cmd_materialize arch-resolver chain differently than my initial
layout. Reformatted to match.

Tests: 7 new in target.rs — OS both variants + unknown rejection,
Target round-trip over every Target::all() value, missing-separator
error, arch error propagation, OS error propagation.
@bryan-minimal

Copy link
Copy Markdown
Member Author

Pushed 0926f00: added FromStr for Target (and OS as a building block) and migrated the ad-hoc Target::all() loop in decode/src/builds.rs. Also fixed the rustfmt diff CI flagged on cmd_materialize — chain layout preference. The new Target::FromStr has a roundtrip test over every Target::all() value so that impl and as_ref stay locked together.

@bryan-minimal
bryan-minimal enabled auto-merge (squash) April 21, 2026 00:02
@bryan-minimal
bryan-minimal merged commit f225092 into main Apr 21, 2026
6 checks passed
@bryan-minimal
bryan-minimal deleted the cmd-dump-target-arch branch April 21, 2026 00:06
twitchyliquid64 pushed a commit that referenced this pull request Apr 22, 2026
* crates/minimal: cmd_dump accepts --arch to override target

Closes #125. Previously `minimal dump --packages` always evaluated
Nickel against the host system's target, so packages whose
`build_deps` use `match { arch = 'Amd64 } => ..., { arch = 'Arm64 } =>
... } target` only surfaced one arm's data in the dump output — the
other arm was erased at evaluation time with no breadcrumb.

`mctx::graph_from_all_packages_with_target(target)` already exists;
cmd_dump simply wasn't plumbed to it. This commit adds a clap arg:

    minimal dump --packages --arch amd64
    minimal dump --packages --arch arm64

Parsing follows `cmd_materialize`'s existing `--arch` convention
(arm64/aarch64 → Arm64, amd64/x86_64 → Amd64). OS is always Linux;
dump consumers want per-arch evaluation of the same pkgs repo, not
cross-OS targets. When `--arch` is absent, defaults to `Target::host()`
(preserves current behavior).

Unblocks the downstream consumers listed in the issue — pkgmgr's
updater can now see per-arch source data for arch-matched packages
(deno, go, gcloud, etc.) by running the dump once per arch, and
supply-chain's scanner gets the same visibility.

Tests: resolve_target helper has unit tests covering the default path,
both arch aliases (amd64/x86_64, arm64/aarch64), and the error case
for unsupported values.

* cmd_dump, cmd_materialize: centralize arch parsing in common::Arch

Addresses review feedback: the str-to-Arch parsing (arm64/aarch64,
amd64/x86_64) was open-coded separately in cmd_dump and cmd_materialize
— moving it to a shared `impl FromStr for Arch` in common means:

- Single source of truth for which aliases we accept. uname -m output
  works verbatim on both macOS (arm64/x86_64) and Linux (aarch64/amd64).
- New consumers can use `"amd64".parse::<Arch>()` instead of
  re-implementing the match.
- Error formatting ("unsupported architecture: X. Use 'amd64' or
  'arm64'.") lives in one place alongside the parse logic.

Both cmd_dump::resolve_target and cmd_materialize's arch resolution
now delegate to this via `str.parse::<Arch>()`. cmd_materialize loses
the anyhow import shadow (only uses anyhow! from below now). Tests
for the parser move to common/target.rs; cmd_dump tests retain their
integration coverage (resolve_target defaults, Linux-OS wrapping,
error propagation).

* common, decode: FromStr for Target, replace ad-hoc loop in builds.rs

Addresses the second review point on #146: decode/src/builds.rs was
looping over Target::all() to find a target by its as_ref() string
("amd64/linux", etc.). Now that Arch has FromStr, Target can too —
consumers outside cmd_dump should use the shared impl instead.

Adds:
- FromStr for OS (linux/macos) with OsParseError
- FromStr for Target, delegates arch/os segment parsing to the
  component FromStrs. Round-trips with Target::as_ref().
- TargetParseError surfaces which segment failed (shape / arch / os)
  so users get a specific message.

decode/src/builds.rs's "target" field parser now does
`target_str.parse::<Target>()` and folds into the existing
Error::InvalidTarget variant on parse failure. No behavior change for
valid inputs.

Also fixes CI fmt-check failure: rustfmt on CI formats the
cmd_materialize arch-resolver chain differently than my initial
layout. Reformatted to match.

Tests: 7 new in target.rs — OS both variants + unknown rejection,
Target round-trip over every Target::all() value, missing-separator
error, arch error propagation, OS error propagation.
norrietaylor added a commit that referenced this pull request May 19, 2026
)

Removes the agentic-workflow artifacts that landed here via the old sync /
caller-wrapper install model (gominimal/min-aw PR #146 retired that path).
This repo will be re-bootstrapped via `gh aw add gominimal/min-aw/<chore>@<release-tag>`
once this PR merges — the gh-aw [sharing-workflows](https://github.github.com/gh-aw/guides/organization-practices/sharing-workflows/)
pattern.

Removed (agentic only):

- `.github/workflows/sync-from-source-of-truth.yml` — retired sync workflow.
- `.github/workflows/worker-fix.yml` + `worker-iterate.yml` — retired caller
  wrappers (no compiled `.lock.yml` was ever committed on this repo).
- `AGENTS.md`, `.github/agents/`, `.github/labels.yml`,
  `.github/copilot-instructions.md` — contract template; will be reinstalled
  by `quick-setup.sh` at re-bootstrap.
- `.github/ISSUE_TEMPLATE/{bug,chore,feature,feedback}.md` — installed by
  the min-aw contract template. The pre-existing `bug_report.md` and
  `feature_request.md` are NOT touched.

Kept (native CI + pre-existing):

- `.github/workflows/ci.yml` — Minimal CI gate.
- `.github/workflows/promote.yml` — release promotion workflow.
- `.github/ISSUE_TEMPLATE/bug_report.md` + `feature_request.md` — pre-existing
  GitHub-default issue templates.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

Feature: minimal dump should accept a target arch override

2 participants