Skip to content

Commit de49198

Browse files
committed
the note about a declared absence was missing a closing parenthesis
Every reader of it saw: note: the C library in this graph (musl declares that it does not supply The C library's name was interpolated from two separate conditionals --- an opening paren, then the name --- and the closing one was never emitted. It is one substitution now. NINE UNIT TESTS COVERED THIS FUNCTION AND NONE OF THEM SAW IT. They asserted `a.find("musl")`, which is true of the broken spelling as well: a criterion aimed at a substring of a sentence cannot see the sentence. The test now asserts the rendered clause, and a second one covers the other side of the same substitution --- an unnamed C library must leave no empty parentheses rather than `graph () declares`. WHAT ACTUALLY FOUND IT was running a real link. The nine unit tests covered the matching rules and the sidecar round-trip, and seven more covered parsing the table, but nothing ran the two together: that a real build writes the sidecar beside build.ninja, that a real link failure reads it back, and that what arrives is a sentence. e2e 744 does that, in four legs --- the linker's own message survives, the row's note reaches it, the sentence reads as one, and an absence is not reported as a defect in the build. Restoring the missing paren fails the third and only the third. The symbol the test uses is one nothing defines. `fork` is the real row in openkal-musl's manifest and every C library on a Linux host defines it, so a test written with it would link and assert nothing.
1 parent 48973cf commit de49198

4 files changed

Lines changed: 147 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ tcsetattr = { form = "accepted-no-effect", note = "openkal 不命名的那些字
105105
的东西。链接点到 `link` 形状里的某一项时,mcpp 把清单读回来:`undefined reference to
106106
'fork'` 因此带着那句说明它是缺陷还是环境限制的话一起到达。
107107

108+
**这条说明此前少一个右括号,而九个单测都没看见。** 渲染出来是
109+
`the C library in this graph (musl declares that it does not supply ...`——C 库的名字由
110+
**两个各自独立的条件**插进去:一个左括号,然后是名字,右括号从来没有被发出过。单测断言的
111+
是 `find("musl")`,那句话里同样有 "musl"。**判据瞄准一句话的子串,就看不见这句话。**
112+
修法是整个括号部分只做一次替换;新增的 e2e 744 真跑一次链接失败并断言整句,把右括号去掉
113+
它就红。
114+
108115
**它是顶层表,而这是量出来的。** 先写成 `[c-abi].absent`——更顺——之后拿**真正发布的
109116
2026.9.18.3 归档**(当时的索引 floor)跑 openkal-musl 0.17.0 将要发布的那份清单:嵌套
110117
写法让每个旧 mcpp **在每个目标上拒绝整份清单**,报 `[c-abi] has no member 'absent'`。

src/build/ninja_backend.cppm

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -785,16 +785,22 @@ std::string c_abi_absent_facility_advice(
785785
}
786786
if (named.empty()) return {};
787787

788+
// ONE SUBSTITUTION FOR THE WHOLE PARENTHETICAL, not an opening paren and
789+
// a name from two separate conditionals. Written that way, the closing
790+
// paren was simply absent and every reader saw `(musl declares that it
791+
// does not supply ...`. The unit tests asserted `find("musl")`, which is
792+
// true of both spellings; it took running a real link to see it.
793+
const std::string who =
794+
cAbiName.empty() ? std::string{} : std::format(" ({})", cAbiName);
788795
return std::format(
789796
"\n"
790-
"note: the C library in this graph{}{} declares that it does not "
797+
"note: the C library in this graph{} declares that it does not "
791798
"supply the following, and the link has just asked for it:{}\n"
792799
" An absence stated in the manifest is a property of the "
793800
"environment this program was built for, not a defect in the build. "
794801
"A program that needs one of these needs a different C environment "
795802
"for this target.\n",
796-
cAbiName.empty() ? "" : " (", cAbiName.empty() ? "" : cAbiName,
797-
named);
803+
who, named);
798804
}
799805

