fix(flags): shell-quote only define-values, pass raw/link flags verbatim

Generalizes d3ad223: C3's shell_quote_arg (#234) was applied to ALL per-unit
flags, breaking two kinds of element that legitimately contain a token-boundary
space — raw descriptor flags (compat.lua `-include mcpp_lua_platform_config.h`
→ gcc saw one malformed arg → aarch64/macos/windows cross-build FAILED) and
mcpp-generated pre-escaped link flags (rpath `$ORIGIN`). join_flags now quotes
ONLY `-D`/`/D` define tokens whose value holds a space (the sole producer of
an intra-value space); everything else is verbatim = 0.0.96 tokenization.
+regression test RawMultiTokenFlagIsNotQuoted; LinkFlagsAreNotReQuoted +
QuotesFlagValueWithSpace still green.

Author: Sunrisepeak

src/build/ninja_backend.cppm

@@ -96,27 +96,34 @@ std::string local_include_flags(const CompileUnit& cu) {
     return flags;
 }
 
-std::string join_flags(const std::vector<std::string>& flags,
-                        bool shellQuote = true) {
-    // mcpp#234: each vector element is already one argv token (e.g. a
-    // manifest define `T=long long` arrives here as the single element
-    // `-DT=long long`, pushed whole by apply_glob_flags) — but joining with
-    // a bare space and no quoting let the embedded space split it into two
-    // words once ninja handed the resolved command line to the shell.
-    // shell_quote_arg is a no-op for tokens with nothing shell-significant
-    // (`-std=c++23`, `-O2`, ...), so plain flags are untouched.
+std::string join_flags(const std::vector<std::string>& flags) {
+    // mcpp#234: a manifest `defines = ["T=long long"]` arrives as the single
+    // element `-DT=long long` (pushed whole by apply_glob_flags) — a space
+    // that is genuinely PART of one argv token. Joining with a bare space let
+    // the shell split it into two words (`-DT=long` + `long`) once ninja
+    // handed the command line to the shell, so a define value with a space
+    // must be shell-quoted.
     //
-    // shellQuote=false for mcpp-GENERATED link flags (lu.linkFlags): those
-    // are already correctly shell-quoted + ninja-escaped at construction
-    // (e.g. the rpath token `-Wl,-rpath,'$$ORIGIN'` from plan.cppm — single
-    // quotes protect it from shell $-expansion, `$$` is ninja's literal `$`).
-    // Re-running shell_quote_arg over such a token DOUBLE-quotes it, baking a
-    // literal `'$ORIGIN'` (quotes included) into the binary's RUNPATH so the
-    // dynamic linker can't resolve dependency .so's placed next to the exe.
+    // But quote ONLY that case. Every OTHER flag element is passed through
+    // verbatim, because two other kinds of element legitimately contain a
+    // space that MUST stay a token boundary, and quoting them breaks the build:
+    //   • raw descriptor flags pack multiple argv tokens into one string
+    //     (compat.lua's `-include mcpp_lua_platform_config.h` — the space
+    //     separates `-include` from its argument; quoting made gcc see one
+    //     malformed arg → "No such file"; broke aarch64/macos/windows builds);
+    //   • mcpp-generated link flags are already shell-quoted + ninja-escaped
+    //     (`-Wl,-rpath,'$$ORIGIN'` — single quotes stop shell $-expansion,
+    //     `$$` is ninja's literal `$`); re-quoting baked a literal `'$ORIGIN'`
+    //     into RUNPATH so dependency .so's next to the exe couldn't resolve.
+    // The `defines` channel is the ONLY producer of a `-D`/`/D`-prefixed token
+    // whose space is intra-value, so gate the quoting on exactly that shape.
     std::string out;
     for (auto const& flag : flags) {
         out += ' ';
-        out += shellQuote ? shell_quote_arg(flag) : flag;
+        const bool defineWithSpace =
+            (flag.starts_with("-D") || flag.starts_with("/D"))
+            && flag.find(' ') != std::string::npos;
+        out += defineWithSpace ? shell_quote_arg(flag) : flag;
     }
     return out;
 }
@@ -898,7 +905,7 @@ std::string emit_ninja_string(const BuildPlan& plan) {
             // binaries run on the build host and use the system -lc++,
             // distributable targets get the static LLVM libc++. See
             // CompileFlags::ldStdlibDefault/ldStdlibTest.
-            std::string unit = join_flags(lu.linkFlags, /*shellQuote=*/false);
+            std::string unit = join_flags(lu.linkFlags);
             unit += (lu.kind == mcpp::build::LinkUnit::TestBinary)
                 ? flags.ldStdlibTest : flags.ldStdlibDefault;
             if (!unit.empty())

tests/unit/test_ninja_backend.cpp

@@ -356,6 +356,32 @@ TEST(NinjaBackend, LinkFlagsAreNotReQuoted) {
     EXPECT_EQ(ninja.find("'\\''"), std::string::npos) << ninja;
 }
 
+// Regression (#234 follow-up): a raw descriptor flag that packs two argv
+// tokens into one string — e.g. compat.lua's `-include <header>` — must pass
+// through VERBATIM so the shell splits it back into `-include` + the header.
+// Blanket shell-quoting wrapped it into one malformed arg, so gcc looked for a
+// file literally named "<space>header" → "No such file" → aarch64/macos/windows
+// cross-builds failed. Only `-D`/`/D` define tokens with an intra-value space
+// get quoted; `-include foo.h` does not.
+TEST(NinjaBackend, RawMultiTokenFlagIsNotQuoted) {
+    auto plan = minimal_plan();
+    plan.compileUnits.push_back({
+        .source = "src/main.c",
+        .object = "obj/main.o",
+        .packageName = "include_flag_test",
+        .packageCflags = {"-include mcpp_lua_platform_config.h"},
+    });
+
+    auto ninja = emit_ninja_string(plan);
+
+    // Verbatim, so the shell re-splits into two args.
+    EXPECT_NE(ninja.find("unit_cflags = -include mcpp_lua_platform_config.h"),
+              std::string::npos) << ninja;
+    // Must NOT be wrapped in quotes (which would make it one malformed arg).
+    EXPECT_EQ(ninja.find("'-include mcpp_lua_platform_config.h'"),
+              std::string::npos) << ninja;
+}
+
 TEST(NinjaBackend, RootPackageCxxflagsAreEmittedOncePerUnit) {
     auto plan = minimal_plan();
     plan.manifest.buildConfig.cxxflags = {"-DROOT_FLAG=1"};