@@ -97,10 +97,33 @@ std::string local_include_flags(const CompileUnit& cu) {
9797}
9898
9999std::string join_flags (const std::vector<std::string>& flags) {
100+ // mcpp#234: a manifest `defines = ["T=long long"]` arrives as the single
101+ // element `-DT=long long` (pushed whole by apply_glob_flags) — a space
102+ // that is genuinely PART of one argv token. Joining with a bare space let
103+ // the shell split it into two words (`-DT=long` + `long`) once ninja
104+ // handed the command line to the shell, so a define value with a space
105+ // must be shell-quoted.
106+ //
107+ // But quote ONLY that case. Every OTHER flag element is passed through
108+ // verbatim, because two other kinds of element legitimately contain a
109+ // space that MUST stay a token boundary, and quoting them breaks the build:
110+ // • raw descriptor flags pack multiple argv tokens into one string
111+ // (compat.lua's `-include mcpp_lua_platform_config.h` — the space
112+ // separates `-include` from its argument; quoting made gcc see one
113+ // malformed arg → "No such file"; broke aarch64/macos/windows builds);
114+ // • mcpp-generated link flags are already shell-quoted + ninja-escaped
115+ // (`-Wl,-rpath,'$$ORIGIN'` — single quotes stop shell $-expansion,
116+ // `$$` is ninja's literal `$`); re-quoting baked a literal `'$ORIGIN'`
117+ // into RUNPATH so dependency .so's next to the exe couldn't resolve.
118+ // The `defines` channel is the ONLY producer of a `-D`/`/D`-prefixed token
119+ // whose space is intra-value, so gate the quoting on exactly that shape.
100120 std::string out;
101121 for (auto const & flag : flags) {
102122 out += ' ' ;
103- out += flag;
123+ const bool defineWithSpace =
124+ (flag.starts_with (" -D" ) || flag.starts_with (" /D" ))
125+ && flag.find (' ' ) != std::string::npos;
126+ out += defineWithSpace ? shell_quote_arg (flag) : flag;
104127 }
105128 return out;
106129}
@@ -396,6 +419,69 @@ std::string emit_ninja_string(const BuildPlan& plan) {
396419 auto append_deps = [&] {
397420 if (msvcDeps) append (" deps = msvc\n " );
398421 };
422+ // mcpp#235: cxx_module/cxx_object had NO depfile at all on non-MSVC —
423+ // only the msvcDeps branch tracked header deps (via /showIncludes). So
424+ // editing a file #include'd inside a module's purview (or a plain
425+ // header pulled in by a .cpp) never invalidated the compile edge: the
426+ // P1689 scan already emits a `.dep`/`.d`-shaped list of textual
427+ // includes, but it was generated and discarded.
428+ //
429+ // Naively mirroring the nasm rule below (`-MD $out.d` + `deps = gcc` +
430+ // `depfile = $out.d`) does NOT work here: GCC's `-fmodules` bolts extra
431+ // "reversed" rules onto ANY -M*/-MF depfile for a TU that imports or
432+ // provides a module — e.g. for a module interface unit:
433+ // obj/m.m.o gcm.cache/m.gcm: src/m.cppm src/vals.inc
434+ // m.c++-module: gcm.cache/m.gcm
435+ // .PHONY: m.c++-module
436+ // gcm.cache/m.gcm:| obj/m.m.o
437+ // and for an importing TU:
438+ // obj/main.o: src/main.cpp gcm.cache/std.gcm gcm.cache/m.gcm
439+ // obj/main.o: m.c++-module std.c++-module
440+ // CXX_IMPORTS += m.c++-module std.c++-module
441+ // Those extra records describe a Make-style dependency graph WITHIN the
442+ // depfile itself (e.g. gcm.cache/m.gcm "having its own inputs"), which
443+ // collides with gcm.cache/m.gcm already being a declared ninja-graph
444+ // OUTPUT of this same edge (`| gcm.cache/m.gcm`) — ninja's depfile
445+ // loader rejects that outright ("inputs may not also have inputs"),
446+ // confirmed empirically (e2e 118 failed the fresh build this way before
447+ // the filter below was added). GCC has no flag to suppress this.
448+ //
449+ // Fix: compile to a scratch `$out.d.raw`, then keep only the FIRST
450+ // record (target + its indented continuation lines — the plain textual
451+ // #include graph, which is all #235 needs) as `$out.d`; module BMI
452+ // deps stay tracked independently via the existing per-edge `dyndep`
453+ // binding, so dropping the reversed/module lines here loses nothing.
454+ // POSIX-only (`awk`): native Windows has no POSIX shell/toolset here
455+ // (see the existing "Windows: skip BMI restat optimization" branch
456+ // below), so a non-MSVC Windows toolchain keeps the pre-#235
457+ // behavior (no depfile) rather than depend on an unavailable filter —
458+ // msvcDeps (cl.exe) is unaffected either way (deps=msvc, no -MMD).
459+ //
460+ // GCC-only: the awk filter strips the "reversed" make-rules that GCC's
461+ // `-fmodules -MMD` bolts onto a module-TU depfile (`gcm.cache/<m>.gcm: |
462+ // <obj>` etc., which ninja rejects as "inputs may not also have inputs").
463+ // Clang's module system (.pcm) does not emit those and its module-TU
464+ // depfile shape is different; applying the GCC-shaped filter there is
465+ // untested and could yield a wrong dep set. Until Clang is verified,
466+ // scope this to GCC — Clang keeps the pre-#235 behavior (no compile-edge
467+ // depfile; header/purview rebuild tracking on Clang is a follow-up, same
468+ // as it was on 0.0.96, so this is not a regression). e2e 118 declares
469+ // `# requires: gcc` and skips where GCC is not the toolchain.
470+ const bool posixDepfile = !msvcDeps && !mcpp::platform::is_windows
471+ && plan.toolchain .compiler == mcpp::toolchain::CompilerId::GCC ;
472+ const std::string mmd_flag = posixDepfile ? " -MMD -MF $out.d.raw " : " " ;
473+ const std::string mmd_filter = posixDepfile
474+ ? " && awk 'NR==1{print;next} /^[^ ]/{exit} {print}' "
475+ " \" $out.d.raw\" > \" $out.d\" && rm -f \" $out.d.raw\" "
476+ : " " ;
477+ auto append_cxx_deps = [&] {
478+ if (posixDepfile) {
479+ append (" deps = gcc\n " );
480+ append (" depfile = $out.d\n " );
481+ } else {
482+ append_deps ();
483+ }
484+ };
399485 // cl.exe needs /TP (our module interfaces are .cppm, unknown to cl) and
400486 // /interface to treat the TU as a module interface unit.
401487 const std::string module_src_flags = msvcDeps ? " /interface /TP" : " " ;
@@ -405,31 +491,38 @@ std::string emit_ninja_string(const BuildPlan& plan) {
405491 append (std::format (" command = "
406492 " $cxx $local_includes $cxxflags $unit_cxxflags{}{} {}\n " ,
407493 module_output_flag, module_src_flags, compile_tail));
408- append_deps ();
494+ append_cxx_deps ();
409495 } else {
410496 append (std::format (" command = "
411497 " if [ -n \" $bmi_out\" ] && [ -f \" $bmi_out\" ]; then "
412498 " cp -p \" $bmi_out\" \" $bmi_out.bak\" ; "
413499 " fi && "
414- " $cxx $local_includes $cxxflags $unit_cxxflags{} {} && "
500+ " $cxx $local_includes $cxxflags $unit_cxxflags{} {}{}{} && "
415501 " if [ -n \" $bmi_out\" ] && [ -f \" $bmi_out.bak\" ] && "
416502 " cmp -s \" $bmi_out\" \" $bmi_out.bak\" ; then "
417503 " mv \" $bmi_out.bak\" \" $bmi_out\" ; "
418504 " else "
419505 " rm -f \" $bmi_out.bak\" ; "
420- " fi\n " , module_output_flag, compile_tail));
506+ " fi\n " , module_output_flag, mmd_flag, compile_tail, mmd_filter));
507+ append_cxx_deps ();
421508 }
422509 append (" description = MOD $out\n " );
423510 if (dyndep)
424511 append (" restat = 1\n " );
425512 append (" \n " );
426513
427514 append (" rule cxx_object\n " );
428- append (std::format (
429- " command = $cxx $local_includes $cxxflags $unit_cxxflags {}\n " ,
430- compile_tail));
515+ if constexpr (mcpp::platform::is_windows) {
516+ append (std::format (
517+ " command = $cxx $local_includes $cxxflags $unit_cxxflags {}\n " ,
518+ compile_tail));
519+ } else {
520+ append (std::format (
521+ " command = $cxx $local_includes $cxxflags $unit_cxxflags {}{}{}\n " ,
522+ mmd_flag, compile_tail, mmd_filter));
523+ }
431524 append (" description = OBJ $out\n " );
432- append_deps ();
525+ append_cxx_deps ();
433526 if (dyndep)
434527 append (" restat = 1\n " );
435528 append (" \n " );
@@ -606,9 +699,10 @@ std::string emit_ninja_string(const BuildPlan& plan) {
606699 // ── Phase 1: scan edges (one .ddi per TU). ──────────────────────
607700 // .ddi is placed beside the object so multi-version mangling can
608701 // namespace by package without producing two `build` rules with
609- // the same `.ddi` output (plan.cppm switches `cu.object` from
610- // `obj/<file>.o` to `obj/<pkg>/<file>.o` whenever a basename
611- // collides across packages — `.ddi` follows that placement).
702+ // the same `.ddi` output (mcpp#233: plan.cppm switches `cu.object`
703+ // from the flat `obj/<file>.o` to a path mirroring the source's
704+ // relative directory under a sanitized-package prefix whenever a
705+ // basename collides — `.ddi` follows that placement).
612706 // Skip .c files: they have no `import`s and don't need P1689 scan;
613707 // running them through cxx_scan would route them through g++ /
614708 // -fmodules which is exactly what C support is here to avoid.
0 commit comments