800806
std::string graph_c_library_isolation_advice(std::string_view output,
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
#!/usr/bin/env bash
2+
# requires: gcc
3+
# 744 -- a `[c-abi-absent]` row with `form = "link"` reaches the LINK, and the
4+
# note it produces is read by a person.
5+
#
6+
# WHAT WAS UNTESTED, AND WHAT IT COST. `c_abi_absent_facility_advice` had nine
7+
# unit tests covering the matching rules and the sidecar round-trip, and the
8+
# table had seven covering the manifest. Nothing ran the two together: that a
9+
# real build WRITES the sidecar beside build.ninja, that a real link failure
10+
# READS it back, and that what arrives is a sentence.
11+
#
12+
# It was not a sentence. The C library's name was interpolated from two
13+
# separate conditionals -- an opening paren, then the name -- and the closing
14+
# paren was never emitted, so every reader saw
15+
#
16+
# note: the C library in this graph (musl declares that it does not supply
17+
#
18+
# The unit test asserted `find("musl")`, which is true of that spelling too.
19+
# A criterion aimed at a substring of a sentence cannot see the sentence.
20+
#
21+
# THE SYMBOL IS DELIBERATELY ONE NOTHING DEFINES. `fork` is the real row in
22+
# openkal-musl's manifest and is defined by every C library on a Linux host,
23+
# so a test written with it would link and assert nothing.
24+
set -e
25+
26+
MCPP="${MCPP:-mcpp}"
27+
work="$(mktemp -d)"
28+
trap 'rm -rf "$work"' EXIT
29+
cd "$work"
30+
31+
mkdir -p libc/src src
32+
cat > libc/src/lib.c <<'EOF'
33+
int fake_c_library_marker(void) { return 0; }
34+
EOF
35+
cat > src/main.c <<'EOF'
36+
extern int mcpp_absent_probe_fn(void);
37+
int main(void) { return mcpp_absent_probe_fn(); }
38+
EOF
39+
cat > mcpp.toml <<'EOF'
40+
[package]
41+
name = "absence-explains-the-link"
42+
version = "0.1.0"
43+
44+
[targets.absence-explains-the-link]
45+
kind = "bin"
46+
main = "src/main.c"
47+
48+
[dependencies]
49+
fakelibc = { path = "libc" }
50+
51+
[build]
52+
allow_host_libs = true
53+
EOF
54+
cat > libc/mcpp.toml <<'EOF'
55+
[package]
56+
name = "fakelibc"
57+
version = "0.1.0"
58+
provides = ["mcpp:c-abi=musl"]
59+
60+
[targets.fakelibc]
61+
kind = "lib"
62+
sources = ["src/*.c"]
63+
64+
[c-abi-absent]
65+
mcpp_absent_probe_fn = { form = "link", note = "this environment has no such operation" }
66+
EOF
67+
68+
out="$("$MCPP" build 2>&1)" && {
69+
echo "FAIL: the link must fail -- nothing defines mcpp_absent_probe_fn" >&2
70+
echo "$out" >&2
71+
exit 1
72+
}
73+
74+
# ── A. the linker's own message is still there, unreplaced ─────────────────
75+
echo "$out" | grep -qi "mcpp_absent_probe_fn" || {
76+
echo "FAIL: the linker's own diagnostic must survive" >&2
77+
echo "$out" >&2
78+
exit 1
79+
}
80+
echo "OK: A (the linker still reports the symbol)"
81+
82+
# ── B. the manifest row reached the link ───────────────────────────────────
83+
echo "$out" | grep -q "this environment has no such operation" || {
84+
echo "FAIL: the [c-abi-absent] row's note must reach the link diagnostic." \
85+
"The sidecar is written beside build.ninja during the build and read" \
86+
"back when the link fails; one of those two did not happen." >&2
87+
echo "$out" >&2
88+
exit 1
89+
}
90+
echo "OK: B (the row's note reached the failure)"
91+
92+
# ── C. the sentence reads as a sentence ────────────────────────────────────
93+
# THE WHOLE CLAUSE, NOT A SUBSTRING OF IT. This is the assertion the unit
94+
# test did not make, and the defect it did not see.
95+
echo "$out" | grep -q "the C library in this graph (musl) declares that it does not supply" || {
96+
echo "FAIL: the note must name the C library in a closed parenthetical." >&2
97+
echo " got: $(echo "$out" | grep -m1 'the C library in this graph')" >&2
98+
exit 1
99+
}
100+
echo "OK: C (the note names the C library and reads as a sentence)"
101+
102+
# ── D. an absence is not reported as a defect in the build ─────────────────
103+
echo "$out" | grep -q "not a defect in the build" || {
104+
echo "FAIL: the note must say an absence is a property of the environment" >&2
105+
echo "$out" >&2
106+
exit 1
107+
}
108+
echo "OK: D (the note distinguishes an absence from a defect)"
109+
110+
echo "OK"

tests/unit/test_build_flags.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,27 @@ TEST(CAbiAbsentAdvice, ALinkAbsenceNamedInTheManifestIsExplained) {
228228
ASSERT_FALSE(a.empty());
229229
EXPECT_NE(a.find("fork"), std::string::npos);
230230
EXPECT_NE(a.find("no process image duplication"), std::string::npos);
231-
EXPECT_NE(a.find("musl"), std::string::npos);
231+
// THE WHOLE CLAUSE, NOT `find("musl")`. The name was interpolated from
232+
// two conditionals -- an opening paren and the name -- and the closing
233+
// one was never emitted, so every reader saw `(musl declares that it
234+
// does not supply ...`. `find("musl")` is true of that sentence too,
235+
// which is why it took a real link to notice. Assert the rendering.
236+
EXPECT_NE(a.find("the C library in this graph (musl) declares"),
237+
std::string::npos) << a;
238+
}
239+
240+
TEST(CAbiAbsentAdvice, AnUnnamedCLibraryLeavesNoEmptyParentheses) {
241+
// The other side of the same substitution: with no name there must be no
242+
// parenthetical at all, rather than `graph () declares`.
243+
std::vector<mcpp::targetside::CAbiAbsentEntry> absent{
244+
{"fork", mcpp::targetside::CAbiAbsentForm::Link, ""},
245+
};
246+
auto a = mcpp::build::c_abi_absent_facility_advice(
247+
"ld.lld: error: undefined symbol: fork\n", "", absent);
248+
ASSERT_FALSE(a.empty());
249+
EXPECT_NE(a.find("the C library in this graph declares"),
250+
std::string::npos) << a;
251+
EXPECT_EQ(a.find("()"), std::string::npos) << a;
232252
}
233253

234254
TEST(CAbiAbsentAdvice, TheGnuSpellingIsMatchedToo) {

0 commit comments

Comments
 (0)