diff --git a/.agents/docs/2026-06-29-feature-capability-model-design.md b/.agents/docs/2026-06-29-feature-capability-model-design.md index 46c5f2a..a205ec3 100644 --- a/.agents/docs/2026-06-29-feature-capability-model-design.md +++ b/.agents/docs/2026-06-29-feature-capability-model-design.md @@ -1,8 +1,9 @@ # Feature System v2 — Capability-Oriented Model (Design) Date: 2026-06-29 -Status: **S1 + S3 implemented & shipped** (see Implementation Status below); -S2 scoped as the documented next stage. +Status: **S1 + S2a + S3 + interface-define propagation implemented & shipped** +(see Implementation Status below); the full eigen[backend-openblas] ecosystem +closed loop is validated end-to-end. Scope: `src/manifest.cppm` (parse), `src/build/prepare.cppm` (feature activation + resolver), `src/cli.cppm` / `src/cli/cmd_build.cppm` (`--cap`), mcpp-index recipe schema. @@ -18,14 +19,42 @@ resolver), `src/cli.cppm` / `src/cli/cmd_build.cppm` (`--cap`), mcpp-index recip `[capabilities]` pins and `--cap`. Tests: `e2e/81_capability_binding.sh` (6 cases), `Manifest.CapabilitiesProvidesRequiresAndPins`, `SynthesizeFromXpkgLua.CapabilitiesAndFeatureDefines`. -- **Stage 2 — optional-dep activation + feature-union unification: NEXT.** - Deliberately deferred from this release. Rationale: activating a *new* - dependency from a feature requires moving feature computation ahead of - dependency resolution (resolution-phase reordering) — a deeper, higher-risk - change. It is also **not required** for the capability/Eigen use case, which - binds over providers that are explicitly declared as dependencies. Shipping - S1+S3 first matches this doc's "each stage independently shippable" intent and - keeps the release low-risk. +- **Stage 2a — feature-activated optional dependencies: DONE.** A dependency + declared under `[feature-deps.]` (TOML) or a feature's nested `deps` + (Lua) is pulled into the worklist only when that feature is active. Feature + activation (including transitive `implies`) is computed ahead of the + resolution worklist via local lambdas in `prepare.cppm` (kept local to avoid a + GCC-16 modules-BMI bug). Tests: `e2e/82_feature_optional_deps.sh`, + `Manifest.FeatureDepsTomlSection`, `SynthesizeFromXpkgLua.FeatureDepsAndImplies`. +- **Interface-define propagation (header-only providers): DONE.** A dependency's + active-feature `defines` are **interface requirements**: they flow into every + consumer's own compile flags along Public/Interface dependency edges, mirroring + `include_dirs`. This is required for header-only libraries whose feature switch + only takes effect in the TU that includes their headers — the canonical case is + Eigen's `use_blas` → `EIGEN_USE_BLAS`, which must be defined when the + *consumer* compiles `a * b`, not only when Eigen's own anchor TU compiles. The + automatic `MCPP_FEATURE_` macro stays private to the owning package (it + is a build signal, not a public contract). Implemented by routing feature + defines through `PackageRoot::publicUsage` and extending the + `computeUsageRequirements()` fixpoint to propagate `cflags`/`cxxflags`. Test: + `e2e/83_feature_defines_propagate.sh`. +- **Stage 2b — feature-union unification across multiple consumers: NEXT.** + Deliberately deferred. When two consumers request different feature sets on the + same dependency, the activated set should be their union (single resolved + instance). The current model activates per the first-seen consumer's request; + divergent transitive feature requests are not yet unified. Not required for the + validated Eigen/OpenBLAS use case. + +### Validated closed loop (eigen[backend-openblas]) + +`mcpp build` of a consumer declaring +`compat.eigen = { features = ["backend-openblas"] }` exercises every stage: +`backend-openblas` → (implies) `use_blas` → `-DEIGEN_USE_BLAS` propagated to the +consumer's TUs + `requires "blas"`; `[feature-deps]` pulls `compat.openblas`, +whose xpkg `install()` hook builds `libopenblas.a` from source (BLAS-only, +`TARGET=GENERIC`, no Fortran) via the `xim:make` build-dep; `provides "blas"` +binds the capability; the provider's `-lopenblas` links. Verified: the produced +binary pulls OpenBLAS's `dgemm_` (not Eigen's built-in GEMM) and runs. --- @@ -100,6 +129,51 @@ Spack, Nixpkgs, pkg-config. Distilled lessons that shaped this design: mutual exclusion structurally — no constraint DSL, no `backend-*` boolean pile. +### Corollary — a feature define is an *interface* requirement + +Rule 1 names *what* a feature may contribute to compilation (a package-owned, +namespaced define); it does not, by itself, fix *where* that define applies. For +a **header-only** provider the answer is forced: the library has no sources of +its own, so its feature switch only takes effect in the translation unit that +*includes its headers* — i.e. in the **consumer**. `EIGEN_USE_BLAS` must be +defined when the consumer compiles `a * b`, not (only) when Eigen's anchor TU +compiles. Therefore a feature's `defines` are treated as **interface +requirements**: they propagate to consumers along Public/Interface dependency +edges, on exactly the same machinery and visibility discipline as a dependency's +public `include_dirs` (`PackageRoot::publicUsage`, the `computeUsageRequirements` +fixpoint). This is the realization of Rule 1 for header-only providers, not an +exception to it — the define stays package-owned and namespaced; only its scope +is corrected. + +Why this does **not** reintroduce the vcpkg failure mode ("flags leak into the +ABI, break composition"): + +- **Only the namespaced, library-owned define crosses the boundary** — never + free-form flags. Link flags / include paths still come from the bound + provider's own build config (Rule 1 intact). +- **Visibility-bounded.** Propagation follows the same Public/Interface edges as + include dirs; a `private` dependency edge keeps the define off the consumer's + public interface. +- **ODR-safe by single-instance propagation.** Activation is unioned onto a + single shared provider instance (Cargo model); propagation flows outward from + that one `publicUsage`, so every consumer of the provider sees the *same* + define set. A header-only library compiled with the switch in one TU and + without it in another would be an ODR violation — single-instance propagation + structurally prevents that split. +- **The automatic `MCPP_FEATURE_` macro is deliberately NOT propagated.** + It is not namespaced by the library (two packages may each declare a + `use_blas` feature → colliding `MCPP_FEATURE_USE_BLAS`), so it stays private to + the owning package as a local build signal. Only the namespaced user define is + an interface contract — which reinforces, rather than relaxes, Rule 1. + +Simplicity note (少即是多): *all* feature defines are interface defines; mcpp does +**not** add a CMake-style PUBLIC/PRIVATE/INTERFACE tri-state for defines — that is +precisely the complexity this design avoids. A define that happens to matter only +to the provider's own `.cpp` still propagates, but lands in consumers as an +unused, namespaced `-D` (harmless). Should a genuinely provider-private feature +define ever be needed, a `private-defines` key is the future-proofing escape +hatch; it is YAGNI today. + ## 4. The model — two primitives ### Primitive ① Feature — additive, composable, does only this diff --git a/mcpp.toml b/mcpp.toml index 91d0e07..7e71554 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,6 +1,6 @@ [package] name = "mcpp" -version = "0.0.71" +version = "0.0.72" description = "Modern C++ build & package management tool" license = "Apache-2.0" authors = ["mcpp-community"] diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 6225b8f..cc7d1b0 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -1343,6 +1343,19 @@ prepare_build(bool print_fingerprint, return changed; }; + auto appendUniqueFlags = + [](std::vector& flags, + const std::vector& additions) -> bool + { + bool changed = false; + for (auto const& f : additions) { + if (std::find(flags.begin(), flags.end(), f) != flags.end()) continue; + flags.push_back(f); + changed = true; + } + return changed; + }; + auto expandIncludeDirs = [&](const std::filesystem::path& packageRoot, const mcpp::manifest::Manifest& manifest) @@ -1424,12 +1437,28 @@ prepare_build(bool print_fingerprint, changed = appendUniquePaths(consumer.privateBuild.includeDirs, dependency.publicUsage.includeDirs) || changed; + // Interface defines (a dependency's active-feature `defines`) + // ride the same edges as include dirs: they must reach the + // consumer's own TUs so header-only switches like + // EIGEN_USE_BLAS take effect where the headers are used. + changed = appendUniqueFlags(consumer.privateBuild.cflags, + dependency.publicUsage.cflags) + || changed; + changed = appendUniqueFlags(consumer.privateBuild.cxxflags, + dependency.publicUsage.cxxflags) + || changed; } if (edge.visibility == mcpp::modgraph::DependencyVisibility::Public || edge.visibility == mcpp::modgraph::DependencyVisibility::Interface) { changed = appendUniquePaths(consumer.publicUsage.includeDirs, dependency.publicUsage.includeDirs) || changed; + changed = appendUniqueFlags(consumer.publicUsage.cflags, + dependency.publicUsage.cflags) + || changed; + changed = appendUniqueFlags(consumer.publicUsage.cxxflags, + dependency.publicUsage.cxxflags) + || changed; } } } @@ -2172,6 +2201,17 @@ prepare_build(bool print_fingerprint, pkg.manifest.buildConfig.cxxflags.push_back(fdef); pkg.privateBuild.cflags.push_back(fdef); pkg.privateBuild.cxxflags.push_back(fdef); + // Interface-propagate the user-declared feature define: + // a header-only dependency's switch (e.g. EIGEN_USE_BLAS) + // only takes effect in the TU that includes its headers, + // so consumers that enable the feature must see it too. + // computeUsageRequirements() flows publicUsage flags into + // each consumer's privateBuild along Public/Interface + // edges, mirroring include_dirs. The automatic + // MCPP_FEATURE_ macro stays private to the owning + // package (it is a build signal, not a public contract). + pkg.publicUsage.cflags.push_back(fdef); + pkg.publicUsage.cxxflags.push_back(fdef); } } // Feature-gated sources (e.g. gtest's gtest_main.cc behind "main"): @@ -2257,6 +2297,13 @@ prepare_build(bool print_fingerprint, apply(packages[i], req); } + // apply() may have added interface defines to packages' publicUsage + // flags (a dependency's active-feature `defines`). Re-run the usage + // fixpoint so those flags flow into each consumer's privateBuild — the + // first pass (above) ran before features were activated. Idempotent: + // include-dir/flag propagation is unique-append. + computeUsageRequirements(); + // ─── Capability binding (Stage 3) ────────────────────────────────── // For each required capability, bind exactly one provider from the // graph. Deterministic: an explicit [capabilities] pin wins; otherwise diff --git a/src/toolchain/fingerprint.cppm b/src/toolchain/fingerprint.cppm index 94ca17b..4a30176 100644 --- a/src/toolchain/fingerprint.cppm +++ b/src/toolchain/fingerprint.cppm @@ -18,7 +18,7 @@ import mcpp.toolchain.detect; export namespace mcpp::toolchain { -inline constexpr std::string_view MCPP_VERSION = "0.0.71"; +inline constexpr std::string_view MCPP_VERSION = "0.0.72"; struct FingerprintInputs { Toolchain toolchain; diff --git a/tests/e2e/83_feature_defines_propagate.sh b/tests/e2e/83_feature_defines_propagate.sh new file mode 100755 index 0000000..660b1dd --- /dev/null +++ b/tests/e2e/83_feature_defines_propagate.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# 83_feature_defines_propagate.sh — Feature System v2: a dependency's active- +# feature `defines` are INTERFACE requirements. When a consumer enables a feature +# on a (header-only) dependency, that feature's `defines` must reach the +# CONSUMER's own translation units — not only the dependency's own compile. This +# is the header-only-library case (e.g. Eigen's `use_blas` → EIGEN_USE_BLAS, +# which only changes behavior in the TU that includes Eigen's headers). The +# define propagates along Public/Interface dependency edges, mirroring +# include_dirs. See .agents/docs/2026-06-29-feature-capability-model-design.md. +# +# No `requires:` capability → runs on all three CI platforms. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +# Header-only dependency: a feature `turbo` carries a package-owned define. +mkdir -p widget/include/widget widget/src +cat > widget/mcpp.toml <<'EOF' +[package] +name = "widget" +version = "0.1.0" + +[features] +default = [] +turbo = { defines = ["WIDGET_TURBO=1"] } + +[build] +include_dirs = ["include"] + +[targets.widget] +kind = "lib" +EOF +cat > widget/include/widget/widget.hpp <<'EOF' +#pragma once +// The macro's value is only meaningful in the TU that includes this header — +// exactly the header-only library situation. +inline int widget_mode() { +#ifdef WIDGET_TURBO + return 1; +#else + return 0; +#endif +} +EOF +cat > widget/src/widget.cppm <<'EOF' +export module widget; +export int widget_anchor() { return 0; } +EOF + +mkdir -p app/src +cat > app/mcpp.toml <<'EOF' +[package] +name = "app" +version = "0.1.0" + +[dependencies] +widget = { path = "../widget", features = ["turbo"] } +EOF +# The consumer's TU asserts the dependency's feature define reached it. If the +# define does NOT propagate, this fails to compile (#error), failing the build. +cat > app/src/main.cpp <<'EOF' +#include +#ifndef WIDGET_TURBO +#error "WIDGET_TURBO did not propagate from widget[turbo] to the consumer" +#endif +int main() { return widget_mode() == 1 ? 0 : 2; } +EOF + +cd app + +# Build: widget[turbo]'s define must reach app/src/main.cpp. A missing +# propagation makes main.cpp hit the #error and the build fails. +"$MCPP" build > b.log 2>&1 || { cat b.log; echo "FAIL: feature define did not propagate to consumer"; exit 1; } + +# Double-check the compile database carries the define on the consumer TU. +grep -q 'WIDGET_TURBO' compile_commands.json || { + echo "FAIL: WIDGET_TURBO missing from consumer compile_commands.json"; cat compile_commands.json; exit 1; } + +# And the produced binary observes turbo mode at runtime. (Binary name is +# platform-dependent — `app` on POSIX, `app.exe` on Windows.) +BIN=$(find target -type f \( -name app -o -name app.exe \) | head -1) +[ -n "$BIN" ] || { echo "FAIL: built binary not found under target/"; exit 1; } +"$BIN"; rc=$? +[ "$rc" -eq 0 ] || { echo "FAIL: binary did not observe WIDGET_TURBO (exit $rc)"; exit 1; } + +echo "OK"