Skip to content

Commit d3ad223

Browse files
committed
fix(flags): do not re-quote mcpp-generated link flags — restore rpath $ORIGIN
C3's shell_quote_arg (#234, for compile-time defines-with-spaces) was applied to per-unit LINK flags too, double-quoting the already-escaped rpath token `-Wl,-rpath,'$$ORIGIN'` → literal `'$ORIGIN'` in RUNPATH → dependency .so's next to the exe couldn't be resolved (e2e 55/56/57/64 regressed vs 0.0.96). join_flags gains shellQuote (default true for user compile flags); the link-flag join passes false. Regression test locks it.
1 parent f0c19fb commit d3ad223

2 files changed

Lines changed: 44 additions & 3 deletions

File tree

src/build/ninja_backend.cppm

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,18 +96,27 @@ std::string local_include_flags(const CompileUnit& cu) {
9696
return flags;
9797
}
9898

99-
std::string join_flags(const std::vector<std::string>& flags) {
99+
std::string join_flags(const std::vector<std::string>& flags,
100+
bool shellQuote = true) {
100101
// mcpp#234: each vector element is already one argv token (e.g. a
101102
// manifest define `T=long long` arrives here as the single element
102103
// `-DT=long long`, pushed whole by apply_glob_flags) — but joining with
103104
// a bare space and no quoting let the embedded space split it into two
104105
// words once ninja handed the resolved command line to the shell.
105106
// shell_quote_arg is a no-op for tokens with nothing shell-significant
106107
// (`-std=c++23`, `-O2`, ...), so plain flags are untouched.
108+
//
109+
// shellQuote=false for mcpp-GENERATED link flags (lu.linkFlags): those
110+
// are already correctly shell-quoted + ninja-escaped at construction
111+
// (e.g. the rpath token `-Wl,-rpath,'$$ORIGIN'` from plan.cppm — single
112+
// quotes protect it from shell $-expansion, `$$` is ninja's literal `$`).
113+
// Re-running shell_quote_arg over such a token DOUBLE-quotes it, baking a
114+
// literal `'$ORIGIN'` (quotes included) into the binary's RUNPATH so the
115+
// dynamic linker can't resolve dependency .so's placed next to the exe.
107116
std::string out;
108117
for (auto const& flag : flags) {
109118
out += ' ';
110-
out += shell_quote_arg(flag);
119+
out += shellQuote ? shell_quote_arg(flag) : flag;
111120
}
112121
return out;
113122
}
@@ -889,7 +898,7 @@ std::string emit_ninja_string(const BuildPlan& plan) {
889898
// binaries run on the build host and use the system -lc++,
890899
// distributable targets get the static LLVM libc++. See
891900
// CompileFlags::ldStdlibDefault/ldStdlibTest.
892-
std::string unit = join_flags(lu.linkFlags);
901+
std::string unit = join_flags(lu.linkFlags, /*shellQuote=*/false);
893902
unit += (lu.kind == mcpp::build::LinkUnit::TestBinary)
894903
? flags.ldStdlibTest : flags.ldStdlibDefault;
895904
if (!unit.empty())

tests/unit/test_ninja_backend.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,38 @@ TEST(NinjaBackend, PlainFlagsPassThroughUnquoted) {
324324
<< ninja;
325325
}
326326

327+
// Regression: mcpp-GENERATED per-unit LINK flags are already correctly
328+
// shell-quoted + ninja-escaped at construction — e.g. the shared-dep rpath
329+
// token `-Wl,-rpath,'$$ORIGIN'` (single quotes stop shell $-expansion, `$$`
330+
// is ninja's literal `$`). join_flags for link flags must NOT re-run
331+
// shell_quote_arg over them: doing so double-quotes the token, baking a
332+
// literal `'$ORIGIN'` (quotes included) into the binary's RUNPATH so the
333+
// dynamic linker can't find dependency .so's next to the exe (e2e 55-57/64).
334+
TEST(NinjaBackend, LinkFlagsAreNotReQuoted) {
335+
auto plan = minimal_plan();
336+
plan.compileUnits.push_back({
337+
.source = "src/main.cpp",
338+
.object = "obj/main.o",
339+
.packageName = "rpath_test",
340+
});
341+
plan.linkUnits.push_back({
342+
.targetName = "app",
343+
.kind = mcpp::build::LinkUnit::Binary,
344+
.objects = {"obj/main.o"},
345+
.linkFlags = {"-Wl,-rpath,'$$ORIGIN'"},
346+
.output = "bin/app",
347+
.entryMain = "src/main.cpp",
348+
});
349+
350+
auto ninja = emit_ninja_string(plan);
351+
352+
// The rpath token passes through verbatim (ninja `$$` = literal `$`).
353+
EXPECT_NE(ninja.find("-Wl,-rpath,'$$ORIGIN'"), std::string::npos) << ninja;
354+
// Must NOT be double-quoted: shell_quote_arg escaping an embedded `'`
355+
// produces the `'\''` sequence, which only appears if it re-quoted.
356+
EXPECT_EQ(ninja.find("'\\''"), std::string::npos) << ninja;
357+
}
358+
327359
TEST(NinjaBackend, RootPackageCxxflagsAreEmittedOncePerUnit) {
328360
auto plan = minimal_plan();
329361
plan.manifest.buildConfig.cxxflags = {"-DROOT_FLAG=1"};

0 commit comments

Comments
 (0)