Skip to content

Commit 9b28fd0

Browse files
committed
fix: address 4 review issues on install integrity
1. is_install_complete: handle mcpplibs nested layout (single subdir with src/ or mcpp.toml), prevents false deletion of old packages 2. copy_from_global: check return value and verify completeness before marking .mcpp_ok; clean partial copies on failure 3. Restore !inst error propagation in resolve_xpkg_path — don't mask xlings launch/protocol errors behind generic "payload missing" 4. Per-command bootstrap gating: check_base_init is deferred to get_cfg(requireBootstrap=true) in build/run/toolchain-install, not run globally in load_or_init. Light commands (self env, toolchain list) skip the check.
1 parent b543fd3 commit 9b28fd0

4 files changed

Lines changed: 44 additions & 13 deletions

File tree

src/cli.cppm

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1139,13 +1139,25 @@ prepare_build(bool print_fingerprint,
11391139
// 3. PATH g++ (with warning)
11401140
std::filesystem::path explicit_compiler;
11411141
std::optional<mcpp::config::GlobalConfig> cfg_opt;
1142-
auto get_cfg = [&]() -> std::expected<mcpp::config::GlobalConfig*, std::string> {
1142+
bool bootstrap_checked = false;
1143+
auto get_cfg = [&](bool requireBootstrap = true) -> std::expected<mcpp::config::GlobalConfig*, std::string> {
11431144
if (!cfg_opt) {
11441145
auto c = mcpp::config::load_or_init(/*quiet=*/false,
11451146
make_bootstrap_progress_callback());
11461147
if (!c) return std::unexpected(c.error().message);
11471148
cfg_opt = std::move(*c);
11481149
}
1150+
// Commands that need bootstrap tools (build, run, toolchain install)
1151+
// pass requireBootstrap=true to get an early, clear error.
1152+
if (requireBootstrap && !bootstrap_checked) {
1153+
bootstrap_checked = true;
1154+
auto problem = mcpp::config::check_base_init(*cfg_opt);
1155+
if (!problem.empty()) {
1156+
return std::unexpected(std::format(
1157+
"{}\n hint: run `mcpp self init --force` to reset and re-initialize",
1158+
problem));
1159+
}
1160+
}
11491161
return &*cfg_opt;
11501162
};
11511163

src/config.cppm

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

561-
// 8. Verify bootstrap completed. Warn (don't block) if something is
562-
// missing — commands like `mcpp self env` should still work even
563-
// if bootstrap tools (patchelf/ninja) failed to download.
564-
auto initProblem = check_base_init(cfg);
565-
if (!initProblem.empty() && !quiet) {
566-
std::println(stderr, "warning: {}", initProblem);
567-
std::println(stderr, " hint: run `mcpp self init --force` to reset and re-initialize");
568-
}
561+
// 8. Bootstrap check is NOT done here — it's deferred to commands that
562+
// actually need bootstrap tools (build, run, toolchain install).
563+
// Light commands (self env, toolchain list) should work even if
564+
// bootstrap is incomplete. Commands call check_base_init() themselves.
569565

570566
return cfg;
571567
}

src/fallback/install_integrity.cppm

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,26 @@ bool is_install_complete(const std::filesystem::path& xpkgDir) {
6767

6868
// Backward compat: no marker but has content directories
6969
// (installed before this feature was added)
70+
// Check top-level content dirs (xim toolchain packages).
7071
for (auto dir : {"bin", "lib", "lib64", "include", "share"}) {
7172
if (std::filesystem::exists(xpkgDir / dir))
7273
return true;
7374
}
75+
// Check for mcpplibs layout: single subdirectory containing src/ or
76+
// mcpp.toml (extracted tarball). E.g. verdir/cmdline-0.0.1/src/...
77+
std::error_code ec;
78+
std::vector<std::filesystem::path> subs;
79+
for (auto& e : std::filesystem::directory_iterator(xpkgDir, ec)) {
80+
if (e.is_directory()) subs.push_back(e.path());
81+
}
82+
if (subs.size() == 1) {
83+
auto& sub = subs[0];
84+
if (std::filesystem::exists(sub / "src")
85+
|| std::filesystem::exists(sub / "mcpp.toml")
86+
|| std::filesystem::exists(sub / "include")
87+
|| std::filesystem::exists(sub / "bin"))
88+
return true;
89+
}
7490
return false;
7591
}
7692

src/pm/package_fetcher.cppm

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -665,25 +665,32 @@ Fetcher::resolve_xpkg_path(std::string_view target,
665665
};
666666
mcpp::log::verbose("fetcher", std::format("xlings install: {}", targets[0]));
667667
auto inst = install(targets, handler);
668-
if (inst && inst->exitCode == 0 && std::filesystem::exists(verdir)) {
668+
if (!inst) {
669+
// xlings launch/protocol failure — propagate the real error,
670+
// don't mask it behind a generic "payload missing".
671+
return std::unexpected(inst.error());
672+
}
673+
if (inst->exitCode == 0 && std::filesystem::exists(verdir)) {
669674
mcpp::fallback::mark_install_complete(verdir);
670675
return make_payload();
671676
}
672677
// Install failed → clean residue so next attempt starts fresh.
673678
mcpp::fallback::clean_incomplete_install(verdir);
674-
if (inst && inst->exitCode != 0) {
679+
if (inst->exitCode != 0) {
675680
mcpp::log::warn("fetcher", std::format(
676681
"xlings install exit={}, trying fallback", inst->exitCode));
677682
}
678683
}
679684

680685
// 4. Copy fallback: xlings may have installed into its global data dir.
681-
mcpp::fallback::copy_xpkg_from_global(verdir);
682-
if (std::filesystem::exists(verdir)) {
686+
bool copyOk = mcpp::fallback::copy_xpkg_from_global(verdir);
687+
if (copyOk && mcpp::fallback::is_install_complete(verdir)) {
683688
mcpp::fallback::mark_install_complete(verdir);
684689
mcpp::log::verbose("fetcher", "resolved via copy fallback");
685690
return make_payload();
686691
}
692+
// Copy failed or incomplete — clean partial copy.
693+
mcpp::fallback::clean_incomplete_install(verdir);
687694

688695
return std::unexpected(CallError{
689696
std::format("xpkg payload missing: {}\n"

0 commit comments

Comments
 (0)