From 9a6fe545272f90986b0f937283e6b530c6bc5799 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Tue, 2 Jun 2026 01:45:01 +0800 Subject: [PATCH 1/2] Implement dependency usage requirements --- .../2026-06-02-imgui-mcpp-dependency-fixes.md | 121 +++++++ ...6-06-02-usage-requirements-architecture.md | 235 +++++++++++++ src/build/flags.cppm | 2 +- src/build/plan.cppm | 12 +- src/cli.cppm | 308 ++++++++++++------ src/manifest.cppm | 14 +- src/modgraph/p1689.cppm | 10 +- src/modgraph/scanner.cppm | 60 +++- src/pm/dep_spec.cppm | 1 + tests/e2e/60_stale_xpkg_cache_reinstall.sh | 160 +++++++++ tests/e2e/61_dependency_visibility.sh | 114 +++++++ tests/unit/test_manifest.cpp | 34 ++ tests/unit/test_modgraph.cpp | 27 ++ tests/unit/test_ninja_backend.cpp | 29 +- 14 files changed, 1006 insertions(+), 121 deletions(-) create mode 100644 .agents/docs/2026-06-02-imgui-mcpp-dependency-fixes.md create mode 100644 .agents/docs/2026-06-02-usage-requirements-architecture.md create mode 100644 tests/e2e/60_stale_xpkg_cache_reinstall.sh create mode 100644 tests/e2e/61_dependency_visibility.sh diff --git a/.agents/docs/2026-06-02-imgui-mcpp-dependency-fixes.md b/.agents/docs/2026-06-02-imgui-mcpp-dependency-fixes.md new file mode 100644 index 0000000..16794c6 --- /dev/null +++ b/.agents/docs/2026-06-02-imgui-mcpp-dependency-fixes.md @@ -0,0 +1,121 @@ +# 2026-06-02 imgui mcpp dependency fixes + +## Scope + +Branch: `codex/imgui-mcpp-dependency-fixes` + +This document tracks the mcpp-side changes discovered while validating the +`imgui-private` package and its `compat.imgui` / `compat.glfw` / +`compat.opengl` dependency chain. The imgui package work stays outside this +branch; this branch is only for mcpp fixes. + +## Root Cause Chain + +The imgui validation exposed two separate mcpp-side problems. + +1. C++ build flags dropped package include dirs. + + `src/build/flags.cppm` built `CompileFlags::cxx` with fewer `{}` format + placeholders than arguments. The final `include_flags` argument was not + emitted into the C++ baseline flags, while `CompileFlags::cc` did include it. + + Impact: module scanning and C++ compilation for the current package could + miss include dirs propagated from direct dependencies. In imgui, this showed + up as scanner failures like `imgui.h` or backend `.cpp` files not found. + + Why gtest did not prove this path: gtest's existing success path did not + exercise the same current-package C++ baseline include propagation shape. It + could compile through its own package metadata while imgui needed direct + dependency include dirs during current-package module/backend scans. + +2. Version dependency lookup reused stale unmarked xpkg directories. + + `loadVersionDep()` accepted an installed path if `install_path()` found a + directory. That bypassed the stricter `.mcpp_ok` install marker semantics + already used in `resolve_xpkg_path()`. + + Impact: an old `compat.glfw@3.4` sandbox directory with layout + `3.4/glfw-3.4/include` and `3.4/glfw-3.4/src` was reused even though the + current index metadata expects `include` and `src` at the version root after + the install hook. mcpp then skipped GLFW's own source files, linked only + imgui and GLFW consumers, and failed with undefined `glfwInit`, + `glfwCreateWindow`, etc. + +## Changes In This Branch + +- `src/build/flags.cppm` + - Fixed the `std::format` placeholder count for `CompileFlags::cxx`, so + `include_flags` is emitted for C++ scan/compile rules. + +- `tests/unit/test_ninja_backend.cpp` + - Added `NinjaBackend.CxxFlagsIncludeBuildIncludeDirs` to lock the C++ include + propagation behavior. + +- `src/cli.cppm` + - In version dependency loading, require either `.mcpp_ok` or a layout that + matches the current index entry's `mcpp` metadata before reusing an existing + xpkg directory. + - Adopt and mark compatible pre-marker installs; clean stale unmarked + directories whose layout no longer matches the index before reinstall. + - Mark the install path complete after a successful version dependency + install. + +- `tests/e2e/60_stale_xpkg_cache_reinstall.sh` + - Added a stale-cache regression that creates an old unmarked xpkg layout and + verifies mcpp reinstalls it instead of linking against the stale contents. + +## Evidence So Far + +- Red test for the include bug: + - `NinjaBackend.CxxFlagsIncludeBuildIncludeDirs` failed before the flags fix + because `flags.cxx` was `-std=c++23 -fmodules -O2` without `-I...`. + +- Green unit suite after the flags fix and current resolver change: + - Command: `mcpp test -- --gtest_filter=NinjaBackend.CxxFlagsIncludeBuildIncludeDirs` + - Result: full mcpp test run completed with `18 passed; 0 failed`. + +- imgui-private validation after clearing stale `compat.glfw@3.4` cache: + - `backend_test ... ok` + - `imgui_test ... ok` + - `test result ok. 2 passed; 0 failed` + +- Red e2e for stale-cache behavior before the resolver fix: + - `tests/e2e/60_stale_xpkg_cache_reinstall.sh` failed with + `undefined reference to stale_value`, proving the stale dependency sources + were not linked. + +- Green e2e after rebuilding the mcpp CLI with the resolver fix: + - Command: + `MCPP=target/x86_64-linux-gnu/85da010ca4e7d6e2/bin/mcpp bash tests/e2e/60_stale_xpkg_cache_reinstall.sh` + - Result: `OK` + +- Boundary regression caught and fixed: + - `tests/e2e/52_local_path_namespaced_index.sh` initially failed after a + marker-only implementation because it intentionally pre-populates a + compatible project-local xpkg layout without `.mcpp_ok`. + - The resolver now adopts an unmarked directory only when the current index's + `mcpp` metadata can resolve the package source/manifest from that root. + +- Latest focused verification with rebuilt CLI: + - `mcpp test`: `18 passed; 0 failed` + - `tests/e2e/52_local_path_namespaced_index.sh`: `OK` + - `tests/e2e/54_package_owned_ldflags.sh`: `OK` + - `tests/e2e/55_dependency_shared_artifact.sh`: `OK` + - `tests/e2e/56_transitive_shared_artifact.sh`: `OK` + - `tests/e2e/57_static_dep_shared_artifact.sh`: `OK` + - `tests/e2e/60_stale_xpkg_cache_reinstall.sh`: `OK` + +## Pending Verification + +- Re-run imgui-private with the rebuilt mcpp binary without manually deleting + package directories. +- Before opening a PR, decide whether to also run the full e2e suite or leave + that to CI after the focused dependency set above. + +## Notes + +- The imgui package itself is currently not the blocker after a fresh GLFW + install. The observed backend tests pass once the dependency cache layout is + corrected. +- The mcpp fixes should be submitted as a separate PR from imgui package/index + updates. diff --git a/.agents/docs/2026-06-02-usage-requirements-architecture.md b/.agents/docs/2026-06-02-usage-requirements-architecture.md new file mode 100644 index 0000000..ca7b734 --- /dev/null +++ b/.agents/docs/2026-06-02-usage-requirements-architecture.md @@ -0,0 +1,235 @@ +# Module-First Usage Requirements Architecture + +## Context + +The ImGui module experiment exposed two separate issues in mcpp: + +1. C++ compile flags computed `include_flags` but did not place them in the + final `cxxflags` string. That is a correctness bug in flag assembly. +2. The current dependency include model is too coarse for a module-first + package ecosystem. `build.include_dirs` is used both to build a package and + as the package's consumer-visible include surface. + +For traditional header packages the symmetric behavior is convenient. For +module packages it is too broad: a package can include upstream headers +internally while exposing only `import package.module;` to consumers. + +This document is the live target for the current PR. The architecture should +not be a thin wrapper around `BuildConfig.includeDirs`; it should make package +usage requirements explicit in the resolved package graph and move consumers +onto that graph incrementally inside this PR. + +## Design Goals + +- Keep existing package descriptors working during migration. +- Treat modules as the primary public surface for module packages. +- Distinguish package-private build requirements from consumer-visible usage + requirements. +- Make scanner, P1689 scan, compile commands, and real compile rules consume + the same resolved usage model. +- Keep link/runtime propagation separate from header/include propagation. +- Avoid forcing users of module packages to see or declare implementation + headers. + +## Terms + +- Private build requirements: + - Requirements needed to compile a package's own sources. + - Example: `mcpplibs.imgui` wrappers need Dear ImGui backend headers + internally. +- Public usage requirements: + - Requirements that consumers inherit when depending on the package. + - Example: `compat.glfw` exports `GLFW/glfw3.h` as a traditional header + surface. +- Link requirements: + - Libraries and linker flags that follow objects into final link units. + - These are independent from whether headers are public. + +## Target Data Model + +The resolved build graph should own usage requirements as first-class data +rather than mutating manifests during dependency resolution: + +```cpp +enum class Visibility { + Private, + Public, + Interface, +}; + +struct UsageRequirements { + std::vector includeDirs; + std::vector cflags; + std::vector cxxflags; + std::vector ldflags; + std::vector modules; +}; + +struct PackageNode { + Manifest manifest; + UsageRequirements privateBuild; + UsageRequirements publicUsage; + UsageRequirements linkUsage; +}; + +struct DependencyEdge { + PackageId from; + PackageId to; + Visibility visibility; +}; +``` + +The important property is that `Manifest` remains the parsed package +description. Resolved requirements live in the package graph and build plan, +not by appending dependency state back into the manifest. + +## Dependency Visibility Semantics + +Use CMake-like visibility: + +- `private` + - The dependency is needed to compile/link the package itself. + - Its usage requirements are not propagated to consumers. +- `public` + - The dependency is needed to compile/link the package itself. + - Its public usage requirements are propagated to consumers. +- `interface` + - The package itself does not compile/link against the dependency. + - Consumers inherit the dependency's public usage requirements. + +Future manifest surface: + +```toml +[dependencies.compat] +imgui = { version = "1.92.8", visibility = "private" } +glfw = { version = "3.4", visibility = "private" } +``` + +Compatibility rule: old dependency declarations default to `public`, matching +current mcpp behavior. + +## Build Graph Behavior + +Resolution should build a package graph: + +1. Parse manifests without rewriting them. +2. Create `PackageNode` entries for root and dependencies. +3. Create `DependencyEdge` entries with resolved visibility. +4. Compute each node's effective private build requirements from: + - the package's own build requirements, + - private/public dependency requirements, + - platform/toolchain requirements. +5. Compute each node's public usage requirements from: + - the package's declared public/interface requirements, + - public/interface dependency requirements. +6. Compute link closure independently from include/header closure. + +Scanner, P1689 scan, compile commands, and Ninja compile rules should all derive +from each compile unit's resolved private build requirements. A module import +edge should require BMI/object/link availability, not implementation header +visibility. + +## ImGui Implications + +`imgui-private` should model upstream headers as private implementation +requirements: + +- `compat.imgui`: private build/link requirement for wrappers and backend + implementation files. +- `compat.glfw`: private build/link requirement for GLFW backend modules. +- `compat.opengl`: private build requirement for OpenGL backend headers. + +Consumers of the module package should only need: + +```cpp +import imgui.core; +import imgui.backend.glfw_opengl3; +``` + +Consumers that want to write direct GLFW/OpenGL header code should declare +`compat.glfw` or `compat.opengl` themselves. + +## Current PR Plan + +1. Parse dependency visibility: + - Add `visibility = "private" | "public" | "interface"` to dependency + specs. + - Default omitted visibility to `public`, preserving existing packages. +2. Introduce resolved usage graph data: + - Add `UsageRequirements` and dependency edge visibility to the package + graph surface currently represented by `PackageRoot`. + - Keep parsed `Manifest` immutable during dependency resolution for include + propagation. +3. Compute usage requirements during dependency resolution: + - Map a package's legacy `build.include_dirs` to its own private build + requirements and public usage requirements. + - For `private` and `public` edges, make dependency public usage visible to + the dependent package's private build. + - For `public` and `interface` edges, propagate dependency public usage to + the dependent package's public usage. +4. Make build consumers use resolved usage: + - Regex scanner, P1689 scanner, compile commands, and Ninja compile rules + should read the same per-package resolved private build include dirs. + - Keep link flag propagation in the existing path for this PR; link usage is + a separate axis in the model and should be migrated after include usage is + proven by tests. + - Keep `build.cflags` and `build.cxxflags` package-private in this PR; + public compile definitions/options need a dedicated manifest surface + before they should propagate to consumers. +5. Preserve the already discovered fixes: + - Keep the C++ `include_flags` assembly regression fix. + - Keep stale unmarked xpkg install recovery. +6. Release path: + - Open PR, wait for CI, and admin squash merge after approval. + - Do not trigger the `0.0.43` release from this PR. + - Defer version bump / release / package-index / xlings-res rollout until + the next requested scheme is implemented and released together. + +## Current PR Scope + +Implement: + +- The C++ `include_flags` assembly fix. +- Focused tests for the C++ include flag behavior. +- The stale unmarked xpkg install recovery already discovered during ImGui + validation. +- Dependency visibility parsing and validation. +- Resolved include usage requirements for `PackageRoot`. +- Dependency-edge include propagation without mutating parsed manifests. +- Scanner/build-plan consumption of resolved include usage. +- PR CI and admin squash merge. + +Leave for follow-up after this PR: + +- Full `linkUsage` migration away from `Manifest::buildConfig.ldflags` + mutation. +- `0.0.43` version bump and release trigger. +- package-index / xlings-res rollout for the released mcpp version. +- Generated module wrapper surface expansion for ImGui. +- mcpp-index publication of ImGui after the module package has a public source + repository and the required mcpp release is available. + +## Live Status + +- Done: dependency specs carry a `visibility` field with parser validation for + TOML descriptors. +- Done: C++ `include_flags` assembly fix and unit coverage. +- Done: stale unmarked xpkg cache recovery path and e2e coverage. +- Done: resolved include usage graph and scanner/build-plan consumers. +- Done: targeted unit/e2e coverage for dependency visibility propagation. +- Done: compatibility checks for existing public transitive include behavior. +- Done: local verification with `mcpp test` and full e2e suite. +- Pending: PR, CI, and admin squash merge. +- Deferred by user decision: `0.0.43` version bump, release trigger, + xim-pkgindex update, and xlings-res rollout. + +## Local Verification + +- `mcpp build`: passed. +- `mcpp test`: passed (`18 passed; 0 failed`). +- `tests/e2e/run_all.sh`: passed (`65 passed, 0 failed, 0 skipped`). +- Targeted checks: + - `tests/e2e/31_transitive_deps.sh`: passed. + - `tests/e2e/50_package_owned_build_flags.sh`: passed. + - `tests/e2e/60_stale_xpkg_cache_reinstall.sh`: passed. + - `tests/e2e/61_dependency_visibility.sh`: passed. diff --git a/src/build/flags.cppm b/src/build/flags.cppm index b9be1fb..235ee37 100644 --- a/src/build/flags.cppm +++ b/src/build/flags.cppm @@ -243,7 +243,7 @@ CompileFlags compute_flags(const BuildPlan& plan) { } std::string cxx_std_flag = plan.cppStandardFlag.empty() ? std::string("-std=c++23") : plan.cppStandardFlag; - f.cxx = std::format("{}{}{}{}{}{}{}{}{}", cxx_std_flag, module_flag, std_module_flag, + f.cxx = std::format("{}{}{}{}{}{}{}{}{}{}", cxx_std_flag, module_flag, std_module_flag, std_compat_module_flag, prebuilt_module_flag, opt_flag, pic_flag, compile_toolchain_flags, b_flag, include_flags); f.cc = std::format("-std={}{}{}{}{}{}", c_std, opt_flag, pic_flag, compile_toolchain_flags, diff --git a/src/build/plan.cppm b/src/build/plan.cppm index d859834..b44fe3d 100644 --- a/src/build/plan.cppm +++ b/src/build/plan.cppm @@ -408,9 +408,15 @@ BuildPlan make_plan(const mcpp::manifest::Manifest& manifest, main_cu.source = *lu.entryMain; main_cu.object = std::filesystem::path("obj") / object_filename_for(*lu.entryMain); main_cu.packageName = qualified_package_name(manifest); - main_cu.localIncludeDirs = local_include_dirs_for_manifest(projectRoot, manifest); - main_cu.packageCflags = manifest.buildConfig.cflags; - main_cu.packageCxxflags = manifest.buildConfig.cxxflags; + if (!packages.empty() && packages[0].usageResolved) { + main_cu.localIncludeDirs = packages[0].privateBuild.includeDirs; + main_cu.packageCflags = packages[0].privateBuild.cflags; + main_cu.packageCxxflags = packages[0].privateBuild.cxxflags; + } else { + main_cu.localIncludeDirs = local_include_dirs_for_manifest(projectRoot, manifest); + main_cu.packageCflags = manifest.buildConfig.cflags; + main_cu.packageCxxflags = manifest.buildConfig.cxxflags; + } // We didn't scan main.cpp earlier (it's not in scanner output unless globbed in). // Best-effort: scan its imports here. diff --git a/src/cli.cppm b/src/cli.cppm index 7eb5e6f..e69fca4 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -631,6 +631,16 @@ std::string canonical_package_build_metadata( s += " ldflag:"; s += flag; } + if (pkg.usageResolved) { + for (auto const& dir : pkg.privateBuild.includeDirs) { + s += " private_include:"; + s += dir.generic_string(); + } + for (auto const& dir : pkg.publicUsage.includeDirs) { + s += " public_include:"; + s += dir.generic_string(); + } + } for (auto const& [path, content] : pkg.manifest.buildConfig.generatedFiles) { s += " genfile:"; s += path.generic_string(); @@ -1567,7 +1577,6 @@ prepare_build(bool print_fingerprint, std::string requestedBy; // human-readable for error messages std::string source; // "version" | "path" | "git" — for type-clash check std::size_t depIndex = 0; // index into dep_manifests/packages-1 (for in-place re-fetch) - std::vector includeDirsAdded; // entries appended to m->buildConfig.includeDirs by this dep std::vector linkFlagsAdded; // entries appended to m->buildConfig.ldflags by this dep }; std::map resolved; @@ -1673,15 +1682,67 @@ prepare_build(bool print_fingerprint, depName, indexPath.string())); } + auto findRawInstalled = [&]() -> std::optional { + if (useProjectEnv) { + if (auto p = mcpp::fetcher::Fetcher::install_path_from_project_data( + *root, ns, shortName, version)) { + return p; + } + } + return fetcher.install_path(ns, shortName, version); + }; + + auto installedLayoutMatchesIndex = [&](const std::filesystem::path& verRoot) -> bool { + if (!luaContent) return false; + + auto field = mcpp::manifest::extract_mcpp_field(*luaContent); + if (field.kind == mcpp::manifest::McppField::StringPath) { + return !mcpp::modgraph::expand_glob(verRoot, field.value).empty(); + } + if (field.kind == mcpp::manifest::McppField::TableBody) { + auto dm = mcpp::manifest::synthesize_from_xpkg_lua( + *luaContent, depName, version); + if (!dm) return false; + for (auto const& [generatedPath, _] : dm->buildConfig.generatedFiles) { + if (!generatedPath.empty()) return true; + } + for (auto const& glob : dm->modules.sources) { + if (!glob.empty() && glob.front() == '!') continue; + if (!mcpp::modgraph::expand_glob(verRoot, glob).empty()) { + return true; + } + } + return false; + } + + for (auto pat : { "mcpp.toml", "*/mcpp.toml" }) { + if (!mcpp::modgraph::expand_glob(verRoot, pat).empty()) { + return true; + } + } + return false; + }; + + auto findCompleteInstalled = [&]() -> std::optional { + auto p = findRawInstalled(); + if (!p) return std::nullopt; + if (mcpp::fallback::is_install_complete(*p)) return p; + if (installedLayoutMatchesIndex(*p)) { + mcpp::fallback::mark_install_complete(*p); + return p; + } + mcpp::fallback::clean_incomplete_install(*p); + return std::nullopt; + }; + + auto markInstalled = [&](const std::filesystem::path& p) { + mcpp::fallback::mark_install_complete(p); + }; + // For custom indices, try project-level xlings data roots first. - std::optional installed; - if (useProjectEnv) { - installed = mcpp::fetcher::Fetcher::install_path_from_project_data( - *root, ns, shortName, version); - } - if (!installed) { - installed = fetcher.install_path(ns, shortName, version); - } + // Existing directories without the mcpp completion marker are treated + // as stale/incomplete on this active resolve path and reinstalled. + std::optional installed = findCompleteInstalled(); if (!installed) { if (luaContent) { @@ -1782,15 +1843,10 @@ prepare_build(bool print_fingerprint, return std::unexpected(err); } // After install, check project data first for custom index packages. - if (useProjectEnv) { - installed = mcpp::fetcher::Fetcher::install_path_from_project_data( - *root, ns, shortName, version); - } - if (!installed) { - installed = fetcher.install_path(ns, shortName, version); - } + installed = findRawInstalled(); if (!installed) return std::unexpected(std::format( "package '{}@{}' install path missing after fetch", depName, version)); + markInstalled(*installed); } std::filesystem::path verRoot = *installed; @@ -1874,84 +1930,137 @@ prepare_build(bool print_fingerprint, return std::pair{effRoot, std::move(*manifest)}; }; - auto appendIncludeDirsTo = [&](mcpp::manifest::Manifest& target, - const std::filesystem::path& depRoot, - const mcpp::manifest::Manifest& depManifest) - -> std::vector + struct DependencyEdge { + std::size_t consumerPackageIndex = 0; + std::size_t dependencyPackageIndex = 0; + mcpp::modgraph::DependencyVisibility visibility = + mcpp::modgraph::DependencyVisibility::Public; + }; + std::vector dependencyEdges; + + auto parseVisibility = [](std::string_view visibility) { + if (visibility == "private") + return mcpp::modgraph::DependencyVisibility::Private; + if (visibility == "interface") + return mcpp::modgraph::DependencyVisibility::Interface; + return mcpp::modgraph::DependencyVisibility::Public; + }; + + auto packageIndexForConsumer = [&](std::size_t consumerDepIndex) { + if (consumerDepIndex == kMainConsumer) return std::size_t{0}; + return consumerDepIndex + 1; + }; + + auto appendUniquePath = + [](std::vector& dirs, + const std::filesystem::path& dir) -> bool { - std::vector added; - auto append_unique = [&](const std::filesystem::path& dir) { - auto& dirs = target.buildConfig.includeDirs; - if (std::find(dirs.begin(), dirs.end(), dir) != dirs.end()) return; - dirs.push_back(dir); - added.push_back(dir); - }; - for (auto& inc : depManifest.buildConfig.includeDirs) { + if (std::find(dirs.begin(), dirs.end(), dir) != dirs.end()) return false; + dirs.push_back(dir); + return true; + }; + + auto appendUniquePaths = + [&](std::vector& dirs, + const std::vector& additions) -> bool + { + bool changed = false; + for (auto const& dir : additions) { + changed = appendUniquePath(dirs, dir) || changed; + } + return changed; + }; + + auto expandIncludeDirs = + [&](const std::filesystem::path& packageRoot, + const mcpp::manifest::Manifest& manifest) + { + std::vector dirs; + for (auto const& inc : manifest.buildConfig.includeDirs) { if (inc.is_absolute()) { - append_unique(inc); + appendUniquePath(dirs, inc); continue; } - auto matches = mcpp::modgraph::expand_dir_glob(depRoot, inc.generic_string()); - if (matches.empty()) continue; - for (auto& d : matches) { - append_unique(d); + for (auto& dir : mcpp::modgraph::expand_dir_glob( + packageRoot, inc.generic_string())) { + appendUniquePath(dirs, dir); } } - return added; + return dirs; }; - auto syncMainPackageIncludes = [&] { - if (!packages.empty()) { - packages[0].manifest.buildConfig.includeDirs = m->buildConfig.includeDirs; - } - }; - - // Append a dep's [build].include_dirs onto the main manifest's, glob- - // expanded against the dep's root. Returns the absolute paths actually - // appended so the caller can later evict them on a SemVer-merge re-fetch. - auto propagateIncludeDirs = [&](const std::filesystem::path& depRoot, - const mcpp::manifest::Manifest& depManifest) - -> std::vector + auto makePackageRoot = + [&](const std::filesystem::path& packageRoot, + const mcpp::manifest::Manifest& manifest) { - auto added = appendIncludeDirsTo(*m, depRoot, depManifest); - syncMainPackageIncludes(); - return added; + mcpp::modgraph::PackageRoot pkg; + pkg.root = packageRoot; + pkg.manifest = manifest; + pkg.usageResolved = true; + + pkg.privateBuild.includeDirs = expandIncludeDirs(packageRoot, manifest); + pkg.privateBuild.cflags = manifest.buildConfig.cflags; + pkg.privateBuild.cxxflags = manifest.buildConfig.cxxflags; + pkg.publicUsage.includeDirs = pkg.privateBuild.includeDirs; + pkg.linkUsage.ldflags = manifest.buildConfig.ldflags; + return pkg; }; - auto propagateIncludeDirsToConsumer = + packages[0] = makePackageRoot(*root, *m); + + auto recordDependencyEdge = [&](std::size_t consumerDepIndex, - const std::filesystem::path& depRoot, - const mcpp::manifest::Manifest& depManifest) + std::size_t dependencyPackageIndex, + const mcpp::manifest::DependencySpec& spec) { - if (consumerDepIndex == kMainConsumer) { - (void)propagateIncludeDirs(depRoot, depManifest); + const auto consumerPackageIndex = packageIndexForConsumer(consumerDepIndex); + if (consumerPackageIndex >= packages.size() + || dependencyPackageIndex >= packages.size()) { return; } - if (consumerDepIndex >= dep_manifests.size() - || consumerDepIndex + 1 >= packages.size()) { + const auto visibility = parseVisibility(spec.visibility); + auto same = [&](const DependencyEdge& edge) { + return edge.consumerPackageIndex == consumerPackageIndex + && edge.dependencyPackageIndex == dependencyPackageIndex + && edge.visibility == visibility; + }; + if (std::find_if(dependencyEdges.begin(), dependencyEdges.end(), same) + != dependencyEdges.end()) { return; } - auto added = appendIncludeDirsTo(*dep_manifests[consumerDepIndex], - depRoot, depManifest); - auto& packageManifest = packages[consumerDepIndex + 1].manifest; - for (auto const& dir : added) { - auto& dirs = packageManifest.buildConfig.includeDirs; - if (std::find(dirs.begin(), dirs.end(), dir) == dirs.end()) { - dirs.push_back(dir); - } - } + dependencyEdges.push_back(DependencyEdge{ + .consumerPackageIndex = consumerPackageIndex, + .dependencyPackageIndex = dependencyPackageIndex, + .visibility = visibility, + }); }; - // Drop earlier include_dirs that came from a now-superseded dep version. - // Erases by value match — safe because the outer code only ever appends, - // and on re-fetch we re-record the new entries afterwards. - auto removeIncludeDirs = [&](const std::vector& paths) { - auto& dirs = m->buildConfig.includeDirs; - for (auto& p : paths) { - auto pos = std::find(dirs.begin(), dirs.end(), p); - if (pos != dirs.end()) dirs.erase(pos); + auto computeUsageRequirements = [&] { + bool changed = true; + while (changed) { + changed = false; + for (auto const& edge : dependencyEdges) { + if (edge.consumerPackageIndex >= packages.size() + || edge.dependencyPackageIndex >= packages.size()) { + continue; + } + auto& consumer = packages[edge.consumerPackageIndex]; + auto const& dependency = packages[edge.dependencyPackageIndex]; + + if (edge.visibility == mcpp::modgraph::DependencyVisibility::Private + || edge.visibility == mcpp::modgraph::DependencyVisibility::Public) { + changed = appendUniquePaths(consumer.privateBuild.includeDirs, + dependency.publicUsage.includeDirs) + || changed; + } + if (edge.visibility == mcpp::modgraph::DependencyVisibility::Public + || edge.visibility == mcpp::modgraph::DependencyVisibility::Interface) { + changed = appendUniquePaths(consumer.publicUsage.includeDirs, + dependency.publicUsage.includeDirs) + || changed; + } + } } - syncMainPackageIncludes(); }; auto normalizeDepLdflag = [](const std::filesystem::path& depRoot, @@ -2235,8 +2344,9 @@ prepare_build(bool print_fingerprint, .packageName = mangled, .version = spec.version, }); - packages.push_back({secStage, *dep_manifests.back()}); - auto added = propagateIncludeDirs(secStage, *dep_manifests.back()); + const auto depPackageIndex = packages.size(); + packages.push_back(makePackageRoot(secStage, *dep_manifests.back())); + recordDependencyEdge(item.consumerDepIndex, depPackageIndex, spec); auto linkFlagsAdded = propagateLinkFlags(secStage, *dep_manifests.back()); ResolvedKey mangledKey{key.ns, mangled}; @@ -2246,7 +2356,6 @@ prepare_build(bool print_fingerprint, .requestedBy = item.requestedBy, .source = "version", .depIndex = dep_manifests.size() - 1, - .includeDirsAdded = std::move(added), .linkFlagsAdded = std::move(linkFlagsAdded), }; @@ -2268,7 +2377,10 @@ prepare_build(bool print_fingerprint, if (*merged == it->second.version) { // The existing pin already satisfies the new constraint — - // no re-fetch needed; just record this consumer. + // no re-fetch needed; just record this consumer edge. + recordDependencyEdge(item.consumerDepIndex, + it->second.depIndex + 1, + spec); continue; } @@ -2309,9 +2421,7 @@ prepare_build(bool print_fingerprint, } } - removeIncludeDirs(it->second.includeDirsAdded); removeLinkFlags(it->second.linkFlagsAdded); - auto added = propagateIncludeDirs(newRoot, newManifest); auto linkFlagsAdded = propagateLinkFlags(newRoot, newManifest); // Replace in dep_manifests + packages. depIndex is the slot @@ -2319,10 +2429,12 @@ prepare_build(bool print_fingerprint, // packages[depIndex+1] is the same dep. *dep_manifests[it->second.depIndex] = std::move(newManifest); packages[it->second.depIndex + 1] = - {newRoot, *dep_manifests[it->second.depIndex]}; + makePackageRoot(newRoot, *dep_manifests[it->second.depIndex]); + recordDependencyEdge(item.consumerDepIndex, + it->second.depIndex + 1, + spec); it->second.version = *merged; - it->second.includeDirsAdded = std::move(added); it->second.linkFlagsAdded = std::move(linkFlagsAdded); if (it->second.depIndex < dep_cache_identities.size()) dep_cache_identities[it->second.depIndex].version = *merged; @@ -2342,15 +2454,14 @@ prepare_build(bool print_fingerprint, continue; } // Same key, same version (or compatible path/git) — already - // processed; still attach its public include dirs to this - // consumer before skipping. Include propagation is per edge, not - // per unique package: two consumers can need the same dep's - // headers even though the dep itself is fetched/scanned once. + // processed; still record the dependency edge before skipping. + // Usage propagation is per edge, not per unique package: two + // consumers can need the same dep's public surface even though + // the dep itself is fetched/scanned once. if (it->second.depIndex + 1 < packages.size()) { - auto const& existing = packages[it->second.depIndex + 1]; - propagateIncludeDirsToConsumer(item.consumerDepIndex, - existing.root, - existing.manifest); + recordDependencyEdge(item.consumerDepIndex, + it->second.depIndex + 1, + spec); } continue; } @@ -2465,22 +2576,12 @@ prepare_build(bool print_fingerprint, } } - // Propagate dep's [build].include_dirs to the main manifest. The - // returned vector is what was actually appended (after glob - // expansion against dep_root) — stash it so a SemVer merge can - // evict these entries on a re-fetch. - auto includeDirsAdded = propagateIncludeDirs(dep_root, *dep_manifest); auto linkFlagsAdded = propagateLinkFlags(dep_root, *dep_manifest); // Move the manifest into stable storage so we can later look it up // by depIndex (the SemVer merger needs to overwrite the slot). dep_manifests.push_back( std::make_unique(std::move(*dep_manifest))); - if (item.consumerDepIndex != kMainConsumer) { - propagateIncludeDirsToConsumer(item.consumerDepIndex, - dep_root, - *dep_manifests.back()); - } dep_cache_identities.push_back({ .indexName = cache_index_name(key.ns), .packageName = name, @@ -2488,7 +2589,9 @@ prepare_build(bool print_fingerprint, ? spec.version : dep_manifests.back()->package.version, }); - packages.push_back({dep_root, *dep_manifests.back()}); + const auto depPackageIndex = packages.size(); + packages.push_back(makePackageRoot(dep_root, *dep_manifests.back())); + recordDependencyEdge(item.consumerDepIndex, depPackageIndex, spec); // Record this dep as resolved so future encounters of the same // (ns, name) hit the fast path (skip / merge / conflict). @@ -2498,7 +2601,6 @@ prepare_build(bool print_fingerprint, .requestedBy = item.requestedBy, .source = sourceKind, .depIndex = dep_manifests.size() - 1, - .includeDirsAdded = std::move(includeDirsAdded), .linkFlagsAdded = std::move(linkFlagsAdded), }; @@ -2518,6 +2620,8 @@ prepare_build(bool print_fingerprint, } } + computeUsageRequirements(); + // Modgraph: regex scanner by default; opt-in to compiler-driven P1689 // scanner via env var MCPP_SCANNER=p1689 (see docs/27). auto scan = [&] { diff --git a/src/manifest.cppm b/src/manifest.cppm index db5671a..c98826e 100644 --- a/src/manifest.cppm +++ b/src/manifest.cppm @@ -497,7 +497,7 @@ std::expected parse_string(std::string_view content, auto is_dep_spec_key = [](std::string_view k) { return k == "path" || k == "version" || k == "git" || k == "rev" || k == "tag" || k == "branch" - || k == "features" || k == "workspace"; + || k == "features" || k == "workspace" || k == "visibility"; }; auto looks_like_inline_dep_spec = [&](const t::Table& sub) { if (sub.empty()) return false; @@ -515,6 +515,16 @@ std::expected parse_string(std::string_view content, if (auto it = sub.find("path"); it != sub.end() && it->second.is_string()) spec.path = it->second.as_string(); if (auto it = sub.find("version"); it != sub.end() && it->second.is_string()) spec.version = it->second.as_string(); if (auto it = sub.find("git"); it != sub.end() && it->second.is_string()) spec.git = it->second.as_string(); + if (auto it = sub.find("visibility"); it != sub.end() && it->second.is_string()) { + spec.visibility = it->second.as_string(); + if (spec.visibility != "public" + && spec.visibility != "private" + && spec.visibility != "interface") { + return std::unexpected(error(origin, std::format( + "[{}.\"{}\"] visibility must be 'public', 'private', or 'interface'", + section, fqName))); + } + } if (auto it = sub.find("rev"); it != sub.end() && it->second.is_string()) { spec.gitRev = it->second.as_string(); spec.gitRefKind = "rev"; @@ -569,7 +579,7 @@ std::expected parse_string(std::string_view content, if (!looks_like_inline_dep_spec(sub)) { return std::unexpected(error(origin, std::format( "[{}.{}] must be a version string or table of " - "(path/version/git/rev/tag/branch/features)", + "(path/version/git/rev/tag/branch/features/visibility)", section, key))); } if (auto r = fill_inline_spec(spec, section, key, sub); !r) return r; diff --git a/src/modgraph/p1689.cppm b/src/modgraph/p1689.cppm index 6062622..b3c01d0 100644 --- a/src/modgraph/p1689.cppm +++ b/src/modgraph/p1689.cppm @@ -49,6 +49,7 @@ scan_file(const std::filesystem::path& source, const std::string& packageName, const mcpp::toolchain::Toolchain& tc, const std::filesystem::path& tmpDir, + const std::vector& includeDirs, std::string_view cppStandardFlag); } // namespace mcpp::modgraph::p1689 @@ -319,6 +320,7 @@ scan_file(const std::filesystem::path& source, const std::string& packageName, const mcpp::toolchain::Toolchain& tc, const std::filesystem::path& tmpDir, + const std::vector& includeDirs, std::string_view cppStandardFlag) { std::error_code ec; @@ -340,8 +342,13 @@ scan_file(const std::filesystem::path& source, std::string std_flag = cppStandardFlag.empty() ? std::string("-std=c++23") : std::string(cppStandardFlag); + std::string include_flags; + for (auto const& dir : includeDirs) { + include_flags += " -I"; + include_flags += shell_escape(dir); + } std::string cmd = std::format( - "{} {} -fmodules{}" + "{} {} -fmodules{}{}" " -fdeps-format=p1689r5" " -fdeps-file={}" " -fdeps-target={}" @@ -350,6 +357,7 @@ scan_file(const std::filesystem::path& source, shell_escape(tc.binaryPath), std_flag, sysroot_flag, + include_flags, shell_escape(ddi), shell_escape(obj), shell_escape(dep), diff --git a/src/modgraph/scanner.cppm b/src/modgraph/scanner.cppm index a29eefd..2b669b4 100644 --- a/src/modgraph/scanner.cppm +++ b/src/modgraph/scanner.cppm @@ -51,12 +51,30 @@ struct ScanResult { ScanResult scan_package(const std::filesystem::path& root, const mcpp::manifest::Manifest& manifest); +enum class DependencyVisibility { + Private, + Public, + Interface, +}; + +struct UsageRequirements { + std::vector includeDirs; + std::vector cflags; + std::vector cxxflags; + std::vector ldflags; + std::vector modules; +}; + // Scan multiple packages (primary + path-based deps) into one combined Graph. // Each SourceUnit retains its own packageName, so validate() applies the // correct naming rules per-package. struct PackageRoot { std::filesystem::path root; mcpp::manifest::Manifest manifest; + UsageRequirements privateBuild; + UsageRequirements publicUsage; + UsageRequirements linkUsage; + bool usageResolved = false; }; ScanResult scan_packages(const std::vector& packages); @@ -337,7 +355,10 @@ local_include_dirs_for(const std::filesystem::path& root, // here — the caller does that after all packages are scanned. void scan_one_into(ScanResult& result, const std::filesystem::path& root, - const mcpp::manifest::Manifest& manifest) + const mcpp::manifest::Manifest& manifest, + const std::vector& localIncludeDirs, + const std::vector& packageCflags, + const std::vector& packageCxxflags) { // Glob exclusion: patterns starting with `!` remove files from the // include set (like .gitignore). @@ -366,8 +387,6 @@ void scan_one_into(ScanResult& result, manifest.package.namespace_.empty() ? manifest.package.name : manifest.package.namespace_ + "." + manifest.package.name; - const auto localIncludeDirs = local_include_dirs_for(root, manifest); - for (auto const& f : all_files) { auto r = scan_file(f, qualifiedName); if (!r) { @@ -375,8 +394,8 @@ void scan_one_into(ScanResult& result, continue; } r->localIncludeDirs = localIncludeDirs; - r->packageCflags = manifest.buildConfig.cflags; - r->packageCxxflags = manifest.buildConfig.cxxflags; + r->packageCflags = packageCflags; + r->packageCxxflags = packageCxxflags; result.graph.units.push_back(std::move(*r)); } } @@ -420,7 +439,9 @@ ScanResult scan_package(const std::filesystem::path& root, const mcpp::manifest::Manifest& manifest) { ScanResult result; - scan_one_into(result, root, manifest); + auto localIncludeDirs = local_include_dirs_for(root, manifest); + scan_one_into(result, root, manifest, localIncludeDirs, + manifest.buildConfig.cflags, manifest.buildConfig.cxxflags); resolve_graph(result); return result; } @@ -428,7 +449,17 @@ ScanResult scan_package(const std::filesystem::path& root, ScanResult scan_packages(const std::vector& packages) { ScanResult result; for (auto const& p : packages) { - scan_one_into(result, p.root, p.manifest); + auto localIncludeDirs = p.usageResolved + ? p.privateBuild.includeDirs + : local_include_dirs_for(p.root, p.manifest); + auto const& packageCflags = p.usageResolved + ? p.privateBuild.cflags + : p.manifest.buildConfig.cflags; + auto const& packageCxxflags = p.usageResolved + ? p.privateBuild.cxxflags + : p.manifest.buildConfig.cxxflags; + scan_one_into(result, p.root, p.manifest, localIncludeDirs, + packageCflags, packageCxxflags); } resolve_graph(result); return result; @@ -445,17 +476,24 @@ ScanResult scan_packages_p1689(const std::vector& packages, for (auto const& g : p.manifest.modules.sources) { for (auto& f : expand_glob(p.root, g)) all_files.insert(f); } - const auto localIncludeDirs = local_include_dirs_for(p.root, p.manifest); + const auto localIncludeDirs = p.usageResolved + ? p.privateBuild.includeDirs + : local_include_dirs_for(p.root, p.manifest); for (auto const& f : all_files) { auto r = mcpp::modgraph::p1689::scan_file( - f, p.manifest.package.name, tc, tmpDir, cppStandardFlag); + f, p.manifest.package.name, tc, tmpDir, + localIncludeDirs, cppStandardFlag); if (!r) { result.errors.push_back(ScanError{ f, 0, r.error() }); continue; } r->localIncludeDirs = localIncludeDirs; - r->packageCflags = p.manifest.buildConfig.cflags; - r->packageCxxflags = p.manifest.buildConfig.cxxflags; + r->packageCflags = p.usageResolved + ? p.privateBuild.cflags + : p.manifest.buildConfig.cflags; + r->packageCxxflags = p.usageResolved + ? p.privateBuild.cxxflags + : p.manifest.buildConfig.cxxflags; result.graph.units.push_back(std::move(*r)); } } diff --git a/src/pm/dep_spec.cppm b/src/pm/dep_spec.cppm index 27ebab0..8ed54d1 100644 --- a/src/pm/dep_spec.cppm +++ b/src/pm/dep_spec.cppm @@ -31,6 +31,7 @@ struct DependencySpec { std::string git; // "https://..." or empty std::string gitRev; // commit / tag / branch (any one) std::string gitRefKind; // "rev" / "tag" / "branch" (for clarity) + std::string visibility = "public"; // public / private / interface bool inheritWorkspace = false; // .workspace = true bool legacyDottedKey = false; // parsed from legacy "ns.name" flat key diff --git a/tests/e2e/60_stale_xpkg_cache_reinstall.sh b/tests/e2e/60_stale_xpkg_cache_reinstall.sh new file mode 100644 index 0000000..c664cbf --- /dev/null +++ b/tests/e2e/60_stale_xpkg_cache_reinstall.sh @@ -0,0 +1,160 @@ +#!/usr/bin/env bash +# requires: gcc fresh-sandbox +# Version dependencies must not reuse stale xpkg directories that predate +# mcpp's .mcpp_ok install marker. Such directories can have an old extracted +# layout that no longer matches the package index's mcpp metadata. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT + +export MCPP_HOME="$TMP/mcpp-home" +source "$(dirname "$0")/_inherit_toolchain.sh" + +INDEX_DIR="$TMP/local-index" +mkdir -p "$INDEX_DIR/pkgs/c" +cat > "$INDEX_DIR/pkgs/c/compat.stale.lua" <<'EOF' +package = { + spec = "1", + namespace = "compat", + name = "compat.stale", + description = "stale cache reinstall test package", + licenses = {"MIT"}, + type = "package", + xpm = { + linux = { + ["1.0.0"] = { + url = "https://example.invalid/stale-1.0.0.tar.gz", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + }, + }, + mcpp = { + language = "c++23", + import_std = false, + sources = { "src/stale.c" }, + targets = { ["stale"] = { kind = "lib" } }, + deps = {}, + }, +} +EOF + +mkdir -p "$TMP/fake-bin" +FAKE_INSTALL_LOG="$TMP/fake-install.log" +cat > "$TMP/fake-bin/xlings" <<'EOF' +#!/usr/bin/env bash +set -e + +if [[ "${1:-}" == "self" && "${2:-}" == "init" ]]; then + mkdir -p "${XLINGS_HOME:?}/subos/default" + printf '{}\n' > "$XLINGS_HOME/subos/default/.xlings.json" + exit 0 +fi + +if [[ "${1:-}" == "update" ]]; then + exit 0 +fi + +if [[ "${1:-}" == "install" ]]; then + printf '%s\n' "$*" > "${FAKE_INSTALL_LOG:?}" + install_root="${XLINGS_PROJECT_DIR:?}/.xlings/data/xpkgs/compat-x-compat.stale/1.0.0" + mkdir -p "$install_root/src" + cat > "$install_root/src/stale.c" <<'SRC' +int stale_value(void) { + return 42; +} +SRC + exit 0 +fi + +if [[ "${1:-}" == "interface" && "${2:-}" == "install_packages" ]]; then + printf '{"kind":"result","exitCode":0}\n' + exit 0 +fi + +exit 0 +EOF +chmod +x "$TMP/fake-bin/xlings" + +cat > "$MCPP_HOME/config.toml" < .mcpp/.xlings/data/xpkgs/compat-x-compat.stale/1.0.0/stale-1.0.0/src/stale.c <<'EOF' +int stale_value(void) { + return 13; +} +EOF + +cat > src/main.cpp <<'EOF' +extern "C" int stale_value(void); +int main() { + return stale_value() == 42 ? 0 : 1; +} +EOF + +cat > mcpp.toml < build.log 2>&1; then + cat build.log + exit 1 +fi + +grep -Fq 'install compat:compat.stale@1.0.0 -y' "$FAKE_INSTALL_LOG" || { + echo "FAIL: stale unmarked xpkg directory should trigger reinstall" + cat build.log + exit 1 +} + +test -f .mcpp/.xlings/data/xpkgs/compat-x-compat.stale/1.0.0/.mcpp_ok || { + echo "FAIL: successful reinstall should be marked complete" + find .mcpp/.xlings/data/xpkgs/compat-x-compat.stale/1.0.0 -maxdepth 2 -print + exit 1 +} + +"$MCPP" run > run.log 2>&1 || { + cat run.log + exit 1 +} + +echo "OK" diff --git a/tests/e2e/61_dependency_visibility.sh b/tests/e2e/61_dependency_visibility.sh new file mode 100644 index 0000000..ec6a0ec --- /dev/null +++ b/tests/e2e/61_dependency_visibility.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# requires: gcc +# Dependency visibility: private deps must be available while compiling the +# dependent package, but their include dirs must not leak to consumers. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +export MCPP_HOME="$TMP/mcpp-home" +source "$(dirname "$0")/_inherit_toolchain.sh" + +cd "$TMP" +mkdir -p hidden/include/hidden hidden/src facade/src app/src + +cat > hidden/include/hidden/value.hpp <<'EOF' +#pragma once +inline int hidden_value() { return 7; } +EOF + +cat > hidden/src/hidden.cppm <<'EOF' +module; +#include +export module hidden; +export int hidden_answer() { return hidden_value(); } +EOF + +cat > hidden/mcpp.toml <<'EOF' +[package] +name = "hidden" +version = "0.1.0" + +[build] +include_dirs = ["include"] + +[modules] +sources = ["src/*.cppm"] + +[targets.hidden] +kind = "lib" +EOF + +cat > facade/src/facade.cppm <<'EOF' +export module facade; +import hidden; +export int facade_answer() { return hidden_answer(); } +EOF + +cat > facade/mcpp.toml <<'EOF' +[package] +name = "facade" +version = "0.1.0" + +[modules] +sources = ["src/*.cppm"] + +[targets.facade] +kind = "lib" + +[dependencies] +hidden = { path = "../hidden", visibility = "private" } +EOF + +cat > app/src/main.cpp <<'EOF' +import facade; +int main() { + return facade_answer() == 7 ? 0 : 1; +} +EOF + +cat > app/mcpp.toml <<'EOF' +[package] +name = "app" +version = "0.1.0" + +[targets.app] +kind = "bin" +main = "src/main.cpp" + +[dependencies] +facade = { path = "../facade" } +EOF + +cd app +"$MCPP" build > build.log 2>&1 || { + cat build.log + echo "private dep should compile its direct consumer" + exit 1 +} +"$MCPP" run > run.log 2>&1 || { + cat run.log + echo "app run failed" + exit 1 +} + +cat > src/main.cpp <<'EOF' +#include +import facade; +int main() { + return hidden_value() == facade_answer() ? 0 : 1; +} +EOF + +if "$MCPP" build > leak.log 2>&1; then + cat leak.log + echo "private dependency include dir leaked to consumer" + exit 1 +fi +grep -q "hidden/value.hpp" leak.log || { + cat leak.log + echo "consumer failed for an unexpected reason" + exit 1 +} + +echo "OK" diff --git a/tests/unit/test_manifest.cpp b/tests/unit/test_manifest.cpp index 5d36739..5c32831 100644 --- a/tests/unit/test_manifest.cpp +++ b/tests/unit/test_manifest.cpp @@ -158,6 +158,40 @@ main = "src/main.cpp" EXPECT_EQ(m->devDependencies.at("gtest").version, "1.15.2"); } +TEST(Manifest, ParsesDependencyVisibility) { + auto m = mcpp::manifest::parse_string(R"( +[package] +name = "x" +version = "0.1.0" +[modules] +sources = ["src/**/*.cppm"] +[targets.x] +kind = "bin" +main = "src/main.cpp" +[dependencies.compat] +imgui = { version = "1.92.8", visibility = "private" } +glfw = { version = "3.4", visibility = "interface" } +opengl = "2026.05.31" +)"); + ASSERT_TRUE(m.has_value()) << m.error().format(); + ASSERT_EQ(m->dependencies.size(), 3u); + EXPECT_EQ(m->dependencies.at("compat.imgui").visibility, "private"); + EXPECT_EQ(m->dependencies.at("compat.glfw").visibility, "interface"); + EXPECT_EQ(m->dependencies.at("compat.opengl").visibility, "public"); +} + +TEST(Manifest, RejectsInvalidDependencyVisibility) { + auto m = mcpp::manifest::parse_string(R"( +[package] +name = "x" +version = "0.1.0" +[dependencies.compat] +imgui = { version = "1.92.8", visibility = "implementation" } +)"); + ASSERT_FALSE(m.has_value()); + EXPECT_NE(m.error().message.find("visibility"), std::string::npos); +} + TEST(Manifest, DefaultTemplateRoundTrip) { auto src = mcpp::manifest::default_template("hello"); auto m = mcpp::manifest::parse_string(src); diff --git a/tests/unit/test_modgraph.cpp b/tests/unit/test_modgraph.cpp index 3d01428..8371871 100644 --- a/tests/unit/test_modgraph.cpp +++ b/tests/unit/test_modgraph.cpp @@ -70,6 +70,33 @@ TEST(Scanner, RecordsPackageLocalIncludeDirs) { std::filesystem::remove_all(dir); } +TEST(Scanner, UsesResolvedPackagePrivateBuildIncludeDirs) { + auto dir = make_tempdir("mcpp-scanner-resolved-includes"); + write(dir / "src" / "foo.cpp", + "int answer() { return 42; }\n"); + std::filesystem::create_directories(dir / "legacy"); + std::filesystem::create_directories(dir / "resolved"); + + mcpp::manifest::Manifest m; + m.package.name = "pkg"; + m.modules.sources = {"src/*.cpp"}; + m.buildConfig.includeDirs = {"legacy"}; + + PackageRoot p{dir, m}; + p.usageResolved = true; + p.privateBuild.includeDirs = {dir / "resolved"}; + + auto r = scan_packages({p}); + ASSERT_TRUE(r.errors.empty()); + ASSERT_EQ(r.graph.units.size(), 1u); + + auto const& dirs = r.graph.units[0].localIncludeDirs; + ASSERT_EQ(dirs.size(), 1u); + EXPECT_EQ(dirs[0], dir / "resolved"); + + std::filesystem::remove_all(dir); +} + TEST(Scanner, PartitionImportFromPrimaryInterface) { // Primary module interface: `export module foo;` → logicalName = "foo". // `import :tls;` resolves to "foo:tls". diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index 01c2740..dddeaa2 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -22,9 +22,21 @@ std::size_t count_occurrences(std::string_view haystack, std::string_view needle return count; } +std::string escaped_include_flag(const std::filesystem::path& path) { + auto s = path.string(); + std::string escaped; + escaped.reserve(s.size()); + for (char c : s) { + if (c == ' ' || c == '$' || c == ':') + escaped.push_back('$'); + escaped.push_back(c); + } + return "-I" + escaped; +} + BuildPlan minimal_plan() { BuildPlan plan; - plan.projectRoot = "/tmp/mcpp-ninja-test"; + plan.projectRoot = std::filesystem::temp_directory_path() / "mcpp-ninja-test"; plan.outputDir = plan.projectRoot / "target" / "test"; plan.manifest.package.name = "objc_rule_test"; plan.manifest.buildConfig.cStandard = "c11"; @@ -100,6 +112,21 @@ TEST(NinjaBackend, CompileCommandsUsesSameCppStandard) { << cdb; } +TEST(NinjaBackend, CxxFlagsIncludeBuildIncludeDirs) { + auto plan = minimal_plan(); + plan.manifest.buildConfig.includeDirs = {"include", "third_party/imgui"}; + + auto flags = compute_flags(plan); + + EXPECT_NE(flags.cxx.find(escaped_include_flag(plan.projectRoot / "include")), + std::string::npos) + << flags.cxx; + EXPECT_NE(flags.cxx.find(escaped_include_flag( + plan.projectRoot / "third_party" / "imgui")), + std::string::npos) + << flags.cxx; +} + TEST(NinjaBackend, RootPackageCxxflagsAreEmittedOncePerUnit) { auto plan = minimal_plan(); plan.manifest.buildConfig.cxxflags = {"-DROOT_FLAG=1"}; From 3bc86e93950c4dfe75332482ad477940eab0c815 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Tue, 2 Jun 2026 01:59:52 +0800 Subject: [PATCH 2/2] Fix ninja include flag test on Windows --- tests/unit/test_ninja_backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index dddeaa2..e1aad6f 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -122,7 +122,7 @@ TEST(NinjaBackend, CxxFlagsIncludeBuildIncludeDirs) { std::string::npos) << flags.cxx; EXPECT_NE(flags.cxx.find(escaped_include_flag( - plan.projectRoot / "third_party" / "imgui")), + plan.projectRoot / std::filesystem::path{"third_party/imgui"})), std::string::npos) << flags.cxx; }