Skip to content

Commit b835590

Browse files
authored
feat(deps): walk transitive dependency graph + detect version conflicts (#17)
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 `<mbedtls/ssl.h>`. 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<unique_ptr>` 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.
1 parent 6b323eb commit b835590

2 files changed

Lines changed: 245 additions & 40 deletions

File tree

src/cli.cppm

Lines changed: 136 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,54 +1027,126 @@ prepare_build(bool print_fingerprint,
10271027
tc->label()));
10281028
}
10291029

1030-
// Resolve dependencies: path-based, then version-based (via xlings).
1031-
// Both end up as PackageRoot entries fed into the same scanner pipeline.
1030+
// Resolve dependencies: walk the **transitive** graph from the main
1031+
// manifest, BFS-style. Each unique `(namespace, shortName)` is fetched
1032+
// once, its `[build].include_dirs` are propagated to the main
1033+
// manifest, and its own `[dependencies]` are queued for processing
1034+
// (its `[dev-dependencies]` are NOT — those are private to the dep's
1035+
// own test runs).
1036+
//
1037+
// Conflict policy: C++ modules require globally-unique module names
1038+
// and ODR-respecting symbols, so the same `(ns, name)` resolved to
1039+
// two different exact versions is an error — mcpp prints both
1040+
// requesting parents and asks the user to align them.
10321041
std::vector<mcpp::modgraph::PackageRoot> packages;
10331042
packages.push_back({*root, *m});
10341043

1035-
std::vector<mcpp::manifest::Manifest> dep_manifests;
1036-
dep_manifests.reserve(m->dependencies.size() + (includeDevDeps ? m->devDependencies.size() : 0));
1044+
// Use a deque + stable storage for dep manifests (vector of unique_ptr
1045+
// so PackageRoot's reference stays valid as new ones are appended).
1046+
std::vector<std::unique_ptr<mcpp::manifest::Manifest>> dep_manifests;
1047+
1048+
struct ResolvedKey {
1049+
std::string ns;
1050+
std::string shortName;
1051+
auto operator<=>(const ResolvedKey&) const = default;
1052+
};
1053+
struct ResolvedRecord {
1054+
std::string version; // empty for path/git deps
1055+
std::string requestedBy; // human-readable for error messages
1056+
std::string source; // "version" | "path" | "git" — for type-clash check
1057+
};
1058+
std::map<ResolvedKey, ResolvedRecord> resolved;
1059+
1060+
struct WorkItem {
1061+
std::string name; // dep map key as written
1062+
mcpp::manifest::DependencySpec spec; // copy (we may mutate version)
1063+
std::string requestedBy; // who asked for it
1064+
};
1065+
std::deque<WorkItem> worklist;
1066+
1067+
// SemVer constraint resolver, shared across the worklist so transitive
1068+
// deps with caret/range constraints (`^1.0`) also get pinned to a
1069+
// concrete version before fetch.
1070+
auto resolveSemver = [&](mcpp::manifest::DependencySpec& s,
1071+
const std::string& depName)
1072+
-> std::expected<void, std::string>
1073+
{
1074+
if (s.isPath() || s.isGit()) return {};
1075+
if (!mcpp::pm::is_version_constraint(s.version)) return {};
1076+
auto cfg = get_cfg();
1077+
if (!cfg) return std::unexpected(cfg.error());
1078+
mcpp::fetcher::Fetcher fetcher(**cfg);
1079+
auto resolved = mcpp::pm::resolve_semver(depName, s.version, fetcher);
1080+
if (!resolved) return std::unexpected(resolved.error());
1081+
mcpp::ui::info("Resolved",
1082+
std::format("{} {} → v{}", depName, s.version, *resolved));
1083+
s.version = std::move(*resolved);
1084+
return {};
1085+
};
10371086

1038-
// Build the unified dep list: regular deps always, dev-deps only when
1039-
// requested (e.g. by `mcpp test`).
1040-
std::vector<std::pair<const std::string*, const mcpp::manifest::DependencySpec*>> allDeps;
1041-
for (auto& [n, s] : m->dependencies) allDeps.emplace_back(&n, &s);
1087+
// Seed the worklist from the main manifest. Dev-deps only when the
1088+
// caller wants them; they're never propagated transitively.
1089+
const std::string mainPkgLabel = m->package.name;
1090+
for (auto& [n, s] : m->dependencies) {
1091+
worklist.push_back({n, s, mainPkgLabel});
1092+
}
10421093
if (includeDevDeps) {
1043-
for (auto& [n, s] : m->devDependencies) allDeps.emplace_back(&n, &s);
1094+
for (auto& [n, s] : m->devDependencies) {
1095+
worklist.push_back({n, s, mainPkgLabel + " (dev-dep)"});
1096+
}
10441097
}
10451098

1046-
// SemVer resolution pass: turn any constraint specs (`^1.2`, `~0.0.1`, etc.)
1047-
// into exact versions by consulting the index. Done up-front so lockfile
1048-
// writing + install + install_path all see the same exact version.
1049-
{
1050-
auto resolveOne = [&](mcpp::manifest::DependencySpec& s,
1051-
const std::string& depName)
1052-
-> std::expected<void, std::string>
1053-
{
1054-
if (s.isPath() || s.isGit()) return {};
1055-
if (!mcpp::pm::is_version_constraint(s.version)) return {};
1056-
auto cfg = get_cfg();
1057-
if (!cfg) return std::unexpected(cfg.error());
1058-
mcpp::fetcher::Fetcher fetcher(**cfg);
1059-
auto resolved = mcpp::pm::resolve_semver(depName, s.version, fetcher);
1060-
if (!resolved) return std::unexpected(resolved.error());
1061-
mcpp::ui::info("Resolved",
1062-
std::format("{} {} → v{}", depName, s.version, *resolved));
1063-
s.version = std::move(*resolved);
1064-
return {};
1099+
while (!worklist.empty()) {
1100+
auto item = std::move(worklist.front());
1101+
worklist.pop_front();
1102+
1103+
const auto& name = item.name;
1104+
auto& spec = item.spec;
1105+
1106+
// Pin SemVer constraint before dedup/fetch.
1107+
if (auto r = resolveSemver(spec, name); !r) {
1108+
return std::unexpected(r.error());
1109+
}
1110+
1111+
ResolvedKey key{
1112+
spec.namespace_.empty()
1113+
? std::string{mcpp::manifest::kDefaultNamespace}
1114+
: spec.namespace_,
1115+
spec.shortName.empty() ? name : spec.shortName,
10651116
};
1066-
for (auto& [n, s] : m->dependencies)
1067-
if (auto r = resolveOne(s, n); !r) return std::unexpected(r.error());
1068-
if (includeDevDeps) {
1069-
for (auto& [n, s] : m->devDependencies)
1070-
if (auto r = resolveOne(s, n); !r) return std::unexpected(r.error());
1117+
const std::string sourceKind =
1118+
spec.isPath() ? "path"
1119+
: spec.isGit() ? "git"
1120+
: "version";
1121+
1122+
if (auto it = resolved.find(key); it != resolved.end()) {
1123+
// Conflict detection.
1124+
if (it->second.source != sourceKind) {
1125+
return std::unexpected(std::format(
1126+
"dependency '{}{}{}' is requested as both a {} dep "
1127+
"(by '{}') and a {} dep (by '{}'). Pick one.",
1128+
key.ns, key.ns.empty() ? "" : ".", key.shortName,
1129+
it->second.source, it->second.requestedBy,
1130+
sourceKind, item.requestedBy));
1131+
}
1132+
if (sourceKind == "version" && it->second.version != spec.version) {
1133+
return std::unexpected(std::format(
1134+
"dependency '{}{}{}' has conflicting versions in the "
1135+
"transitive graph:\n"
1136+
" '{}' requested by '{}'\n"
1137+
" '{}' requested by '{}'\n"
1138+
"C++ modules require a single global version of each "
1139+
"package. Pick a version compatible with both consumers, "
1140+
"or ask one upstream to widen its dep range.",
1141+
key.ns, key.ns.empty() ? "" : ".", key.shortName,
1142+
it->second.version, it->second.requestedBy,
1143+
spec.version, item.requestedBy));
1144+
}
1145+
// Same key, same version (or compatible path/git) — already
1146+
// processed; skip.
1147+
continue;
10711148
}
1072-
}
10731149

1074-
// Reuse get_cfg defined above for dep resolution (same lambda).
1075-
for (auto const& [namePtr, specPtr] : allDeps) {
1076-
const auto& name = *namePtr;
1077-
const auto& spec = *specPtr;
10781150
std::filesystem::path dep_root;
10791151

10801152
if (spec.isPath()) {
@@ -1307,8 +1379,32 @@ prepare_build(bool print_fingerprint,
13071379
for (auto& d : matches) m->buildConfig.includeDirs.push_back(d);
13081380
}
13091381

1310-
dep_manifests.push_back(std::move(*dep_manifest));
1311-
packages.push_back({dep_root, dep_manifests.back()});
1382+
// Record this dep as resolved so future encounters of the same
1383+
// (ns, name) hit the fast path (skip / conflict check).
1384+
resolved[key] = ResolvedRecord{
1385+
.version = sourceKind == "version" ? spec.version : "",
1386+
.requestedBy = item.requestedBy,
1387+
.source = sourceKind,
1388+
};
1389+
1390+
// Stable storage so the PackageRoot reference below stays valid
1391+
// when the worklist appends more deps and the vector grows.
1392+
dep_manifests.push_back(
1393+
std::make_unique<mcpp::manifest::Manifest>(std::move(*dep_manifest)));
1394+
packages.push_back({dep_root, *dep_manifests.back()});
1395+
1396+
// Recurse: the dep's own [dependencies] become new worklist items.
1397+
// dev-dependencies are intentionally NOT walked — those are
1398+
// private to the dep's test runs, not part of its public ABI.
1399+
const std::string thisDepLabel = std::format(
1400+
"{}{}{}@{}",
1401+
key.ns,
1402+
key.ns.empty() ? "" : ".",
1403+
key.shortName,
1404+
sourceKind == "version" ? spec.version : sourceKind);
1405+
for (auto& [child_name, child_spec] : dep_manifests.back()->dependencies) {
1406+
worklist.push_back({child_name, child_spec, thisDepLabel});
1407+
}
13121408
}
13131409

13141410
// Modgraph: regex scanner by default; opt-in to compiler-driven P1689

tests/e2e/31_transitive_deps.sh

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
#!/usr/bin/env bash
2+
# 31_transitive_deps.sh — transitive dependency walker:
3+
# * a path-dep that itself declares a path-dep is fully resolved
4+
# (consumer doesn't need to list the grandchild explicitly)
5+
# * the grandchild's [build].include_dirs propagate so its headers
6+
# are visible while compiling its parent.
7+
set -e
8+
9+
TMP=$(mktemp -d)
10+
trap "rm -rf $TMP" EXIT
11+
export MCPP_HOME="$TMP/mcpp-home"
12+
13+
# ── 1. Grandchild: a header-providing C lib whose `[build].include_dirs`
14+
# is what consumers care about. Plays the role of mbedtls in the
15+
# llmapi → tinyhttps → mbedtls chain.
16+
mkdir -p "$TMP/grandchild" && cd "$TMP/grandchild"
17+
"$MCPP" new gc > /dev/null
18+
cd gc
19+
mkdir -p include/gc
20+
cat > include/gc/gc.h <<'EOF'
21+
#pragma once
22+
inline int gc_answer(void) { return 42; }
23+
EOF
24+
rm -f src/main.cpp
25+
cat > src/gc.cppm <<'EOF'
26+
export module gc;
27+
EOF
28+
cat > mcpp.toml <<'EOF'
29+
[package]
30+
name = "gc"
31+
version = "0.1.0"
32+
[build]
33+
include_dirs = ["include"]
34+
[targets.gc]
35+
kind = "lib"
36+
EOF
37+
38+
# ── 2. Child: depends on grandchild via path; its own .cppm pulls
39+
# <gc/gc.h>, which can only work if gc's include_dirs reach the
40+
# child's compile rule (transitive include propagation).
41+
mkdir -p "$TMP/child" && cd "$TMP/child"
42+
"$MCPP" new ch > /dev/null
43+
cd ch
44+
rm -f src/main.cpp
45+
cat > src/ch.cppm <<'EOF'
46+
module;
47+
#include <gc/gc.h>
48+
export module ch;
49+
export int ch_answer() { return gc_answer(); }
50+
EOF
51+
cat > mcpp.toml <<EOF
52+
[package]
53+
name = "ch"
54+
version = "0.1.0"
55+
[targets.ch]
56+
kind = "lib"
57+
58+
[dependencies]
59+
gc = { path = "$TMP/grandchild/gc" }
60+
EOF
61+
62+
# ── 3. Top: depends ONLY on child. Should still pull grandchild
63+
# transitively without an explicit declaration.
64+
mkdir -p "$TMP/top" && cd "$TMP/top"
65+
"$MCPP" new top > /dev/null
66+
cd top
67+
cat > src/main.cpp <<'EOF'
68+
import std;
69+
import ch;
70+
int main() {
71+
std::println("answer={}", ch_answer());
72+
return ch_answer() == 42 ? 0 : 1;
73+
}
74+
EOF
75+
cat > mcpp.toml <<EOF
76+
[package]
77+
name = "top"
78+
version = "0.1.0"
79+
80+
[dependencies]
81+
ch = { path = "$TMP/child/ch" }
82+
EOF
83+
84+
"$MCPP" build > build.log 2>&1 || { cat build.log; echo "transitive build failed"; exit 1; }
85+
out="$("$MCPP" run 2>&1 | tail -1)"
86+
[[ "$out" == "answer=42" ]] || { echo "unexpected output: $out"; exit 1; }
87+
88+
# ── 4. Same dep referenced through two parallel paths is allowed
89+
# (no version conflict — same path, same package).
90+
mkdir -p "$TMP/top2" && cd "$TMP/top2"
91+
"$MCPP" new top2 > /dev/null
92+
cd top2
93+
cat > src/main.cpp <<'EOF'
94+
import std;
95+
import ch;
96+
int main() { std::println("answer={}", ch_answer()); return ch_answer() == 42 ? 0 : 1; }
97+
EOF
98+
cat > mcpp.toml <<EOF
99+
[package]
100+
name = "top2"
101+
version = "0.1.0"
102+
103+
[dependencies]
104+
ch = { path = "$TMP/child/ch" }
105+
gc = { path = "$TMP/grandchild/gc" }
106+
EOF
107+
"$MCPP" build > build-top2.log 2>&1 || { cat build-top2.log; echo "duplicate-but-consistent dep failed"; exit 1; }
108+
109+
echo "OK"

0 commit comments

Comments
 (0)