From 4b0aa9e6afa97ac0fe72344a470e80b295c7aac5 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 29 Jul 2026 17:21:52 +0800 Subject: [PATCH] fix: resolve SemVer constraints through the index route, not the registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolve_semver` / `try_merge_semver` took a `Fetcher&` and called `read_xpkg_lua` on it, which only ever reads the shared registry under `/data`. Candidate selection, meanwhile, dispatches across all three descriptor transports (local path index, project clone, registry). The split meant a package served by a project `[indices]` entry resolved fine as an exact version and failed the moment the same dependency carried a constraint: dependency 'acme.util' has SemVer constraint '^2.0' but the index entry isn't cloned locally yet — run `mcpp index update` first — advice that does nothing for a local path index, leaving no way forward. Both functions now take `const IndexRoute&`, so the descriptor is reached the same way everywhere. `mcpp new --template` passes a route with no project indices, which is what it always meant: there is no project yet. The not-found message drops the `mcpp index update` hint when a local path index is what would have answered. `prepare.cppm`'s `index_route` lambda moves above `resolveSemver` so the resolver call can use it; nothing else about it changes. Closes #308 --- src/build/prepare.cppm | 40 +++---- src/pm/resolver.cppm | 53 +++++---- src/scaffold/create.cppm | 6 +- tests/e2e/169_semver_project_index.sh | 154 ++++++++++++++++++++++++++ 4 files changed, 213 insertions(+), 40 deletions(-) create mode 100755 tests/e2e/169_semver_project_index.sh diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index cdeedce1..e1b0d926 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -1411,6 +1411,21 @@ prepare_build(bool print_fingerprint, }; std::deque worklist; + // Index routing — WHICH index answers for a namespace and how its + // descriptors are read — lives in mcpp.pm.index_route, shared with the + // `mcpp add` existence gate so the two cannot disagree about which + // packages are real (#305/#307). `cfg` is filled in per call: the route is + // rebuilt on demand because `root` moves when a workspace member is + // selected above. + auto index_route = [&](mcpp::config::GlobalConfig* cfg = nullptr) { + return mcpp::pm::IndexRoute{ &m->indices, *root, cfg }; + }; + auto findIndexForNs = [&](const std::string& ns) + -> const mcpp::pm::IndexSpec* + { + return index_route().find_for_ns(ns); + }; + // 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. @@ -1422,11 +1437,12 @@ prepare_build(bool print_fingerprint, 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); - // 0.0.10+: use structured namespace from DependencySpec. + // 0.0.10+: use structured namespace from DependencySpec. The route (not + // a bare Fetcher) is what reaches a descriptor served by a project + // `[indices]` entry — see #308. auto resolved = mcpp::pm::resolve_semver( s.namespace_, s.shortName.empty() ? depName : s.shortName, - s.version, fetcher, targetPlatform); + s.version, index_route(*cfg), targetPlatform); if (!resolved) return std::unexpected(resolved.error()); mcpp::ui::info("Resolved", std::format("{} {} → v{}", depName, s.version, *resolved)); @@ -1439,21 +1455,6 @@ prepare_build(bool print_fingerprint, // different version is needed. Returns the dep's effective root (where // mcpp.toml lives) and a fully loaded manifest. using LoadedDep = std::pair; - // Index routing — WHICH index answers for a namespace and how its - // descriptors are read — lives in mcpp.pm.index_route, shared with the - // `mcpp add` existence gate so the two cannot disagree about which - // packages are real (#305/#307). `cfg` is filled in per call: the route is - // rebuilt on demand because `root` moves when a workspace member is - // selected above. - auto index_route = [&](mcpp::config::GlobalConfig* cfg = nullptr) { - return mcpp::pm::IndexRoute{ &m->indices, *root, cfg }; - }; - auto findIndexForNs = [&](const std::string& ns) - -> const mcpp::pm::IndexSpec* - { - return index_route().find_for_ns(ns); - }; - // Identity-first candidate probe. A candidate is DISAMBIGUATED by the // DECLARED (namespace, name) of whatever descriptor the index holds — never // by whether a canonically-named file `..lua` happens to exist on @@ -2568,13 +2569,12 @@ prepare_build(bool print_fingerprint, // PR adds multi-version mangling as a Level-1 fallback). auto cfg = get_cfg(); if (!cfg) return std::unexpected(cfg.error()); - mcpp::fetcher::Fetcher fetcher(**cfg); auto merged = mcpp::pm::try_merge_semver( key.ns, key.shortName, it->second.constraint, item.originalConstraint, - fetcher, targetPlatform); + index_route(*cfg), targetPlatform); if (!merged) { // Level 1 fallback: multi-version mangling. Two // versions can't be reconciled by SemVer, but they diff --git a/src/pm/resolver.cppm b/src/pm/resolver.cppm index fcc89f03..16d01a26 100644 --- a/src/pm/resolver.cppm +++ b/src/pm/resolver.cppm @@ -2,9 +2,15 @@ // using the package's xpkg lua descriptor as the version inventory. // // Part of the package-management subsystem refactor (PR-R4 in -// `.agents/docs/2026-05-08-pm-subsystem-architecture.md`). Strictly -// pulled out of `cli.cppm` with no behavior change; the same -// signatures, the same error strings, the same platform key picking. +// `.agents/docs/2026-05-08-pm-subsystem-architecture.md`), originally +// pulled out of `cli.cppm` verbatim. +// +// The descriptor is reached through `mcpp.pm.index_route`, never through a +// bare `Fetcher`. A `Fetcher` only ever reads the shared registry, so while +// these functions took one, a dependency served by a project +// `[indices]` entry resolved fine as an exact version and then failed the +// moment the same dependency carried a SemVer constraint — candidate +// selection could see the package, version resolution could not (#308). // // Implementation note: `resolve_semver` is **not** declared inline on // purpose. Inlining it across modules makes every importer @@ -20,7 +26,8 @@ import mcpp.manifest; import mcpp.platform; import mcpp.platform.axis; import mcpp.pm.compat; -import mcpp.pm.package_fetcher; +import mcpp.pm.dep_spec; +import mcpp.pm.index_route; import mcpp.version_req; export namespace mcpp::pm { @@ -45,7 +52,7 @@ bool is_version_constraint(std::string_view v); std::expected resolve_semver(std::string_view ns, std::string_view shortName, std::string_view constraint, - mcpp::pm::Fetcher& fetcher, + const mcpp::pm::IndexRoute& route, const mcpp::platform::PlatformKey& platform); // Try to AND-merge two version constraints and resolve to a single @@ -54,7 +61,7 @@ std::expected try_merge_semver(std::string_view ns, std::string_view shortName, std::string_view a, std::string_view b, - mcpp::pm::Fetcher& fetcher, + const mcpp::pm::IndexRoute& route, const mcpp::platform::PlatformKey& platform); // ─── Legacy overloads (COMPAT, remove in 1.0.0) ───────────────────── @@ -62,13 +69,13 @@ try_merge_semver(std::string_view ns, std::string_view shortName, std::expected resolve_semver(std::string_view name, std::string_view constraint, - mcpp::pm::Fetcher& fetcher); + const mcpp::pm::IndexRoute& route); std::expected try_merge_semver(std::string_view name, std::string_view a, std::string_view b, - mcpp::pm::Fetcher& fetcher); + const mcpp::pm::IndexRoute& route); } // namespace mcpp::pm @@ -88,18 +95,26 @@ bool is_version_constraint(std::string_view v) { std::expected resolve_semver(std::string_view ns, std::string_view shortName, std::string_view constraint, - mcpp::pm::Fetcher& fetcher, + const mcpp::pm::IndexRoute& route, const mcpp::platform::PlatformKey& platform) { namespace vr = mcpp::version_req; auto qname = mcpp::pm::compat::qualified_name(ns, shortName); - auto luaContent = fetcher.read_xpkg_lua(ns, shortName); + auto luaContent = route.read(mcpp::pm::DependencyCoordinate{ + .namespace_ = std::string(ns), .shortName = std::string(shortName) }); if (!luaContent) { + // `mcpp index update` is only advice worth giving when the shared + // registry (or a not-yet-cloned git index) is what would have answered. + // A project `[indices] path = …` is whatever the user has on disk, and + // telling them to refresh it sends them nowhere. + auto* idx = route.find_for_ns(ns); + const bool refreshable = !idx || idx->is_builtin() || !idx->is_local(); return std::unexpected(std::format( - "dependency '{}' has SemVer constraint '{}' but the index entry " - "isn't cloned locally yet — run `mcpp index update` first", - qname, constraint)); + "dependency '{}' has SemVer constraint '{}' but no readable index " + "entry for it{}", + qname, constraint, + refreshable ? " — run `mcpp index update` first" : "")); } auto req = vr::parse_req(constraint); @@ -145,7 +160,7 @@ std::expected try_merge_semver(std::string_view ns, std::string_view shortName, std::string_view a, std::string_view b, - mcpp::pm::Fetcher& fetcher, + const mcpp::pm::IndexRoute& route, const mcpp::platform::PlatformKey& platform) { auto canon = [](std::string_view v) -> std::string { @@ -162,7 +177,7 @@ try_merge_semver(std::string_view ns, std::string_view shortName, else if (!cb.empty()) merged = cb; else merged = "*"; - return resolve_semver(ns, shortName, merged, fetcher, platform); + return resolve_semver(ns, shortName, merged, route, platform); } // ─── Legacy overloads (COMPAT, remove in 1.0.0) ───────────────────── @@ -170,13 +185,13 @@ try_merge_semver(std::string_view ns, std::string_view shortName, std::expected resolve_semver(std::string_view name, std::string_view constraint, - mcpp::pm::Fetcher& fetcher) + const mcpp::pm::IndexRoute& route) { auto resolved = mcpp::pm::compat::resolve_package_name(name, ""); // Legacy overload: no target is threaded through it, so it names the host // axis explicitly rather than inheriting a silent default (#254). return resolve_semver(resolved.namespace_, resolved.shortName, - constraint, fetcher, + constraint, route, mcpp::platform::HostPlatform::current()); } @@ -184,12 +199,12 @@ std::expected try_merge_semver(std::string_view name, std::string_view a, std::string_view b, - mcpp::pm::Fetcher& fetcher) + const mcpp::pm::IndexRoute& route) { auto resolved = mcpp::pm::compat::resolve_package_name(name, ""); // Legacy overload — see the resolve_semver note above. return try_merge_semver(resolved.namespace_, resolved.shortName, - a, b, fetcher, + a, b, route, mcpp::platform::HostPlatform::current()); } diff --git a/src/scaffold/create.cppm b/src/scaffold/create.cppm index c11e8f64..4a026b27 100644 --- a/src/scaffold/create.cppm +++ b/src/scaffold/create.cppm @@ -14,6 +14,7 @@ import mcpp.fetcher; import mcpp.fetcher.progress; import mcpp.manifest; import mcpp.pm.compat; +import mcpp.pm.index_route; import mcpp.platform.axis; import mcpp.pm.resolver; import mcpp.scaffold; @@ -76,7 +77,10 @@ fetch_template_package(const mcpp::scaffold::TemplateSpec& spec) { if (version.empty()) { // `mcpp add` has no target concept — the axis it means is the host, // named explicitly rather than inherited from a default (#254). - auto v = mcpp::pm::resolve_semver(ns, shortName, "*", fetcher, + // A template package is always a registry package — `mcpp new` has no + // project yet, so there are no `[indices]` to route through. + mcpp::pm::IndexRoute registryOnly{ nullptr, {}, &*cfg }; + auto v = mcpp::pm::resolve_semver(ns, shortName, "*", registryOnly, mcpp::platform::HostPlatform::current()); if (!v) return std::unexpected(v.error()); version = *v; diff --git a/tests/e2e/169_semver_project_index.sh b/tests/e2e/169_semver_project_index.sh new file mode 100755 index 00000000..a5d5568c --- /dev/null +++ b/tests/e2e/169_semver_project_index.sh @@ -0,0 +1,154 @@ +#!/usr/bin/env bash +# requires: gcc fresh-sandbox +# #308: a SemVer constraint on a package served by a project `[indices]` entry +# must resolve. Version resolution used to read the shared registry directly +# instead of routing like candidate selection does, so the exact-version form +# (`gadget = "2.0.0"`) worked while the constraint form (`gadget = "^2.0"`) failed +# with "run `mcpp index update` first" — advice that does nothing for a local +# path index. Both forms are asserted here so the two cannot drift apart again. +# +# No network: the index is a local path and the package payload is pre-seeded +# into the project's xlings data dir, so nothing is ever downloaded. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT + +export MCPP_HOME="$TMP/mcpp-home" +source "$(dirname "$0")/_inherit_toolchain.sh" + +mkdir -p "$TMP/proj" +cd "$TMP/proj" + +# ── A project-local index carrying acme.gadget 2.0.0 and 2.1.0 ──────────── +mkdir -p local-index/pkgs/a +cat > local-index/pkgs/a/acme.gadget.lua <<'EOF' +package = { + spec = "1", + namespace = "acme", + name = "acme.gadget", + description = "Package reachable only through a project [indices] entry", + licenses = {"MIT"}, + type = "package", + xpm = { + linux = { + ["2.0.0"] = { + url = "https://example.invalid/gadget-2.0.0.tar.gz", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + ["2.1.0"] = { + url = "https://example.invalid/gadget-2.1.0.tar.gz", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + }, + macosx = { + ["2.0.0"] = { + url = "https://example.invalid/gadget-2.0.0.tar.gz", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + ["2.1.0"] = { + url = "https://example.invalid/gadget-2.1.0.tar.gz", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + }, + windows = { + ["2.0.0"] = { + url = "https://example.invalid/gadget-2.0.0.zip", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + ["2.1.0"] = { + url = "https://example.invalid/gadget-2.1.0.zip", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + }, + }, + mcpp = { + language = "c++23", + import_std = false, + sources = { "src/gadget.cppm" }, + targets = { ["gadget"] = { kind = "lib" } }, + deps = {}, + }, +} +EOF + +# ── Pre-seed the payload so resolution is the only thing under test ───── +mkdir -p .mcpp/.xlings/data/xpkgs/acme.gadget/2.1.0/src +cat > .mcpp/.xlings/data/xpkgs/acme.gadget/2.1.0/src/gadget.cppm <<'EOF' +export module gadget; + +export int gadget_value() { + return 42; +} +EOF + +mkdir -p src +cat > src/main.cpp <<'EOF' +import gadget; + +int main() { + return gadget_value() == 42 ? 0 : 1; +} +EOF + +cat > mcpp.toml <<'EOF' +[package] +name = "proj" +version = "0.1.0" + +[indices] +acme = { path = "local-index" } + +[dependencies.acme] +gadget = "^2.0" + +[targets.proj] +kind = "bin" +main = "src/main.cpp" +EOF + +"$MCPP" build > build.log 2>&1 || { + cat build.log + echo "FAIL: SemVer constraint did not resolve against the project [indices] entry" + exit 1 +} + +# The constraint must have been pinned to the newest matching version, not +# merely tolerated. +grep -q '→ v2.1.0' build.log || { + cat build.log + echo "FAIL: expected '^2.0' to resolve to v2.1.0" + exit 1 +} +grep -q 'index update' build.log && { + cat build.log + echo "FAIL: a local path index must never be answered with 'mcpp index update'" + exit 1 +} +grep -q '\[package\."acme.gadget"\]' mcpp.lock || { + cat mcpp.lock 2>/dev/null || true + echo "FAIL: expected acme.gadget lock entry" + exit 1 +} +# Only 2.1.0 is seeded above, so a build that got here at all consumed the +# resolved version rather than falling back to the constraint's lower bound. +# (The lockfile records the CONSTRAINT, not the pin — pre-existing behaviour +# shared with registry deps, not something this test is asserting about.) + +"$MCPP" run > run.log 2>&1 || { cat run.log; echo "FAIL: run failed"; exit 1; } + +# ── The exact-version form keeps working through the same route ───────── +mkdir -p .mcpp/.xlings/data/xpkgs/acme.gadget/2.0.0/src +cp .mcpp/.xlings/data/xpkgs/acme.gadget/2.1.0/src/gadget.cppm \ + .mcpp/.xlings/data/xpkgs/acme.gadget/2.0.0/src/gadget.cppm +sed -i.bak 's/^gadget = "\^2\.0"$/gadget = "2.0.0"/' mcpp.toml && rm -f mcpp.toml.bak +grep -qE '^gadget = "2\.0\.0"$' mcpp.toml || { cat mcpp.toml; echo "FAIL: rewrite to exact version did not apply"; exit 1; } +rm -f mcpp.lock +"$MCPP" build > build2.log 2>&1 || { + cat build2.log + echo "FAIL: exact version through a project [indices] entry regressed" + exit 1 +} +grep -q '2\.0\.0' mcpp.lock || { cat mcpp.lock; echo "FAIL: expected 2.0.0 pin"; exit 1; } + +echo "OK"