Skip to content

Commit c99bd26

Browse files
committed
fix(cli): rename run's positional to bin, which frees --target
The collision was real — `mcpp run q` became `mcpp run --target=q` — but retreating to `--target-triple` on this one subcommand paid for it with a lasting inconsistency. The actual cause is narrower than "one word, two axes": ParsedArgs::value() falls back from an unset option to a positional OF THE SAME NAME, and that positional was named `target`. cmd_run never reads the name (it takes positional(0) by index) — it only labels the slot in --help, where `bin` is the more accurate word. So `--target` works on `run` like everywhere else, `--target-triple` stays as an alias because it shipped in 2026.8.19.1, and e2e/130 now pins both spellings while e2e/73 pins the positional.
1 parent b2c38ca commit c99bd26

6 files changed

Lines changed: 48 additions & 25 deletions

File tree

.agents/docs/2026-08-19-baremetal-phase3-usable-plan.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,5 +352,12 @@ error: test result: FAILED. 2 passed; 1 failed
352352
```
353353

354354
`e2e/73` 立刻红。而位置参数上**原本就有一条注释写着这个碰撞** —— 我为了对称把它
355-
覆盖了。⇒ `run` 保持 `--target-triple`(`test` 没有这个位置参数,`--target` 正常)。
356-
**一致性是判据之一,但它排在「不能坏」后面。**
355+
覆盖了。
356+
357+
**第一版修法(`run` 退回只认 `--target-triple`)也不对** —— 它把「不能坏」买回来了,
358+
代价是留下一条真实的不一致。**真因不是这个词有两个轴,而是位置参数的名字选错了**:
359+
`ParsedArgs::value()` 在选项未设置时会**按同名回落到位置参数**,而这个位置参数的名字
360+
`cmd_run` 从来不读(它按下标取 `positional(0)`),只出现在 `--help` 里。
361+
**把位置参数改名 `bin`**(在 `--help` 里本来就更准),`--target` 就自由了。
362+
`--target-triple` 作为 2026.8.19.1 已发布的拼写保留为别名。
363+
**一致性是判据之一,它排在「不能坏」后面 —— 但排在后面不等于要放弃。**

docs/05-mcpp-toml.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ can produce them.
906906

907907
```bash
908908
mcpp build --target riscv64-none-elf
909-
mcpp run --target-triple riscv64-none-elf # via [target.<triple>].runner
909+
mcpp run --target riscv64-none-elf # via [target.<triple>].runner
910910
```
911911

912912
**What changes on a freestanding target**

docs/zh/05-mcpp-toml.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ cxxflags = ["-march=x86-64-v2"]
804804

805805
```bash
806806
mcpp build --target riscv64-none-elf
807-
mcpp run --target-triple riscv64-none-elf # 经 [target.<triple>].runner
807+
mcpp run --target riscv64-none-elf # 经 [target.<triple>].runner
808808
```
809809

810810
**freestanding target 上有什么不同**

src/cli.cppm

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -316,29 +316,29 @@ int run(int argc, char** argv) {
316316
.action(wrap_rc(cmd_build)))
317317
.subcommand(cl::App("run")
318318
.description("Build + run a binary target (after `--`, args are passed to it)")
319-
// NB: this positional is a BINARY NAME from [[bin]]/src layout —
320-
// unrelated to `--target <triple>` (the cross-target axis).
321-
.arg(cl::Arg("target").help("Binary name (optional)"))
322-
// ⚠️ Same word, two axes — and they were ALREADY named this way:
323-
// the positional above is a binary name, this option is the cross
324-
// target triple, exactly as in `mcpp build --target`. Spelling the
325-
// option differently here would make the one command that needs
326-
// both the one command where the flag is not called --target.
327-
// ⚠️ NOT `--target`, and this is not a style choice.
319+
// ⚠️ Named `bin`, NOT `target`, and the rename is load-bearing.
328320
//
329-
// The POSITIONAL above is already called target — it is the binary
330-
// NAME — and the parser keys both by that word, so declaring the
331-
// option makes `mcpp run <binary>` set the cross-target instead:
321+
// This positional is a BINARY NAME from [[bin]]/src layout. It was
322+
// called `target` — the same word as the cross-target axis — and
323+
// ParsedArgs::value() falls back from an unset option to a
324+
// positional OF THE SAME NAME, so adding `--target` here made
325+
// every ordinary invocation read the binary name as a triple:
332326
//
333327
// $ mcpp run q
334328
// error: unknown target 'q'
335329
//
336-
// Measured, after trying exactly that for the sake of matching
337-
// `mcpp build --target`. `run` is the one subcommand that cannot
338-
// spell it that way, and the existing comment on the positional
339-
// said so before this was attempted.
340-
.option(cl::Option("target-triple").takes_value().value_name("TRIPLE")
330+
// The name is never read back (cmd_run takes positional(0) by
331+
// index); it only labels this slot and shows up in --help, where
332+
// `bin` is the more accurate word anyway. So renaming it is what
333+
// lets `run` spell the flag `--target` like every other
334+
// subcommand, instead of being the one command that cannot.
335+
.arg(cl::Arg("bin").help("Binary name (optional)"))
336+
.option(cl::Option("target").takes_value().value_name("TRIPLE")
341337
.help("Cross target triple (same axis as `mcpp build --target`)"))
338+
// Kept as an alias: it shipped in 2026.8.19.1 as the only spelling
339+
// `run` accepted, and scripts written against it must keep working.
340+
.option(cl::Option("target-triple").takes_value().value_name("TRIPLE")
341+
.help("Alias for --target"))
342342
.option(cl::Option("package").short_name('p').takes_value().value_name("NAME")
343343
.help("Run only the named workspace member (single-member; no --workspace fan-out)"))
344344
.option(cl::Option("cache").takes_value().value_name("MODE")

src/cli/cmd_build.cppm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,10 @@ export int cmd_run(const mcpplibs::cmdline::ParsedArgs& parsed,
166166
bool no_cache = parsed.is_flag_set("no-cache");
167167
if (auto c = parsed.value("cache")) cache_mode = *c;
168168
else if (no_cache) cache_mode = "off";
169-
// ⚠️ `--target-triple`, never `--target`: this subcommand's POSITIONAL is
170-
// called target and means the binary NAME, and the parser keys both by
171-
// that word. See the option's declaration in cli.cppm.
169+
// Both spellings; see cli.cppm for why the positional had to be renamed
170+
// before `--target` could exist here at all.
172171
std::string target_triple;
172+
if (auto tt = parsed.value("target")) target_triple = *tt;
173173
if (auto tt = parsed.value("target-triple")) target_triple = *tt;
174174
return mcpp::build::build_run_target(targetName, passthrough, package_filter,
175175
cache_mode, no_cache, target_triple);

tests/e2e/130_freestanding_riscv_build_and_run.sh

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,26 @@ readelf -h "$img" | grep -q '0x80200000' || {
159159
readelf -h "$img" | grep -i entry; exit 1; }
160160

161161
# ── run ─────────────────────────────────────────────────────────────────────
162-
"$MCPP" run --target-triple riscv64-none-elf > run.log 2>&1 || true
162+
# ⚠️ BOTH spellings, and the pairing is the test.
163+
#
164+
# `run` also takes a POSITIONAL binary name, and the arg parser falls back
165+
# from an unset option to a positional of the same name. While that positional
166+
# was itself called `target`, adding `--target` here silently turned
167+
# `mcpp run <binary>` into `mcpp run --target=<binary>`:
168+
#
169+
# $ mcpp run q
170+
# error: unknown target 'q'
171+
#
172+
# `--target-triple` shipped first and stays an alias, so both are pinned; the
173+
# positional case is pinned by 73_issue131_per_target_cxxflag.sh.
174+
"$MCPP" run --target riscv64-none-elf > run.log 2>&1 || true
163175
grep -q 'MCPP-FREESTANDING-OK' run.log || {
164176
cat run.log; echo "the firmware did not run (or produced no output)"; exit 1; }
165177

178+
"$MCPP" run --target-triple riscv64-none-elf > alias.log 2>&1 || true
179+
grep -q 'MCPP-FREESTANDING-OK' alias.log || {
180+
cat alias.log; echo "--target-triple (the 2026.8.19.1 spelling) stopped working"; exit 1; }
181+
166182
# ── the runner is REQUIRED, and its absence must say so ─────────────────────
167183
# Two-sided: without this, "run worked" could equally mean mcpp exec'd the
168184
# image directly and something else printed the line.

0 commit comments

Comments
 (0)