Skip to content

Commit 4838952

Browse files
committed
feat(build): feature-dep 与已声明依赖同键时按加法合并
`mergeActiveFeatureDeps` 用的是 `try_emplace`,键已存在就丢弃 feature 的 spec。这条规则对 version/path/git 是对的(条件段不该静默覆盖无条件段),对 `tools` / `reexport` 却丢掉了这个 feature 存在的全部理由。 gRPC 就是反例,而且是本 issue 的头号用例:它**无条件**依赖 compat.protobuf, 而 `codegen` feature 需要往**同一条边**加 `tools = ["protoc"], reexport = true`。 挪到无条件条目上不可行 —— 那会让每个 grpc 消费者都构建 protoc(~157 个额外 TU),而这正是 tools 默认关闭要避免的。 于是:tools / features 取并集,host-module / reexport 取或,身份字段不合并。 与逐边 feature 请求本来遵循的规则一致。 e2e 193 补第 3 段覆盖它,并已用「不合并 tools」验证会变红 (`rulepkg: no codegen tool`)。
1 parent c1a3e45 commit 4838952

5 files changed

Lines changed: 104 additions & 1 deletion

File tree

‎.agents/docs/2026-08-06-provisions-and-build-inputs.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,18 @@ visible(P) = ⋃ {P→D} [ own(P→D) ∪ exported(D) ]
146146
2. **provision 的存在性绑定 feature 激活**,与 `featureDefines`(`prepare.cppm:3812`)同一时机。feature 关闭时不应有任何工具被构建。
147147
3. **`reexport` 只在依赖侧有意义。** root 写它无害但无效果(root 没有消费者)。
148148

149+
##### feature-dep 与已声明依赖同键时按加法合并
150+
151+
`mergeActiveFeatureDeps` 原本用 `try_emplace`,即「键已存在就丢弃 feature 的
152+
spec」。gRPC 恰好是反例:它**无条件**依赖 `compat.protobuf`,而 `codegen`
153+
feature 需要往**同一条边**加 `tools = ["protoc"], reexport = true`。把这个请求
154+
挪到无条件条目上不可行(那会让每个消费者都构建 protoc),丢弃又会静默丢掉这个
155+
feature 存在的全部理由。
156+
157+
因此:`tools` / `features` 取并集,`host-module` / `reexport` 取或;
158+
`version` / `path` / `git` 这类**身份字段不合并**,「条件段绝不静默覆盖无条件
159+
段」这条纪律保持不变。与逐边 feature 请求本来就遵循的规则一致。
160+
149161
#### D1.3 裸名:走索引那套命名空间阶梯,而不是「谁最后写谁赢」
150162

151163
`env_var_name`(`tool_store.cppm:154`)今天同时发长名与短名:`compat.protobuf` → `MCPP_DEP_COMPAT_PROTOBUF_BIN_PROTOC` 与 `MCPP_DEP_PROTOBUF_BIN_PROTOC`。今天只有 root 亲自声明的工具进环境,撞车在用户眼皮底下;传递传播之后,两条互不相识的库各自提供同名短名工具时,谁赢取决于 vector 的追加顺序,且无任何诊断。这从另一扇门放回了本设计要保住的「版本错配不可表达」性质。

‎docs/05-mcpp-toml.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,12 @@ int main() { return grpcgen::generate_all() ? 0 : 1; }
11061106
- **One hop per declaration.** A re-exported provision reaches the consumers of
11071107
the package that declared it. For it to travel further, the next package must
11081108
re-export in turn — each package decides only what *it* hands on.
1109+
- **A feature may add a request to a dependency you already declare.** gRPC
1110+
depends on protobuf unconditionally and its `codegen` feature adds
1111+
`tools = ["protoc"], reexport = true` to that same edge. `tools` and
1112+
`features` union, `host-module` and `reexport` OR together; `version` /
1113+
`path` / `git` do not merge, so a feature still cannot silently override the
1114+
unconditional entry's identity.
11091115
- **Visibility, not execution.** `dep_bin()` returns a path; whether anything
11101116
runs is still the consumer's `build.mcpp`'s decision. Nothing changes about
11111117
who builds the tool or how the tool store is keyed.

‎docs/zh/05-mcpp-toml.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,10 @@ int main() { return grpcgen::generate_all() ? 0 : 1; }
847847
命名空间里塞东西。「把某样东西交给消费者」是一条供应链主张,必须写下来。
848848
- **一次声明只走一跳。** 被再导出的提供物到达声明它的那个包的消费者;要继续
849849
往上走,下一个包必须自己也写 `reexport`。每个包只决定**它**交出什么。
850+
- **feature 可以往一条已经声明过的依赖上追加请求。** gRPC 无条件依赖 protobuf,
851+
而它的 `codegen` feature 往同一条边加 `tools = ["protoc"], reexport = true`。
852+
`tools` 与 `features` 取并集,`host-module` 与 `reexport` 取或;`version` /
853+
`path` / `git` 不合并 —— feature 仍然无法静默覆盖无条件条目的身份。
850854
- **传播的是可见性,不是执行。** `dep_bin()` 只返回路径,跑不跑仍由消费者的
851855
`build.mcpp` 决定;谁构建了这个工具、tool store 怎么做键,都不改变。
852856
- **裸名由阶梯决定,而不是靠运气。** 一旦两个库都能再导出,它们可能同时提供

