Skip to content

Commit 28b2c42

Browse files
committed
fix: address round-2 review — 4 issues
1. toolchain install now calls check_base_init() before proceeding, failing early if patchelf/ninja bootstrap is incomplete 2. Preserve original xlings install error (exit code + message) in final error instead of masking with generic "payload missing" 3. One-time legacy migration: migrate_legacy_installs() scans xpkgs and writes .mcpp_ok markers for old complete packages on first run. is_install_complete() still has legacy fallback for un-migrated packages but migration ensures it's rarely needed. 4. Clean trailing whitespace in .agents/docs/*.md
1 parent 9b28fd0 commit 28b2c42

7 files changed

Lines changed: 91 additions & 23 deletions

.agents/docs/2026-05-22-copy-priority-analysis.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ resolve_xpkg_path(target, autoInstall):
148148

149149
### 方案 C:install 优先 + install 直接调用(非 NDJSON)
150150

151-
之前排查发现 NDJSON interface 路径安装大包不可靠。`install_with_progress()`
151+
之前排查发现 NDJSON interface 路径安装大包不可靠。`install_with_progress()`
152152
已有"直接调用" fallback(`std::system("xlings install ... -y")`)。
153153

154154
将工具链安装改为使用 `install_with_progress()`(直接调用模式)而非

