Skip to content

Commit cd753e3

Browse files
committed
fix(build): report the staged file's absolute path and drop ninja's trailing pad
Two cosmetics found by driving a real staging failure end-to-end through ninja: the `FAILED: <target>` line ninja emits carries a trailing space, and `$out` is relative because ninja runs staging with cwd = the build directory — the reader of that diagnostic is the person who has to go unlock the file.
1 parent 9f396d6 commit cd753e3

3 files changed

Lines changed: 13 additions & 3 deletions

File tree

src/build/ninja_backend.cppm

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,9 @@ std::string filter_ninja_output(std::string_view output,
349349
if (auto close = target.find(']'); close != std::string::npos)
350350
target = ltrim_copy(target.substr(close + 1));
351351
}
352+
while (!target.empty()
353+
&& std::isspace(static_cast<unsigned char>(target.back())))
354+
target.pop_back();
352355
if (target.empty()) continue;
353356
filtered += "failed: ";
354357
filtered += target;

src/build/stage.cppm

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ std::error_code write_once(const std::filesystem::path& src,
124124
std::string failure_message(const std::filesystem::path& src,
125125
const std::filesystem::path& dst,
126126
const std::error_code& ec) {
127+
// ninja runs staging with cwd = the build directory, so `$out` is relative.
128+
// Print it absolute: the reader has to go find (or unlock) this file.
129+
std::error_code aec;
130+
auto shown = std::filesystem::absolute(dst, aec);
131+
if (aec) shown = dst;
127132
return std::format(
128133
"cannot stage file into the build directory\n"
129134
" file: {}\n"
@@ -135,7 +140,7 @@ std::string failure_message(const std::filesystem::path& src,
135140
" editor/IDE indexer, antivirus, or — for a .dll — a still-running\n"
136141
" program from a previous `mcpp run`. Close it or restart clangd,\n"
137142
" then re-run the build.",
138-
dst.string(), src.string(), ec.value(), ec.message());
143+
shown.string(), src.string(), ec.value(), ec.message());
139144
}
140145

141146
} // namespace

tests/unit/test_ninja_backend.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -876,7 +876,7 @@ TEST(NinjaBackend, FilterKeepsStagingDiagnosticsAndFailedTarget) {
876876
std::string raw =
877877
"ninja: Entering directory `target/x86_64-linux-gnu/fp'\n"
878878
"[1/3] STAGE pcm.cache/std.pcm\n"
879-
"FAILED: [code=1] pcm.cache/std.pcm\n"
879+
"FAILED: [code=1] pcm.cache/std.pcm \n"
880880
"/opt/mcpp/bin/mcpp stage --output pcm.cache/std.pcm /cache/std.pcm\n"
881881
"error: cannot stage file into the build directory\n"
882882
"hint: another process has this file memory-mapped, loaded or open.\n"
@@ -885,7 +885,9 @@ TEST(NinjaBackend, FilterKeepsStagingDiagnosticsAndFailedTarget) {
885885
auto filtered = filter_ninja_output(raw, prefixes);
886886

887887
// Which output failed is now preserved (it used to be dropped entirely).
888-
EXPECT_NE(filtered.find("failed: pcm.cache/std.pcm"), std::string::npos) << filtered;
888+
// ninja pads the target list with a trailing space; the normalized line
889+
// must not inherit it.
890+
EXPECT_NE(filtered.find("failed: pcm.cache/std.pcm\n"), std::string::npos) << filtered;
889891
EXPECT_EQ(filtered.find("[code=1]"), std::string::npos) << filtered;
890892
// The diagnostic survives; the echoed command line does not.
891893
EXPECT_NE(filtered.find("error: cannot stage file"), std::string::npos) << filtered;

0 commit comments

Comments
 (0)