fix: [build].defines is a BuildInputs member, and [build] stops swallowing unknown keys (2026.7.28.1)

Review follow-up on #297. The original fix routed `[build].defines` to the
P1689 scan correctly — the mechanism was right and the fold sites already
mirrored the #229 conditional-merge funnel. Two things were left open.

1. `defines` lived on BuildConfig, not BuildInputs.

   The cfg axis carries `ConditionalConfig::inputs`, a BuildInputs — that is
   the whole point of the type (#258): "membership here is the answer, and it
   is a type rather than a hand-kept list, so it cannot drift." A BuildConfig-
   only field is therefore inexpressible under `[target.'cfg(...)'.build]`,
   and since the conditional reader only reads keys it knows, a platform-only
   macro vanished without a word — #296's own failure mode, one section over.
   The fold's comment claimed conditional defines landed; they could not.

   `defines` is moved to BuildInputs, append() carries it, the conditional
   reader reads it, and the emptiness gate counts it (a section with only
   `defines` was being dropped before it was ever evaluated).

2. The root cause was never "defines wasn't folded" — it was that `[build]`
   read its keys with a bare if-chain and silently discarded everything else.

   That is #131's footgun, which `[targets.<name>]` already guards against
   with exactly the comment that describes #296: "a `[targets.x] cxxflags`
   typo on an older mcpp just vanished." `[build]` and
   `[target.<pred>.build]` now get the same treatment — a schema warning, an
   error under --strict. Verified against all 116 mcpp.toml in the repo and
   every heredoc manifest in tests/: zero false positives. (`build.default_*`
   in the e2e fixtures is the GLOBAL config.toml, parsed by config.cppm, and
   is untouched by this.)

Also: the xpkg descriptor listed `defines` as a did-you-mean redirect to
`flags`. The two manifest surfaces are two spellings of one schema, so a key
accepted in an mcpp.toml must not be an unknown-key error in a descriptor —
`defines` is now a real key there and in `target_cfg`, and only the singular
misspelling still redirects.

Tests — the new call sites had no coverage at all, and two thirds of the
existing fold sites were untested:
  - conditional `defines` parse, defines-only section, append() merge,
    both unknown-key guards, and a manifest exercising every supported
    `[build]` key so the guard can't fire on one (129 -> 136 unit tests)
  - e2e 167 rewritten: toolchain-neutral (a LOCAL module, not `import std`,
    so it runs on Windows where #296 was reported), #error guards as the
    assertion, plus NAME=value, a value containing a space (the -D quoting
    path from #234), the cfg axis, a path DEPENDENCY's own defines, and
    non-propagation to the consumer
  - e2e 167 no longer masks a failing `mcpp run` behind `tail`'s exit status
  - e2e 93 asserts `defines` parses as a known descriptor key

Version bumped to 2026.7.28.1 (check_version_pins.sh green; the bootstrap
MCPP_PIN stays at the released 2026.7.27.1).

Author: Sunrisepeak

docs/05-mcpp-toml.md

@@ -183,7 +183,33 @@ then only guaranteed to run on the build machine's version and above). A lower
 floor (11–13) requires a self-built libc++ archive (already verified to work, a
 data-level switch, available on request).
 
-`defines` desugars each entry to `-D<x>` on both the C and C++ compile channels and reaches every TU in the package — including module interface units — so it affects the P1689 module scan. For macros that should only affect a single binary's entry source, use `[targets.<name>].defines` instead.
+`defines` takes **bare** macro names (no `-D`) and desugars each entry to `-D<x>` on
+both the C and C++ compile channels. It reaches every TU in the package — module
+interface units included — so it also reaches the P1689 module scan, which is what
+makes a macro-guarded `import` resolvable. Assembly units pick it up too. It is a
+build input like any other, so `[target.'cfg(...)'.build]` can carry it:
+
+```toml
+[build]
+defines = ["APP_NAME=\"demo\""]
+
+[target.'cfg(windows)'.build]
+defines = ["USE_WIN32", "WINVER=0x0A00"]
+```
+
+Picking the right axis:
+
+| You want the macro on… | Use |
+|---|---|
+| every TU of this package | `[build].defines` (here) |
+| one binary's own entry source only | `[targets.<name>].defines` |
+| a specific set of files | `[build].flags` with a `glob` + `defines` |
+| every TU **and** every consumer's TUs | `[features.<name>].defines` (an interface contribution) |
+
+`[build].defines` is private to the package: it does not propagate to consumers.
+
+Unsupported keys under `[build]` are reported as a warning (an error under
+`--strict`) rather than silently ignored.
 
 Do not configure the C++ standard via `build.cxxflags = ["-std=..."]`. Instead use:
 

docs/zh/05-mcpp-toml.md

@@ -167,7 +167,31 @@ cargo/rustc、cc 等同样尊重该变量)> 本字段(项目默认,类似 SwiftP
 系统 libc++(产物只保证在构建机同版本及以上运行)。更低 floor(11–13)
 需自建 libc++ 归档(已验证可行,数据级切换,按需提供)。
 
-`defines` 把每个条目脱糖为 `-D<x>`,同时作用于 C 和 C++ 编译通道,并覆盖包内每个 TU(包括模块接口单元),因此会影响 P1689 模块扫描。若只想让某个二进制入口源读到宏,请改用 `[targets.<name>].defines`。
+`defines` 接受**裸**宏名(不带 `-D`),把每个条目脱糖为 `-D<x>`,同时作用于 C 和
+C++ 编译通道。它覆盖包内每个 TU(含模块接口单元),因此也会进入 P1689 模块扫描
+—— 这正是被宏保护的 `import` 能被解析的前提。汇编单元同样能拿到。它是普通的构建
+输入,所以 `[target.'cfg(...)'.build]` 也能承载它:
+
+```toml
+[build]
+defines = ["APP_NAME=\"demo\""]
+
+[target.'cfg(windows)'.build]
+defines = ["USE_WIN32", "WINVER=0x0A00"]
+```
+
+选择合适的轴:
+
+| 想让宏作用于… | 用 |
+|---|---|
+| 本包的每个 TU | `[build].defines`(本节) |
+| 仅某个二进制自己的入口源 | `[targets.<name>].defines` |
+| 指定的一批文件 | `[build].flags` 配 `glob` + `defines` |
+| 本包每个 TU **以及**消费者的 TU | `[features.<name>].defines`(接口贡献) |
+
+`[build].defines` 是包私有的:不会传播给消费者。
+
+`[build]` 下不支持的键会作为警告报出(`--strict` 下为错误),而不是被静默忽略。
 
 C++ 标准不要通过 `build.cxxflags = ["-std=..."]` 配置。请使用:
 

mcpp.toml

@@ -1,6 +1,6 @@
 [package]
 name        = "mcpp"
-version     = "2026.7.27.1"
+version     = "2026.7.28.1"
 description = "Modern C++ build & package management tool"
 license     = "Apache-2.0"
 authors     = ["mcpp-community"]

src/build/prepare.cppm

@@ -443,10 +443,19 @@ void merge_conditional_build_inputs(mcpp::manifest::Manifest& m,
 }
 
 // Desugar `[build].defines` into `-D<x>` on both C and C++ flag channels.
-// This must run AFTER conditional `[target.'cfg(...)'.build]` sections are
-// merged (so conditional defines land too) and BEFORE the manifest is
-// snapshotted into packages[] / fingerprinted. Idempotent: clearing the
-// `defines` vector after folding makes repeated calls harmless.
+//
+// ORDER (both halves are load-bearing): this must run AFTER
+// merge_conditional_build_inputs — `defines` is a BuildInputs member, so a
+// matching `[target.'cfg(...)'.build] defines` has been appended by then and
+// folds in the same pass, landing after the unconditional entries so GNU
+// last-wins gives the conditional rule precedence — and BEFORE the manifest is
+// snapshotted into packages[] / fingerprinted, because that snapshot (not the
+// manifest) is what the P1689 scan, the compile edges and compute_fingerprint
+// actually read.
+//
+// Idempotent: clearing the vector after folding makes repeated calls harmless.
+// Both `cflags` and `cxxflags` get the macro; assembly units pick it up for
+// free via the -D/-U/-I subset the ninja backend filters out of packageCflags.
 void fold_build_defines_into_flags(mcpp::manifest::BuildConfig& bc) {
     for (auto const& d : bc.defines) {
         bc.cflags.push_back("-D" + d);

src/manifest/toml.cppm

@@ -889,6 +889,38 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
             if (val.is_string()) m.xlings.envs[k] = val.as_string();
     if (auto v = doc->get_string("build.macos_deployment_target"))
         m.buildConfig.macosDeploymentTarget = *v;
+
+    // Surface unsupported [build] keys instead of silently dropping them.
+    // #296 is #131's footgun one section over: `[build] defines` on an mcpp
+    // that did not know the key simply vanished, and the build then failed
+    // much later with a module-graph divergence naming neither the key nor
+    // the manifest. An unknown key in a section this central is always a typo
+    // or a version mismatch, never an intentional no-op — so say so. Same
+    // policy as [targets.<name>] above: a warning, an error under --strict.
+    //
+    // MUST stay in sync with the `doc->get_*("build.<key>")` reads above.
+    static constexpr std::string_view kKnownBuildKeys[] = {
+        "allow_host_libs", "c_standard", "cflags", "cxxflags",
+        "default-profile", "defines", "dialect_cxxflags", "flags",
+        "include_dirs", "include_dirs_after", "ldflags",
+        "macos_deployment_target", "profile", "sources", "static_stdlib",
+        "target",
+    };
+    if (auto* bt = doc->get_table("build")) {
+        for (auto& [key, _] : *bt) {
+            bool known = false;
+            for (auto k : kKnownBuildKeys) if (key == k) { known = true; break; }
+            if (!known) {
+                m.schemaWarnings.push_back(std::format(
+                    "[build] has unsupported key '{}' (ignored). Supported keys: "
+                    "sources, cflags, cxxflags, ldflags, defines, flags, "
+                    "include_dirs, include_dirs_after, dialect_cxxflags, "
+                    "c_standard, target, static_stdlib, allow_host_libs, "
+                    "profile, macos_deployment_target.", key));
+            }
+        }
+    }
+
     for (auto const& flag : m.buildConfig.cxxflags) {
         if (starts_with_std_flag(flag)) {
             return std::unexpected(error(origin,
@@ -992,6 +1024,12 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
                 read_list("cxxflags", cc.inputs.cxxflags);
                 read_list("ldflags",  cc.inputs.ldflags);
                 read_list("sources",  cc.inputs.sources);
+                // #296: package-level macros are a build input like any other,
+                // so the cfg axis carries them too — a platform-only macro
+                // (`[target.'cfg(windows)'.build] defines = ["USE_WIN32"]`)
+                // must reach the scan and the compile exactly like an
+                // unconditional one.
+                read_list("defines",  cc.inputs.defines);
                 // #258: per-glob flags and include dirs, through the SAME entry
                 // grammar `[build].flags` uses — a conditional section is just
                 // a set of build inputs, so it reads the same way.
@@ -1004,6 +1042,31 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
                             cc.inputs.globFlags))
                         return std::unexpected(error(origin, *err));
                 }
+                // The conditional axis carries BuildInputs and nothing else, so
+                // its vocabulary is exactly that struct's members — a key
+                // outside it (`static_stdlib`, `target`, a profile knob) is not
+                // conditionable and would otherwise vanish without a word, the
+                // #296 failure mode. MUST stay in sync with the reads above and
+                // with types.cppm's BuildInputs.
+                static constexpr std::string_view kKnownConditionalBuildKeys[] = {
+                    "cflags", "cxxflags", "defines", "flags",
+                    "include_dirs", "include_dirs_after", "ldflags", "sources",
+                };
+                for (auto& [key, _] : bt) {
+                    bool known = false;
+                    for (auto k : kKnownConditionalBuildKeys)
+                        if (key == k) { known = true; break; }
+                    if (!known) {
+                        m.schemaWarnings.push_back(std::format(
+                            "[target.{}.build] has unsupported key '{}' (ignored). "
+                            "A conditional section may only contribute build INPUTS: "
+                            "sources, cflags, cxxflags, ldflags, defines, flags, "
+                            "include_dirs, include_dirs_after. Selection knobs "
+                            "(target, linkage, static_stdlib) and profile settings "
+                            "are resolved before the predicate is evaluated and "
+                            "cannot be conditioned.", triple, key));
+                    }
+                }
             }
             // [target.<predicate>.{dependencies,dev-dependencies,build-dependencies}]
             // parsed via the shared table-based loader (same selectors/namespaces
@@ -1020,6 +1083,7 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
             if (auto r = read_deps("build-dependencies", cc.buildDependencies); !r) return std::unexpected(r.error());
             if (!cc.inputs.cflags.empty() || !cc.inputs.cxxflags.empty()
                 || !cc.inputs.ldflags.empty() || !cc.inputs.sources.empty()
+                || !cc.inputs.defines.empty()
                 || !cc.inputs.globFlags.empty() || !cc.inputs.includeDirs.empty()
                 || !cc.inputs.includeDirsAfter.empty()
                 || !cc.dependencies.empty() || !cc.devDependencies.empty()

src/manifest/types.cppm

@@ -171,6 +171,17 @@ struct BuildInputs {
     std::vector<std::string>           cflags;
     std::vector<std::string>           cxxflags;
     std::vector<std::string>           ldflags;
+    // #296: package-level preprocessor macros. Unlike per-target `defines`
+    // (which only reach the binary's own entry TU), these reach EVERY TU in
+    // the package — module interface units included — so they participate in
+    // the P1689 module scan, which is what makes a macro-guarded `import`
+    // resolvable. Desugared to `-D<x>` on both the C and C++ channels
+    // (fold_build_defines_into_flags in prepare.cppm) after the conditional
+    // merge and before the manifest is snapshotted into packages[] /
+    // fingerprinted. A member HERE rather than on BuildConfig so the cfg axis
+    // can carry it: `[target.'cfg(windows)'.build] defines = [...]` must work,
+    // and membership of this type is what guarantees it (see above).
+    std::vector<std::string>           defines;
     std::vector<GlobFlags>             globFlags;      // flags = [...] (ordered)
     std::vector<std::filesystem::path> includeDirs;    // relative to package root
     // #249: emitted as -idirafter (searched after the toolchain's system dirs)
@@ -185,6 +196,7 @@ inline void append(BuildInputs& dst, const BuildInputs& src) {
     dst.cflags.insert(dst.cflags.end(), src.cflags.begin(), src.cflags.end());
     dst.cxxflags.insert(dst.cxxflags.end(), src.cxxflags.begin(), src.cxxflags.end());
     dst.ldflags.insert(dst.ldflags.end(), src.ldflags.begin(), src.ldflags.end());
+    dst.defines.insert(dst.defines.end(), src.defines.begin(), src.defines.end());
     dst.globFlags.insert(dst.globFlags.end(),
                          src.globFlags.begin(), src.globFlags.end());
     dst.includeDirs.insert(dst.includeDirs.end(),
@@ -203,13 +215,6 @@ inline void append(BuildInputs& dst, const BuildInputs& src) {
 // is read in ~150 places, and a BuildConfig genuinely IS a set of build
 // inputs plus the selection axis and resolved policy scalars.
 struct BuildConfig : BuildInputs {
-    // Package-level preprocessor defines. Unlike per-target `defines` (which
-    // only affect the binary's entry TU), these reach every TU in this
-    // package — including module interface units — so they participate in
-    // the P1689 module scan. Desugared to `-D<x>` on both C and C++ compiles
-    // before the manifest is snapshot into BuildPlan / fingerprint.
-    std::vector<std::string>           defines;
-
     // feature name → extra source globs gated by that feature. A glob listed
     // here is EXCLUDED from the default build and only compiled/linked when the
     // feature is active for this package (resolved in prepare_build). Lets a

src/manifest/xpkg.cppm

@@ -202,7 +202,7 @@ namespace mcpp::manifest {
 // Kept as one array so the did-you-mean suggester speaks the same vocabulary
 // the parser accepts.
 inline constexpr std::string_view kKnownXpkgKeys[] = {
-    "cflags", "c_standard", "cxxflags", "deps", "features", "flags",
+    "cflags", "c_standard", "cxxflags", "defines", "deps", "features", "flags",
     "generated_files", "import_std", "include_dirs", "include_dirs_after",
     "language", "ldflags",
     "linux", "macosx", "modules", "provides", "runtime", "scan_overrides",
@@ -215,8 +215,10 @@ inline constexpr std::pair<std::string_view, std::string_view> kXpkgKeyAliases[]
     { "dependencies", "deps" },
     { "dependency",   "deps" },
     { "requires",     "deps" },   // cargo muscle memory
-    { "define",       "flags" },
-    { "defines",      "flags" },
+    // #296: `defines` is a real key now (package-level bare macros). Only the
+    // singular misspelling needs redirecting, and it points at `defines` — the
+    // old redirect to `flags` predates the key existing.
+    { "define",       "defines" },
     { "feature",      "features" },
     { "source",       "sources" },
     { "target",       "targets" },
@@ -1261,6 +1263,7 @@ synthesize_from_xpkg_lua(std::string_view luaContent,
                         : sub == "cxxflags" ? &cc.inputs.cxxflags
                         : sub == "ldflags"  ? &cc.inputs.ldflags
                         : sub == "sources"  ? &cc.inputs.sources
+                        : sub == "defines"  ? &cc.inputs.defines
                         : nullptr;
                     // Unknown sub-keys stay a HARD ERROR here. The shared
                     // BuildInputs parser must not be read as licence to relax
@@ -1273,8 +1276,8 @@ synthesize_from_xpkg_lua(std::string_view luaContent,
                     if (!dst && !pathDst) {
                         return std::unexpected(ManifestError{
                             std::format("unknown target_cfg key '{}' (expected "
-                                        "cflags/cxxflags/ldflags/sources/flags/"
-                                        "include_dirs/include_dirs_after)", sub),
+                                        "cflags/cxxflags/ldflags/sources/defines/"
+                                        "flags/include_dirs/include_dirs_after)", sub),
                             m.sourcePath, 0, 0});
                     }
                     if (!cur.consume('=') || !cur.consume('{')) {
@@ -1298,6 +1301,7 @@ synthesize_from_xpkg_lua(std::string_view luaContent,
                 cur.consume('}');
                 if (!cc.inputs.cflags.empty() || !cc.inputs.cxxflags.empty()
                     || !cc.inputs.ldflags.empty() || !cc.inputs.sources.empty()
+                    || !cc.inputs.defines.empty()
                     || !cc.inputs.globFlags.empty()
                     || !cc.inputs.includeDirs.empty()
                     || !cc.inputs.includeDirsAfter.empty())
@@ -1517,19 +1521,28 @@ synthesize_from_xpkg_lua(std::string_view luaContent,
             }
             cur.consume('}');
         }
-        else if (key == "cflags" || key == "cxxflags" || key == "ldflags") {
+        else if (key == "cflags" || key == "cxxflags" || key == "ldflags"
+                 || key == "defines") {
             // `{ "-Dfoo", "-Wall", ... }` — appended to the per-rule baseline
             // by ninja_backend. cflags goes to the C rule (.c files), cxxflags
             // to C++ rule (.cpp/.cc/.cxx/.cppm), ldflags to link commands.
+            //
+            // #296: `defines` carries BARE macro names (`{ "USE_FOO", "N=1" }`,
+            // no `-D`) and desugars onto BOTH compile channels at prepare time,
+            // which is what lets it reach the P1689 scan. Same key, same
+            // meaning as `[build].defines` in an mcpp.toml — the two manifest
+            // surfaces are two spellings of one schema, so a key that exists in
+            // one must not be a did-you-mean error in the other.
             if (!cur.consume('{')) {
                 return std::unexpected(ManifestError{
                     std::format("expected '{{' after `{} =`", key),
                     m.sourcePath, 0, 0});
             }
             cur.skip_ws_and_comments();
-            auto& target = (key == "cflags")
-                ? m.buildConfig.cflags
-                : (key == "cxxflags" ? m.buildConfig.cxxflags : m.buildConfig.ldflags);
+            auto& target = (key == "cflags")   ? m.buildConfig.cflags
+                         : (key == "cxxflags") ? m.buildConfig.cxxflags
+                         : (key == "defines")  ? m.buildConfig.defines
+                         :                       m.buildConfig.ldflags;
             while (!cur.eof() && cur.peek() != '}') {
                 auto s = cur.read_string();
                 if (key == "cxxflags" && starts_with_std_flag(s)) {

src/toolchain/fingerprint.cppm

@@ -18,7 +18,7 @@ import mcpp.toolchain.detect;
 
 export namespace mcpp::toolchain {
 
-inline constexpr std::string_view MCPP_VERSION = "2026.7.27.1";
+inline constexpr std::string_view MCPP_VERSION = "2026.7.28.1";
 
 struct FingerprintInputs {
     Toolchain                       toolchain;