From 3e419e40040505aacc28a8d6ee702e031134f914 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Sat, 9 May 2026 18:21:42 +0800 Subject: [PATCH] feat(deps): walk transitive dependency graph + detect version conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this patch the dep loop only iterated `m->dependencies` once — direct deps' `[build].include_dirs` propagated, but their own `[dependencies]` didn't get fetched, didn't appear in the modgraph, and their includes weren't visible while compiling them. Concretely: a project depending on `mcpplibs.tinyhttps` would have to also explicitly list `mbedtls = "3.6.1"`, otherwise tinyhttps's `tls.cppm` failed to find ``. Replace the single-pass loop with a worklist algorithm: worklist = seed from main manifest's [dependencies] (and [dev-dependencies] when --tests) resolved = map<(ns, name), {version, requestedBy, source}> while worklist not empty: item = pop pin SemVer constraint to a concrete version (one shared lambda) key = (item.ns, item.shortName) if resolved[key] exists: compare source kinds (path/git/version) — clash = error compare exact versions — clash = error same → skip (already processed) fetch the dep, load its manifest, propagate its include_dirs, record into resolved[key] and packages for each child in dep_manifest.dependencies: worklist.push(child, requestedBy = this dep) `[dev-dependencies]` are seeded from the main manifest only; transitive walks intentionally skip them because dev-deps are private to the package's own test runs, not part of its public ABI. Conflict messages name both requesters so users can see the path through the graph that produced the disagreement, e.g. dependency 'mcpp.D' has conflicting versions in the transitive graph: '1.0.0' requested by 'B@2.0' '2.0.0' requested by 'C@1.5' C++ modules require a single global version of each package. (Single-version semantics is forced by C++ modules — module names + ODR are global, multiple versions of the same package can't coexist in one build. Same constraint as cargo for non-Rust targets, vcpkg, conan, etc.) Storage detail: dep manifests are now held by `vector` so PackageRoot's reference into the manifest stays stable when the worklist appends new entries during the walk. Coverage: * `tests/e2e/31_transitive_deps.sh` — three-level path-dep chain (top → ch → gc): top declares only ch, gc's include_dirs reach ch's compile rule, link succeeds, runtime returns the right answer. Also exercises a duplicate-but-consistent dep reachable through two edges. * Verified end-to-end against `mcpplibs.llmapi`: a fresh project that declares only `mcpplibs.llmapi = "0.2.4"` builds and links cleanly, with mbedtls + mcpplibs.tinyhttps pulled automatically by the walker. --- src/cli.cppm | 176 ++++++++++++++++++++++++-------- tests/e2e/31_transitive_deps.sh | 109 ++++++++++++++++++++ 2 files changed, 245 insertions(+), 40 deletions(-) create mode 100755 tests/e2e/31_transitive_deps.sh diff --git a/src/cli.cppm b/src/cli.cppm index 7259a49..16247a3 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -1027,54 +1027,126 @@ prepare_build(bool print_fingerprint, tc->label())); } - // Resolve dependencies: path-based, then version-based (via xlings). - // Both end up as PackageRoot entries fed into the same scanner pipeline. + // Resolve dependencies: walk the **transitive** graph from the main + // manifest, BFS-style. Each unique `(namespace, shortName)` is fetched + // once, its `[build].include_dirs` are propagated to the main + // manifest, and its own `[dependencies]` are queued for processing + // (its `[dev-dependencies]` are NOT — those are private to the dep's + // own test runs). + // + // Conflict policy: C++ modules require globally-unique module names + // and ODR-respecting symbols, so the same `(ns, name)` resolved to + // two different exact versions is an error — mcpp prints both + // requesting parents and asks the user to align them. std::vector packages; packages.push_back({*root, *m}); - std::vector dep_manifests; - dep_manifests.reserve(m->dependencies.size() + (includeDevDeps ? m->devDependencies.size() : 0)); + // Use a deque + stable storage for dep manifests (vector of unique_ptr + // so PackageRoot's reference stays valid as new ones are appended). + std::vector> dep_manifests; + + struct ResolvedKey { + std::string ns; + std::string shortName; + auto operator<=>(const ResolvedKey&) const = default; + }; + struct ResolvedRecord { + std::string version; // empty for path/git deps + std::string requestedBy; // human-readable for error messages + std::string source; // "version" | "path" | "git" — for type-clash check + }; + std::map resolved; + + struct WorkItem { + std::string name; // dep map key as written + mcpp::manifest::DependencySpec spec; // copy (we may mutate version) + std::string requestedBy; // who asked for it + }; + std::deque worklist; + + // SemVer constraint resolver, shared across the worklist so transitive + // deps with caret/range constraints (`^1.0`) also get pinned to a + // concrete version before fetch. + auto resolveSemver = [&](mcpp::manifest::DependencySpec& s, + const std::string& depName) + -> std::expected + { + if (s.isPath() || s.isGit()) return {}; + if (!mcpp::pm::is_version_constraint(s.version)) return {}; + auto cfg = get_cfg(); + if (!cfg) return std::unexpected(cfg.error()); + mcpp::fetcher::Fetcher fetcher(**cfg); + auto resolved = mcpp::pm::resolve_semver(depName, s.version, fetcher); + if (!resolved) return std::unexpected(resolved.error()); + mcpp::ui::info("Resolved", + std::format("{} {} → v{}", depName, s.version, *resolved)); + s.version = std::move(*resolved); + return {}; + }; - // Build the unified dep list: regular deps always, dev-deps only when - // requested (e.g. by `mcpp test`). - std::vector> allDeps; - for (auto& [n, s] : m->dependencies) allDeps.emplace_back(&n, &s); + // Seed the worklist from the main manifest. Dev-deps only when the + // caller wants them; they're never propagated transitively. + const std::string mainPkgLabel = m->package.name; + for (auto& [n, s] : m->dependencies) { + worklist.push_back({n, s, mainPkgLabel}); + } if (includeDevDeps) { - for (auto& [n, s] : m->devDependencies) allDeps.emplace_back(&n, &s); + for (auto& [n, s] : m->devDependencies) { + worklist.push_back({n, s, mainPkgLabel + " (dev-dep)"}); + } } - // SemVer resolution pass: turn any constraint specs (`^1.2`, `~0.0.1`, etc.) - // into exact versions by consulting the index. Done up-front so lockfile - // writing + install + install_path all see the same exact version. - { - auto resolveOne = [&](mcpp::manifest::DependencySpec& s, - const std::string& depName) - -> std::expected - { - if (s.isPath() || s.isGit()) return {}; - if (!mcpp::pm::is_version_constraint(s.version)) return {}; - auto cfg = get_cfg(); - if (!cfg) return std::unexpected(cfg.error()); - mcpp::fetcher::Fetcher fetcher(**cfg); - auto resolved = mcpp::pm::resolve_semver(depName, s.version, fetcher); - if (!resolved) return std::unexpected(resolved.error()); - mcpp::ui::info("Resolved", - std::format("{} {} → v{}", depName, s.version, *resolved)); - s.version = std::move(*resolved); - return {}; + while (!worklist.empty()) { + auto item = std::move(worklist.front()); + worklist.pop_front(); + + const auto& name = item.name; + auto& spec = item.spec; + + // Pin SemVer constraint before dedup/fetch. + if (auto r = resolveSemver(spec, name); !r) { + return std::unexpected(r.error()); + } + + ResolvedKey key{ + spec.namespace_.empty() + ? std::string{mcpp::manifest::kDefaultNamespace} + : spec.namespace_, + spec.shortName.empty() ? name : spec.shortName, }; - for (auto& [n, s] : m->dependencies) - if (auto r = resolveOne(s, n); !r) return std::unexpected(r.error()); - if (includeDevDeps) { - for (auto& [n, s] : m->devDependencies) - if (auto r = resolveOne(s, n); !r) return std::unexpected(r.error()); + const std::string sourceKind = + spec.isPath() ? "path" + : spec.isGit() ? "git" + : "version"; + + if (auto it = resolved.find(key); it != resolved.end()) { + // Conflict detection. + if (it->second.source != sourceKind) { + return std::unexpected(std::format( + "dependency '{}{}{}' is requested as both a {} dep " + "(by '{}') and a {} dep (by '{}'). Pick one.", + key.ns, key.ns.empty() ? "" : ".", key.shortName, + it->second.source, it->second.requestedBy, + sourceKind, item.requestedBy)); + } + if (sourceKind == "version" && it->second.version != spec.version) { + return std::unexpected(std::format( + "dependency '{}{}{}' has conflicting versions in the " + "transitive graph:\n" + " '{}' requested by '{}'\n" + " '{}' requested by '{}'\n" + "C++ modules require a single global version of each " + "package. Pick a version compatible with both consumers, " + "or ask one upstream to widen its dep range.", + key.ns, key.ns.empty() ? "" : ".", key.shortName, + it->second.version, it->second.requestedBy, + spec.version, item.requestedBy)); + } + // Same key, same version (or compatible path/git) — already + // processed; skip. + continue; } - } - // Reuse get_cfg defined above for dep resolution (same lambda). - for (auto const& [namePtr, specPtr] : allDeps) { - const auto& name = *namePtr; - const auto& spec = *specPtr; std::filesystem::path dep_root; if (spec.isPath()) { @@ -1307,8 +1379,32 @@ prepare_build(bool print_fingerprint, for (auto& d : matches) m->buildConfig.includeDirs.push_back(d); } - dep_manifests.push_back(std::move(*dep_manifest)); - packages.push_back({dep_root, dep_manifests.back()}); + // Record this dep as resolved so future encounters of the same + // (ns, name) hit the fast path (skip / conflict check). + resolved[key] = ResolvedRecord{ + .version = sourceKind == "version" ? spec.version : "", + .requestedBy = item.requestedBy, + .source = sourceKind, + }; + + // Stable storage so the PackageRoot reference below stays valid + // when the worklist appends more deps and the vector grows. + dep_manifests.push_back( + std::make_unique(std::move(*dep_manifest))); + packages.push_back({dep_root, *dep_manifests.back()}); + + // Recurse: the dep's own [dependencies] become new worklist items. + // dev-dependencies are intentionally NOT walked — those are + // private to the dep's test runs, not part of its public ABI. + const std::string thisDepLabel = std::format( + "{}{}{}@{}", + key.ns, + key.ns.empty() ? "" : ".", + key.shortName, + sourceKind == "version" ? spec.version : sourceKind); + for (auto& [child_name, child_spec] : dep_manifests.back()->dependencies) { + worklist.push_back({child_name, child_spec, thisDepLabel}); + } } // Modgraph: regex scanner by default; opt-in to compiler-driven P1689 diff --git a/tests/e2e/31_transitive_deps.sh b/tests/e2e/31_transitive_deps.sh new file mode 100755 index 0000000..98d1d7d --- /dev/null +++ b/tests/e2e/31_transitive_deps.sh @@ -0,0 +1,109 @@ +#!/usr/bin/env bash +# 31_transitive_deps.sh — transitive dependency walker: +# * a path-dep that itself declares a path-dep is fully resolved +# (consumer doesn't need to list the grandchild explicitly) +# * the grandchild's [build].include_dirs propagate so its headers +# are visible while compiling its parent. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +export MCPP_HOME="$TMP/mcpp-home" + +# ── 1. Grandchild: a header-providing C lib whose `[build].include_dirs` +# is what consumers care about. Plays the role of mbedtls in the +# llmapi → tinyhttps → mbedtls chain. +mkdir -p "$TMP/grandchild" && cd "$TMP/grandchild" +"$MCPP" new gc > /dev/null +cd gc +mkdir -p include/gc +cat > include/gc/gc.h <<'EOF' +#pragma once +inline int gc_answer(void) { return 42; } +EOF +rm -f src/main.cpp +cat > src/gc.cppm <<'EOF' +export module gc; +EOF +cat > mcpp.toml <<'EOF' +[package] +name = "gc" +version = "0.1.0" +[build] +include_dirs = ["include"] +[targets.gc] +kind = "lib" +EOF + +# ── 2. Child: depends on grandchild via path; its own .cppm pulls +# , which can only work if gc's include_dirs reach the +# child's compile rule (transitive include propagation). +mkdir -p "$TMP/child" && cd "$TMP/child" +"$MCPP" new ch > /dev/null +cd ch +rm -f src/main.cpp +cat > src/ch.cppm <<'EOF' +module; +#include +export module ch; +export int ch_answer() { return gc_answer(); } +EOF +cat > mcpp.toml < /dev/null +cd top +cat > src/main.cpp <<'EOF' +import std; +import ch; +int main() { + std::println("answer={}", ch_answer()); + return ch_answer() == 42 ? 0 : 1; +} +EOF +cat > mcpp.toml < build.log 2>&1 || { cat build.log; echo "transitive build failed"; exit 1; } +out="$("$MCPP" run 2>&1 | tail -1)" +[[ "$out" == "answer=42" ]] || { echo "unexpected output: $out"; exit 1; } + +# ── 4. Same dep referenced through two parallel paths is allowed +# (no version conflict — same path, same package). +mkdir -p "$TMP/top2" && cd "$TMP/top2" +"$MCPP" new top2 > /dev/null +cd top2 +cat > src/main.cpp <<'EOF' +import std; +import ch; +int main() { std::println("answer={}", ch_answer()); return ch_answer() == 42 ? 0 : 1; } +EOF +cat > mcpp.toml < build-top2.log 2>&1 || { cat build-top2.log; echo "duplicate-but-consistent dep failed"; exit 1; } + +echo "OK"