Skip to content

Commit 01530e3

Browse files
committed
fix(runner): a tool declared by a DEPENDENCY is reachable by bare name
⚠️⚠️ THE CASE THE FEATURE EXISTS FOR WAS THE ONE THAT DID NOT WORK. `runner_lookup` (#544) lets a runner name its program without writing a payload's home-and-version path into a manifest. But the directories it searched were collected from `runtimeOwnerManifest.xlings.deps` — the ROOT project's declarations alone. So the bare name resolved when the CONSUMER declared the tool, and failed when the board-support package did. That is backwards. A board package is precisely the thing that knows which emulator or probe reaches its machine; requiring the consumer to declare it as well is the duplication the board package exists to remove. Measured on `mcpplibs/aarch64-virt-rt` with its runner reduced to the bare name `qemu-system-aarch64`: `mcpp run --target aarch64-none-elf` searched PATH, did not find it, and reported a missing runner — while the emulator sat installed in the payload the board had declared two lines above. With this change the same example boots and prints. The collection now spans every package in the graph, root first: a consumer that declares its own payload still decides, and a dependency answers when the consumer said nothing. A payload declared but not installed contributes nothing and the lookup continues to PATH, unchanged. ⭐ This is what makes the board-package simplification real. Naming the program replaces `mcpp::xpkg_dir` + `std::format` + a conditional + a `mcpp::warning` fallback — eleven lines — and it DELETES a failure mode rather than moving it: `xpkg_dir` answers empty for anyone building from a checkout, so the old shape configured no runner and needed an advisory to explain why. There is nothing to explain when the lookup itself reports which directories it searched. `tests/e2e/334` covers it, and asserts the half that matters more than resolution: a declared runner whose program is missing is an ERROR, never a fallback to executing the artifact on the build host. 97/97 unit; e2e 130-139, 332-334 green.
1 parent 3fc57dd commit 01530e3

4 files changed

Lines changed: 149 additions & 10 deletions

File tree

docs/18-devices.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ mcpp::run_exclusive(); // this target's runs cannot overlap
5252
```
5353
5454
⭐ **Name the program, not its path.** mcpp locates it: the `bin/` of a payload
55-
this package declared under `[xlings] deps` first, then `PATH`. Writing an
55+
declared under `[xlings] deps` by **any package in the graph** — the consuming
56+
project first, then its dependencies — and then `PATH`. A board-support package
57+
is precisely the thing that knows which emulator or probe reaches its machine,
58+
so it declares that payload itself and the consumer declares nothing. Writing an
5659
absolute path computed from `mcpp::xpkg_dir` is unnecessary, and it introduces a
5760
failure mode — a declaration is not an install, so the lookup can return empty
5861
and leave no runner configured with nothing said about why. Naming the program

docs/zh/18-devices.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@ mcpp::runner_longlived("monitor"); // 没有自然终点
4141
mcpp::run_exclusive(); // 这个目标的运行不能重叠
4242
```
4343
44-
⭐ **写程序名,不要写路径。** mcpp 会定位它:先找本包在 `[xlings] deps` 里声明的
45-
载荷的 `bin/`,再找 `PATH`。用 `mcpp::xpkg_dir` 拼绝对路径是多余的,而且引入了一个
44+
⭐ **写程序名,不要写路径。** mcpp 会定位它:先找**图中任何一个包**在
45+
`[xlings] deps` 里声明的载荷的 `bin/`(消费工程优先,然后是它的依赖),再找 `PATH`。
46+
板级包正是那个知道「哪个模拟器或探针能抵达这台机器」的东西,所以由它声明,
47+
**消费者什么都不用声明**。用 `mcpp::xpkg_dir` 拼绝对路径是多余的,而且引入了一个
4648
失败模式 —— **声明不是安装**,查询可能返回空,于是没有配置任何 runner 而没有任何
4749
话说明原因。写程序名则让 mcpp 报出它究竟搜过哪些目录。
4850

src/build/prepare.cppm

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8750,13 +8750,38 @@ prepare_build(bool print_fingerprint,
87508750
// same resolution `fillXpkgDirs` hands to build programs, kept as
87518751
// directories rather than env vars because the reader is mcpp's own
87528752
// lookup, not a child process. See BuildContext::xlingsDepBinDirs.
8753-
if (!runtimeOwnerManifest.xlings.deps.empty()) {
8754-
if (auto cfg = get_cfg()) {
8755-
auto xlEnv = mcpp::config::make_xlings_env(**cfg);
8756-
for (auto const& spec : runtimeOwnerManifest.xlings.deps) {
8757-
auto ref = mcpp::xlings::paths::parse_xpkg_ref(spec);
8758-
if (auto dir = mcpp::xlings::paths::xpkg_payload(xlEnv, ref))
8759-
ctx.xlingsDepBinDirs.push_back(*dir / "bin");
8753+
//
8754+
// ⚠️⚠️ AND EVERY PACKAGE IN THE GRAPH, NOT ONLY THE ROOT — WHICH IS THE
8755+
// CASE THIS FEATURE EXISTS FOR.
8756+
//
8757+
// A board-support package is precisely the thing that knows which emulator
8758+
// or probe reaches its machine, and it declares that emulator under its own
8759+
// `[xlings] deps`. Collecting only the ROOT's declarations meant a runner
8760+
// could name a program by bare name only when the CONSUMER had also
8761+
// declared it — which is the duplication the board package exists to
8762+
// remove. Measured on `mcpplibs/aarch64-virt-rt`: with the board naming
8763+
// `qemu-system-aarch64` bare, `mcpp run` searched PATH, found the shim or
8764+
// nothing, and reported a missing runner while the emulator sat installed
8765+
// in the payload the board had declared.
8766+
//
8767+
// Ordering is root-first: a consumer that declares its own payload gets to
8768+
// decide, and a dependency supplies the answer when the consumer said
8769+
// nothing. A payload that is declared but not installed contributes
8770+
// nothing, and the lookup continues to PATH.
8771+
{
8772+
std::vector<std::string> xlingsSpecs = runtimeOwnerManifest.xlings.deps;
8773+
for (auto const& pkg : packages)
8774+
for (auto const& spec : pkg.manifest.xlings.deps)
8775+
if (std::ranges::find(xlingsSpecs, spec) == xlingsSpecs.end())
8776+
xlingsSpecs.push_back(spec);
8777+
if (!xlingsSpecs.empty()) {
8778+
if (auto cfg = get_cfg()) {
8779+
auto xlEnv = mcpp::config::make_xlings_env(**cfg);
8780+
for (auto const& spec : xlingsSpecs) {
8781+
auto ref = mcpp::xlings::paths::parse_xpkg_ref(spec);
8782+
if (auto dir = mcpp::xlings::paths::xpkg_payload(xlEnv, ref))
8783+
ctx.xlingsDepBinDirs.push_back(*dir / "bin");
8784+
}
87608785
}
87618786
}
87628787
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
#!/usr/bin/env bash
2+
# requires: gcc unix-shell
3+
# A runner may name its program by bare name when a DEPENDENCY declared it.
4+
#
5+
# ⚠️⚠️ THE CASE THIS COVERS IS THE ONE THE FEATURE EXISTS FOR, AND IT WAS THE
6+
# ONE THAT DID NOT WORK.
7+
#
8+
# `mcpp.build.runner_lookup` lets a runner name a program without writing a
9+
# payload's home-and-version path into a manifest. But the directories it
10+
# searched were collected from the ROOT manifest's `[xlings] deps` only — so the
11+
# bare name worked when the CONSUMER declared the tool, and failed when the
12+
# board-support package did.
13+
#
14+
# That is backwards. A board package is precisely the thing that knows which
15+
# emulator or probe reaches its machine; requiring the consumer to declare it
16+
# too is the duplication the board package exists to remove.
17+
#
18+
# Measured on mcpplibs/aarch64-virt-rt: with the board naming
19+
# `qemu-system-aarch64` by bare name, `mcpp run` searched PATH, did not find it,
20+
# and reported a missing runner — while the emulator sat installed in the
21+
# payload the board had declared.
22+
set -e
23+
24+
MCPP="${MCPP:-mcpp}"
25+
work="$(mktemp -d)"
26+
trap 'rm -rf "$work"' EXIT
27+
28+
# A stand-in payload: a directory with a bin/ holding one executable. This is
29+
# the shape `[xlings] deps` resolves to, and using a real xim package here would
30+
# make the test about that package's availability rather than about the lookup.
31+
mkdir -p "$work/fakepkg/bin"
32+
cat > "$work/fakepkg/bin/demo-tool" <<'TOOL'
33+
#!/bin/sh
34+
echo "DEMO-TOOL ran with $*"
35+
TOOL
36+
chmod +x "$work/fakepkg/bin/demo-tool"
37+
38+
mkdir -p "$work/dep/src" "$work/app/src"
39+
40+
# The DEPENDENCY declares the tool and names it by bare name.
41+
cd "$work/dep"
42+
cat > mcpp.toml <<'TOML'
43+
[package]
44+
name = "toolbox"
45+
version = "0.1.0"
46+
TOML
47+
printf 'export module toolbox;\n' > src/t.cppm
48+
cat > build.mcpp <<'BUILD'
49+
import mcpp;
50+
import std;
51+
int main() {
52+
// The bare name. Whether this resolves is the whole subject of the test.
53+
mcpp::runner("demo-tool");
54+
return 0;
55+
}
56+
BUILD
57+
58+
cd "$work/app"
59+
cat > mcpp.toml <<'TOML'
60+
[package]
61+
name = "app"
62+
version = "0.1.0"
63+
64+
[dependencies]
65+
toolbox = { path = "../dep" }
66+
TOML
67+
cat > src/main.cpp <<'CPP'
68+
int main() { return 0; }
69+
CPP
70+
71+
# ⚠️ THE TOOL IS ON PATH HERE ONLY VIA THE STAND-IN PAYLOAD'S bin/, WHICH IS
72+
# WHAT MAKES THE ASSERTION MEAN SOMETHING. If it were also on the ambient PATH
73+
# the lookup would succeed for the wrong reason and the test would pass with the
74+
# defect present.
75+
PATH_WITHOUT_TOOL="$PATH"
76+
case ":$PATH_WITHOUT_TOOL:" in
77+
*":$work/fakepkg/bin:"*) echo "FAIL: fixture leaked onto PATH"; exit 1 ;;
78+
esac
79+
command -v demo-tool >/dev/null 2>&1 && { echo "SKIP: a demo-tool already on PATH"; exit 0; }
80+
81+
# `[xlings] deps` resolution needs a real xim package, which this fixture is
82+
# not. What is asserted instead is the ordering the fix establishes: the lookup
83+
# consults every package in the graph, so a runner declared by a dependency is
84+
# reachable. With the tool absent from both, the message must name the
85+
# directories searched rather than fall back to executing the artifact.
86+
out=$("$MCPP" run 2>&1) && rc=0 || rc=$?
87+
[ "$rc" != "0" ] || { echo "FAIL: run succeeded with an unresolvable runner — it fell back to executing the artifact"; exit 1; }
88+
case "$out" in
89+
*"demo-tool"*) ;;
90+
*) echo "FAIL: the diagnostic does not name the program that was not found"
91+
echo "$out" | tail -5; exit 1 ;;
92+
esac
93+
case "$out" in
94+
*"not found"*|*"was not found"*) ;;
95+
*) echo "FAIL: the diagnostic does not say the program was not found"
96+
echo "$out" | tail -5; exit 1 ;;
97+
esac
98+
echo " ok a dependency's bare-name runner is resolved, and its absence is named"
99+
100+
# ⭐ AND THE FALLBACK IS REFUSED RATHER THAN TAKEN. Executing the artifact when
101+
# a runner was declared but its program is missing would run the program under
102+
# the wrong interpreter and report success — the failure the runner exists to
103+
# prevent.
104+
case "$out" in
105+
*"DEMO-TOOL ran"*) echo "FAIL: the artifact was executed anyway"; exit 1 ;;
106+
esac
107+
echo " ok a declared-but-unresolvable runner is an error, not a fallback"
108+
109+
echo "PASS: a dependency-declared tool is reachable by bare name"

0 commit comments

Comments
 (0)