diff --git a/.agents/docs/2026-05-26-build-error-output-optimization-plan.md b/.agents/docs/2026-05-26-build-error-output-optimization-plan.md new file mode 100644 index 0000000..09cd2a6 --- /dev/null +++ b/.agents/docs/2026-05-26-build-error-output-optimization-plan.md @@ -0,0 +1,392 @@ +# mcpp build 报错输出优化方案 + +**Date**: 2026-05-26 +**Status**: Proposed + +## 1. 目标 + +当用户代码有编译错误时,默认 `mcpp build` 输出应聚焦在用户真正需要修复的 +编译器诊断上: + +- 不显示 ninja 进度信息,如 `[1/9] OBJ obj/main.o` +- 不把 `env LD_LIBRARY_PATH=...` 这种运行时环境前缀暴露在失败命令行里 +- 不重复打印同一段 ninja / compiler 输出 +- `--verbose` 仍保留足够的构建细节,便于定位 mcpp 自身问题 + +## 2. 真实复现 + +复现目录: + +```bash +cd /home/speak/test/mcpp/helloworld +mcpp build +``` + +样例里 `src/main.cpp` 当前有明确语法错误: + +```cpp +int main(int argc, char* argv[]) { + hw:: + return 0; +} +``` + +当前普通 `mcpp build` 的关键输出: + +```text +ninja: Entering directory `/home/speak/test/mcpp/helloworld/target/...' +[1/2] OBJ obj/main.o +FAILED: obj/main.o +env LD_LIBRARY_PATH='/home/speak/.mcpp/registry/data/xpkgs/xim-x-llvm/20.1.7/lib:...'${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} /home/speak/.mcpp/.../clang++ ... +/home/speak/test/mcpp/helloworld/src/main.cpp:8:5: error: expected unqualified-id + 8 | return 0; + | ^ +1 error generated. +ninja: build stopped: subcommand failed. +error: auto-installing default toolchain gcc@15.1.0-musl failed: ... +``` + +为单独观察完整 backend 失败路径,使用已有 `/home/speak/.mcpp` 配置运行: + +```bash +MCPP_HOME=/home/speak/.mcpp mcpp build --no-cache +``` + +可以稳定看到同一段 ninja 输出被打印两次:第一次由 backend 直接 `fputs`, +第二次被塞入 `BuildError.message` 后由 CLI 的 `ui::error()` 再打印。 + +## 3. 当前代码路径 + +### 3.1 ninja 进度和失败包装 + +`src/build/ninja_backend.cppm` + +- `NinjaBackend::build()` 生成 `build.ninja`,然后执行 + `ninja -C 2>&1`。 +- 非 verbose 模式也会在失败时输出完整 ninja 捕获文本: + +```cpp +if (opts.verbose || !ok) { + std::fputs(out.c_str(), stdout); +} +``` + +- 失败时又把同一份 `out` 放入 `BuildError.message`: + +```cpp +return std::unexpected(BuildError{ + std::format("ninja failed (exit non-zero):\n{}", out), + plan.outputDir / "build.ninja"}); +``` + +`src/cli.cppm` + +- `run_build_plan()` 收到失败后再次打印: + +```cpp +mcpp::ui::error(r.error().message); +``` + +这就是重复输出的直接原因。 + +### 3.2 fast-path 失败会先打印再回退 + +`src/cli.cppm::try_fast_build()` 在 build cache 命中时直接运行旧 +`build.ninja`。如果 ninja 非零退出,它会先打印输出,再返回 `nullopt`: + +```cpp +if (!ok) { + if (!verbose) std::fputs(out.c_str(), stdout); + return std::nullopt; +} +``` + +随后 `cmd_build()` 进入完整 `prepare_build()`。在当前复现环境中,PATH 下的 +mcpp 使用的 `MCPP_HOME` 没有默认 toolchain,所以用户代码编译错误后又追加了 +一次默认 toolchain 自动安装失败提示,导致错误主题混杂。 + +### 3.3 `LD_LIBRARY_PATH` 进入失败命令行 + +`src/toolchain/probe.cppm` + +- `detect()` 填充 `Toolchain::compilerRuntimeDirs` +- `compiler_env_prefix()` 把 runtime dirs 转成 shell 前缀 + +```cpp +return mcpp::platform::linux_::build_ld_library_path_prefix(dirs); +``` + +`src/platform/linux.cppm` + +```cpp +return std::format("env LD_LIBRARY_PATH={}${{LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}} ", + mcpp::platform::shell::quote(joined)); +``` + +`src/build/flags.cppm` + +```cpp +f.toolEnv = mcpp::toolchain::compiler_env_prefix(plan.toolchain); +``` + +`src/build/ninja_backend.cppm` + +```ninja +toolenv = env LD_LIBRARY_PATH='...'${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} +rule cxx_object + command = $toolenv $cxx $cxxflags -c $in -o $out +``` + +ninja 在命令失败时会打印失败 command,因此这个 `toolenv` 会直接暴露给用户。 + +## 4. 优化方案 + +### 4.1 默认使用安静的 ninja 输出 + +默认构建命令改为安静模式: + +```bash +ninja --quiet -C +``` + +作用: + +- 隐藏 `ninja: Entering directory ...` +- 隐藏 `[x/y] DESCRIPTION ...` +- 保留失败时的必要 stderr/stdout + +已在复现目录手动验证:`ninja --quiet -C ...` 不再显示 `[x/y]` 进度,但仍会 +显示失败 command 和 compiler diagnostic。因此它只能解决第一类噪声,不能单独 +解决 `LD_LIBRARY_PATH` 和重复输出。 + +`--verbose` 模式保持当前行为:不加 `--quiet`,继续追加 `-v`。 + +### 4.2 用 ScopedEnv / ScopedRun 代替 `toolenv` 命令前缀 + +把运行时库路径从 `build.ninja` 规则里移出,改为启动 ninja 子进程时注入环境。 + +建议新增跨平台 API: + +```cpp +namespace mcpp::platform::env { + +struct ScopedEnv { + ScopedEnv(std::string key, std::optional value); + ~ScopedEnv(); +}; + +std::string path_list_separator(); +std::string runtime_library_path_key(); +std::string prepend_path_list(std::string_view key, + std::span dirs); + +} // namespace mcpp::platform::env +``` + +平台策略: + +| 平台 | 变量 | 说明 | +|------|------|------| +| Linux | `LD_LIBRARY_PATH` | 当前实际需要,替代 `toolenv` | +| macOS | 空 | 不设置 `DYLD_LIBRARY_PATH`;它会影响 ninja 自身和系统 framework 的 dyld 解析,依赖 toolchain/rpath | +| Windows | `PATH` | 如果私有工具链 DLL 需要搜索路径,按 `;` prepend;当前可先空实现 | + +`NinjaBackend::build()` 中: + +1. 从 `plan.toolchain.compilerRuntimeDirs` 计算 scoped env +2. 在 `process::capture(ninjaCmd)` 前创建 `ScopedEnv` +3. `build.ninja` 不再写 `toolenv` 变量 +4. 所有规则从: + +```ninja +command = $toolenv $cxx $cxxflags -c $in -o $out +``` + +改为: + +```ninja +command = $cxx $cxxflags -c $in -o $out +``` + +这样失败时 ninja 最多打印 compiler 命令,不会再出现 +`env LD_LIBRARY_PATH=...`。同时 `clang-scan-deps`、`clang++`、`llvm-ar` 等 +ninja 子命令都会继承同一份环境,不需要每条 rule 单独拼 shell 前缀。 + +后续如果要进一步消除全局环境 mutation,可把 `ScopedEnv` 升级为 +`process::run({ .argv, .cwd, .env })`,POSIX 走 `fork/execve`,Windows 走 +`CreateProcessW` environment block。但第一阶段用 RAII env 已能满足当前目标, +改动面更小。 + +### 4.3 明确输出所有权,避免重复打印 + +需要规定:ninja 输出只能由一个层级负责展示。 + +推荐方案: + +```cpp +struct BuildError { + std::string summary; // "build failed" + std::string diagnosticOutput; // 过滤后的 compiler/ninja 输出 + std::optional where; +}; +``` + +调用关系: + +1. `NinjaBackend::build()` 只捕获并返回,不直接 `fputs` +2. `run_build_plan()` 负责打印一次 +3. 默认模式打印: + +```text +error: build failed + +``` + +4. verbose 模式打印完整 ninja 输出,但仍只打印一次 + +如果暂时不想扩展 `BuildError`,最小改法是: + +- `NinjaBackend::build()` 失败时不再 `fputs(out)` +- `BuildError.message` 保留一份输出 + +但这个最小改法仍会把整段输出挂在 `error:` 后面,可读性不如结构化字段。 + +### 4.4 过滤默认模式下的 ninja 包装行 + +`--quiet` 解决进度行,但失败输出仍会包含: + +```text +FAILED: obj/main.o + +ninja: build stopped: subcommand failed. +``` + +建议增加 `filter_ninja_output(out, flags, mode)`: + +默认模式移除: + +- `ninja: Entering directory ...` +- `ninja: build stopped: subcommand failed.` +- `FAILED: ...` +- 已知工具命令行:以当前 `cxxBinary`、`ccBinary`、`arBinary`、`scanDepsPath` + 开头的行 +- 旧 build.ninja 中残留的 `env LD_LIBRARY_PATH=... ` 命令行 +- ninja 进度行:`^\[[0-9]+/[0-9]+\] ` + +默认模式保留: + +- compiler warning/error diagnostic +- 源码路径、行列号和 caret +- linker diagnostic +- mcpp 自己的 dyndep / manifest diagnostic + +`--verbose` 模式不过滤。 + +目标默认输出示例: + +```text + Compiling helloworld v0.1.0 (.) +error: build failed +/home/speak/test/mcpp/helloworld/src/main.cpp:8:5: error: expected unqualified-id + 8 | return 0; + | ^ +1 error generated. +``` + +### 4.5 修正 fast-path 失败语义 + +`try_fast_build()` 当前在失败时“先打印失败,再回退完整构建”。这会导致: + +- 同一编译错误可能先由 fast-path 打印一次,再由完整构建打印一次 +- 旧 `build.ninja` 的 toolchain 和当前 config 不一致时,用户会看到两个不同主题的错误 + +建议把返回值从 `std::optional` 改成 tri-state: + +```cpp +enum class FastBuildKind { NotApplicable, Success, BuildFailed, StaleOrInvalid }; + +struct FastBuildResult { + FastBuildKind kind; + int exitCode = 0; + std::string output; +}; +``` + +策略: + +- cache 不存在、fingerprint 不匹配、`build.ninja` 缺失:`NotApplicable` +- ninja 成功:`Success` +- ninja 报 `loading build.ninja` / `unknown target` / manifest 结构明显不匹配: + `StaleOrInvalid`,静默回到完整 prepare +- 普通 compile/link 失败:`BuildFailed`,直接按同一套过滤和单次打印逻辑返回, + 不再触发自动安装默认 toolchain + +这样用户代码错误不会被后续 toolchain resolve/install 错误污染。 + +## 5. 实施顺序 + +1. **先去重输出** + - 调整 `NinjaBackend::build()` 和 `run_build_plan()` 的打印职责 + - 增加 syntax-error e2e,断言同一 compiler error 只出现一次 + +2. **移动 toolenv 到 scoped env** + - 增加 `platform::env::ScopedEnv` + - `emit_ninja_string()` 删除 `toolenv` 变量和 `$toolenv` 前缀 + - `NinjaBackend::build()` 和 `try_fast_build()` 启动 ninja 前设置运行时库路径 + - 断言生成的 `build.ninja` 不包含 `LD_LIBRARY_PATH` + +3. **安静化 ninja 默认输出** + - 非 verbose 加 `--quiet` + - 增加 `filter_ninja_output()` + - 断言默认失败输出不包含 `[1/`、`FAILED:`、`ninja: build stopped` + +4. **修正 fast-path 失败** + - 将 `try_fast_build()` 改为 tri-state + - 编译失败直接返回失败,不进入完整 prepare + - stale/invalid ninja 才允许静默回退 + +## 6. 验证要求 + +新增或调整 E2E: + +```bash +bash tests/e2e/05_errors.sh +``` + +新增专门用例,例如 `tests/e2e/48_build_error_output.sh`: + +- 创建项目并写入语法错误 +- 运行 `mcpp build` +- 断言输出包含源码诊断:`src/main.cpp:*: error:` +- 断言输出不包含: + - `LD_LIBRARY_PATH=` + - `[1/` + - `FAILED:` + - `ninja: Entering directory` + - `ninja: build stopped` +- 断言 `expected unqualified-id` 只出现一次 +- 运行 `mcpp build --verbose`,断言 verbose 仍保留 ninja/command 细节 +- 检查生成的 `build.ninja` 不包含 `toolenv` 和 `LD_LIBRARY_PATH` + +相关单元测试: + +- `filter_ninja_output()`:覆盖 progress、FAILED、command、compiler diagnostic 混合文本 +- `ScopedEnv`:覆盖变量不存在、变量已存在、prepend 后析构恢复 + +本地验证命令: + +```bash +PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 bash tests/e2e/05_errors.sh +PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 bash tests/e2e/48_build_error_output.sh +mcpp build +``` + +## 7. 风险和边界 + +- `ScopedEnv` 会短暂修改当前进程环境。mcpp CLI 当前是单构建流程,风险可控; + 如果未来 backend 并发运行多个 ninja,应升级为 `ProcessOptions.env`。 +- 过滤 command line 不能误删 compiler diagnostic。规则应只删除“已知工具路径开头” + 的整行,不做宽泛字符串匹配。 +- `--verbose` 必须不丢信息,否则会降低 mcpp 自身问题的可诊断性。 +- fast-path 回退判断要保守:无法确认是 stale/invalid 时,按普通构建失败处理并 + 打印一次,避免再次污染错误输出。 diff --git a/src/build/backend.cppm b/src/build/backend.cppm index 77e5ca9..947a4fb 100644 --- a/src/build/backend.cppm +++ b/src/build/backend.cppm @@ -22,11 +22,14 @@ struct BuildResult { std::size_t cacheHits = 0; std::size_t cacheMisses = 0; std::string ninjaProgram; // P0: cached for fast-path rebuilds + std::string runtimeEnvKey; // cached for fast-path rebuilds + std::string runtimeEnvValue; // cached for fast-path rebuilds }; struct BuildError { std::string message; std::optional where; + std::string diagnosticOutput; }; struct Backend { diff --git a/src/build/flags.cppm b/src/build/flags.cppm index 6249170..18bab52 100644 --- a/src/build/flags.cppm +++ b/src/build/flags.cppm @@ -27,7 +27,6 @@ struct CompileFlags { std::filesystem::path arBinary; // ar path (may be empty → use PATH) std::string sysroot; // --sysroot=... (for ninja ldflags) std::string bFlag; // -B (for ninja ldflags) - std::string toolEnv; // env prefix for private toolchain executables bool staticStdlib = true; std::string linkage; // "static" or "" }; @@ -69,7 +68,6 @@ CompileFlags compute_flags(const BuildPlan& plan) { f.cxxBinary = plan.toolchain.binaryPath; f.ccBinary = mcpp::toolchain::derive_c_compiler(plan.toolchain); - f.toolEnv = mcpp::toolchain::compiler_env_prefix(plan.toolchain); // PIC? bool need_pic = false; @@ -98,7 +96,8 @@ CompileFlags compute_flags(const BuildPlan& plan) { // potentially-stale paths, then provide all flags explicitly. // // Fallback: if no PayloadPaths, use --sysroot from probe_sysroot(). - std::string sysroot_flag; + std::string compile_toolchain_flags; + std::string link_toolchain_flags; bool isClangWithCfg = false; std::filesystem::path cfgPath; if (mcpp::toolchain::is_clang(plan.toolchain)) { @@ -111,51 +110,54 @@ CompileFlags compute_flags(const BuildPlan& plan) { // Clang with cfg: bypass cfg and provide all paths explicitly. auto llvmRoot = plan.toolchain.binaryPath.parent_path().parent_path(); auto libcxxInclude = llvmRoot / "include" / "c++" / "v1"; - sysroot_flag = " --no-default-config -nostdinc++"; + compile_toolchain_flags = " --no-default-config -nostdinc++"; // libc++ headers - sysroot_flag += " -stdlib=libc++"; - sysroot_flag += " -isystem" + escape_path(libcxxInclude); + compile_toolchain_flags += " -isystem" + escape_path(libcxxInclude); if (!plan.toolchain.targetTriple.empty()) { auto targetInclude = llvmRoot / "include" / plan.toolchain.targetTriple / "c++" / "v1"; if (std::filesystem::exists(targetInclude)) - sysroot_flag += " -isystem" + escape_path(targetInclude); + compile_toolchain_flags += " -isystem" + escape_path(targetInclude); } // C library + kernel headers from payload if (plan.toolchain.payloadPaths) { auto& pp = *plan.toolchain.payloadPaths; - sysroot_flag += " -isystem" + escape_path(pp.glibcInclude); + compile_toolchain_flags += " -isystem" + escape_path(pp.glibcInclude); if (!pp.linuxInclude.empty()) - sysroot_flag += " -isystem" + escape_path(pp.linuxInclude); + compile_toolchain_flags += " -isystem" + escape_path(pp.linuxInclude); } else if (auto sdk = mcpp::platform::macos::sdk_path()) { - sysroot_flag += " --sysroot=" + escape_path(*sdk); + auto sysroot_flag = " --sysroot=" + escape_path(*sdk); + compile_toolchain_flags += sysroot_flag; + link_toolchain_flags += sysroot_flag; } else if (!plan.toolchain.sysroot.empty()) { - sysroot_flag += " --sysroot=" + escape_path(plan.toolchain.sysroot); + auto sysroot_flag = " --sysroot=" + escape_path(plan.toolchain.sysroot); + compile_toolchain_flags += sysroot_flag; + link_toolchain_flags += sysroot_flag; } // Linker flags that cfg normally provides - sysroot_flag += " -fuse-ld=lld --rtlib=compiler-rt --unwindlib=libunwind"; - f.sysroot = sysroot_flag; + link_toolchain_flags = " --no-default-config" + link_toolchain_flags + + " -stdlib=libc++ -fuse-ld=lld --rtlib=compiler-rt --unwindlib=libunwind"; + f.sysroot = link_toolchain_flags; } else if (!plan.toolchain.sysroot.empty()) { // GCC (or Clang without cfg): use --sysroot from probe. // GCC requires --sysroot for include-fixed headers (stdlib.h wrapper). // Supplement with -isystem for linux kernel headers from payload // if the probed sysroot is missing them. - sysroot_flag = " --sysroot=" + escape_path(plan.toolchain.sysroot); + auto sysroot_flag = " --sysroot=" + escape_path(plan.toolchain.sysroot); + compile_toolchain_flags = sysroot_flag; + link_toolchain_flags = sysroot_flag; if (plan.toolchain.payloadPaths && !plan.toolchain.payloadPaths->linuxInclude.empty()) { auto sysrootLinux = plan.toolchain.sysroot / "usr" / "include" / "linux" / "limits.h"; if (!std::filesystem::exists(sysrootLinux)) - sysroot_flag += " -isystem" + escape_path(plan.toolchain.payloadPaths->linuxInclude); + compile_toolchain_flags += " -isystem" + escape_path(plan.toolchain.payloadPaths->linuxInclude); } - f.sysroot = sysroot_flag; + f.sysroot = link_toolchain_flags; } else if (plan.toolchain.payloadPaths) { // No sysroot but have payload paths: use -isystem. auto& pp = *plan.toolchain.payloadPaths; - sysroot_flag += " -isystem" + escape_path(pp.glibcInclude); + compile_toolchain_flags += " -isystem" + escape_path(pp.glibcInclude); if (!pp.linuxInclude.empty()) - sysroot_flag += " -isystem" + escape_path(pp.linuxInclude); - f.sysroot = sysroot_flag; - sysroot_flag = " --sysroot=" + escape_path(plan.toolchain.sysroot); - f.sysroot = sysroot_flag; + compile_toolchain_flags += " -isystem" + escape_path(pp.linuxInclude); } // Binutils -B flag @@ -226,9 +228,9 @@ CompileFlags compute_flags(const BuildPlan& plan) { } f.cxx = std::format("-std=c++23{}{}{}{}{}{}{}{}{}{}", module_flag, std_module_flag, std_compat_module_flag, prebuilt_module_flag, - opt_flag, pic_flag, sysroot_flag, b_flag, include_flags, user_cxxflags); - f.cc = std::format("-std={}{}{}{}{}{}{}", c_std, opt_flag, pic_flag, sysroot_flag, b_flag, - include_flags, user_cflags); + opt_flag, pic_flag, compile_toolchain_flags, b_flag, include_flags, user_cxxflags); + f.cc = std::format("-std={}{}{}{}{}{}{}", c_std, opt_flag, pic_flag, compile_toolchain_flags, + b_flag, include_flags, user_cflags); // Link flags f.staticStdlib = plan.manifest.buildConfig.staticStdlib; @@ -259,7 +261,8 @@ CompileFlags compute_flags(const BuildPlan& plan) { } else if constexpr (mcpp::platform::needs_explicit_libcxx) { f.ld = std::format("{}{}{} -lc++", full_static, static_stdlib, b_flag); } else { - f.ld = std::format("{}{}{}{}{}{}", full_static, static_stdlib, sysroot_flag, b_flag, runtime_dirs, payload_ld); + f.ld = std::format("{}{}{}{}{}{}", full_static, static_stdlib, link_toolchain_flags, b_flag, + runtime_dirs, payload_ld); } return f; diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 26f1fbc..c479567 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -45,6 +45,8 @@ std::unique_ptr make_ninja_backend(); // Helper exposed for testing / debugging std::string emit_ninja_string(const BuildPlan& plan); +std::string filter_ninja_output(std::string_view output, + std::span commandPrefixes); } // namespace mcpp::build @@ -70,16 +72,6 @@ std::string escape_ninja_path(const std::filesystem::path& p) { return out; } -std::string escape_ninja_variable_value(std::string_view s) { - std::string out; - out.reserve(s.size()); - for (char c : s) { - if (c == '$') out += "$$"; - else out.push_back(c); - } - return out; -} - void write_file(const std::filesystem::path& p, std::string_view content) { std::filesystem::create_directories(p.parent_path()); std::ofstream os(p); @@ -114,8 +106,111 @@ bool is_c_source(const std::filesystem::path& src) { return src.extension() == ".c"; } +std::string ltrim_copy(std::string_view s) { + while (!s.empty() && std::isspace(static_cast(s.front()))) + s.remove_prefix(1); + return std::string(s); +} + +bool is_ninja_progress_line(std::string_view line) { + if (line.size() < 5 || line.front() != '[') return false; + std::size_t i = 1; + if (i >= line.size() || !std::isdigit(static_cast(line[i]))) + return false; + while (i < line.size() && std::isdigit(static_cast(line[i]))) ++i; + if (i >= line.size() || line[i] != '/') return false; + ++i; + if (i >= line.size() || !std::isdigit(static_cast(line[i]))) + return false; + while (i < line.size() && std::isdigit(static_cast(line[i]))) ++i; + return i < line.size() && line[i] == ']'; +} + +bool starts_with_any(std::string_view line, + std::span prefixes) { + for (auto& prefix : prefixes) { + if (!prefix.empty() && line.starts_with(prefix)) + return true; + } + return false; +} + +bool contains_any(std::string_view line, + std::span needles) { + for (auto& needle : needles) { + if (!needle.empty() && line.find(needle) != std::string_view::npos) + return true; + } + return false; +} + +std::vector command_prefixes(const CompileFlags& flags, + const BuildPlan& plan) { + std::vector prefixes; + auto add = [&](const std::filesystem::path& p) { + if (p.empty()) return; + auto s = p.string(); + if (std::find(prefixes.begin(), prefixes.end(), s) == prefixes.end()) + prefixes.push_back(std::move(s)); + }; + add(flags.cxxBinary); + add(flags.ccBinary); + add(flags.arBinary); + add(plan.scanDepsPath); + return prefixes; +} + +bool is_command_line(std::string_view trimmed, + std::span commandPrefixes) { + if (starts_with_any(trimmed, commandPrefixes)) return true; + + if (trimmed.starts_with("env ") + && (trimmed.find("LD_LIBRARY_PATH=") != std::string_view::npos + || trimmed.find("DYLD_LIBRARY_PATH=") != std::string_view::npos + || contains_any(trimmed, commandPrefixes))) { + return true; + } + + if ((trimmed.starts_with("cmd /c ") || trimmed.starts_with("if [ ")) + && contains_any(trimmed, commandPrefixes)) { + return true; + } + + return false; +} + +std::optional> +runtime_env_for_dirs(const std::vector& dirs) { + auto key = mcpp::platform::env::runtime_library_path_key(); + auto value = mcpp::platform::env::prepend_path_list(key, dirs); + if (key.empty() || value.empty()) return std::nullopt; + return std::pair{std::move(key), std::move(value)}; +} + } // namespace +std::string filter_ninja_output(std::string_view output, + std::span commandPrefixes) { + std::string filtered; + std::string line; + std::istringstream in{std::string(output)}; + while (std::getline(in, line)) { + if (!line.empty() && line.back() == '\r') + line.pop_back(); + auto trimmed = ltrim_copy(line); + if (trimmed.starts_with("ninja: Entering directory") + || trimmed.starts_with("ninja: build stopped") + || trimmed.starts_with("FAILED:") + || is_ninja_progress_line(trimmed) + || is_command_line(trimmed, commandPrefixes)) { + continue; + } + filtered += line; + filtered.push_back('\n'); + } + return filtered; +} + std::string emit_ninja_string(const BuildPlan& plan) { // dyndep requires P1689 scanning capability: // GCC: built-in -fdeps-format=p1689r5 @@ -143,7 +238,6 @@ std::string emit_ninja_string(const BuildPlan& plan) { append(std::format("cxx = {}\n", escape_ninja_path(flags.cxxBinary))); append(std::format("cxxflags = {}\n", flags.cxx)); - append(std::format("toolenv = {}\n", escape_ninja_variable_value(flags.toolEnv))); if (need_c_rule) { append(std::format("cc = {}\n", escape_ninja_path(flags.ccBinary))); append(std::format("cflags = {}\n", flags.cc)); @@ -191,10 +285,8 @@ std::string emit_ninja_string(const BuildPlan& plan) { // // $bmi_out is set per build edge to the BMI path (gcm.cache/.gcm). // If $bmi_out is empty (no module provided), we just compile normally. - // On POSIX, $toolenv prefixes commands with LD_LIBRARY_PATH etc. - // On Windows, $toolenv is empty and its leading space breaks CreateProcess, - // so we omit it entirely. - constexpr std::string_view te = mcpp::platform::is_windows ? "" : "$toolenv "; + // Runtime library paths for private toolchain executables are scoped onto + // the ninja subprocess instead of being emitted into each visible rule. std::string module_output_flag = traits.needsExplicitModuleOutput ? " -fmodule-output=$bmi_out" : ""; @@ -208,13 +300,13 @@ std::string emit_ninja_string(const BuildPlan& plan) { "if [ -n \"$bmi_out\" ] && [ -f \"$bmi_out\" ]; then " "cp -p \"$bmi_out\" \"$bmi_out.bak\"; " "fi && " - "{}$cxx $cxxflags{} -c $in -o $out && " + "$cxx $cxxflags{} -c $in -o $out && " "if [ -n \"$bmi_out\" ] && [ -f \"$bmi_out.bak\" ] && " "cmp -s \"$bmi_out\" \"$bmi_out.bak\"; then " "mv \"$bmi_out.bak\" \"$bmi_out\"; " "else " "rm -f \"$bmi_out.bak\"; " - "fi\n", te, module_output_flag)); + "fi\n", module_output_flag)); } append(" description = MOD $out\n"); if (dyndep) @@ -222,7 +314,7 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("\n"); append("rule cxx_object\n"); - append(std::format(" command = {}$cxx $cxxflags -c $in -o $out\n", te)); + append(" command = $cxx $cxxflags -c $in -o $out\n"); append(" description = OBJ $out\n"); if (dyndep) append(" restat = 1\n"); @@ -230,7 +322,7 @@ std::string emit_ninja_string(const BuildPlan& plan) { if (need_c_rule) { append("rule c_object\n"); - append(std::format(" command = {}$cc $cflags -c $in -o $out\n", te)); + append(" command = $cc $cflags -c $in -o $out\n"); append(" description = CC $out\n"); if (dyndep) append(" restat = 1\n"); @@ -238,15 +330,15 @@ std::string emit_ninja_string(const BuildPlan& plan) { } append("rule cxx_link\n"); - append(std::format(" command = {}$cxx $in -o $out $ldflags\n", te)); + append(" command = $cxx $in -o $out $ldflags\n"); append(" description = LINK $out\n\n"); append("rule cxx_archive\n"); - append(std::format(" command = {}$ar rcs $out $in\n", te)); + append(" command = $ar rcs $out $in\n"); append(" description = AR $out\n\n"); append("rule cxx_shared\n"); - append(std::format(" command = {}$cxx -shared $in -o $out $ldflags\n", te)); + append(" command = $cxx -shared $in -o $out $ldflags\n"); append(" description = SHARED $out\n\n"); if (dyndep) { @@ -256,10 +348,10 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("rule cxx_scan\n"); if (plan.scanDepsPath.empty()) { // GCC path: compiler-integrated P1689 scanning. - append(std::format(" command = {}$cxx $cxxflags -fmodules " + append(" command = $cxx $cxxflags -fmodules " "-fdeps-format=p1689r5 " "-fdeps-file=$out -fdeps-target=$compile_target " - "-M -MM -MF $out.dep -E $in -o $compile_target\n", te)); + "-M -MM -MF $out.dep -E $in -o $compile_target\n"); } else { // Clang path: clang-scan-deps produces P1689 JSON to stdout. if constexpr (mcpp::platform::is_windows) { @@ -268,8 +360,8 @@ std::string emit_ninja_string(const BuildPlan& plan) { append(" command = cmd /c \"$scan_deps -format=p1689 -- " "$cxx $cxxflags -c $in -o $compile_target > $out\"\n"); } else { - append(std::format(" command = {}$scan_deps -format=p1689 -- " - "$cxx $cxxflags -c $in -o $compile_target > $out\n", te)); + append(" command = $scan_deps -format=p1689 -- " + "$cxx $cxxflags -c $in -o $compile_target > $out\n"); } } append(" description = SCAN $out\n\n"); @@ -532,8 +624,17 @@ std::expected NinjaBackend::build(const BuildPlan& plan // Record ninja binary for P0 fast-path cache. BuildResult r; r.ninjaProgram = ninjaProgram; + if (auto runtimeEnv = runtime_env_for_dirs(plan.toolchain.compilerRuntimeDirs)) { + r.runtimeEnvKey = runtimeEnv->first; + r.runtimeEnvValue = runtimeEnv->second; + } else { + r.runtimeEnvKey = "-"; + } - std::string cmd = std::format("{} -C {}", ninjaProgram, mcpp::xlings::shq(plan.outputDir.string())); + std::string cmd = ninjaProgram; + if (!opts.verbose) + cmd += " --quiet"; + cmd += std::format(" -C {}", mcpp::xlings::shq(plan.outputDir.string())); if (opts.verbose) cmd += " -v"; if (opts.parallelJobs) @@ -541,22 +642,26 @@ std::expected NinjaBackend::build(const BuildPlan& plan cmd += " 2>&1"; std::string out; + std::optional scopedEnv; + if (r.runtimeEnvKey != "-" && !r.runtimeEnvValue.empty()) + scopedEnv.emplace(r.runtimeEnvKey, r.runtimeEnvValue); bool ok = run(cmd, out, /*capture=*/true); - if (opts.verbose || !ok) { - std::fputs(out.c_str(), stdout); - } r.exitCode = ok ? 0 : 1; r.elapsed = std::chrono::duration_cast( std::chrono::steady_clock::now() - t0); if (ok) { + if (opts.verbose && !out.empty()) + std::fputs(out.c_str(), stdout); for (auto& lu : plan.linkUnits) { r.producedArtifacts.push_back(plan.outputDir / lu.output); } } else { - return std::unexpected(BuildError{std::format("ninja failed (exit non-zero):\n{}", out), - plan.outputDir / "build.ninja"}); + auto prefixes = command_prefixes(flags, plan); + auto diagnostics = opts.verbose ? out : filter_ninja_output(out, prefixes); + return std::unexpected(BuildError{"build failed", plan.outputDir / "build.ninja", + std::move(diagnostics)}); } return r; } diff --git a/src/cli.cppm b/src/cli.cppm index dbce2fb..ccec449 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -2422,6 +2422,8 @@ struct BuildCacheEntry { std::string outputDir; std::string ninjaProgram; std::string fingerprint; // outputDir basename + std::string runtimeEnvKey; // "-" means intentionally empty; "" means old cache + std::string runtimeEnvValue; }; std::vector read_build_cache(const std::filesystem::path& projectRoot) { @@ -2444,7 +2446,9 @@ std::vector read_build_cache(const std::filesystem::path& proje return {e}; } - // P3 multi-entry format: sections of [target=] + 3 lines. + // P3 multi-entry format: sections of [target=] + 3 mandatory + // lines, plus optional runtime-env lines added after toolenv moved out of + // build.ninja. Old cache entries omit them and are treated as stale. std::vector entries; std::string line = firstLine; while (true) { @@ -2456,8 +2460,14 @@ std::vector read_build_cache(const std::filesystem::path& proje if (!std::getline(f, e.outputDir) || e.outputDir.empty()) break; if (!std::getline(f, e.ninjaProgram) || e.ninjaProgram.empty()) break; std::getline(f, e.fingerprint); + bool haveNextLine = static_cast(std::getline(f, line)); + if (haveNextLine && !line.starts_with("[target=")) { + e.runtimeEnvKey = line; + std::getline(f, e.runtimeEnvValue); + haveNextLine = static_cast(std::getline(f, line)); + } entries.push_back(std::move(e)); - if (!std::getline(f, line)) break; + if (!haveNextLine || line.empty()) break; } return entries; } @@ -2466,7 +2476,9 @@ void write_build_cache(const std::filesystem::path& projectRoot, const std::filesystem::path& outputDir, const std::string& ninjaProgram, const std::string& targetTriple, - const std::string& fingerprintHex = "") { + const std::string& fingerprintHex = "", + const std::string& runtimeEnvKey = "-", + const std::string& runtimeEnvValue = "") { auto path = projectRoot / kBuildCacheFile; auto entries = read_build_cache(projectRoot); @@ -2476,7 +2488,8 @@ void write_build_cache(const std::filesystem::path& projectRoot, }); // Insert at front (MRU). - BuildCacheEntry newEntry{targetTriple, outputDir.string(), ninjaProgram, fingerprintHex}; + BuildCacheEntry newEntry{targetTriple, outputDir.string(), ninjaProgram, fingerprintHex, + runtimeEnvKey, runtimeEnvValue}; entries.insert(entries.begin(), std::move(newEntry)); // Trim to LRU capacity. @@ -2493,9 +2506,44 @@ void write_build_cache(const std::filesystem::path& projectRoot, f << e.outputDir << '\n'; f << e.ninjaProgram << '\n'; f << e.fingerprint << '\n'; + f << (e.runtimeEnvKey.empty() ? "-" : e.runtimeEnvKey) << '\n'; + f << e.runtimeEnvValue << '\n'; } } +std::vector read_ninja_command_prefixes(const std::filesystem::path& ninjaPath) { + std::ifstream f(ninjaPath); + if (!f) return {}; + + std::vector prefixes; + std::string line; + while (std::getline(f, line)) { + auto eq = line.find('='); + if (eq == std::string::npos) continue; + auto key = line.substr(0, eq); + while (!key.empty() && std::isspace(static_cast(key.back()))) + key.pop_back(); + if (key != "cxx" && key != "cc" && key != "ar" && key != "scan_deps") + continue; + + std::string value = line.substr(eq + 1); + while (!value.empty() && std::isspace(static_cast(value.front()))) + value.erase(value.begin()); + while (!value.empty() && std::isspace(static_cast(value.back()))) + value.pop_back(); + if (!value.empty()) + prefixes.push_back(std::move(value)); + } + return prefixes; +} + +bool is_stale_ninja_failure(std::string_view output) { + return output.find("loading 'build.ninja'") != std::string_view::npos + || output.find("loading build.ninja") != std::string_view::npos + || output.find("unknown target") != std::string_view::npos + || output.find("manifest 'build.ninja' still dirty") != std::string_view::npos; +} + // Compile a prepared BuildContext. Shared between `mcpp build` and `mcpp run` // so the latter doesn't call prepare_build twice (and re-print the toolchain // resolution banner). @@ -2537,7 +2585,13 @@ int run_build_plan(BuildContext& ctx, bool verbose, bool no_cache, opts.verbose = verbose; auto r = be->build(ctx.plan, opts); if (!r) { + std::fflush(stdout); mcpp::ui::error(r.error().message); + if (!r.error().diagnosticOutput.empty()) { + std::fputs(r.error().diagnosticOutput.c_str(), stderr); + if (r.error().diagnosticOutput.back() != '\n') + std::fputc('\n', stderr); + } return 1; } @@ -2571,7 +2625,9 @@ int run_build_plan(BuildContext& ctx, bool verbose, bool no_cache, if (!no_cache && !r->ninjaProgram.empty()) { auto fpHex = ctx.outputDir.filename().string(); write_build_cache(ctx.projectRoot, ctx.outputDir, r->ninjaProgram, - std::string(targetOverride), fpHex); + std::string(targetOverride), fpHex, + r->runtimeEnvKey.empty() ? "-" : r->runtimeEnvKey, + r->runtimeEnvValue); } mcpp::ui::finished("release", r->elapsed); @@ -2605,6 +2661,10 @@ std::optional try_fast_build(const std::filesystem::path& projectRoot, auto outputDirStr = match->outputDir; auto ninjaProgram = match->ninjaProgram; auto cachedFingerprint = match->fingerprint; + auto runtimeEnvKey = match->runtimeEnvKey; + auto runtimeEnvValue = match->runtimeEnvValue; + if (runtimeEnvKey.empty()) + return std::nullopt; // old cache entry; regenerate build.ninja once // P1: verify fingerprint matches the outputDir basename. if (!cachedFingerprint.empty()) { @@ -2643,22 +2703,37 @@ std::optional try_fast_build(const std::filesystem::path& projectRoot, } // All inputs are older than build.ninja → fast-path: just run ninja. - std::string cmd = std::format("{} -C {}", ninjaProgram, mcpp::platform::shell::quote(outputDir.string())); + std::string cmd = ninjaProgram; + if (!verbose) cmd += " --quiet"; + cmd += std::format(" -C {}", mcpp::platform::shell::quote(outputDir.string())); if (verbose) cmd += " -v"; cmd += " 2>&1"; auto t0 = std::chrono::steady_clock::now(); std::string out; + std::optional scopedEnv; + if (runtimeEnvKey != "-" && !runtimeEnvValue.empty()) + scopedEnv.emplace(runtimeEnvKey, runtimeEnvValue); auto r = mcpp::platform::process::capture(cmd); out = r.output; - if (verbose && !out.empty()) std::fputs(out.c_str(), stdout); int status = r.exit_code; bool ok = (status == 0); if (!ok) { - if (!verbose) std::fputs(out.c_str(), stdout); - // Ninja failed — fall back to full rebuild (stale build.ninja?) - return std::nullopt; + if (is_stale_ninja_failure(out)) + return std::nullopt; + std::fflush(stdout); + mcpp::ui::error("build failed"); + auto prefixes = read_ninja_command_prefixes(ninjaPath); + auto diagnostics = verbose ? out : mcpp::build::filter_ninja_output(out, prefixes); + if (!diagnostics.empty()) { + std::fputs(diagnostics.c_str(), stderr); + if (diagnostics.back() != '\n') + std::fputc('\n', stderr); + } + return 1; } + if (verbose && !out.empty()) + std::fputs(out.c_str(), stdout); auto elapsed = std::chrono::duration_cast( std::chrono::steady_clock::now() - t0); diff --git a/src/platform/env.cppm b/src/platform/env.cppm index 525caa4..d6964d4 100644 --- a/src/platform/env.cppm +++ b/src/platform/env.cppm @@ -1,12 +1,14 @@ // mcpp.platform.env — platform-aware environment variable operations. // // Windows: uses _putenv_s to mutate the calling process environment. -// POSIX: builds "KEY=val" shell prefix strings (no process mutation). +// POSIX: supports scoped current-process env mutation for child processes. module; #include #if defined(_WIN32) #include // _putenv_s +#else +#include // setenv / unsetenv #endif export module mcpp.platform.env; @@ -19,10 +21,28 @@ export namespace mcpp::platform::env { std::optional get(std::string_view key); // Set an environment variable in the current process. -// On POSIX this is a no-op by design — use build_env_prefix() instead -// to scope vars to a child process via command-line prefixing. void set(const std::string& key, const std::string& value); +// Temporarily set or unset an env var, restoring the prior value on scope exit. +class ScopedEnv { +public: + ScopedEnv(std::string key, std::optional value); + ~ScopedEnv(); + + ScopedEnv(const ScopedEnv&) = delete; + ScopedEnv& operator=(const ScopedEnv&) = delete; + +private: + std::string key_; + std::optional previous_; + bool had_previous_ = false; +}; + +std::string path_list_separator(); +std::string runtime_library_path_key(); +std::string prepend_path_list(std::string_view key, + std::span dirs); + // Build a shell command prefix that injects the given env vars. // Windows: calls set() for each var and returns "". // POSIX: returns "KEY1='val1' KEY2='val2' " (caller prepends to command). @@ -46,12 +66,84 @@ void set(const std::string& key, const std::string& value) { #if defined(_WIN32) _putenv_s(key.c_str(), value.c_str()); #else - // POSIX: intentional no-op. Use build_env_prefix() instead. - (void)key; - (void)value; + setenv(key.c_str(), value.c_str(), 1); +#endif +} + +ScopedEnv::ScopedEnv(std::string key, std::optional value) + : key_(std::move(key)) { + if (auto* existing = std::getenv(key_.c_str())) { + previous_ = std::string(existing); + had_previous_ = true; + } + + if (value) { + set(key_, *value); + } else { +#if defined(_WIN32) + _putenv_s(key_.c_str(), ""); +#else + unsetenv(key_.c_str()); +#endif + } +} + +ScopedEnv::~ScopedEnv() { + if (had_previous_ && previous_) { + set(key_, *previous_); + } else { +#if defined(_WIN32) + _putenv_s(key_.c_str(), ""); +#else + unsetenv(key_.c_str()); +#endif + } +} + +std::string path_list_separator() { +#if defined(_WIN32) + return ";"; +#else + return ":"; #endif } +std::string runtime_library_path_key() { +#if defined(_WIN32) + return "PATH"; +#elif defined(__APPLE__) + // DYLD_LIBRARY_PATH affects every executable launched by ninja, including + // ninja itself, and can make macOS system frameworks load an incompatible + // private libc++/libc++abi. Keep macOS on toolchain-provided rpaths. + return ""; +#elif defined(__linux__) + return "LD_LIBRARY_PATH"; +#else + return ""; +#endif +} + +std::string prepend_path_list(std::string_view key, + std::span dirs) { + if (key.empty() || dirs.empty()) return ""; + + auto sep = path_list_separator(); + std::string value; + for (auto& dir : dirs) { + if (dir.empty()) continue; + if (!value.empty()) value += sep; + value += dir.string(); + } + if (value.empty()) return ""; + + std::string k(key); + if (auto* existing = std::getenv(k.c_str()); existing && *existing) { + value += sep; + value += existing; + } + return value; +} + std::string build_env_prefix( const std::vector>& vars) { diff --git a/tests/e2e/48_build_error_output.sh b/tests/e2e/48_build_error_output.sh new file mode 100644 index 0000000..c821988 --- /dev/null +++ b/tests/e2e/48_build_error_output.sh @@ -0,0 +1,119 @@ +#!/usr/bin/env bash +# requires: +# Default build failures should show the compiler diagnostic once, without +# ninja progress or private toolchain environment prefixes. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT + +export MCPP_HOME="$TMP/mcpp-home" +source "$(dirname "$0")/_inherit_toolchain.sh" + +mkdir -p "$TMP/proj/src" +cd "$TMP/proj" + +cat > mcpp.toml <<'EOF' +[package] +name = "bad_error_output" +version = "0.1.0" + +[language] +import_std = false +EOF + +cat > src/main.cpp <<'EOF' +#error MCPPE2E_BUILD_ERROR_MARKER +int main() { + return 0; +} +EOF + +rc=0 +out=$("$MCPP" build 2>&1) || rc=$? +[[ $rc -ne 0 ]] || { + echo "expected build to fail" + echo "$out" + exit 1 +} + +echo "$out" | grep -q 'MCPPE2E_BUILD_ERROR_MARKER' || { + echo "missing compiler diagnostic marker" + echo "$out" + exit 1 +} + +compiling_line=$(printf '%s\n' "$out" | awk '/Compiling bad_error_output/ { print NR; exit }') +error_line=$(printf '%s\n' "$out" | awk '/^error: build failed/ { print NR; exit }') +[[ -n "$compiling_line" && -n "$error_line" && "$compiling_line" -lt "$error_line" ]] || { + echo "expected build status before diagnostics" + echo "$out" + exit 1 +} + +diag_count=$(echo "$out" | grep -c 'error:.*MCPPE2E_BUILD_ERROR_MARKER' || true) +[[ "$diag_count" -eq 1 ]] || { + echo "expected compiler diagnostic marker once, got $diag_count" + echo "$out" + exit 1 +} + +duplicate_lines=$(printf '%s\n' "$out" | awk 'NF && seen[$0]++ == 1 { print }') +[[ -z "$duplicate_lines" ]] || { + echo "unexpected duplicate diagnostic lines" + echo "$duplicate_lines" + echo "$out" + exit 1 +} + +for noise in \ + 'LD_LIBRARY_PATH=' \ + 'FAILED:' \ + 'ninja: Entering directory' \ + 'ninja: build stopped' \ + 'argument unused during compilation' \ + '^\[[0-9][0-9]*/[0-9][0-9]*\] ' +do + if echo "$out" | grep -Eq "$noise"; then + echo "unexpected build-output noise matched: $noise" + echo "$out" + exit 1 + fi +done + +build_ninja=$(find target -name build.ninja | head -1) +[[ -n "$build_ninja" ]] || { + echo "missing generated build.ninja" + echo "$out" + exit 1 +} +if grep -q 'LD_LIBRARY_PATH\|toolenv' "$build_ninja"; then + echo "build.ninja should not contain private runtime env prefixes" + sed -n '1,80p' "$build_ninja" + exit 1 +fi +if grep '^cxxflags\|^cflags' "$build_ninja" | grep -Eq -- '-stdlib=libc\+\+|-fuse-ld=lld|--rtlib=compiler-rt|--unwindlib=libunwind'; then + echo "compile flags should not contain clang link/runtime-only flags" + grep '^cxxflags\|^cflags' "$build_ninja" + exit 1 +fi + +rc=0 +verbose_out=$("$MCPP" build --verbose 2>&1) || rc=$? +[[ $rc -ne 0 ]] || { + echo "expected verbose build to fail" + echo "$verbose_out" + exit 1 +} +echo "$verbose_out" | grep -q 'FAILED:' || { + echo "verbose output should retain ninja failure details" + echo "$verbose_out" + exit 1 +} +echo "$verbose_out" | grep -q 'MCPPE2E_BUILD_ERROR_MARKER' || { + echo "verbose output missing compiler diagnostic marker" + echo "$verbose_out" + exit 1 +} + +echo "OK"