Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@
没有 `[workspace.target.<triple>]`:根里普通的 `[target.<triple>]` 本来就按 triple
被成员继承,为同一能力再加一种拼法只增加接口面。

⚠️ **继承作用到每一个成员,包括「作为兄弟成员的 `path` 依赖被编译」的那个**
—— 也就是成员互相依赖这种最普通的形态。而 `path` 依赖里**不是成员**的那些
(vendored 副本、example)不会获得这些标志:成员资格问的是 workspace 自己的
`members` 列表,不是「这个路径在不在 workspace 目录下」。

⚠️ `[workspace.build] include_dirs` 里的相对路径按 **workspace 根**锚定(#224):
它是在根 manifest 里写的,按各成员自己的目录解析会指向不存在的地方。

- **依赖声明了高于当前图的标准时会说出来。** C++ 模块图只有一个标准,依赖自己的
`standard` 不生效 —— 这是对的;缺的是它一直不说。degraded 级别(`--strict` 提升),
且**只对工程作者自己拥有的 manifest 生效**:索引里带 mcpp 段的描述符 782 个全都声明了
Expand Down
64 changes: 62 additions & 2 deletions src/build/prepare.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
113 changes: 80 additions & 33 deletions src/project.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down
36 changes: 36 additions & 0 deletions tests/e2e/321_workspace_inheritance.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Loading