Skip to content

feat: reject mcpp add for packages not present in the index #307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Sunrisepeak merged 2 commits into from
Jul 29, 2026
Merged

feat: reject mcpp add for packages not present in the index #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5032fe6
feat: reject mcpp add for packages not present in the index
ZheFeng7110 Jul 29, 2026
fabffe3
fix: route `mcpp add`'s existence check through the real index resolu…
Sunrisepeak Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 23 additions & 73 deletions src/build/prepare.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import mcpp.fetcher;
import mcpp.fetcher.progress;
import mcpp.pm.resolver;
import mcpp.pm.index_spec;
import mcpp.pm.index_route;
import mcpp.pm.mangle;
import mcpp.pm.compat;
import mcpp.pm.dep_spec;
Expand Down Expand Up @@ -694,20 +695,10 @@ prepare_build(bool print_fingerprint,
m->targetOverrides[triple] = entry;
}
}
// Inherit workspace indices if member doesn't define any. A
// relative `[indices].path` was declared at the WORKSPACE root,
// so it must resolve against `*root` (still the workspace root
// here), not the member directory — otherwise every member
// needs its own `../`-prefixed copy of the same declaration
// (#224).
if (m->indices.empty() && !wsManifest->indices.empty()) {
m->indices = wsManifest->indices;
for (auto& [_, idx] : m->indices) {
if (idx.is_local() && idx.path.is_relative()) {
idx.path = std::filesystem::weakly_canonical(*root / idx.path);
}
}
}
// Inherit workspace indices if member doesn't define any. `*root`
// is still the workspace root here, which is what a relative
// `[indices].path` was written against (#224).
mcpp::project::inherit_workspace_indices(*m, *wsManifest, *root);

mcpp::ui::status("Workspace", std::format("building member '{}'", targetMember));
root = memberDir;
Expand All @@ -730,14 +721,7 @@ prepare_build(bool print_fingerprint,
}
}
// Inherit workspace indices if member doesn't define any
if (m->indices.empty() && !wsm->indices.empty()) {
m->indices = wsm->indices;
for (auto& [_, idx] : m->indices) {
if (idx.is_local() && idx.path.is_relative()) {
idx.path = std::filesystem::weakly_canonical(wsRoot / idx.path);
}
}
}
mcpp::project::inherit_workspace_indices(*m, *wsm, wsRoot);
}
}
}
Expand Down Expand Up @@ -1455,28 +1439,19 @@ 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<std::filesystem::path, mcpp::manifest::Manifest>;
// Helper: find the IndexSpec for a namespace from the manifest's [indices].
// Returns nullptr if the namespace maps to the default/builtin index.
// 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*
{
if (ns.empty() || ns == std::string(mcpp::pm::kDefaultNamespace)) {
// R6: `[indices] default = {...}` (normalized to
// kDefaultNamespace by toml.cppm) redirects the default
// namespace — return it when present instead of unconditionally
// falling back to the builtin index.
auto it = m->indices.find(std::string(mcpp::pm::kDefaultNamespace));
return it == m->indices.end() ? nullptr : &it->second;
}
if (auto it = m->indices.find(ns); it != m->indices.end()) {
return &it->second;
}
auto root = ns.substr(0, ns.find('.'));
for (auto& [idxName, spec] : m->indices) {
if (idxName == ns) return &spec;
if (idxName == root) return &spec;
}
return nullptr;
return index_route().find_for_ns(ns);
};

