Skip to content

Commit 370d8e2

Browse files
committed
fix: runner was reported as an ignored key while being honoured, and CI installed the emulator into one home
Two things CI found that local runs could not. * `[target.<triple>].runner` drew "unsupported key 'runner' (ignored)". The unknown-key sweep is about SCALARS — "a scalar that does nothing" — and it skipped tables but not arrays, so an array key the parser reads a few lines earlier was announced as ignored. Saying a working key does nothing is worse than either statement being true on its own. Two tests pin it: the key parses and warns about nothing, and the two shapes that would run nothing (an empty array, a bare string) are still errors. * The bare-metal job installed the emulator into the ambient xlings home only, and `mcpp run` answered [error] xlings: 'qemu-system-riscv64' is not installed even though the shim was on PATH. A shim dispatches against whichever home owns it, and `mcpp run` goes through that shim — so the emulator has to be in the home MCPP uses, exactly like the sysroot two steps below it. Installed into both now, with the `--version` probe kept as the before-the-fact check.
1 parent b54809c commit 370d8e2

3 files changed

Lines changed: 66 additions & 3 deletions

File tree

.github/workflows/ci-linux-e2e.yml

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,18 @@ jobs:
132132
133133
- name: Install the emulator (xim:qemu-riscv)
134134
run: |
135+
# ⚠️ BOTH homes. The shim on PATH dispatches against whichever home
136+
# owns it, and `mcpp run` runs the runner through that shim — so an
137+
# emulator installed only in the ambient xlings home answers
138+
# "xlings: 'qemu-system-riscv64' is not installed" when mcpp asks.
139+
# Measured: the job installed it once, the shim resolved, and the
140+
# run still failed.
135141
"$XLINGS_BIN" install xim:qemu-riscv -y
136-
# Assert it is reachable BEFORE the suite runs. Without this the
137-
# capability probe simply would not add `qemu-riscv`, and the test
138-
# would skip — which is what this job exists to prevent.
142+
XLINGS_HOME="${MCPP_HOME:-$HOME/.mcpp}/registry" \
143+
"$XLINGS_BIN" install xim:qemu-riscv -y
144+
# Assert it is reachable AND runnable BEFORE the tests. Without this
145+
# the capability probe simply would not add `qemu-riscv` and the
146+
# tests would skip — which is what this job exists to prevent.
139147
command -v qemu-system-riscv64
140148
qemu-system-riscv64 --version | head -1
141149
# The target sysroot, into the home MCPP uses. Test 131's BSP

src/manifest/toml.cppm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,6 +1460,12 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
14601460
};
14611461
for (auto& [key, value] : body) {
14621462
if (value.is_table()) continue; // the conditional channel
1463+
// ...and arrays, which this sweep was never about. It checks
1464+
// SCALARS ("a scalar that does nothing"), and `runner` is an
1465+
// array read a few lines above — reaching here it was reported
1466+
// as "unsupported key 'runner' (ignored)" while in fact being
1467+
// honoured, which is worse than either being true.
1468+
if (value.is_array()) continue;
14631469
bool known = false;
14641470
for (auto k : kKnownTargetScalars) if (key == k) { known = true; break; }
14651471
if (known) continue;

tests/unit/test_manifest.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3590,3 +3590,52 @@ defines = ["FOO=1"]
35903590
// ...and the field itself is gone, so nothing downstream can read it.
35913591
EXPECT_EQ(m->targetOverrides.at("x86_64-linux-gnu").cxxRuntime, "");
35923592
}
3593+
3594+
// ── [target.<triple>].runner ────────────────────────────────────────────────
3595+
3596+
TEST(Manifest, TargetRunnerParsesAndDoesNotWarn) {
3597+
constexpr auto src = R"(
3598+
[package]
3599+
name = "fw"
3600+
version = "0.1.0"
3601+
3602+
[target.riscv64-none-elf]
3603+
runner = ["qemu-system-riscv64", "-machine", "virt", "-kernel"]
3604+
)";
3605+
auto m = mcpp::manifest::parse_string(src);
3606+
ASSERT_TRUE(m.has_value()) << m.error().format();
3607+
auto it = m->targetOverrides.find("riscv64-none-elf");
3608+
ASSERT_NE(it, m->targetOverrides.end());
3609+
ASSERT_EQ(it->second.runner.size(), 4u);
3610+
EXPECT_EQ(it->second.runner.front(), "qemu-system-riscv64");
3611+
EXPECT_EQ(it->second.runner.back(), "-kernel");
3612+
// ⚠️ The unknown-key sweep is about SCALARS ("a scalar that does
3613+
// nothing"). It skipped tables but not arrays, so this key was reported as
3614+
// "unsupported key 'runner' (ignored)" while in fact being honoured —
3615+
// worse than either statement being true. Seen in CI.
3616+
EXPECT_TRUE(m->schemaWarnings.empty())
3617+
<< (m->schemaWarnings.empty() ? "" : m->schemaWarnings[0]);
3618+
}
3619+
3620+
TEST(Manifest, TargetRunnerRejectsShapesThatWouldRunNothing) {
3621+
// An empty template would exec nothing and report success.
3622+
constexpr auto empty = R"(
3623+
[package]
3624+
name = "fw"
3625+
version = "0.1.0"
3626+
[target.riscv64-none-elf]
3627+
runner = []
3628+
)";
3629+
EXPECT_FALSE(mcpp::manifest::parse_string(empty).has_value());
3630+
3631+
// A bare string is the shape a user reaches for first; taking it would
3632+
// mean guessing where the word boundaries are.
3633+
constexpr auto scalar = R"(
3634+
[package]
3635+
name = "fw"
3636+
version = "0.1.0"
3637+
[target.riscv64-none-elf]
3638+
runner = "qemu-system-riscv64 -kernel"
3639+
)";
3640+
EXPECT_FALSE(mcpp::manifest::parse_string(scalar).has_value());
3641+
}

0 commit comments

Comments
 (0)