fix: mcpp self config --mirror X hangs on fresh MCPP_HOME#76
Merged
Conversation
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.
0d33212 to
6a0f23a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
mcpp self config --mirror GLOBALon a freshMCPP_HOMEhangs because:cmd_self_config→config::load_or_initload_or_initstep 5 seeds.xlings.jsonwith the hardcoded default"CN"— regardless of--mirrorxlings self init+ensure_patchelf+ensure_ninja) reads that mirror and downloads via the CN proxycmd_self_configcallxlings::config_set_mirrorto update the file — too late, the user's choice never reached the very network roundtrip they were trying to redirectFix (~15 lines of real logic)
Plumb the user's mirror choice through to the seed step so the first write of
.xlings.jsonalready has the right value, and the bootstrap that immediately follows downloads via the chosen mirror.src/config.cppmwrite_default_xlings_jsongrowsmirror_overrideparameter (default empty).load_or_initgrowsinitial_mirrorparameter (default empty; all 14 existing call sites unchanged).seed_xlings_json, overriding the"CN"default for that fresh seed.src/cli.cppm—cmd_self_configvalidates--mirrorfirst, then passes it asload_or_init(..., mirror).xlings::config_set_mirrorstill runs afterwards (no-op on fresh since the seed already has the right value; required for the already-initialized case where the seed is skipped).src/xlings.cppm— TODO comment atseed_xlings_json's"CN"default about the long-term direction (flip default to GLOBAL, or auto-detect on first init).For an already-initialized
MCPP_HOME(.xlings.jsonalready exists), the seed is skipped — no behavior change for that path.Test
New
tests/e2e/46_self_config_mirror_no_bootstrap.sh:MCPP_HOMEmcpp self config --mirror GLOBAL.xlings.jsonmirror isGLOBAL(proves seed override took effect; the OLD code seededCNand only switched toGLOBALpost-bootstrap)CNvia a second--mirrorcall to exercise the already-initialized pathExisting
tests/e2e/38_self_config_mirror.shstill passes — it always callsmcpp self envfirst (noinitial_mirroroverride), so the "default seed is CN" invariant is preserved.Diff size
Risk
Minimal. Both new parameters default to empty/unchanged behavior. Only
cmd_self_configopts in.Test plan