Skip to content

Commit 42064ef

Browse files
committed
fix: .mcpp_ok only written after verified binary exists
Round-3 review fixes: 1. Bootstrap marker: mark_install_complete() only called after verifying the actual binary (bin/patchelf, bin/ninja) exists, not just after ensure_*() returns (which may have failed). 2. Remove automatic migrate_legacy_installs() from load_or_init(). Heuristic-based marker writing could stamp half-extracted packages as complete. Legacy heuristic remains in is_install_complete() as read-only fallback (won't delete old packages), but .mcpp_ok is only written on explicit success paths or via `mcpp self init`.
1 parent 28b2c42 commit 42064ef

1 file changed

Lines changed: 22 additions & 14 deletions

File tree

src/config.cppm

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -545,25 +545,33 @@ std::expected<GlobalConfig, ConfigError> load_or_init(
545545
#if !defined(__APPLE__) && !defined(_WIN32)
546546
// patchelf is ELF-only; macOS uses Mach-O and Windows uses PE.
547547
ensure_sandbox_patchelf(cfg, quiet, onBootstrapProgress);
548-
mcpp::fallback::mark_install_complete(
549-
mcpp::xlings::paths::xim_tool(bsEnv, "patchelf",
550-
mcpp::xlings::pinned::kPatchelfVersion));
548+
// Only mark complete if the actual binary exists (not just the dir).
549+
{
550+
auto pBin = mcpp::xlings::paths::xim_tool(bsEnv, "patchelf",
551+
mcpp::xlings::pinned::kPatchelfVersion) / "bin" / "patchelf";
552+
if (std::filesystem::exists(pBin))
553+
mcpp::fallback::mark_install_complete(pBin.parent_path().parent_path());
554+
}
551555
#endif
552556
ensure_sandbox_ninja(cfg, quiet, onBootstrapProgress);
553-
auto nRoot = mcpp::xlings::paths::xim_tool_root(bsEnv, "ninja");
554-
std::error_code ec;
555-
if (std::filesystem::exists(nRoot)) {
556-
for (auto& v : std::filesystem::directory_iterator(nRoot, ec))
557-
mcpp::fallback::mark_install_complete(v.path());
557+
{
558+
auto nRoot = mcpp::xlings::paths::xim_tool_root(bsEnv, "ninja");
559+
auto ninja_name = std::string("ninja") + std::string(mcpp::platform::exe_suffix);
560+
std::error_code ec;
561+
if (std::filesystem::exists(nRoot)) {
562+
for (auto& v : std::filesystem::directory_iterator(nRoot, ec)) {
563+
if (std::filesystem::exists(v.path() / ninja_name))
564+
mcpp::fallback::mark_install_complete(v.path());
565+
}
566+
}
558567
}
559568
}
560569

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-
}
570+
// 8. Legacy migration is NOT done automatically here — it would write
571+
// .mcpp_ok markers based on heuristics, which could mark half-extracted
572+
// packages as complete. Migration only runs via explicit `mcpp self init`.
573+
// The is_install_complete() fallback still recognizes legacy packages
574+
// read-only (won't delete them), but won't stamp them as verified.
567575

568576
// 9. Bootstrap check is NOT done here — it's deferred to commands that
569577
// actually need bootstrap tools (build, run, toolchain install).

0 commit comments

Comments
 (0)