// Identity-first candidate probe. A candidate is DISAMBIGUATED by the
Expand Down Expand Up @@ -1510,19 +1485,7 @@ prepare_build(bool print_fingerprint,
{
auto cfg = get_cfg();
if (!cfg) return std::nullopt;

auto* idxSpec = findIndexForNs(coord.namespace_);
if (idxSpec && idxSpec->is_local()) {
auto indexPath = mcpp::config::resolve_project_index_path(*root, *idxSpec);
return mcpp::fetcher::Fetcher::read_xpkg_lua_from_path(
indexPath, coord.namespace_, coord.shortName);
}
if (idxSpec && !idxSpec->is_builtin()) {
return mcpp::fetcher::Fetcher::read_xpkg_lua_from_project_data(
*root, coord.namespace_, coord.shortName);
}
mcpp::fetcher::Fetcher fetcher(**cfg);
return fetcher.read_xpkg_lua(coord.namespace_, coord.shortName);
return index_route(*cfg).read(coord);
};

auto xpkgLuaMatchesCandidate =
Expand Down Expand Up @@ -1613,14 +1576,10 @@ prepare_build(bool print_fingerprint,
// package that would have materialized a moment later. Local path
// indices and the builtin index are both readable here, so they stay
// under the strict rule below.
bool anyLazyGitIndex = false;
for (auto& c : candidates) {
auto* idx = findIndexForNs(c.namespace_);
if (idx && !idx->is_builtin() && !idx->is_local()) {
anyLazyGitIndex = true;
break;
}
}
bool anyLazyGitIndex = std::ranges::any_of(candidates,
[&](const mcpp::pm::DependencyCoordinate& c) {
return index_route().lazy_git(c.namespace_);
});

// T9 (#278) — no candidate resolved. This used to fall through to
// `candidates.front()` SILENTLY, so mcpp carried on with a namespace
Expand All @@ -1641,17 +1600,8 @@ prepare_build(bool print_fingerprint,
// error string (see Fetcher::scan_fqns_with_short_name).
std::string hint;
if (auto cfg = get_cfg()) {
mcpp::fetcher::Fetcher fetcher(**cfg);
auto roots = fetcher.builtin_index_roots();
for (auto& [idxName, idxSpec] : m->indices) {
if (idxSpec.is_local()) {
roots.push_back(
mcpp::config::resolve_project_index_path(
*root, idxSpec));
}
}
auto fqns = mcpp::fetcher::Fetcher::scan_fqns_with_short_name(
roots, candidates.front().shortName);
auto fqns = mcpp::pm::cross_namespace_matches(
index_route(*cfg), candidates.front().shortName);
if (!fqns.empty()) {
hint += "\n a package with this name exists under "
"another namespace:";
Expand Down
144 changes: 140 additions & 4 deletions src/pm/commands.cppm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,68 @@
// into the pm subsystem so `cli.cppm` is responsible only for the
// global CLI framework + non-pm commands.
//
// Strict zero-behavior-change move: every line below is identical to
// what previously lived in `cli.cppm`, only the surrounding namespace
// has changed.
// The bodies started life as a strict zero-behavior-change move out of
// `cli.cppm`; `cmd_add` has since grown the index existence gate (#305).

export module mcpp.pm.commands;

import std;
import mcpp.manifest; // kDefaultNamespace alias
import mcpp.config;
import mcpp.fetcher.progress; // bootstrap progress for load_or_init
import mcpp.manifest; // kDefaultNamespace alias
import mcpp.lockfile; // load / write (still via shim)
import mcpp.platform.axis; // HostPlatform for the published-version check
import mcpp.pm.dep_spec; // DependencyCoordinate
import mcpp.pm.dependency_selector; // same candidates the manifest parser derives
import mcpp.pm.index_route; // shared index routing (with mcpp.build.prepare)
import mcpp.pm.resolver; // is_version_constraint
import mcpp.project; // shared find_manifest_root
import mcpp.ui;
import mcpp.xlings; // index freshness
import mcpplibs.cmdline;

namespace mcpp::pm::commands::detail {

// Render candidate coordinates the way prepare's resolution error does, so a
// rejected `mcpp add` and a failed `mcpp build` name the same identities.
inline std::string format_tried(
const std::vector<mcpp::pm::DependencyCoordinate>& candidates) {
std::string tried;
for (auto& c : candidates) {
if (!tried.empty()) tried += ", ";
tried += c.namespace_.empty()
? std::format("(no namespace, {})", c.shortName)
: std::format("({}, {})", c.namespace_, c.shortName);
}
return tried;
}

// A version the descriptor does not publish is worth flagging but not worth
// refusing over: version tables are per-OS, so "absent for this host" is not
// "absent". Say what IS published and let the user decide — the alternative is
// a hard failure on a dependency that resolves fine on the platform it was
// added for.
inline void warn_unpublished_version(std::string_view lua,
std::string_view display,
const std::string& version) {
if (mcpp::pm::is_version_constraint(version)) return;
auto versions = mcpp::manifest::list_xpkg_versions(
lua, mcpp::platform::HostPlatform::current());
if (versions.empty()) return;
if (std::ranges::find(versions, version) != versions.end()) return;

std::string avail;
for (auto& v : versions) {
if (!avail.empty()) avail += ", ";
avail += v;
}
mcpp::ui::warning(std::format(
"'{}' has no version {} published for this platform — available: {}",
display, version, avail));
}

} // namespace mcpp::pm::commands::detail

export namespace mcpp::pm::commands {

inline int cmd_add(const mcpplibs::cmdline::ParsedArgs& parsed) {
Expand Down Expand Up @@ -68,6 +117,93 @@ inline int cmd_add(const mcpplibs::cmdline::ParsedArgs& parsed) {
return 2;
}

// ── Existence gate (#305) ──────────────────────────────────────────
// Refuse to write a dependency no index can serve, so a typo fails here
// instead of halfway into the next `mcpp build`. Two rules keep the gate
// from refusing packages that are perfectly real:
//
// • It probes the SAME candidates the manifest parser will derive from
// the key about to be written. A dotted selector is a namespace path,
// not a name: `capi.lua` means `(mcpplibs.capi, lua)` then
// `(capi, lua)`, and a literal `(mcpplibs, "capi.lua")` probe can
// never match one — `package.name` is a single atomic segment
// (SPEC-001 §3.2), so nothing in any index is named "capi.lua".
// • It reads through mcpp.pm.index_route, the same routing
// `mcpp.build.prepare` resolves dependencies with. A package served by
// a project `[indices]` entry therefore counts as present, and a
// namespace no readable index can speak for is reported as unverified
// rather than rejected: refusing an add is only correct when absence
// was actually proven.
{
auto selector = explicitNamespace
? mcpp::pm::make_direct_dependency_selector(ns, shortName, nameSpec)
: mcpp::pm::resolve_dependency_selector(
nameSpec,
mcpp::pm::DependencySelectorMode::OmittedMcpplibsPriority);

auto cfg = mcpp::config::load_or_init(
/*quiet=*/false, mcpp::fetcher::make_bootstrap_progress_callback());
if (!cfg) {
mcpp::ui::error(cfg.error().message);
return 4;
}

auto indices = mcpp::pm::effective_indices(*root);
mcpp::pm::IndexRoute route{ &indices, *root, &*cfg };
auto found = mcpp::pm::lookup_descriptor(route, selector.candidates);

// Does the shared registry answer for any identity we tried? It is the
// only index a refresh can do anything about — a project
// `[indices] path = …` is whatever the user has on disk.
const bool registryInvolved = std::ranges::any_of(selector.candidates,
[&](const mcpp::pm::DependencyCoordinate& c) {
auto* idx = route.find_for_ns(c.namespace_);
return !idx || idx->is_builtin();
});

// Only pay for a refresh when the answer was "no" and the registry
// that would have answered is stale — a package that is already on
// disk costs zero network round-trips.
if (!found.hit && found.conclusive && registryInvolved) {
auto xlEnv = mcpp::config::make_xlings_env(*cfg);
if (!mcpp::xlings::is_index_fresh(xlEnv, cfg->searchTtlSeconds)) {
mcpp::ui::status("Updating", "package index (auto-refresh)");
mcpp::xlings::ensure_index_fresh(
xlEnv, cfg->searchTtlSeconds, /*quiet=*/true);
found = mcpp::pm::lookup_descriptor(route, selector.candidates);
}
}

if (!found.hit && found.conclusive) {
std::string hint;
if (!selector.candidates.empty()) {
for (auto& fqn : mcpp::pm::cross_namespace_matches(
route, selector.candidates.front().shortName)) {
hint += "\n " + fqn;
}
}
if (!hint.empty()) {
hint = "\n a package with this name exists under another "
"namespace:" + hint;
}
if (registryInvolved) {
hint += "\n hint: `mcpp index update` if it was published "
"recently";
}
mcpp::ui::error(std::format(
"package '{}' not found in any configured index\n tried: {}{}",
nameSpec, detail::format_tried(selector.candidates), hint));
return 2;
}
if (!found.hit) {
mcpp::ui::warning(std::format(
"'{}' could not be verified — no readable index covers that "
"namespace yet; adding it unchecked", nameSpec));
} else {
detail::warn_unpublished_version(found.hit->lua, nameSpec, version);
}
}

std::ifstream in(manifestPath);
std::stringstream ss; ss << in.rdbuf();
std::string text = ss.str();
Expand Down
Loading
Loading