‎src/build/prepare.cppm‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3148,7 +3148,34 @@ prepare_build(bool print_fingerprint,
31483148
for (auto& f : activateFeatures(pm, requested, seedDefault)) {
31493149
auto it = pm.featureDeps.find(f);
31503150
if (it == pm.featureDeps.end()) continue;
3151-
for (auto& [k, spec] : it->second) pm.dependencies.try_emplace(k, spec);
3151+
for (auto& [k, spec] : it->second) {
3152+
auto [pos, fresh] = pm.dependencies.try_emplace(k, spec);
3153+
if (fresh) continue;
3154+
// #359: the key already exists unconditionally, and dropping
3155+
// the feature's spec here loses REQUESTS the feature exists to
3156+
// make. gRPC is the shape: it depends on compat.protobuf
3157+
// always, and its `codegen` feature has to add
3158+
// `tools = ["protoc"], reexport = true` to that same edge —
3159+
// which is precisely what must NOT be paid for by a consumer
3160+
// who did not ask for codegen, so moving it to the
3161+
// unconditional entry is not an option either.
3162+
//
3163+
// Additive fields merge; identity fields (version/path/git) do
3164+
// not, keeping "a conditional section never silently
3165+
// overrides an unconditional one" intact. Same rule the
3166+
// per-edge feature request already follows.
3167+
auto& dst = pos->second;
3168+
for (auto const& t : spec.tools)
3169+
if (std::find(dst.tools.begin(), dst.tools.end(), t)
3170+
== dst.tools.end())
3171+
dst.tools.push_back(t);
3172+
for (auto const& f2 : spec.features)
3173+
if (std::find(dst.features.begin(), dst.features.end(), f2)
3174+
== dst.features.end())
3175+
dst.features.push_back(f2);
3176+
dst.hostModule = dst.hostModule || spec.hostModule;
3177+
dst.reexport = dst.reexport || spec.reexport;
3178+
}
31523179
}
31533180
};
31543181

‎tests/e2e/193_provision_reexport.sh‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,58 @@ if grep -q "rulepkg" <(nm -C "target"/*/*/bin/app 2>/dev/null || true); then
179179
echo "FAIL: the rule package was linked into the consumer's binary"; exit 1
180180
fi
181181

182+
# ── 3. a feature ADDING a request to an already-declared dependency ────────
183+
# The real shape: gRPC depends on protobuf always, and its `codegen` feature
184+
# has to add `tools = ["protoc"], reexport = true` to that SAME edge. Moving
185+
# the request to the unconditional entry is not an option — it would make every
186+
# consumer build protoc — and dropping the feature's spec (try_emplace keeps
187+
# the existing key) would silently lose the request.
188+
cd "$TMP"
189+
cat > lib/mcpp.toml <<'EOF'
190+
[package]
191+
name = "mylib"
192+
version = "0.1.0"
193+
194+
[build]
195+
sources = ["src/mylib.cpp"]
196+
197+
# Unconditional: the library links against this package no matter what.
198+
[dependencies]
199+
toolpkg = { path = "../toolpkg" }
200+
rulepkg = { path = "../rulepkg", host-module = true, reexport = true }
201+
202+
# The feature adds a REQUEST to the edge above, and nothing else.
203+
[feature-deps.codegen]
204+
toolpkg = { path = "../toolpkg", tools = ["codegen"], reexport = true }
205+
EOF
206+
cat > app/mcpp.toml <<'EOF'
207+
[package]
208+
name = "app"
209+
version = "0.1.0"
210+
211+
[dependencies]
212+
mylib = { path = "../lib", features = ["codegen"] }
213+
EOF
214+
cd app && rm -rf target
215+
"$MCPP" build > b3.log 2>&1 || {
216+
cat b3.log
217+
echo "FAIL: a feature could not add a tool request to an existing dependency"
218+
exit 1
219+
}
220+
out="$("$MCPP" run 2>&1 | grep '^ANSWER=' | tail -1)"
221+
[[ "$out" == "ANSWER=42" ]] || { echo "FAIL: expected ANSWER=42, got '$out'"; exit 1; }
222+
223+
# … and without the feature, nothing is built.
224+
cd "$TMP"
225+
sed 's/, features = \["codegen"\]//' app/mcpp.toml > app/mcpp.toml.new
226+
mv app/mcpp.toml.new app/mcpp.toml
227+
cd app && rm -rf target
228+
if "$MCPP" build > b4.log 2>&1; then
229+
cat b4.log
230+
echo "FAIL: the build succeeded without the codegen feature — the tool ran anyway"
231+
exit 1
232+
fi
233+
grep -q "host tool" b4.log && {
234+
cat b4.log; echo "FAIL: a tool was built for a consumer that did not enable the feature"; exit 1; }
235+
182236
echo "PASS: 193_provision_reexport"

0 commit comments

Comments
 (0)