.agents/docs/2026-05-22-ensure-base-init-design.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,12 @@ config::load_or_init()
9595
std::expected<void, ConfigError>
9696
ensure_base_init_ok(const GlobalConfig& cfg) {
9797
auto xlEnv = make_xlings_env(cfg);
98-
98+
9999
struct Check {
100100
std::string_view name;
101101
std::filesystem::path path;
102102
};
103-
103+
104104
Check checks[] = {
105105
{"xlings binary", cfg.xlingsBinary},
106106
{"sandbox marker", xlEnv.home / "subos" / "default" / ".xlings.json"},
@@ -109,7 +109,7 @@ ensure_base_init_ok(const GlobalConfig& cfg) {
109109
{"ninja", mcpp::xlings::paths::xim_tool(xlEnv, "ninja",
110110
mcpp::xlings::pinned::kNinjaVersion) / "bin" / "ninja"},
111111
};
112-
112+
113113
for (auto& c : checks) {
114114
if (!std::filesystem::exists(c.path)) {
115115
return std::unexpected(ConfigError{std::format(
@@ -127,7 +127,7 @@ ensure_base_init_ok(const GlobalConfig& cfg) {
127127
```cpp
128128
int cmd_self_init(const ParsedArgs& parsed) {
129129
bool force = parsed.is_flag_set("force");
130-
130+
131131
if (force) {
132132
// 删除 bootstrap 状态(不删整个 ~/.mcpp/,保留 config.toml 和 toolchain)
133133
auto xlHome = cfg->xlingsHome();
@@ -140,12 +140,12 @@ int cmd_self_init(const ParsedArgs& parsed) {
140140
std::filesystem::remove_all(xpkgs / prefix, ec);
141141
}
142142
}
143-
143+
144144
// 重新 bootstrap(复用 load_or_init 的 bootstrap 逻辑)
145145
ensure_init(xlEnv, false);
146146
ensure_patchelf(xlEnv, false, nullptr);
147147
ensure_ninja(xlEnv, false, nullptr);
148-
148+
149149
// 验证
150150
auto check = ensure_base_init_ok(*cfg);
151151
if (!check) {

.agents/docs/2026-05-22-fallback-architecture-design.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ resolve()
331331
└── 返回 error
332332
333333
install()(只有 resolve 失败才走到)
334-
334+
335335
resolve()(第二次,install 后)
336336
├── sandbox 有?→ 返回
337337
├── ~/.xlings/ 有?→ copy → 返回
@@ -366,11 +366,11 @@ auto resolve_quick = [&]() -> std::optional<XpkgPayload> {
366366

367367
auto resolve_with_copy_fallback = [&]() -> std::expected<XpkgPayload, CallError> {
368368
if (auto p = resolve_quick()) return *p;
369-
369+
370370
// FALLBACK: copy from global xlings
371371
mcpp::log::verbose("fetcher", "[fallback:xpkg.copy_from_global]");
372372
// ... copy logic (existing code)
373-
373+
374374
if (auto p = resolve_quick()) return *p;
375375
return std::unexpected(CallError{"xpkg payload missing: " + verdir.string()});
376376
};

src/cli.cppm

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3726,6 +3726,17 @@ int cmd_toolchain(const mcpplibs::cmdline::ParsedArgs& parsed) {
37263726
}
37273727

37283728
if (subname == "install") {
3729+
// Toolchain install needs patchelf (ELF fixup) and ninja (build).
3730+
// Fail early if bootstrap is incomplete rather than producing a
3731+
// broken toolchain with missing fixups.
3732+
auto bsProblem = mcpp::config::check_base_init(*cfg);
3733+
if (!bsProblem.empty()) {
3734+
mcpp::ui::error(std::format(
3735+
"{}\n hint: run `mcpp self init --force` to reset and re-initialize",
3736+
bsProblem));
3737+
return 1;
3738+
}
3739+
37293740
// Accept three input shapes — they all collapse to (compiler, version):
37303741
// mcpp toolchain install gcc 16.1.0 → ("gcc", "16.1.0")
37313742
// mcpp toolchain install gcc@16.1.0 → ("gcc", "16.1.0")

src/config.cppm

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,14 @@ std::expected<GlobalConfig, ConfigError> load_or_init(
558558
}
559559
}
560560

561-
// 8. Bootstrap check is NOT done here — it's deferred to commands that
561+
// 8. One-time migration: mark legacy xpkgs (installed before .mcpp_ok
562+
// was introduced) so they aren't mistaken for incomplete installs.
563+
{
564+
auto xpkgsBase = cfg.xlingsHome() / "data" / "xpkgs";
565+
mcpp::fallback::migrate_legacy_installs(xpkgsBase);
566+
}
567+
568+
// 9. Bootstrap check is NOT done here — it's deferred to commands that
562569
// actually need bootstrap tools (build, run, toolchain install).
563570
// Light commands (self env, toolchain list) should work even if
564571
// bootstrap is incomplete. Commands call check_base_init() themselves.

src/fallback/install_integrity.cppm

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,28 +52,29 @@ bool clean_incomplete_install(const std::filesystem::path& xpkgDir);
5252
// Returns number of directories cleaned.
5353
int clean_all_incomplete(const std::filesystem::path& xpkgsBase);
5454

55+
// One-time migration: scan xpkgs and write .mcpp_ok markers for all
56+
// old packages that look complete but lack a marker. After migration,
57+
// is_install_complete() uses strict marker-only semantics. Returns
58+
// number of packages migrated.
59+
int migrate_legacy_installs(const std::filesystem::path& xpkgsBase);
60+
5561
} // namespace mcpp::fallback
5662

5763
// ─── Implementation ─────────────────────────────────────────────────
5864

5965
namespace mcpp::fallback {
6066

61-
bool is_install_complete(const std::filesystem::path& xpkgDir) {
62-
if (!std::filesystem::exists(xpkgDir)) return false;
67+
namespace {
6368

64-
// Primary: .mcpp_ok marker
65-
if (std::filesystem::exists(xpkgDir / std::string(kInstallMarker)))
66-
return true;
67-
68-
// Backward compat: no marker but has content directories
69-
// (installed before this feature was added)
70-
// Check top-level content dirs (xim toolchain packages).
69+
// Heuristic: does this directory look like a complete xpkg?
70+
// Used ONLY for legacy migration (pre-.mcpp_ok packages).
71+
bool looks_complete_legacy(const std::filesystem::path& xpkgDir) {
72+
// xim toolchain/tool packages: top-level bin/lib/lib64/include/share
7173
for (auto dir : {"bin", "lib", "lib64", "include", "share"}) {
7274
if (std::filesystem::exists(xpkgDir / dir))
7375
return true;
7476
}
75-
// Check for mcpplibs layout: single subdirectory containing src/ or
76-
// mcpp.toml (extracted tarball). E.g. verdir/cmdline-0.0.1/src/...
77+
// mcpplibs layout: single subdirectory containing src/ or mcpp.toml
7778
std::error_code ec;
7879
std::vector<std::filesystem::path> subs;
7980
for (auto& e : std::filesystem::directory_iterator(xpkgDir, ec)) {
@@ -90,6 +91,22 @@ bool is_install_complete(const std::filesystem::path& xpkgDir) {
9091
return false;
9192
}
9293

94+
} // namespace
95+
96+
bool is_install_complete(const std::filesystem::path& xpkgDir) {
97+
if (!std::filesystem::exists(xpkgDir)) return false;
98+
99+
// Strict: .mcpp_ok marker is the sole authority.
100+
// Legacy packages should have been migrated (marker added) during
101+
// the first load_or_init() after upgrading to this version.
102+
if (std::filesystem::exists(xpkgDir / std::string(kInstallMarker)))
103+
return true;
104+
105+
// Fallback for un-migrated packages (e.g. first run after upgrade
106+
// before migration has a chance to run).
107+
return looks_complete_legacy(xpkgDir);
108+
}
109+
93110
void mark_install_complete(const std::filesystem::path& xpkgDir) {
94111
auto marker = xpkgDir / std::string(kInstallMarker);
95112
if (std::filesystem::exists(marker)) return;
@@ -124,4 +141,24 @@ int clean_all_incomplete(const std::filesystem::path& xpkgsBase) {
124141
return cleaned;
125142
}
126143

144+
int migrate_legacy_installs(const std::filesystem::path& xpkgsBase) {
145+
if (!std::filesystem::exists(xpkgsBase)) return 0;
146+
147+
int migrated = 0;
148+
std::error_code ec;
149+
for (auto& pkgDir : std::filesystem::directory_iterator(xpkgsBase, ec)) {
150+
if (!pkgDir.is_directory()) continue;
151+
for (auto& verDir : std::filesystem::directory_iterator(pkgDir.path(), ec)) {
152+
if (!verDir.is_directory()) continue;
153+
auto marker = verDir.path() / std::string(kInstallMarker);
154+
if (std::filesystem::exists(marker)) continue;
155+
if (looks_complete_legacy(verDir.path())) {
156+
mark_install_complete(verDir.path());
157+
++migrated;
158+
}
159+
}
160+
}
161+
return migrated;
162+
}
163+
127164
} // namespace mcpp::fallback

src/pm/package_fetcher.cppm

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,7 @@ Fetcher::resolve_xpkg_path(std::string_view target,
659659
mcpp::fallback::clean_incomplete_install(verdir);
660660

661661
// 3. Install via xlings (primary path).
662+
std::string installError; // preserved for final error message
662663
if (autoInstall) {
663664
std::vector<std::string> targets {
664665
std::format("{}:{}@{}", parsed.indexName, parsed.packageName, parsed.version)
@@ -677,8 +678,14 @@ Fetcher::resolve_xpkg_path(std::string_view target,
677678
// Install failed → clean residue so next attempt starts fresh.
678679
mcpp::fallback::clean_incomplete_install(verdir);
679680
if (inst->exitCode != 0) {
681+
installError = std::format(
682+
"xlings install of '{}:{}@{}' failed (exit {})",
683+
parsed.indexName, parsed.packageName, parsed.version,
684+
inst->exitCode);
685+
if (inst->error)
686+
installError += ": " + inst->error->message;
680687
mcpp::log::warn("fetcher", std::format(
681-
"xlings install exit={}, trying fallback", inst->exitCode));
688+
"{}, trying fallback", installError));
682689
}
683690
}
684691

@@ -692,6 +699,12 @@ Fetcher::resolve_xpkg_path(std::string_view target,
692699
// Copy failed or incomplete — clean partial copy.
693700
mcpp::fallback::clean_incomplete_install(verdir);
694701

702+
// 5. All paths exhausted — return the most informative error.
703+
if (!installError.empty()) {
704+
return std::unexpected(CallError{std::format(
705+
"{}\n hint: check network and retry, or `mcpp self init --force`",
706+
installError)});
707+
}
695708
return std::unexpected(CallError{
696709
std::format("xpkg payload missing: {}\n"
697710
" hint: check network and retry, or `mcpp self init --force`",

0 commit comments

Comments
 (0)