Skip to content

Commit 60f6bb3

Browse files
authored
fix(run): exec target/ninja directly, scope LD_LIBRARY_PATH to child (#133)
* fix(run): exec target/ninja directly, scope LD_LIBRARY_PATH to child mcpp run/test/build injected the bundled-glibc LD_LIBRARY_PATH into mcpp's own process (setenv) before std::system/capture, so the host /bin/sh it spawned inherited it. On newer-glibc distros sh then loaded the bundled (older) libc, which could not satisfy host libtinfo's GLIBC_2.42 symbols, and aborted before the target ran (reported as a 'sh:' version error). Add platform::process::run_exec/capture_exec (no shell; child-scoped env) and route the run, test, fast-path-ninja, and full-build-ninja launches through them. The produced binary is already self-contained (bundled --dynamic-linker + RUNPATH via gcc-specs/clang-cfg patch), so the loader env was redundant for normal binaries anyway. - store BuildResult::ninjaProgram raw (execvp needs argv, not a shell string); quote only at the remaining shell site; defensive unquote for legacy caches. - tests/unit/test_process_run_exec.cpp: proves parent env is never mutated. - tests/e2e/74: hostile LD_LIBRARY_PATH + bundled-interp guard. - ci-fresh-install: linux-distro-matrix across fedora/arch/tumbleweed/ debian-testing (+ old-glibc ubuntu-2004/debian-11). * fix(process): use posix_spawn (parent-built envp); fix Windows/macOS CI surfaced two cross-platform bugs in the first cut: - macOS: fork()+setenv()-in-child mutated the test's parent-env expectation (post-fork setenv is async-signal-unsafe on macOS). Switch POSIX run_exec/ capture_exec to posix_spawnp with an envp built in the PARENT — the child env never touches the parent, and there is no post-fork setenv. - Windows: capture_exec force-quoted argv[0], so popen's cmd.exe /c "..." stripped the outer quotes and mangled the ninja path ('filename syntax incorrect'). Keep argv[0] raw (per platform.shell guidance) and quote only later args. Unit tests gated to POSIX (they reference /bin/*); Windows path is covered by the integration build that launches ninja via capture_exec. Verified on Linux: unit 19/19, e2e 74, fast-path rebuild. * fix(process): shell env-prefix launcher (no spawn) — fixes Windows/macOS Both spawn-based attempts regressed non-Linux: posix_spawn returned an error on the macOS runner (run_exec rc!=0), and _spawnvpe segfaulted mcpp run on Windows (0xC0000005). The leak is Linux-only (macOS injects no runtime env; Windows has no glibc symbol versioning), so changing the launch primitive on every platform was the wrong call. Reimplement run_exec/capture_exec on top of the existing shell helpers: - run_exec: std::system with the env as a prefix via build_env_prefix (POSIX) / _putenv_s (Windows). The shell std::system spawns starts clean, so a bundled-glibc LD_LIBRARY_PATH reaches only the target child, never /bin/sh — which fixes the original crash. - capture_exec: command_from_argv + ' 2>&1' through the existing capture_with_env (same prefix semantics). No spawn primitives, no per-platform exec code. Linux: unit 19/19, e2e 74, hostile-LD_LIBRARY_PATH run, and fast-path rebuild all green. * test(process): use /bin/sh not /bin/true (absent on macOS) The macOS CI failures across every launcher attempt were this test, not the implementation: /bin/true lives at /usr/bin/true on macOS, so run_exec returned 127 and the rc==0 assertion failed. Use /bin/sh -c 'exit 0' (present on Linux and macOS). * refactor(process): hybrid launcher — Linux posix_spawn, macOS/Windows shell Per review: use a direct exec (posix_spawn) on Linux, where the leak actually lives and where it can be iterated locally, and keep the proven std::system shell path on macOS/Windows (which inject no runtime env / have no glibc symbol versioning). This gives Linux the clean child-only env isolation with no shell quoting/signal/injection surface, while not risking the platforms that can't be reproduced here. A TODO(launcher-unify) marks unifying onto spawn later if macOS/Windows ever need the same isolation. Linux verified: unit 19/19, e2e 74, hostile-LD_LIBRARY_PATH run, fast-path, passthrough args.
1 parent 435e1fa commit 60f6bb3

7 files changed

Lines changed: 1106 additions & 53 deletions

File tree

.agents/docs/2026-06-19-runtime-launch-and-multi-distro-plan.md

Lines changed: 705 additions & 0 deletions
Large diffs are not rendered by default.

.github/workflows/ci-fresh-install.yml

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,83 @@ jobs:
120120
mcpp clean
121121
mcpp run
122122
123+
# ──────────────────────────────────────────────────────────────────
124+
# Newer/rolling-glibc distros — reproduction surface for the
125+
# bundled-glibc-vs-host-libtinfo `sh:` crash (host glibc > bundled).
126+
# Plus older-glibc legs (the safe reverse direction) proving the
127+
# musl-static mcpp + self-contained toolchain run end-to-end on old
128+
# hosts. Runs the released mcpp inside distro containers.
129+
# ──────────────────────────────────────────────────────────────────
130+
linux-distro-matrix:
131+
name: Linux distro (${{ matrix.distro }})
132+
runs-on: ubuntu-24.04
133+
container:
134+
image: ${{ matrix.image }}
135+
timeout-minutes: 45
136+
strategy:
137+
fail-fast: false
138+
matrix:
139+
include:
140+
- distro: fedora-latest
141+
image: fedora:latest
142+
setup: dnf -y install curl bash tar gzip xz git findutils binutils file glibc-langpack-en
143+
- distro: arch
144+
image: archlinux:latest
145+
setup: pacman -Sy --noconfirm curl bash tar gzip xz git binutils file
146+
- distro: tumbleweed
147+
image: opensuse/tumbleweed:latest
148+
setup: zypper -n install curl bash tar gzip xz git binutils file
149+
- distro: debian-testing
150+
image: debian:testing
151+
setup: apt-get update && apt-get -y install curl bash tar gzip xz-utils git ca-certificates binutils findutils file
152+
- distro: ubuntu-2004
153+
image: ubuntu:20.04
154+
setup: apt-get update && DEBIAN_FRONTEND=noninteractive apt-get -y install curl bash tar gzip xz-utils git ca-certificates binutils findutils file
155+
- distro: debian-11
156+
image: debian:11
157+
setup: apt-get update && apt-get -y install curl bash tar gzip xz-utils git ca-certificates binutils findutils file
158+
env:
159+
XLINGS_NON_INTERACTIVE: '1'
160+
HOME: /root
161+
steps:
162+
- uses: actions/checkout@v4
163+
164+
- name: Install prerequisites (${{ matrix.distro }})
165+
run: ${{ matrix.setup }}
166+
167+
- name: Install xlings + mcpp
168+
run: |
169+
curl -fsSL https://raw.githubusercontent.com/openxlings/xlings/main/tools/other/quick_install.sh | bash -s v0.4.38
170+
echo "$HOME/.xlings/subos/current/bin" >> "$GITHUB_PATH"
171+
172+
- name: Configure mcpp
173+
run: |
174+
export PATH="$HOME/.xlings/subos/current/bin:$PATH"
175+
xlings update
176+
xlings install mcpp -y -g
177+
mcpp --version
178+
mcpp self config --mirror GLOBAL
179+
180+
- name: "Regression: new → run (loader env must not crash /bin/sh)"
181+
run: |
182+
export PATH="$HOME/.xlings/subos/current/bin:$PATH"
183+
cd "$(mktemp -d)"
184+
mcpp new hello_distro
185+
cd hello_distro
186+
mcpp run
187+
188+
- name: "Self-containment: produced binary uses bundled loader"
189+
run: |
190+
export PATH="$HOME/.xlings/subos/current/bin:$PATH"
191+
cd "$(mktemp -d)" && mcpp new hc && cd hc && mcpp build
192+
bin="$(find target -type f -name hc | head -1)"
193+
interp="$(file "$bin" | grep -o 'interpreter [^,]*' | awk '{print $2}')"
194+
echo "interp=$interp"
195+
case "$interp" in
196+
*/.mcpp/*|*/registry/*|*xpkgs*) echo "OK bundled loader" ;;
197+
*) echo "FAIL host loader: $interp"; exit 1 ;;
198+
esac
199+
123200
# ──────────────────────────────────────────────────────────────────
124201
# macOS: llvm@20.1.7
125202
# ──────────────────────────────────────────────────────────────────

src/build/execute.cppm

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ export std::optional<int> try_fast_build(const std::filesystem::path& projectRoo
274274

275275
auto outputDirStr = match->outputDir;
276276
auto ninjaProgram = match->ninjaProgram;
277+
// Legacy caches stored a shell-quoted path; execvp needs the raw path.
278+
if (ninjaProgram.size() >= 2 && ninjaProgram.front() == '\''
279+
&& ninjaProgram.back() == '\'')
280+
ninjaProgram = ninjaProgram.substr(1, ninjaProgram.size() - 2);
277281
auto cachedFingerprint = match->fingerprint;
278282
auto runtimeEnvKey = match->runtimeEnvKey;
279283
auto runtimeEnvValue = match->runtimeEnvValue;
@@ -317,19 +321,21 @@ export std::optional<int> try_fast_build(const std::filesystem::path& projectRoo
317321
}
318322

319323
// All inputs are older than build.ninja → fast-path: just run ninja.
320-
std::string cmd = ninjaProgram;
321-
if (!verbose) cmd += " --quiet";
322-
cmd += std::format(" -C {}", mcpp::platform::shell::quote(outputDir.string()));
323-
if (verbose) cmd += " -v";
324-
cmd += " 2>&1";
324+
std::vector<std::string> argv{ninjaProgram};
325+
if (!verbose) argv.push_back("--quiet");
326+
argv.push_back("-C");
327+
argv.push_back(outputDir.string());
328+
if (verbose) argv.push_back("-v");
325329

326-
auto t0 = std::chrono::steady_clock::now();
327-
std::string out;
328-
std::optional<mcpp::platform::env::ScopedEnv> scopedEnv;
330+
std::vector<std::pair<std::string, std::string>> childEnv;
329331
if (runtimeEnvKey != "-" && !runtimeEnvValue.empty())
330-
scopedEnv.emplace(runtimeEnvKey, runtimeEnvValue);
331-
auto r = mcpp::platform::process::capture(cmd);
332-
out = r.output;
332+
childEnv.emplace_back(runtimeEnvKey, runtimeEnvValue);
333+
334+
auto t0 = std::chrono::steady_clock::now();
335+
// capture_exec merges stderr into the captured output (replacing `2>&1`),
336+
// so is_stale_ninja_failure / filter_ninja_output still see ninja errors.
337+
auto r = mcpp::platform::process::capture_exec(argv, childEnv);
338+
std::string out = r.output;
333339
int status = r.exit_code;
334340
bool ok = (status == 0);
335341
if (!ok) {
@@ -386,19 +392,21 @@ export int build_run_target(const std::optional<std::string>& targetName,
386392
std::format("`{}`", mcpp::ui::shorten_path(exe, pathCtx)));
387393
std::println("");
388394
std::fflush(stdout);
389-
std::string cmd = mcpp::platform::shell::quote(exe.string());
390-
for (auto& a : passthrough) cmd += " " + mcpp::platform::shell::quote(a);
395+
std::vector<std::string> argv;
396+
argv.push_back(exe.string());
397+
for (auto& a : passthrough) argv.push_back(a);
391398

392-
std::optional<mcpp::platform::env::ScopedEnv> runtimeEnv;
399+
std::vector<std::pair<std::string, std::string>> childEnv;
393400
auto runtimeEnvKey = mcpp::platform::env::runtime_library_path_key();
394401
auto runtimeEnvValue = mcpp::platform::env::prepend_path_list(
395402
runtimeEnvKey, ctx->plan.runtimeLibraryDirs);
396-
if (!runtimeEnvKey.empty() && !runtimeEnvValue.empty()) {
397-
runtimeEnv.emplace(runtimeEnvKey, runtimeEnvValue);
398-
}
403+
if (!runtimeEnvKey.empty() && !runtimeEnvValue.empty())
404+
childEnv.emplace_back(runtimeEnvKey, runtimeEnvValue);
399405

400-
int rc = std::system(cmd.c_str());
401-
return mcpp::platform::process::extract_exit_code(rc) == 0 ? 0 : 1;
406+
// Direct exec (no /bin/sh): the loader env reaches ONLY the target child,
407+
// never mcpp or a host shell. Fixes the bundled-glibc-vs-host-libtinfo
408+
// crash on newer-glibc distros.
409+
return mcpp::platform::process::run_exec(argv, childEnv) == 0 ? 0 : 1;
402410
}
403411

404412
// `mcpp test` driver: discover tests/**/*.cpp, synthesize targets, build
@@ -505,38 +513,40 @@ export int run_tests(std::span<const std::string> passthrough,
505513
int failed = 0;
506514
std::vector<std::string> failures;
507515

508-
std::optional<mcpp::platform::env::ScopedEnv> runtimeEnv;
509516
auto runtimeEnvKey = mcpp::platform::env::runtime_library_path_key();
510517
auto runtimeEnvValue = mcpp::platform::env::prepend_path_list(
511518
runtimeEnvKey, ctx->plan.runtimeLibraryDirs);
512-
if (!runtimeEnvKey.empty() && !runtimeEnvValue.empty()) {
513-
runtimeEnv.emplace(runtimeEnvKey, runtimeEnvValue);
514-
}
515519

516520
for (auto& lu : ctx->plan.linkUnits) {
517521
if (lu.kind != mcpp::build::LinkUnit::TestBinary) continue;
518522
auto exe = ctx->outputDir / lu.output;
519523
mcpp::ui::status("Running", std::format("bin/{}", lu.targetName));
520524

521-
// Prepend the sandbox's subos/default/bin to PATH so tools
522-
// bootstrapped during sandbox init (patchelf, ninja, etc.) are
523-
// visible to test binaries that shell out to them. The
524-
// toolchain binary's path encodes the registry root — derive it.
525-
std::string pathPrefix;
525+
std::vector<std::string> argv;
526+
argv.push_back(exe.string());
527+
for (auto& a : passthrough) argv.push_back(a);
528+
529+
std::vector<std::pair<std::string, std::string>> childEnv;
530+
if (!runtimeEnvKey.empty() && !runtimeEnvValue.empty())
531+
childEnv.emplace_back(runtimeEnvKey, runtimeEnvValue);
532+
533+
// Prepend the sandbox's subos/default/bin to the CHILD PATH so test
534+
// binaries that shell out to bootstrapped tools (patchelf, ninja) find
535+
// them — applied to the child only, not via a leaky shell prefix.
526536
if constexpr (!mcpp::platform::is_windows) {
527537
if (auto xpkgs = mcpp::xlings::paths::xpkgs_from_compiler(ctx->tc.binaryPath)) {
528538
// xpkgs is <registry>/data/xpkgs → registry = xpkgs/../..
529539
auto registryDir = xpkgs->parent_path().parent_path();
530540
auto sandboxBin = registryDir / "subos" / "default" / "bin";
531-
if (std::filesystem::exists(sandboxBin))
532-
pathPrefix = std::format("PATH={}:\"$PATH\" ",
533-
mcpp::platform::shell::quote(sandboxBin.string()));
541+
if (std::filesystem::exists(sandboxBin)) {
542+
std::array<std::filesystem::path, 1> extra{sandboxBin};
543+
auto pathVal = mcpp::platform::env::prepend_path_list("PATH", extra);
544+
if (!pathVal.empty()) childEnv.emplace_back("PATH", pathVal);
545+
}
534546
}
535547
}
536548

537-
std::string cmd = pathPrefix + mcpp::platform::shell::quote(exe.string());
538-
for (auto& a : passthrough) cmd += " " + mcpp::platform::shell::quote(a);
539-
int exitCode = mcpp::platform::process::extract_exit_code(std::system(cmd.c_str()));
549+
int exitCode = mcpp::platform::process::run_exec(argv, childEnv);
540550

541551
if (exitCode == 0) {
542552
std::println("{} ... ok", lu.targetName);

src/build/ninja_backend.cppm

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -714,15 +714,11 @@ std::expected<BuildResult, BuildError> NinjaBackend::build(const BuildPlan& plan
714714
ninjaBin = *nb;
715715
}
716716

717-
std::string ninjaProgram;
718-
if (!ninjaBin.empty()) {
719-
if constexpr (mcpp::platform::is_windows)
720-
ninjaProgram = ninjaBin.string();
721-
else
722-
ninjaProgram = mcpp::platform::shell::quote(ninjaBin.string());
723-
} else {
724-
ninjaProgram = "ninja";
725-
}
717+
// Raw program path (no shell quoting): recorded in the fast-path cache and
718+
// exec'd directly via capture_exec/execvp, which take argv (not a shell
719+
// string). Shell-using call sites must quote it locally.
720+
std::string ninjaProgram = ninjaBin.empty() ? std::string("ninja")
721+
: ninjaBin.string();
726722

727723
// Record ninja binary for P0 fast-path cache.
728724
BuildResult r;
@@ -734,21 +730,27 @@ std::expected<BuildResult, BuildError> NinjaBackend::build(const BuildPlan& plan
734730
r.runtimeEnvKey = "-";
735731
}
736732

737-
std::string cmd = ninjaProgram;
733+
// Direct exec (no /bin/sh): argv, not a shell string. capture_exec merges
734+
// stderr into the captured output (replacing the old `2>&1`), and applies
735+
// the runtime env to the child ONLY — so a bundled-glibc LD_LIBRARY_PATH
736+
// can never poison the host shell (the newer-glibc `sh:` crash class).
737+
std::vector<std::string> nargv{ninjaProgram};
738738
if (!opts.verbose)
739-
cmd += " --quiet";
740-
cmd += std::format(" -C {}", mcpp::xlings::shq(plan.outputDir.string()));
739+
nargv.push_back("--quiet");
740+
nargv.push_back("-C");
741+
nargv.push_back(plan.outputDir.string());
741742
if (opts.verbose)
742-
cmd += " -v";
743+
nargv.push_back("-v");
743744
if (opts.parallelJobs)
744-
cmd += std::format(" -j{}", opts.parallelJobs);
745-
cmd += " 2>&1";
745+
nargv.push_back(std::format("-j{}", opts.parallelJobs));
746746

747-
std::string out;
748-
std::optional<mcpp::platform::env::ScopedEnv> scopedEnv;
747+
std::vector<std::pair<std::string, std::string>> nenv;
749748
if (r.runtimeEnvKey != "-" && !r.runtimeEnvValue.empty())
750-
scopedEnv.emplace(r.runtimeEnvKey, r.runtimeEnvValue);
751-
bool ok = run(cmd, out, /*capture=*/true);
749+
nenv.emplace_back(r.runtimeEnvKey, r.runtimeEnvValue);
750+
751+
auto cap = mcpp::platform::process::capture_exec(nargv, nenv);
752+
std::string out = cap.output;
753+
bool ok = (cap.exit_code == 0);
752754

753755
r.exitCode = ok ? 0 : 1;
754756
r.elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(

0 commit comments

Comments
 (0)