From e43ea8917b4f9134e72ec1cd1fe053da441f77d9 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Sat, 9 May 2026 08:46:28 +0800 Subject: [PATCH] feat: lib-root convention (`src/.cppm` + optional `[lib].path`) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces a Cargo-style "lib root" — a single primary module-interface file that aggregates the package's public API. Two recognised forms: * Convention (no config): `src/.cppm` where `` is the last `.`-segment of `[package].name`. Examples: mcpplibs.tinyhttps → src/tinyhttps.cppm mcpplibs.cmdline → src/cmdline.cppm gtest → src/gtest.cppm * Override: `[lib] path = "src/foo.cppm"` (cargo-style). Either way, the file must `export module ;` (no `:partition` suffix) — partitions go in sibling files and are re-exported from the lib root, mirroring Rust's `lib.rs` aggregating `pub mod`s. Lib-root checks fire only for projects that ship a `kind = "lib"` (or `shared`) target. Pure binaries (mcpp itself, scaffolded `mcpp new`) are unaffected. Validation policy: - explicit `[lib].path` pointing at a missing file → ERROR - convention miss (no `src/.cppm`) → WARNING (soft on-ramp; existing libs aren't broken — they get a polite reminder) - lib-root file exists but `export module …:partition;` → ERROR (lib root must be the primary module, never a partition) - lib-root file exports a different module name than [package].name → ERROR Existing libs that already follow the convention (mcpplibs.cmdline, mcpplibs.templates, mcpplibs.tinyhttps) keep working unchanged. The soft warning gives anyone who doesn't a clear path to compliance without a hard break. Coverage: 4 manifest unit tests + 5 validate tests + a manual smoke on tinyhttps (zero warnings) and a deliberate file-rename reproduction of the warning path. --- src/cli.cppm | 6 +- src/manifest.cppm | 65 ++++++++++++++++++ src/modgraph/validate.cppm | 92 ++++++++++++++++++++++++++ tests/unit/test_manifest.cpp | 61 +++++++++++++++++ tests/unit/test_modgraph.cpp | 124 +++++++++++++++++++++++++++++++++++ 5 files changed, 347 insertions(+), 1 deletion(-) diff --git a/src/cli.cppm b/src/cli.cppm index 454c824..7259a49 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -1332,7 +1332,11 @@ prepare_build(bool print_fingerprint, std::println(stderr, "warning: {}", w.format()); } - auto report = mcpp::modgraph::validate(scan.graph, *m); + auto report = mcpp::modgraph::validate(scan.graph, *m, *root); + for (auto& w : report.warnings) { + if (w.path.empty()) std::println(stderr, "warning: {}", w.message); + else std::println(stderr, "warning: {}: {}", w.path.string(), w.message); + } if (!report.ok()) { std::string msg = "validation errors:\n"; for (auto& e : report.errors) { diff --git a/src/manifest.cppm b/src/manifest.cppm index 05e4f96..c10c01a 100644 --- a/src/manifest.cppm +++ b/src/manifest.cppm @@ -97,6 +97,29 @@ struct TargetEntry { std::string linkage; // "static" | "dynamic" | "" (= auto by libc) }; +// `[lib]` — library "root" interface convention. +// +// Convention-over-configuration: a library package's primary module +// interface lives at `src/.cppm`, where `` is +// the last dotted segment of `[package].name` (e.g. `mcpplibs.tinyhttps` +// → `src/tinyhttps.cppm`). That file declares `export module +// ;` and re-exports the public partitions. The lib +// root then drives: +// * `[modules].exports` default (the lib root's module = the only +// externally-visible base module), +// * `mcpp publish` xpkg generation (consumer just `import ;`), +// * downstream tooling (docs / explain) entry point. +// +// Override the convention with `[lib].path = "src/foo.cppm"` (cargo-style) +// — the file must still `export module ;` (no partition). +// +// Lib-root is only meaningful for projects that ship a `kind = "lib"` +// target. Pure-binary projects (mcpp itself, scaffolded `mcpp new`) +// don't trigger any lib-root checks. +struct LibConfig { + std::filesystem::path path; // explicit override; empty = use convention +}; + // `[pack]` — `mcpp pack` configuration. See docs/35-pack-design.md. // // `default_mode` picks the bundling strategy when the user runs bare @@ -140,6 +163,9 @@ struct Manifest { // [pack] — `mcpp pack` config (see docs/35-pack-design.md). PackConfig packConfig; + // [lib] — library root interface convention (M5.x+). + LibConfig lib; + // M5.0: post-parse computed/inferred state bool usesModules = true; // refined by scanner bool usesImportStd = true; // refined by scanner @@ -182,6 +208,19 @@ McppField extract_mcpp_field(std::string_view luaContent); std::vector list_xpkg_versions(std::string_view luaContent, std::string_view platform); +// Resolve the lib-root path for a manifest: +// 1. `[lib].path` if explicitly set (cargo-style override), +// 2. otherwise the convention `src/.cppm`, where +// `` is the last `.`-segment of [package].name +// (e.g. `mcpplibs.tinyhttps` → `src/tinyhttps.cppm`). +// The returned path is relative to the package root unless the user +// passed an absolute path in `[lib].path`. +std::filesystem::path resolve_lib_root_path(const Manifest& manifest); + +// True if the manifest declares at least one `kind = "lib"` target. +// Lib-root convention only applies when this returns true. +bool has_lib_target(const Manifest& manifest); + // Synthesize a Manifest from an xpkg .lua file's `mcpp = {}` segment. // Used when a fetched dep has no source/mcpp.toml — the index entry's // `mcpp = {}` workaround block carries the missing build info. @@ -494,6 +533,11 @@ std::expected parse_string(std::string_view content, if (auto v = doc->get_string_array("build.cxxflags")) m.buildConfig.cxxflags = *v; if (auto v = doc->get_string("build.c_standard")) m.buildConfig.cStandard = *v; + // [lib] — library root convention (cargo-style). + if (auto v = doc->get_string("lib.path")) { + m.lib.path = *v; + } + // [pack] — `mcpp pack` configuration. See docs/35-pack-design.md. if (auto v = doc->get_string("pack.default_mode")) { const auto& s = *v; @@ -1207,4 +1251,25 @@ license = "Apache-2.0" )", packageName); } +bool has_lib_target(const Manifest& manifest) { + for (auto& t : manifest.targets) { + if (t.kind == Target::Library || t.kind == Target::SharedLibrary) { + return true; + } + } + return false; +} + +std::filesystem::path resolve_lib_root_path(const Manifest& manifest) { + if (!manifest.lib.path.empty()) { + return manifest.lib.path; + } + // Convention: src/.cppm + std::string tail = manifest.package.name; + if (auto p = tail.rfind('.'); p != std::string::npos) { + tail = tail.substr(p + 1); + } + return std::filesystem::path("src") / (tail + ".cppm"); +} + } // namespace mcpp::manifest diff --git a/src/modgraph/validate.cppm b/src/modgraph/validate.cppm index 4a3f43e..8ad2407 100644 --- a/src/modgraph/validate.cppm +++ b/src/modgraph/validate.cppm @@ -24,6 +24,14 @@ struct ValidateReport { ValidateReport validate(const Graph& g, const mcpp::manifest::Manifest& manifest); +// Same as `validate` plus a project-root path used to verify that the +// lib-root convention file actually exists on disk. Pass an empty path +// to disable the on-disk check (used by unit tests that build a Graph +// in memory without writing source files). +ValidateReport validate(const Graph& g, + const mcpp::manifest::Manifest& manifest, + const std::filesystem::path& projectRoot); + bool is_public_package_name(std::string_view name); bool is_forbidden_top_module(std::string_view name); @@ -48,6 +56,13 @@ bool is_forbidden_top_module(std::string_view name) { ValidateReport validate(const Graph& g, const mcpp::manifest::Manifest& manifest) +{ + return validate(g, manifest, /*projectRoot=*/{}); +} + +ValidateReport validate(const Graph& g, + const mcpp::manifest::Manifest& manifest, + const std::filesystem::path& projectRoot) { ValidateReport r; @@ -107,6 +122,83 @@ ValidateReport validate(const Graph& g, } } + // 2.5 Lib-root convention (M5.x+). + // + // For projects that ship a `kind = "lib"` target, expect a primary + // module-interface file at either `[lib].path` (explicit override) or + // `src/.cppm` (default convention). The file must + // declare `export module ;` (no partition suffix); + // partitions go in sibling files and are aggregated by re-exporting + // from the lib root, à la `lib.rs` in cargo. + // + // Pure-binary projects (mcpp itself, scaffolded `mcpp new`) skip this + // check — they have no lib-root concept. + if (mcpp::manifest::has_lib_target(manifest)) { + auto lib_root_rel = mcpp::manifest::resolve_lib_root_path(manifest); + const bool was_explicit = !manifest.lib.path.empty(); + + // On-disk existence check (skipped when projectRoot is empty — + // unit tests can build Graphs in memory without writing files). + if (!projectRoot.empty()) { + auto lib_root_abs = lib_root_rel.is_absolute() + ? lib_root_rel + : (projectRoot / lib_root_rel); + std::error_code ec; + const bool exists = std::filesystem::exists(lib_root_abs, ec); + if (!exists) { + if (was_explicit) { + // Explicit `[lib].path` pointing at a missing file is + // always an error. + r.errors.push_back({lib_root_rel, std::format( + "[lib].path '{}' does not exist", lib_root_rel.string())}); + } else { + // Convention miss is a warning — gives existing projects + // a soft on-ramp before they rename / move files. + r.warnings.push_back({lib_root_rel, std::format( + "lib target without conventional lib root '{}' " + "(create the file or set [lib].path)", + lib_root_rel.string())}); + } + } + } + + // Even without on-disk verification we can still cross-check the + // graph: if a unit at the lib-root path is present, it must + // export `` exactly (no partition). + const mcpp::modgraph::SourceUnit* lib_unit = nullptr; + for (auto& u : g.units) { + // Match relative or absolute — projectRoot may be empty in + // tests, so we just compare path tails. + auto u_rel = u.path.is_absolute() && !projectRoot.empty() + ? std::filesystem::relative(u.path, projectRoot) + : u.path; + if (u_rel == lib_root_rel || u.path == lib_root_rel) { + lib_unit = &u; + break; + } + } + if (lib_unit) { + if (!lib_unit->provides) { + r.errors.push_back({lib_unit->path, std::format( + "lib root '{}' must declare `export module {};`", + lib_root_rel.string(), manifest.package.name)}); + } else { + const auto& m = lib_unit->provides->logicalName; + if (m.find(':') != std::string::npos) { + r.errors.push_back({lib_unit->path, std::format( + "lib root '{}' exports a partition '{}' — must be the " + "primary module '{}' (no `:partition` suffix)", + lib_root_rel.string(), m, manifest.package.name)}); + } else if (m != manifest.package.name) { + r.errors.push_back({lib_unit->path, std::format( + "lib root '{}' exports module '{}', expected '{}' " + "(must match [package].name)", + lib_root_rel.string(), m, manifest.package.name)}); + } + } + } + } + // 3. Topology auto topo = topo_sort(g); if (!topo) { diff --git a/tests/unit/test_manifest.cpp b/tests/unit/test_manifest.cpp index 54e79cc..7087973 100644 --- a/tests/unit/test_manifest.cpp +++ b/tests/unit/test_manifest.cpp @@ -330,6 +330,67 @@ package = { EXPECT_EQ(b.version, "0.0.2"); } +TEST(Manifest, LibRootInferredFromPackageName) { + constexpr auto src = R"( +[package] +name = "mcpplibs.tinyhttps" +version = "0.2.0" +[targets.tinyhttps] +kind = "lib" +)"; + auto m = mcpp::manifest::parse_string(src); + ASSERT_TRUE(m.has_value()) << m.error().format(); + EXPECT_TRUE(m->lib.path.empty()); + EXPECT_TRUE(mcpp::manifest::has_lib_target(*m)); + auto root = mcpp::manifest::resolve_lib_root_path(*m); + EXPECT_EQ(root.string(), "src/tinyhttps.cppm"); +} + +TEST(Manifest, LibRootBareNameNoNamespace) { + constexpr auto src = R"( +[package] +name = "gtest" +version = "1.0.0" +[targets.gtest] +kind = "lib" +)"; + auto m = mcpp::manifest::parse_string(src); + ASSERT_TRUE(m.has_value()) << m.error().format(); + auto root = mcpp::manifest::resolve_lib_root_path(*m); + EXPECT_EQ(root.string(), "src/gtest.cppm"); +} + +TEST(Manifest, LibRootExplicitOverride) { + constexpr auto src = R"( +[package] +name = "mcpplibs.tinyhttps" +version = "0.2.0" +[lib] +path = "src/api.cppm" +[targets.tinyhttps] +kind = "lib" +)"; + auto m = mcpp::manifest::parse_string(src); + ASSERT_TRUE(m.has_value()) << m.error().format(); + EXPECT_EQ(m->lib.path.string(), "src/api.cppm"); + auto root = mcpp::manifest::resolve_lib_root_path(*m); + EXPECT_EQ(root.string(), "src/api.cppm"); +} + +TEST(Manifest, HasLibTargetFalseForBareBinaryManifest) { + // No [targets.*] declared → parse_string leaves targets empty. + // load() would later infer a bin/lib from sources, but parse_string + // alone leaves it bare; either way no lib target. + constexpr auto src = R"( +[package] +name = "mcpp" +version = "0.0.2" +)"; + auto m = mcpp::manifest::parse_string(src); + ASSERT_TRUE(m.has_value()) << m.error().format(); + EXPECT_FALSE(mcpp::manifest::has_lib_target(*m)); +} + TEST(ListXpkgVersions, IgnoresCommentedEntries) { constexpr auto src = R"( package = { diff --git a/tests/unit/test_modgraph.cpp b/tests/unit/test_modgraph.cpp index f872b0f..416b21c 100644 --- a/tests/unit/test_modgraph.cpp +++ b/tests/unit/test_modgraph.cpp @@ -152,6 +152,130 @@ TEST(Validate, ForbiddenTopName) { EXPECT_FALSE(rep.ok()); } +TEST(Validate, LibRootHappyPath) { + // Project: lib target "tinyhttps", convention puts the lib root at + // src/tinyhttps.cppm exporting `mcpplibs.tinyhttps`. Two partition + // siblings sit alongside. + Graph g; + SourceUnit root; + root.path = "src/tinyhttps.cppm"; + root.packageName = "mcpplibs.tinyhttps"; + root.provides = ModuleId{"mcpplibs.tinyhttps"}; + g.units.push_back(root); + g.producerOf["mcpplibs.tinyhttps"] = 0; + SourceUnit p1; + p1.path = "src/tls.cppm"; + p1.packageName = "mcpplibs.tinyhttps"; + p1.provides = ModuleId{"mcpplibs.tinyhttps:tls"}; + g.units.push_back(p1); + g.producerOf["mcpplibs.tinyhttps:tls"] = 1; + + mcpp::manifest::Manifest m; + m.package.name = "mcpplibs.tinyhttps"; + mcpp::manifest::Target t; + t.name = "tinyhttps"; + t.kind = mcpp::manifest::Target::Library; + m.targets.push_back(t); + + auto rep = validate(g, m); // empty projectRoot → on-disk check skipped + EXPECT_TRUE(rep.ok()) << "errors:" << [&]{ + std::string s; for (auto& e : rep.errors) s += "\n " + e.message; return s; + }(); +} + +TEST(Validate, LibRootExportsPartitionIsError) { + // Lib root file at the conventional path exports `:foo` (a partition + // suffix) — must be rejected: lib root must be the primary module. + Graph g; + SourceUnit u; + u.path = "src/tinyhttps.cppm"; + u.packageName = "mcpplibs.tinyhttps"; + u.provides = ModuleId{"mcpplibs.tinyhttps:something"}; + g.units.push_back(u); + g.producerOf["mcpplibs.tinyhttps:something"] = 0; + + mcpp::manifest::Manifest m; + m.package.name = "mcpplibs.tinyhttps"; + mcpp::manifest::Target t; + t.name = "tinyhttps"; + t.kind = mcpp::manifest::Target::Library; + m.targets.push_back(t); + + auto rep = validate(g, m); + EXPECT_FALSE(rep.ok()); + bool found = false; + for (auto& e : rep.errors) { + if (e.message.find("partition") != std::string::npos + && e.message.find("primary module") != std::string::npos) { found = true; break; } + } + EXPECT_TRUE(found) << "expected lib-root partition error"; +} + +TEST(Validate, LibRootWrongModuleNameIsError) { + // Lib root file exports a module that doesn't match [package].name. + Graph g; + SourceUnit u; + u.path = "src/tinyhttps.cppm"; + u.packageName = "mcpplibs.tinyhttps"; + u.provides = ModuleId{"some.other.module"}; + g.units.push_back(u); + g.producerOf["some.other.module"] = 0; + + mcpp::manifest::Manifest m; + m.package.name = "mcpplibs.tinyhttps"; + mcpp::manifest::Target t; + t.name = "tinyhttps"; + t.kind = mcpp::manifest::Target::Library; + m.targets.push_back(t); + + auto rep = validate(g, m); + EXPECT_FALSE(rep.ok()); + bool found = false; + for (auto& e : rep.errors) { + if (e.message.find("expected 'mcpplibs.tinyhttps'") != std::string::npos) { + found = true; break; + } + } + EXPECT_TRUE(found) << "expected expected-module-name error"; +} + +TEST(Validate, LibRootNotEnforcedForBinaryProject) { + // Pure-binary project: no lib target → no lib-root checks. Even if a + // file at src/.cppm exists exporting an unrelated module, no + // error should fire. + Graph g; + mcpp::manifest::Manifest m; + m.package.name = "myapp"; + mcpp::manifest::Target t; + t.name = "myapp"; + t.kind = mcpp::manifest::Target::Binary; + t.main = "src/main.cpp"; + m.targets.push_back(t); + + auto rep = validate(g, m); + EXPECT_TRUE(rep.ok()); +} + +TEST(Validate, LibRootMissingFileWithExplicitPathIsError) { + Graph g; + mcpp::manifest::Manifest m; + m.package.name = "myorg.foo"; + m.lib.path = "src/does-not-exist.cppm"; + mcpp::manifest::Target t; + t.name = "foo"; + t.kind = mcpp::manifest::Target::Library; + m.targets.push_back(t); + + // Pass a non-empty projectRoot so the on-disk check is enabled. + auto rep = validate(g, m, std::filesystem::current_path()); + EXPECT_FALSE(rep.ok()); + bool found = false; + for (auto& e : rep.errors) { + if (e.message.find("does not exist") != std::string::npos) { found = true; break; } + } + EXPECT_TRUE(found) << "expected explicit-path-missing error"; +} + TEST(TopoSort, DetectsCycle) { Graph g; g.units.resize(2);