diff --git a/.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md b/.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md index d811699d..0ba08af1 100644 --- a/.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md +++ b/.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md @@ -1747,3 +1747,21 @@ and when the axes have different owners, the uniform version is wrong.** The measurement that `system` builds and runs was correct and load-bearing for the library axis; carrying it across to the toolchain axis is what produced a proposal the maintainer had already declined on the issue. + +**Fourth round: does the new key reach everything it says it reaches?** + +Asked of the shipped implementation rather than of the design, and it did not. + +| claim as shipped | what measurement showed | where | +|---|---|---| +| `[workspace.build]` applies to every member | Only to the member the command names. A sibling compiled as its `path` dependency — the ordinary workspace shape — got none of the flags, in the same command. `[workspace.package] standard` hid it, because the standard is imposed graph-wide from the root for BMI compatibility and reached the sibling anyway | §5.5 | +| a member may omit `version` when the workspace supplies it | True where the command names it; false where it is reached as a sibling's dependency, which refused it for a field the workspace does provide | §5.5 | +| `[workspace.build] include_dirs` inherits | It did, verbatim — so a relative path written at the workspace root was resolved against each MEMBER's directory. #224 for a third key, found by re-reading the merge | §5.5, D11 | + +The shape all three share: **a rule was implemented at the one place its first +consumer reads it, and the key has more than one consumer.** The inheritance +site, the dependency load site and the package-assembly site each read a +different half, and a fix placed at any one of them is silent at the other two. +That is the same sentence as §2's defect, one layer up — which is why "who else +reads this?" is the question worth asking of every new key, and why the +denominator in each new test is a case that a single-site fix would still pass. diff --git a/CHANGELOG.md b/CHANGELOG.md index 23973ebe..4458f15c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,14 @@ 没有 `[workspace.target.]`:根里普通的 `[target.]` 本来就按 triple 被成员继承,为同一能力再加一种拼法只增加接口面。 + ⚠️ **继承作用到每一个成员,包括「作为兄弟成员的 `path` 依赖被编译」的那个** + —— 也就是成员互相依赖这种最普通的形态。而 `path` 依赖里**不是成员**的那些 + (vendored 副本、example)不会获得这些标志:成员资格问的是 workspace 自己的 + `members` 列表,不是「这个路径在不在 workspace 目录下」。 + + ⚠️ `[workspace.build] include_dirs` 里的相对路径按 **workspace 根**锚定(#224): + 它是在根 manifest 里写的,按各成员自己的目录解析会指向不存在的地方。 + - **依赖声明了高于当前图的标准时会说出来。** C++ 模块图只有一个标准,依赖自己的 `standard` 不生效 —— 这是对的;缺的是它一直不说。degraded 级别(`--strict` 提升), 且**只对工程作者自己拥有的 manifest 生效**:索引里带 mcpp 段的描述符 782 个全都声明了 diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 4a1a9aa6..3e6a49fe 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -4443,8 +4443,38 @@ prepare_build(bool print_fingerprint, auto makePackageRoot = [&](const std::filesystem::path& packageRoot, - const mcpp::manifest::Manifest& manifest) + const mcpp::manifest::Manifest& manifestIn) { + // `[workspace.build]` APPLIES TO EVERY MEMBER, INCLUDING ONE REACHED AS + // ANOTHER MEMBER'S `path` DEPENDENCY — WHICH IS THE ORDINARY SHAPE. + // + // Inheritance runs where the command's own manifest is loaded, so + // `mcpp build -p appb` gave `appb` the workspace flags and gave `liba` + // none, even though `liba` is a member of the same workspace and is + // being compiled by the same command. Measured before this: `-DWS_FLAG` + // on the consumer's TUs and not on the sibling's. + // + // `[workspace.package] standard` did not have the problem, because the + // standard is imposed graph-wide from the root for BMI-compatibility + // reasons — which is exactly why the gap was invisible until a + // `[build]` flag was inheritable too. + // + // Applied HERE because this is the one funnel both dependency-assembly + // sites go through, and because the include directories a few lines + // below are captured from the manifest at this moment: a later mutation + // would reach the flags and silently not the include dirs. + // + // Only for MEMBERS. An index or git dependency is not part of the + // workspace and must not acquire its flags. + mcpp::manifest::Manifest manifest = manifestIn; + if (wsManifest && !runtimeWorkspaceRoot.empty() + && mcpp::project::is_workspace_member(*wsManifest, + runtimeWorkspaceRoot, + packageRoot)) { + mcpp::project::inherit_workspace_build(manifest, *wsManifest, + runtimeWorkspaceRoot); + } + mcpp::modgraph::PackageRoot pkg; pkg.root = packageRoot; pkg.manifest = manifest; @@ -5442,13 +5472,43 @@ prepare_build(bool print_fingerprint, "{} dependency '{}' (at '{}') has no mcpp.toml", spec.isGit() ? "git" : "path", name, dep_root.string())); } - auto dm = mcpp::manifest::load(dep_root / "mcpp.toml"); + // A MEMBER IS A MEMBER HOWEVER IT IS REACHED. + // + // A workspace member that omits `package.version` because + // `[workspace.package]` supplies it is legal — and it is reached + // here as a sibling's `path` dependency, which is the ordinary + // shape rather than an exotic one. Loading it as an anonymous path + // dependency would refuse it for a field the workspace does + // provide, and the message would name the member's manifest rather + // than the table that answers. + // + // `is_workspace_member` asks the workspace's own `members` list, so + // a vendored copy or an example living inside the tree is still + // refused for a missing version, exactly as before. + const bool depIsMember = + wsManifest && !runtimeWorkspaceRoot.empty() + && mcpp::project::is_workspace_member( + *wsManifest, runtimeWorkspaceRoot, dep_root); + auto dm = mcpp::manifest::load( + dep_root / "mcpp.toml", + {.insideWorkspace = depIsMember}); if (!dm) { return std::unexpected(std::format( "dependency '{}' (at '{}'): {}", name, dep_root.string(), dm.error().format())); } dep_manifest = std::move(*dm); + // The metadata half of the inheritance. The `[build]` half runs in + // `makePackageRoot`, where the include directories are captured; + // splitting them is what keeps each one at the point its consumer + // reads it. + if (depIsMember) { + mcpp::project::inherit_workspace_package( + *dep_manifest, *wsManifest); + if (auto bad = mcpp::project::workspace_inheritance_error( + *dep_manifest, dep_root)) + return std::unexpected(*bad); + } // #229: path/git-dep half of the L1 cfg funnel — mirrors the // loadVersionDep call site above (loadFrom's L1 cfg merge, ~1740 // lines up). Before this fix, path/git deps never ran this merge diff --git a/src/project.cppm b/src/project.cppm index e5f61b85..990f0605 100644 --- a/src/project.cppm +++ b/src/project.cppm @@ -128,6 +128,59 @@ export void inherit_workspace_indices(mcpp::manifest::Manifest& member, } } +// Is `candidate` one of this workspace's declared members? +// +// The membership test is the workspace's OWN `members` list resolved against +// the workspace root — not "is this path under the workspace directory". A +// `path` dependency can live inside the tree without being a member (a vendored +// copy, an example, a scratch package), and a member's flags are exactly what +// it must not acquire. +export bool is_workspace_member(const mcpp::manifest::Manifest& workspace, + const std::filesystem::path& wsRoot, + const std::filesystem::path& candidate) { + if (!workspace.workspace.present) return false; + std::error_code ec; + auto want = std::filesystem::weakly_canonical(candidate, ec); + if (ec) { ec.clear(); want = candidate.lexically_normal(); } + for (auto const& m : workspace.workspace.members) { + auto member = std::filesystem::weakly_canonical(wsRoot / m, ec); + if (ec) { ec.clear(); member = (wsRoot / m).lexically_normal(); } + if (member == want) return true; + } + return false; +} + +export void inherit_workspace_build(mcpp::manifest::Manifest& member, + const mcpp::manifest::Manifest& workspace, + const std::filesystem::path& wsRoot); + +// The `[workspace.package]` half on its own — metadata, no paths, so no anchor +// argument. Second caller: a member reached as a sibling's `path` dependency, +// which may legally omit `version` because this table supplies it. +export void inherit_workspace_package(mcpp::manifest::Manifest& member, + const mcpp::manifest::Manifest& workspace) { + const auto& inh = workspace.workspace.inherited; + // `standardDeclared` and not `standard != "c++23"`: a member that + // deliberately pins c++23 under a c++26 workspace must keep it, and that is + // indistinguishable from the default without the bit. + if (inh.standardDeclared && !member.package.standardDeclared) { + member.package.standard = inh.standard; + member.language.standard = inh.standard; + member.package.standardDeclared = true; + // `cppStandard` was normalised by the parser from the member's own + // value; it has to be re-derived, or the inherited spelling would sit + // in `package.standard` while every build surface kept reading the + // default out of the normalised copy. + if (auto cfg = mcpp::manifest::normalize_cpp_standard(inh.standard)) + member.cppStandard = *cfg; + } + if (member.package.version.empty()) member.package.version = inh.version; + if (member.package.license.empty()) member.package.license = inh.license; + if (member.package.description.empty()) member.package.description = inh.description; + if (member.package.repo.empty()) member.package.repo = inh.repo; + if (member.package.authors.empty()) member.package.authors = inh.authors; +} + // EVERYTHING A MEMBER INHERITS FROM ITS WORKSPACE ROOT, IN ONE FUNCTION. // // There are two inheritance SITES in prepare_build — the command issued at the @@ -158,40 +211,34 @@ export void inherit_workspace_config(mcpp::manifest::Manifest& member, member.targetOverrides[triple] = entry; inherit_workspace_indices(member, workspace, wsRoot); - const auto& inh = workspace.workspace.inherited; - - // `[workspace.package]`. The standard is the load-bearing one: a C++ module - // graph has ONE standard, so a workspace that states it once is how a - // monorepo stops depending on every member remembering to. - // - // `standardDeclared` and not `standard != "c++23"`: a member that - // deliberately pins c++23 under a c++26 workspace must keep it, and that is - // indistinguishable from the default without the bit. - if (inh.standardDeclared && !member.package.standardDeclared) { - member.package.standard = inh.standard; - member.language.standard = inh.standard; - member.package.standardDeclared = true; - // `cppStandard` was normalised by the parser from the member's own - // value; it has to be re-derived, or the inherited spelling would sit - // in `package.standard` while every build surface kept reading the - // default out of the normalised copy. Same class of defect as a - // recorded field with no reader, one struct over. - if (auto cfg = mcpp::manifest::normalize_cpp_standard(inh.standard)) - member.cppStandard = *cfg; - } - if (member.package.version.empty()) member.package.version = inh.version; - // (the "still missing after inheritance" refusal is in - // `workspace_inheritance_error` below — one predicate, both call sites) - if (member.package.license.empty()) member.package.license = inh.license; - if (member.package.description.empty()) member.package.description = inh.description; - if (member.package.repo.empty()) member.package.repo = inh.repo; - if (member.package.authors.empty()) member.package.authors = inh.authors; + // The two halves, each with a second caller of its own: a member reached + // as a sibling.s `path` dependency needs both, at two different points. + // (The "still missing after inheritance" refusal is + // `workspace_inheritance_error`, called by each site.) + inherit_workspace_package(member, workspace); + inherit_workspace_build(member, workspace, wsRoot); +} - // `[workspace.build]`. Vectors append workspace-FIRST so a member's own - // flag lands later on the command line, where the compiler lets it win. - if (inh.buildPresent) { - auto& b = member.buildConfig; - const auto& w = inh.build; +// The `[workspace.build]` half on its own. +// +// SEPARATE BECAUSE IT HAS A SECOND CALLER. `inherit_workspace_config` runs for +// the manifest the command names; this runs additionally for every OTHER member +// pulled in as a `path` dependency — which is what workspace members are to each +// other, and therefore the ordinary case rather than an exotic one. Without the +// second call, `mcpp build -p appb` gave `appb` the workspace flags and gave the +// sibling `liba` none, while compiling both in the same command. +// +// `[workspace.package] standard` needs no second call: the standard is imposed +// graph-wide from the root for BMI-compatibility reasons, which is precisely +// why this gap stayed invisible until a `[build]` key became inheritable too. +export void inherit_workspace_build(mcpp::manifest::Manifest& member, + const mcpp::manifest::Manifest& workspace, + const std::filesystem::path& wsRoot) { + const auto& inh = workspace.workspace.inherited; + if (!inh.buildPresent) return; + auto& b = member.buildConfig; + const auto& w = inh.build; + { auto prepend = [](auto& dst, const auto& src) { if (src.empty()) return; dst.insert(dst.begin(), src.begin(), src.end()); diff --git a/tests/e2e/321_workspace_inheritance.sh b/tests/e2e/321_workspace_inheritance.sh index 186a3e0a..e7d988f6 100755 --- a/tests/e2e/321_workspace_inheritance.sh +++ b/tests/e2e/321_workspace_inheritance.sh @@ -149,4 +149,40 @@ grep -q "package.version" novers.log || { exit 1 } +# ── a SIBLING member reached as a `path` dependency inherits too ──────────── +# +# THE ORDINARY WORKSPACE SHAPE, AND THE ONE THAT WAS MISSED. Inheritance runs +# where the command's own manifest is loaded, so `mcpp build -p consumer` gave +# the consumer the workspace flags and gave the sibling none — while compiling +# both in the same command. `[workspace.package] standard` hid the gap, because +# the standard is imposed graph-wide from the root for BMI compatibility and +# reached the sibling anyway. +# +# The negative is in the same fixture on purpose: a `path` dependency that is +# NOT a member (a vendored copy, an example) must not acquire the flags, and a +# fix that inherited to every path dependency would pass the positive alone. +cat > mcpp.toml <<'EOF' +[workspace] +members = ["silent", "pinned", "adds", "consumer"] + +[workspace.package] +standard = 26 +version = "0.4.2" + +[workspace.build] +cxxflags = ["-DFROM_WORKSPACE=1"] +EOF +mkdir -p consumer/src vendored/src +printf '[package]\nname = "consumer"\n\n[dependencies]\nadds = { path = "../adds" }\nvend = { path = "../vendored" }\n' > consumer/mcpp.toml +printf 'import addslib;\nimport vend;\nint main(){ return addslib()+vend_v()==3 ? 0 : 1; }\n' > consumer/src/main.cpp +# `adds` becomes a library so it can be imported; its own source asserts it saw +# the workspace flag while being built as somebody else's dependency. +printf '[package]\nname = "adds"\n\n[targets.adds]\nkind = "lib"\n\n[build]\ncxxflags = ["-DFROM_MEMBER=1"]\n' > adds/mcpp.toml +rm -f adds/src/main.cpp +printf '#ifndef FROM_WORKSPACE\n#error "a SIBLING member built as a path dependency did not inherit"\n#endif\nexport module addslib;\nexport int addslib(){ return 1; }\n' > adds/src/addslib.cppm +printf '[package]\nname = "vend"\nversion = "0.1.0"\n\n[targets.vend]\nkind = "lib"\n' > vendored/mcpp.toml +printf '#ifdef FROM_WORKSPACE\n#error "a NON-member path dependency must not acquire workspace flags"\n#endif\nexport module vend;\nexport int vend_v(){ return 2; }\n' > vendored/src/vend.cppm + +"$MCPP" build -p consumer > sibling.log 2>&1 || { cat sibling.log; exit 1; } + echo "PASS: 321_workspace_inheritance"