Skip to content

Commit 7f8d3e4

Browse files
authored
fix: mcpp self config --mirror X hangs on fresh MCPP_HOME (#76)
Root cause: on a fresh MCPP_HOME, write_default_xlings_json seeds .xlings.json with the hardcoded `"CN"` default, regardless of --mirror. Then load_or_init's xlings sandbox bootstrap (ensure_init / patchelf / ninja install) reads that mirror and downloads via CN. Only after the bootstrap finishes does cmd_self_config invoke xlings::config_set_mirror to update the file — too late: the user's --mirror choice never reached the very network roundtrip they were trying to redirect. For an overseas user the CN mirror is slow or unreachable, so the "download" phase hangs. Fix (~15 lines): plumb the user's mirror choice through to the seed. - `config::write_default_xlings_json` grows `mirror_override` param. - `config::load_or_init` grows `initial_mirror` param (default empty; all existing callers unchanged). Passed straight through to write_default_xlings_json, so when set it overrides the "CN" default for the FRESH seed. - `cmd_self_config` parses --mirror first, validates, then forwards it as load_or_init's initial_mirror. The subsequent xlings::config_set_mirror still runs (idempotent on fresh, required for the already-initialized case where seed was skipped). - TODO comment at seed_xlings_json's `"CN"` default about the long-term direction (flip default to GLOBAL, or auto-detect). For an already-initialized MCPP_HOME (`.xlings.json` already exists), the seed is skipped — no behavior change there. config_set_mirror handles that case as before. Test: tests/e2e/46_self_config_mirror_no_bootstrap.sh asserts that after a fresh `mcpp self config --mirror GLOBAL`, the seeded .xlings.json has `mirror: GLOBAL` (not the default CN). Then round-trips back to CN to exercise the post-init path. Existing 38_self_config_mirror.sh still passes: it always calls `mcpp self env` first, which doesn't pass an initial_mirror, so the "default seed is CN" invariant is preserved.
1 parent a349d71 commit 7f8d3e4

4 files changed

Lines changed: 107 additions & 14 deletions

File tree

src/cli.cppm

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4355,26 +4355,40 @@ std::string upper_ascii(std::string s) {
43554355
}
43564356

43574357
int cmd_self_config(const mcpplibs::cmdline::ParsedArgs& parsed) {
4358-
auto cfg = mcpp::config::load_or_init(/*quiet=*/false, make_bootstrap_progress_callback());
4358+
auto mirror = parsed.option_or_empty("mirror").value();
4359+
if (!mirror.empty()) {
4360+
mirror = upper_ascii(std::move(mirror));
4361+
if (mirror != "CN" && mirror != "GLOBAL") {
4362+
mcpp::ui::error(std::format(
4363+
"invalid mirror '{}'; expected CN or GLOBAL", mirror));
4364+
return 2;
4365+
}
4366+
}
4367+
4368+
// When --mirror is given AND this is a fresh MCPP_HOME, seed .xlings.json
4369+
// with the user's choice on the very first write so the immediately-
4370+
// following xlings sandbox bootstrap (patchelf / ninja download) uses
4371+
// their mirror — not the historical CN default that an overseas user
4372+
// is trying to redirect away from. For an already-initialized MCPP_HOME
4373+
// the seed is skipped and config_set_mirror below updates the existing
4374+
// file via xlings.
4375+
//
4376+
// TODO(mirror-default): the default "CN" lives in
4377+
// mcpp::xlings::seed_xlings_json — see the matching note there for the
4378+
// long-term plan (flip default to GLOBAL, or auto-detect on first init).
4379+
auto cfg = mcpp::config::load_or_init(
4380+
/*quiet=*/false, make_bootstrap_progress_callback(), mirror);
43594381
if (!cfg) {
43604382
mcpp::ui::error(cfg.error().message);
43614383
return 4;
43624384
}
43634385

43644386
auto env = mcpp::config::make_xlings_env(*cfg);
4365-
auto mirror = parsed.option_or_empty("mirror").value();
43664387
if (mirror.empty()) {
43674388
auto rc = mcpp::xlings::config_show(env);
43684389
return rc == 0 ? 0 : 1;
43694390
}
43704391

4371-
mirror = upper_ascii(std::move(mirror));
4372-
if (mirror != "CN" && mirror != "GLOBAL") {
4373-
mcpp::ui::error(std::format(
4374-
"invalid mirror '{}'; expected CN or GLOBAL", mirror));
4375-
return 2;
4376-
}
4377-
43784392
auto rc = mcpp::xlings::config_set_mirror(env, mirror, /*quiet=*/true);
43794393
if (rc != 0) {
43804394
mcpp::ui::error(std::format("failed to set xlings mirror to {}", mirror));

src/config.cppm

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,17 @@ using BootstrapFile = mcpp::xlings::BootstrapFile;
186186
using BootstrapProgress = mcpp::xlings::BootstrapProgress;
187187
using BootstrapProgressCallback = mcpp::xlings::BootstrapProgressCallback;
188188

189+
// `initial_mirror`: when seeding a *fresh* .xlings.json (file did not
190+
// already exist), use this as the mirror value instead of the default.
191+
// Empty means "use the default". Used by `mcpp self config --mirror X`
192+
// so that the very first network roundtrip (during the immediately-
193+
// following xlings sandbox bootstrap) goes through the user's chosen
194+
// mirror, not the historical CN default. No effect when .xlings.json
195+
// already exists — that case is handled by xlings::config_set_mirror.
189196
std::expected<GlobalConfig, ConfigError> load_or_init(
190197
bool quiet = false,
191-
BootstrapProgressCallback onBootstrapProgress = {});
198+
BootstrapProgressCallback onBootstrapProgress = {},
199+
std::string_view initial_mirror = {});
192200

193201
// Pretty-print resolved config for `mcpp env` command.
194202
void print_env(const GlobalConfig& cfg);
@@ -310,7 +318,8 @@ default_backend = "ninja"
310318
}
311319

312320
bool write_default_xlings_json(const std::filesystem::path& path,
313-
const std::vector<IndexRepo>& repos)
321+
const std::vector<IndexRepo>& repos,
322+
std::string_view mirror_override = {})
314323
{
315324
// Delegate to xlings module. Convert IndexRepo vec to pair span.
316325
std::vector<std::pair<std::string,std::string>> pairs;
@@ -320,7 +329,10 @@ bool write_default_xlings_json(const std::filesystem::path& path,
320329
// construct a temporary Env with home = path.parent_path().
321330
mcpp::xlings::Env env;
322331
env.home = path.parent_path();
323-
mcpp::xlings::seed_xlings_json(env, pairs);
332+
if (mirror_override.empty())
333+
mcpp::xlings::seed_xlings_json(env, pairs);
334+
else
335+
mcpp::xlings::seed_xlings_json(env, pairs, mirror_override);
324336
return std::filesystem::exists(path);
325337
}
326338

@@ -385,7 +397,8 @@ void ensure_sandbox_patchelf(const GlobalConfig& cfg, bool quiet,
385397

386398
std::expected<GlobalConfig, ConfigError> load_or_init(
387399
bool quiet,
388-
BootstrapProgressCallback onBootstrapProgress)
400+
BootstrapProgressCallback onBootstrapProgress,
401+
std::string_view initial_mirror)
389402
{
390403
GlobalConfig cfg;
391404

@@ -507,7 +520,7 @@ std::expected<GlobalConfig, ConfigError> load_or_init(
507520
// without losing xlings' active subos or version bindings.
508521
auto xjson = cfg.xlingsHome() / ".xlings.json";
509522
if (!std::filesystem::exists(xjson)) {
510-
write_default_xlings_json(xjson, cfg.indexRepos);
523+
write_default_xlings_json(xjson, cfg.indexRepos, initial_mirror);
511524
} else {
512525
mcpp::fallback::migrate_xlings_json_index_names(xjson);
513526
}

src/xlings.cppm

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,20 @@ int install_with_progress(const Env& env, std::string_view target,
197197
// ─── Sandbox lifecycle ──────────────────────────────────────────────
198198

199199
// Write .xlings.json seed file.
200+
//
201+
// TODO(mirror-default): the default `"CN"` is the historical setting for
202+
// the project's initial Chinese-mainland user base, but it bites overseas
203+
// users (and CI on GitHub-hosted runners) — the first network roundtrip
204+
// goes through a CN mirror that is slow/unreachable for them. The
205+
// `mcpp self config --mirror X` flow now passes the user's choice as an
206+
// override through to here, so they can pick the right mirror BEFORE the
207+
// first download. Longer term, consider:
208+
// (a) flip the default to "GLOBAL" and have CN users opt in via
209+
// `mcpp self config --mirror CN` (smaller blast radius once docs
210+
// cover the switch); or
211+
// (b) auto-detect on first init (env hint like LANG, a quick HEAD probe
212+
// to github.com vs. ghproxy with a tight timeout, and pin the
213+
// winning value into .xlings.json).
200214
void seed_xlings_json(const Env& env,
201215
std::span<const std::pair<std::string,std::string>> repos,
202216
std::string_view mirror = "CN");
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
#!/usr/bin/env bash
2+
# requires:
3+
# 46_self_config_mirror_fresh_seed.sh — `mcpp self config --mirror X` on a
4+
# fresh MCPP_HOME must seed .xlings.json with X from the very first write,
5+
# so the immediately-following sandbox bootstrap (patchelf / ninja
6+
# download) uses the user's chosen mirror instead of the historical CN
7+
# default.
8+
#
9+
# This is the user-visible fix for the "fresh-install mirror config hangs
10+
# on overseas networks" report: previously the seed always wrote
11+
# `"mirror": "CN"` regardless of --mirror, and bootstrap downloads went
12+
# through an unreachable CN proxy before xlings::config_set_mirror had a
13+
# chance to change the value.
14+
set -e
15+
16+
TMP=$(mktemp -d)
17+
trap "rm -rf $TMP" EXIT
18+
19+
export MCPP_HOME="$TMP/fresh"
20+
21+
# Sanity: file doesn't exist yet — we're truly fresh.
22+
[[ ! -f "$MCPP_HOME/registry/.xlings.json" ]] || {
23+
echo "FAIL: precondition — registry/.xlings.json already exists"; exit 1; }
24+
25+
"$MCPP" self config --mirror GLOBAL > "$TMP/out.log" 2>&1 || {
26+
cat "$TMP/out.log"
27+
echo "FAIL: 'self config --mirror GLOBAL' exited non-zero on fresh MCPP_HOME"
28+
exit 1
29+
}
30+
31+
# After the command the seeded .xlings.json must reflect GLOBAL, NOT the
32+
# default CN. This is the property the fix guarantees on the seed path —
33+
# the OLD code seeded "CN" first and only switched to GLOBAL via a
34+
# post-bootstrap xlings call, leaving the patchelf/ninja download going
35+
# through CN.
36+
grep -q '"mirror": "GLOBAL"' "$MCPP_HOME/registry/.xlings.json" || {
37+
cat "$MCPP_HOME/registry/.xlings.json"
38+
echo "FAIL: .xlings.json mirror should be GLOBAL after fresh-init --mirror"
39+
exit 1
40+
}
41+
42+
# Round-trip back to CN — also via --mirror — exercises the post-init
43+
# path (xlings config --mirror) since .xlings.json now exists.
44+
"$MCPP" self config --mirror CN > "$TMP/cn.log" 2>&1 || {
45+
cat "$TMP/cn.log"; echo "FAIL: switching back to CN failed"; exit 1; }
46+
grep -q '"mirror": "CN"' "$MCPP_HOME/registry/.xlings.json" || {
47+
cat "$MCPP_HOME/registry/.xlings.json"
48+
echo "FAIL: round-trip to CN did not stick"
49+
exit 1
50+
}
51+
52+
echo "OK"

0 commit comments

Comments
 (0)