Skip to content

Commit 3eb6ea2

Browse files
speak-agentclaude
andcommitted
the absent-facility advice reaches both channels a failed build reports through
A build reports failure on two paths: the one with a plan, and the fast path, which has none by construction. `ninja_backend.cppm` already records why that matters for `link_failure_advice` --- advice attached to only one of them "would appear or not depending on whether build.ninja happened to be up to date, which is exactly the kind of 'same decision in two places' this codebase keeps paying for" --- and the new `[c-abi.absent]` advice had been attached to one. The list travels between them in a file beside build.ninja, written when the plan emits it and opened only after a build has already failed. The fast path's purpose is to read as little as possible, so it reads nothing extra until there is a failure to explain. A graph that declares no absence removes the file rather than leaving it: a stale one would let the fast path explain a failure with a list the current graph never declared. Co-authored-by: Claude Code <noreply@anthropic.com>
1 parent 790b303 commit 3eb6ea2

3 files changed

Lines changed: 129 additions & 0 deletions

File tree

src/build/execute.cppm

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,6 +1258,19 @@ std::optional<int> run_ninja_fast(const std::string& ninjaProgram,
12581258
if (auto advice = mcpp::build::graph_c_library_isolation_advice(out);
12591259
!advice.empty())
12601260
std::fputs(advice.c_str(), stderr);
1261+
// THE SAME ADVICE THE PLAN PATH GIVES, FROM THE LIST THE PLAN WROTE
1262+
// DOWN. This path has no `BuildPlan` by construction, so the C
1263+
// library's `[c-abi.absent]` table reaches it through a file beside
1264+
// build.ninja rather than through a resolution it exists to skip.
1265+
// Advice attached to one path only appears or not depending on
1266+
// whether build.ninja happened to be up to date.
1267+
{
1268+
auto [cAbiName, absent] = mcpp::build::read_c_abi_absent_sidecar(
1269+
ninjaPath.parent_path());
1270+
if (auto advice = mcpp::build::c_abi_absent_facility_advice(
1271+
out, cAbiName, absent); !advice.empty())
1272+
std::fputs(advice.c_str(), stderr);
1273+
}
12611274
return 1;
12621275
}
12631276
if (verbose && !out.empty())

src/build/ninja_backend.cppm

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,26 @@ std::string c_abi_absent_facility_advice(
111111
std::string_view output, std::string_view cAbiName,
112112
const std::vector<mcpp::targetside::CAbiAbsentEntry>& absent);
113113

114+
// THE SAME ADVICE ON BOTH PATHS, WHICH IS WHY THE LIST IS WRITTEN DOWN.
115+
//
116+
// A build reports failure through two channels: this one, which has a plan,
117+
// and the fast path (`mcpp.build.execute`), which deliberately has none. An
118+
// advice attached to only one of them appears or not depending on whether
119+
// build.ninja happened to be up to date, which is the same decision in two
120+
// places wearing a different hat. The plan writes the list beside build.ninja;
121+
// the fast path reads it back and calls the same function.
122+
//
123+
// It is a plain file rather than a field in an existing record because the
124+
// fast path's whole purpose is to read as little as possible: this one is
125+
// opened only after a build has already failed.
126+
void write_c_abi_absent_sidecar(
127+
const std::filesystem::path& outputDir, std::string_view cAbiName,
128+
const std::vector<mcpp::targetside::CAbiAbsentEntry>& absent);
129+
130+
// `{name, absent}` read back, or an empty pair when no build wrote one.
131+
std::pair<std::string, std::vector<mcpp::targetside::CAbiAbsentEntry>>
132+
read_c_abi_absent_sidecar(const std::filesystem::path& outputDir);
133+
114134
// mcpp#662: the compile-side sibling of `link_failure_advice`, same shape —
115135
// text-matched against RAW ninja output (command lines included; the caller
116136
// must not pass the filtered form), returning advice to APPEND, never
@@ -686,6 +706,48 @@ std::string link_failure_advice(std::string_view output) {
686706
" `std::align_val_t`, which are the ones most often forgotten.\n";
687707
}
688708

709+
namespace {
710+
constexpr std::string_view kCAbiAbsentSidecar = ".mcpp-c-abi-absent";
711+
}
712+
713+
void write_c_abi_absent_sidecar(
714+
const std::filesystem::path& outputDir, std::string_view cAbiName,
715+
const std::vector<mcpp::targetside::CAbiAbsentEntry>& absent) {
716+
const auto path = outputDir / kCAbiAbsentSidecar;
717+
std::error_code ec;
718+
if (absent.empty()) { std::filesystem::remove(path, ec); return; }
719+
std::ofstream f(path, std::ios::binary | std::ios::trunc);
720+
if (!f) return;
721+
f << cAbiName << '\n';
722+
for (auto const& e : absent)
723+
f << mcpp::targetside::c_abi_absent_form_name(e.form) << '\t'
724+
<< e.name << '\t' << e.note << '\n';
725+
}
726+
727+
std::pair<std::string, std::vector<mcpp::targetside::CAbiAbsentEntry>>
728+
read_c_abi_absent_sidecar(const std::filesystem::path& outputDir) {
729+
std::pair<std::string, std::vector<mcpp::targetside::CAbiAbsentEntry>> out;
730+
std::ifstream f(outputDir / kCAbiAbsentSidecar, std::ios::binary);
731+
if (!f) return out;
732+
std::getline(f, out.first);
733+
std::string line;
734+
while (std::getline(f, line)) {
735+
if (line.empty()) continue;
736+
const auto a = line.find('\t');
737+
if (a == std::string::npos) continue;
738+
const auto b = line.find('\t', a + 1);
739+
if (b == std::string::npos) continue;
740+
auto form = mcpp::targetside::parse_c_abi_absent_form(line.substr(0, a));
741+
if (!form) continue;
742+
mcpp::targetside::CAbiAbsentEntry e;
743+
e.form = *form;
744+
e.name = line.substr(a + 1, b - a - 1);
745+
e.note = line.substr(b + 1);
746+
out.second.push_back(std::move(e));
747+
}
748+
return out;
749+
}
750+
689751
std::string c_abi_absent_facility_advice(
690752
std::string_view output, std::string_view cAbiName,
691753
const std::vector<mcpp::targetside::CAbiAbsentEntry>& absent) {
@@ -3097,6 +3159,14 @@ std::expected<BuildResult, BuildError> NinjaBackend::build(const BuildPlan& plan
30973159
plan.outputDir});
30983160

