diff --git a/mcpp.toml b/mcpp.toml index 07e487b7..54d943b5 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,6 +1,6 @@ [package] name = "mcpp" -version = "2026.8.5.4" +version = "2026.8.6.1" description = "Modern C++ build & package management tool" license = "Apache-2.0" authors = ["mcpp-community"] diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 8bfe86ae..4f0b3e68 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -2602,12 +2602,20 @@ prepare_build(bool print_fingerprint, matches = mcpp::modgraph::expand_glob(verRoot, pat); if (!matches.empty()) break; } + // Name the directory actually searched. `` was a literal + // placeholder, so the message could not distinguish "the package + // is Form B and you forgot the mcpp field" from "the verdir mcpp + // resolved is not this package's at all" — the second is what a + // cross-namespace install_path hit produces, and it sent this + // investigation down the wrong path for a while. if (matches.empty()) return std::unexpected(std::format( "dependency '{}': index entry has no `mcpp = ...` field, " - "and no mcpp.toml was found at /mcpp.toml or " - "/*/mcpp.toml — add an explicit `mcpp = \"\"` " - "or `mcpp = {{ ... }}` block to the .lua descriptor.", - depName)); + "and no mcpp.toml was found at '{}/mcpp.toml' or " + "'{}/*/mcpp.toml' — add an explicit `mcpp = \"\"` " + "or `mcpp = {{ ... }}` block to the .lua descriptor. " + "(If that directory belongs to a DIFFERENT package, the " + "install step resolved the wrong verdir.)", + depName, verRoot.string(), verRoot.string())); if (matches.size() > 1) return std::unexpected(std::format( "dependency '{}': default mcpp.toml lookup matched {} " "files; pin one with explicit `mcpp = \"\"`.", diff --git a/src/fallback/legacy_dirs.cppm b/src/fallback/legacy_dirs.cppm index 0bde7d78..528334bf 100644 --- a/src/fallback/legacy_dirs.cppm +++ b/src/fallback/legacy_dirs.cppm @@ -1,7 +1,34 @@ // mcpp.fallback.legacy_dirs — legacy xpkg directory scan. // -// Last-resort fallback scan (COMPAT, remove in 1.0.0): walk xpkgs/ -// for any directory ending with -x- or -x-. +// Last-resort fallback scan (COMPAT, remove in 1.0.0): walk xpkgs/ for a +// directory that holds the requested package under an older naming layout. +// +// WHY THE SCAN IS NAMESPACE-BOUND +// +// The bare `-x-` arm used to match ANY prefix, so a lookup for +// `ocornut:imgui` happily returned `compat-x-imgui` — a DIFFERENT package that +// merely shares a short name. `Fetcher::install_path` then treats that verdir +// as the requested package's, which means: +// +// * the install is skipped (the package "already exists"), and +// * whatever is inside the other namespace's verdir is what gets read. +// +// It stayed invisible while the two packages carried unrelated versions — +// `compat:imgui@1.92.8` next to `mcpplibs:imgui@0.0.6` never collide, because +// install_path also matches on version. Aligning package versions to upstream +// (mcpp-index#163: `imgui@0.0.6` really was ImGui 1.92.8) makes them coincide, +// and the bug becomes reachable. +// +// The observed failure was loud only by luck: `compat.imgui` is a Form B +// descriptor, so the wrong verdir had no mcpp.toml and the build stopped with +// "index entry has no `mcpp = ...` field" — a diagnostic naming the wrong +// cause. Between two Form A packages the wrong verdir DOES have a manifest, +// and the build would have silently compiled the wrong package. +// +// So the shortName arm now requires the directory's own namespace prefix to be +// one the caller actually asked for. That keeps the legacy layouts it exists +// for (`-x-`, `-x-`) and refuses the one it +// was never meant to serve: some other namespace's package. export module mcpp.fallback.legacy_dirs; @@ -9,23 +36,38 @@ import std; export namespace mcpp::fallback { -// Scan the xpkgs base directory for a legacy install directory whose -// name ends with "-x-" or "-x-". -// Returns the matching directory name (not the full path) if found. +// Scan the xpkgs base directory for a legacy install directory holding +// (namespace, shortName). Returns the matching directory name (not the full +// path) if found. +// +// `acceptedPrefixes` are the directory prefixes (the part before `-x-`) that +// may satisfy a bare short-name match — the requested namespace, and the index +// name for the old index-prefixed layout. A fully-qualified `-x-.` +// match carries the namespace in the suffix itself and needs no prefix check. std::optional scan_legacy_install_dirs(const std::filesystem::path& xpkgsBase, std::string_view qualifiedName, - std::string_view shortName) { + std::string_view shortName, + const std::vector& acceptedPrefixes) { std::error_code ec; std::string suffix1 = std::format("-x-{}", qualifiedName); std::string suffix2 = std::format("-x-{}", shortName); + auto prefix_accepted = [&](const std::string& dirname) { + auto cut = dirname.size() - suffix2.size(); + std::string_view prefix{dirname.data(), cut}; + for (auto& p : acceptedPrefixes) + if (prefix == p) return true; + return false; + }; + for (auto& entry : std::filesystem::directory_iterator(xpkgsBase, ec)) { if (!entry.is_directory()) continue; auto dirname = entry.path().filename().string(); if (dirname.ends_with(suffix1)) return dirname; - if (suffix2 != suffix1 && dirname.ends_with(suffix2)) + if (suffix2 != suffix1 && dirname.ends_with(suffix2) + && prefix_accepted(dirname)) return dirname; } return std::nullopt; diff --git a/src/pm/package_fetcher.cppm b/src/pm/package_fetcher.cppm index d911488f..7b487665 100644 --- a/src/pm/package_fetcher.cppm +++ b/src/pm/package_fetcher.cppm @@ -1173,10 +1173,22 @@ Fetcher::install_path(std::string_view ns, std::string_view shortName, if (auto p = try_dir(dirName)) return *p; } - // Last-resort fallback scan (COMPAT, remove in 1.0.0): walk xpkgs/ for - // any directory ending with -x- or -x-. + // Last-resort fallback scan (COMPAT, remove in 1.0.0): walk xpkgs/ for a + // directory holding this package under an older layout. + // + // The bare `-x-` arm is bound to the namespaces the caller + // actually asked for. Unbound, it returned ANY namespace's directory with + // a matching short name — `ocornut:imgui` resolving to `compat-x-imgui` — + // and install_path's contract is "the verdir for THIS package", so the + // install was skipped and another package's tree read in its place. See + // the header of mcpp.fallback.legacy_dirs for why version alignment is + // what made it reachable. auto qname = mcpp::pm::compat::qualified_name(ns, shortName); - if (auto legacy = mcpp::fallback::scan_legacy_install_dirs(base, qname, shortName)) { + std::vector acceptedPrefixes; + if (!ns.empty()) acceptedPrefixes.emplace_back(ns); + if (!cfg_.defaultIndex.empty()) acceptedPrefixes.emplace_back(cfg_.defaultIndex); + if (auto legacy = mcpp::fallback::scan_legacy_install_dirs( + base, qname, shortName, acceptedPrefixes)) { if (auto p = try_dir(*legacy)) return *p; } return std::nullopt; diff --git a/src/version.cppm b/src/version.cppm index cf953421..3741777b 100644 --- a/src/version.cppm +++ b/src/version.cppm @@ -31,6 +31,6 @@ import std; export namespace mcpp { -inline constexpr std::string_view MCPP_VERSION = "2026.8.5.4"; +inline constexpr std::string_view MCPP_VERSION = "2026.8.6.1"; } // namespace mcpp diff --git a/tests/e2e/191_link_scale.sh b/tests/e2e/191_link_scale.sh new file mode 100755 index 00000000..bf61cb9a --- /dev/null +++ b/tests/e2e/191_link_scale.sh @@ -0,0 +1,140 @@ +#!/usr/bin/env bash +# 191_link_scale.sh — a link edge with more objects than a command line can hold. +# +# WHAT THIS COVERS THAT NOTHING ELSE DID +# +# mcpp#346: no CI job builds a package of the opencv/ffmpeg magnitude. Every +# job builds either mcpp itself (tens of TUs) or a synthetic e2e project +# (single digits). Link-line length, the response-file path, and ninja graph +# size over a large object set therefore had ZERO coverage, and the whole +# command-length defect family surfaced in the ecosystem rather than in CI: +# +# #274 ninja goals argv 50781 chars vs cmd.exe 8191 +# #247 Windows CreateProcess 32 KiB +# #345 POSIX MAX_ARG_STRLEN 128 KiB (ninja spawns `sh -c ""`) +# #360 link.exe LNK1170, response-file LINE capped at 128 KiB +# +# 190 asserts the SHAPE of the generated rule (`rspfile_content = $in_newline`, +# 25 objects). This asserts the SCALE: it builds a link edge whose object list +# does not fit in a command line on any supported platform, and requires it to +# link and run. A regression that puts objects back on the command line fails +# here for the same reason opencv-module failed in the ecosystem — except in +# 20 seconds, with no external package, and with a diagnostic that names the +# cause. +# +# WHY THE SIZE ASSERTION IS PART OF THE TEST +# +# The regime is what gives this test its value, and the regime depends on +# incidental things — object naming, the disambiguation prefix, how many files +# the loop below writes. If any of them shrinks the object list back under the +# ceiling, the test would keep passing while covering nothing. So the response +# file's size is asserted directly: below the ceiling the test reports that it +# has stopped covering the axis, rather than passing quietly. +# +# VERIFIED TO FAIL WITHOUT THE FIX +# +# Reverting the generated `cxx_link` rule to its pre-#345 inline form on this +# exact project reproduces the original symptom verbatim: +# +# ninja: fatal: posix_spawn: Argument list too long +# +# COST +# +# C sources, one trivial function each. Measured at 1.3s wall on a developer +# machine (1400 TUs, parallel). Windows is the expensive leg — per-process +# spawn cost dominates there — and is bounded by the suite's 600s per-test +# timeout. +# +# The names are padded so each object path is ~100 bytes, which is what +# reaches the 128 KiB ceiling at a file count this small: padding trades +# compile time (expensive) for path length (free). Padding is bounded on the +# other side by Windows MAX_PATH — 100 bytes relative, plus the temp directory +# and the build directory, stays near 200 of the 260 available. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +# ── The ceiling this test has to clear ──────────────────────────────────── +# POSIX MAX_ARG_STRLEN, the largest of the command-line limits in +# src/build/cmdlimits.cppm — clearing the largest clears all of them, so one +# number works for every platform the suite runs on. +CEILING=$((128 * 1024)) + +PAD=$(printf 'x%.0s' $(seq 1 88)) +N=1400 + +mkdir -p scale/src +cat > scale/mcpp.toml <<'EOF' +[package] +name = "scale" +version = "0.1.0" + +[build] +c_standard = "c11" +EOF + +i=1 +while [ "$i" -le "$N" ]; do + printf 'int f%04d(void) { return %d; }\n' "$i" "$i" > "scale/src/f${i}_${PAD}.c" + i=$((i + 1)) +done +# The bin target is inferred from src/main.cpp; the objects it links are the C +# TUs above. Referencing the FIRST and the LAST of them is what makes a +# truncated object list a link error rather than a smaller binary: a response +# file cut short at any point drops one of these two. +cat > scale/src/main.cpp < b.log 2>&1 || { tail -40 b.log; echo "FAIL: build $N objects"; exit 1; } +echo " ok: built $((N + 1)) objects" + +ninja_file=$(find target -name build.ninja | head -1) +[ -n "$ninja_file" ] || { echo "FAIL: no build.ninja"; exit 1; } +bdir=$(dirname "$ninja_file") +bin_rel=$(cd "$bdir" && ls bin/ 2>/dev/null | head -1) +[ -n "$bin_rel" ] || { echo "FAIL: nothing was linked"; exit 1; } + +# Relink with the response file kept, so its real contents can be inspected — +# the same technique as 190, at a size that matters. +(cd "$bdir" && rm -f "bin/$bin_rel" && ninja -d keeprsp "bin/$bin_rel" > relink.log 2>&1) || { + tail -20 "$bdir/relink.log"; echo "FAIL: relink under -d keeprsp"; exit 1; } + +rsp=$(find "$bdir" -name '*.rsp' | head -1) +[ -n "$rsp" ] || { echo "FAIL: -d keeprsp left no response file"; exit 1; } + +bytes=$(wc -c < "$rsp" | tr -d ' ') +objects=$(( $(wc -l < "$rsp" | tr -d ' ') + 1 )) + +# 1. NON-VACUITY: the object list must not fit in a command line. Without +# this, everything below could pass on a link edge small enough that the +# inline form would have worked too. +[ "$bytes" -gt "$CEILING" ] || { + echo "response file is $bytes bytes over $objects objects; the ceiling is $CEILING" + echo "FAIL: this test no longer reaches the regime it exists to cover." + echo " Raise N or PAD until the object list exceeds the ceiling again." + exit 1; } +echo " ok: object list is $bytes bytes ($objects objects), past the ${CEILING}-byte command-line ceiling" + +# 2. No single line approaches link.exe's per-line cap (the LNK1170 axis) — +# asserted here at real scale rather than 190's 25 objects. +longest=$(awk '{ if (length($0) > m) m = length($0) } END { print m+0 }' "$rsp") +[ "$longest" -lt 4096 ] || { + echo "FAIL: longest response-file line is $longest chars — objects are not one per line" + exit 1; } +echo " ok: longest response-file line $longest chars" + +# 3. The objects were really consumed. main.cpp calls into both ends of the +# object list, so a truncated response file cannot reach this point — it +# fails at link time with an undefined reference. Running the binary closes +# the remaining gap: that the values arriving at runtime are the ones the +# two TUs return. +"./$bdir/bin/$bin_rel" || { echo "FAIL: linked binary did not run cleanly"; exit 1; } +echo " ok: linked binary runs" + +echo "OK" diff --git a/tests/e2e/192_install_path_namespace.sh b/tests/e2e/192_install_path_namespace.sh new file mode 100755 index 00000000..72a7c9e6 --- /dev/null +++ b/tests/e2e/192_install_path_namespace.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# requires: gcc fresh-sandbox +# 192_install_path_namespace.sh — a package must never be satisfied from +# ANOTHER namespace's install directory. +# +# `Fetcher::install_path(ns, shortName, version)` answers "where is THIS +# package installed". Its last-resort legacy scan matched any directory ending +# in `-x-`, whatever namespace that directory belonged to, so a +# lookup for `acme:widget@1.5.0` returned `compat-x-widget/1.5.0`. The caller +# then skips the install ("already present") and reads the other package's +# tree. +# +# WHY IT WAS UNREACHABLE UNTIL NOW +# +# install_path also matches on version, so two packages sharing a short name +# collided only if they also shared a version — and the ecosystem's module +# layers carried packaging counters (`imgui@0.0.6`) while the compat packages +# carried upstream versions (`compat.imgui@1.92.8`). Aligning the module layers +# to upstream (mcpp-index#163) makes them coincide, which is how this surfaced. +# +# WHY THE SILENT SHAPE IS THE ONE THAT MATTERS +# +# Discovered against a Form B neighbour, so the wrong verdir had no mcpp.toml +# and the build stopped — with a diagnostic naming the wrong cause. Between two +# packages whose sources live in the verdir, there is no error at all: the +# build compiles the wrong package's source. That is what this test pins. +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" + +# ── Two packages, same short name, same version, different namespaces ───── +mkdir -p local-index/pkgs/a +cat > local-index/pkgs/a/acme.widget.lua <<'EOF' +package = { + spec = "1", + namespace = "acme", + name = "widget", + description = "acme's widget", + licenses = {"MIT"}, + type = "package", + xpm = { + linux = { ["1.5.0"] = { url = "https://example.invalid/w.tar.gz", sha256 = "0000000000000000000000000000000000000000000000000000000000000000" } }, + macosx = { ["1.5.0"] = { url = "https://example.invalid/w.tar.gz", sha256 = "0000000000000000000000000000000000000000000000000000000000000000" } }, + windows = { ["1.5.0"] = { url = "https://example.invalid/w.zip", sha256 = "0000000000000000000000000000000000000000000000000000000000000000" } }, + }, + mcpp = { + language = "c++23", + import_std = false, + sources = { "src/widget.cppm" }, + targets = { ["widget"] = { kind = "lib" } }, + deps = {}, + }, +} +EOF + +# ── Only the FOREIGN namespace's payload is on disk ─────────────────────── +# `compat:widget@1.5.0` is installed; `acme:widget@1.5.0` is not. Nothing may +# hand acme's lookup this directory. +# +# It goes in the GLOBAL store ($MCPP_HOME/registry/data/xpkgs), which is what +# `Fetcher::install_path` scans — the project's own .mcpp/.xlings tree is a +# different root reached by `install_path_from_project_data`. Seeding the wrong +# one makes this test pass on a broken binary, which is exactly what the first +# draft did. +mkdir -p "$MCPP_HOME/registry/data/xpkgs/compat-x-widget/1.5.0/src" +cat > "$MCPP_HOME/registry/data/xpkgs/compat-x-widget/1.5.0/src/widget.cppm" <<'EOF' +export module widget; +// If this ever reaches a build that asked for acme:widget, the wrong package +// was compiled. The value is the tell. +export int widget_value() { return 1; } +EOF + +mkdir -p src +cat > src/main.cpp <<'EOF' +import widget; +int main() { return widget_value() == 2 ? 0 : 1; } +EOF + +cat > mcpp.toml <<'EOF' +[package] +name = "consumer" +version = "0.1.0" + +[dependencies.acme] +widget = "1.5.0" + +[indices] +acme = { path = "local-index" } +EOF + +# The build must NOT succeed: acme:widget is not installed and its url is +# unreachable by design. What it must never do is satisfy the dependency from +# compat's directory. +if "$MCPP" build > b.log 2>&1; then + echo "--- build log ---"; cat b.log + echo "FAIL: build succeeded, so acme:widget was satisfied from somewhere —" + echo " the only widget payload on disk belongs to compat." + exit 1 +fi + +# Distinguish "correctly refused" from "compiled the wrong package and then +# failed for an unrelated reason": compat's source must never be compiled. +if grep -qE "compat-x-widget/1\.5\.0/src/widget\.cppm" b.log; then + grep -nE "compat-x-widget" b.log | head -5 + echo "FAIL: the build reached compat's source while resolving acme:widget" + exit 1 +fi +echo " ok: acme:widget was not satisfied from compat-x-widget" + +# And the diagnostic has to name the directory it looked in — `` as a +# literal placeholder cannot distinguish a missing mcpp field from a wrong +# verdir, which is what made this take a while to find. +if grep -q "" b.log; then + echo "FAIL: diagnostic still prints the literal placeholder ''" + exit 1 +fi +echo " ok: diagnostics name a real path" + +echo "OK" diff --git a/tests/unit/test_fallback_legacy_dirs.cpp b/tests/unit/test_fallback_legacy_dirs.cpp new file mode 100644 index 00000000..f8c7871f --- /dev/null +++ b/tests/unit/test_fallback_legacy_dirs.cpp @@ -0,0 +1,73 @@ +#include + +import std; +import mcpp.fallback.legacy_dirs; + +// The legacy install-dir scan must not cross namespaces. +// +// `Fetcher::install_path` contract is "the verdir for THIS package". Its +// last-resort scan used to match any directory ending in `-x-`, +// regardless of which namespace that directory belongs to. A lookup for +// `ocornut:imgui` therefore returned `compat-x-imgui` — a different package +// that merely shares a short name — and the caller then (a) skipped the +// install because the package "already exists" and (b) read the other +// package's tree. +// +// It was unreachable while the two carried unrelated versions +// (`compat:imgui@1.92.8` vs `mcpplibs:imgui@0.0.6`), because install_path also +// matches on version. Aligning package versions to upstream (mcpp-index#163) +// makes them coincide. + +namespace { + +std::filesystem::path make_xpkgs(std::initializer_list dirs) { + auto base = std::filesystem::temp_directory_path() + / std::format("legacy-dirs-{}", + std::chrono::steady_clock::now() + .time_since_epoch().count()); + for (auto d : dirs) std::filesystem::create_directories(base / std::string(d)); + return base; +} + +} // namespace + +TEST(FallbackLegacyDirs, DoesNotReturnAnotherNamespacesDirectory) { + auto base = make_xpkgs({"compat-x-imgui"}); + // Asking for ocornut:imgui. Only compat's directory exists, and it is NOT + // an answer to this question. + auto hit = mcpp::fallback::scan_legacy_install_dirs( + base, "ocornut.imgui", "imgui", {"ocornut", "mcpplibs"}); + EXPECT_FALSE(hit.has_value()) + << "returned '" << hit.value_or("") << "' for ocornut:imgui"; + std::filesystem::remove_all(base); +} + +TEST(FallbackLegacyDirs, StillFindsOwnNamespaceShortNameLayout) { + auto base = make_xpkgs({"compat-x-imgui", "ocornut-x-imgui"}); + auto hit = mcpp::fallback::scan_legacy_install_dirs( + base, "ocornut.imgui", "imgui", {"ocornut", "mcpplibs"}); + ASSERT_TRUE(hit.has_value()); + EXPECT_EQ(*hit, "ocornut-x-imgui"); + std::filesystem::remove_all(base); +} + +TEST(FallbackLegacyDirs, StillFindsIndexPrefixedLegacyLayout) { + // The old layout this arm exists for: -x-. + auto base = make_xpkgs({"mcpplibs-x-tinyhttps"}); + auto hit = mcpp::fallback::scan_legacy_install_dirs( + base, "mcpplibs.tinyhttps", "tinyhttps", {"mcpplibs", "mcpplibs"}); + ASSERT_TRUE(hit.has_value()); + EXPECT_EQ(*hit, "mcpplibs-x-tinyhttps"); + std::filesystem::remove_all(base); +} + +TEST(FallbackLegacyDirs, QualifiedSuffixNeedsNoPrefixAllowance) { + // `-x-.` carries the namespace in the suffix itself, so it is + // accepted whatever the prefix is — that is the legacy FQN layout. + auto base = make_xpkgs({"someindex-x-ocornut.imgui"}); + auto hit = mcpp::fallback::scan_legacy_install_dirs( + base, "ocornut.imgui", "imgui", {"ocornut"}); + ASSERT_TRUE(hit.has_value()); + EXPECT_EQ(*hit, "someindex-x-ocornut.imgui"); + std::filesystem::remove_all(base); +}