30993161
auto ninja_path = plan.outputDir / "build.ninja";
3162+
// Written beside build.ninja and not into it: the fast path replays the
3163+
// ninja file without a plan, and the advice a failed link needs has to
3164+
// reach both channels or it appears depending on whether build.ninja
3165+
// happened to be up to date.
3166+
write_c_abi_absent_sidecar(
3167+
plan.outputDir, plan.targetSide.cAbi.interfaceName,
3168+
plan.targetSide.cAbiDecl ? plan.targetSide.cAbiDecl->absent
3169+
: std::vector<mcpp::targetside::CAbiAbsentEntry>{});
31003170
auto manifest = emit_ninja_string(plan);
31013171
stage("emit-ninja");
31023172

@@ -3465,6 +3535,8 @@ std::expected<BuildResult, BuildError> NinjaBackend::build(const BuildPlan& plan
34653535
diagnostics += c_abi_absent_facility_advice(
34663536
out, plan.targetSide.cAbi.interfaceName,
34673537
plan.targetSide.cAbiDecl->absent);
3538+
// (the fast path reads the same list from the sidecar this build
3539+
// wrote beside build.ninja — see `write_c_abi_absent_sidecar`)
34683540
return std::unexpected(BuildError{"build failed", plan.outputDir / "build.ninja",
34693541
std::move(diagnostics)});
34703542
}

tests/unit/test_build_flags.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,47 @@ TEST(CAbiAbsentAdvice, ALongerSymbolWithTheSamePrefixIsNotExplained) {
281281
EXPECT_FALSE(mcpp::build::c_abi_absent_facility_advice(
282282
"ld.lld: error: undefined symbol: open\n", "musl", absent).empty());
283283
}
284+
285+
TEST(CAbiAbsentAdvice, TheListSurvivesAWriteAndReadBesideBuildNinja) {
286+
// The two paths that report a failed build --- the one with a plan and the
287+
// fast path, which has none by construction --- must give the same advice,
288+
// or it appears depending on whether build.ninja happened to be up to
289+
// date. The list travels between them in a file.
290+
Tmp dir;
291+
std::vector<mcpp::targetside::CAbiAbsentEntry> absent{
292+
{"fork", mcpp::targetside::CAbiAbsentForm::Link, "no image duplication"},
293+
{"mprotect", mcpp::targetside::CAbiAbsentForm::Enosys, ""},
294+
};
295+
mcpp::build::write_c_abi_absent_sidecar(dir.path, "musl", absent);
296+
auto [name, back] = mcpp::build::read_c_abi_absent_sidecar(dir.path);
297+
EXPECT_EQ(name, "musl");
298+
ASSERT_EQ(back.size(), 2u);
299+
EXPECT_EQ(back[0].name, "fork");
300+
EXPECT_EQ(back[0].form, mcpp::targetside::CAbiAbsentForm::Link);
301+
EXPECT_EQ(back[0].note, "no image duplication");
302+
EXPECT_EQ(back[1].form, mcpp::targetside::CAbiAbsentForm::Enosys);
303+
EXPECT_TRUE(back[1].note.empty());
304+
// And the advice built from the read-back list is the advice the plan
305+
// path would have given.
306+
EXPECT_FALSE(mcpp::build::c_abi_absent_facility_advice(
307+
"ld.lld: error: undefined symbol: fork\n", name, back).empty());
308+
}
309+
310+
TEST(CAbiAbsentAdvice, ADirectoryNoBuildWroteToYieldsNothing) {
311+
Tmp dir;
312+
auto [name, back] = mcpp::build::read_c_abi_absent_sidecar(dir.path);
313+
EXPECT_TRUE(name.empty());
314+
EXPECT_TRUE(back.empty());
315+
}
316+
317+
TEST(CAbiAbsentAdvice, AGraphThatDeclaresNoAbsenceLeavesNoSidecar) {
318+
// A build whose C library states nothing must not leave a file behind for
319+
// the next build to read: the fast path would then explain a failure with
320+
// a list the current graph never declared.
321+
Tmp dir;
322+
mcpp::build::write_c_abi_absent_sidecar(dir.path, "musl",
323+
{{"fork", mcpp::targetside::CAbiAbsentForm::Link, ""}});
324+
mcpp::build::write_c_abi_absent_sidecar(dir.path, "", {});
325+
auto [name, back] = mcpp::build::read_c_abi_absent_sidecar(dir.path);
326+
EXPECT_TRUE(back.empty());
327+
}

0 commit comments

Comments
 (0)