Skip to content

Commit 8a2d003

Browse files
committed
fix: strict marker semantics for cleanup, remove dead migration code
Round-4 review fixes: 1. clean_incomplete_install() now uses marker-only check: - Has .mcpp_ok → keep (verified install) - No marker but looks_complete_legacy → keep (pre-upgrade package) - No marker, no legacy content → clean (genuinely incomplete) This prevents half-extracted packages that happen to have bin/lib from escaping cleanup. 2. Remove migrate_legacy_installs() — it was dead code (declared but never called). The legacy fallback in is_install_complete() handles old packages read-only without writing markers.
1 parent 42064ef commit 8a2d003

2 files changed

Lines changed: 29 additions & 40 deletions

File tree

src/config.cppm

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -567,13 +567,7 @@ std::expected<GlobalConfig, ConfigError> load_or_init(
567567
}
568568
}
569569

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.
575-
576-
// 9. Bootstrap check is NOT done here — it's deferred to commands that
570+
// 8. Bootstrap check is NOT done here — it's deferred to commands that
577571
// actually need bootstrap tools (build, run, toolchain install).
578572
// Light commands (self env, toolchain list) should work even if
579573
// bootstrap is incomplete. Commands call check_base_init() themselves.

src/fallback/install_integrity.cppm

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,12 @@ void mark_install_complete(const std::filesystem::path& xpkgDir);
4848
bool clean_incomplete_install(const std::filesystem::path& xpkgDir);
4949

5050
// Scan an xpkgs base directory and clean ALL incomplete installations.
51+
// Only cleans directories without .mcpp_ok marker AND without legacy
52+
// content (won't delete pre-upgrade packages).
5153
// Used by `mcpp self init`.
5254
// Returns number of directories cleaned.
5355
int clean_all_incomplete(const std::filesystem::path& xpkgsBase);
5456

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-
6157
} // namespace mcpp::fallback
6258

6359
// ─── Implementation ─────────────────────────────────────────────────
@@ -93,17 +89,21 @@ bool looks_complete_legacy(const std::filesystem::path& xpkgDir) {
9389

9490
} // namespace
9591

92+
// Strict: has .mcpp_ok marker (written only on verified success).
93+
bool has_marker(const std::filesystem::path& xpkgDir) {
94+
return std::filesystem::exists(xpkgDir / std::string(kInstallMarker));
95+
}
96+
9697
bool is_install_complete(const std::filesystem::path& xpkgDir) {
9798
if (!std::filesystem::exists(xpkgDir)) return false;
9899

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;
100+
// Primary: .mcpp_ok marker — the only fully trusted signal.
101+
if (has_marker(xpkgDir)) return true;
104102

105-
// Fallback for un-migrated packages (e.g. first run after upgrade
106-
// before migration has a chance to run).
103+
// Read-only legacy fallback: recognize pre-upgrade packages so they
104+
// aren't treated as missing by resolve_xpkg_path(). Does NOT write
105+
// any marker. Does NOT prevent clean_incomplete_install() from
106+
// cleaning — that function uses marker-only semantics.
107107
return looks_complete_legacy(xpkgDir);
108108
}
109109

@@ -116,8 +116,22 @@ void mark_install_complete(const std::filesystem::path& xpkgDir) {
116116

117117
bool clean_incomplete_install(const std::filesystem::path& xpkgDir) {
118118
if (!std::filesystem::exists(xpkgDir)) return false;
119-
if (is_install_complete(xpkgDir)) return false;
120119

120+
// Marker-only: if .mcpp_ok exists, this is a verified install — keep it.
121+
// Legacy packages (no marker but has content) are NOT cleaned here;
122+
// they're recognized by is_install_complete() for read-only compat.
123+
if (has_marker(xpkgDir)) return false;
124+
125+
// No marker. If it looks like a legacy complete package, don't clean
126+
// it either — it predates the marker system.
127+
if (looks_complete_legacy(xpkgDir)) {
128+
mcpp::log::debug("integrity", std::format(
129+
"legacy package without marker, skipping cleanup: {}",
130+
xpkgDir.string()));
131+
return false;
132+
}
133+
134+
// No marker, no legacy content — this is genuinely incomplete.
121135
mcpp::log::verbose("integrity",
122136
std::format("cleaning incomplete install: {}", xpkgDir.string()));
123137
std::error_code ec;
@@ -141,24 +155,5 @@ int clean_all_incomplete(const std::filesystem::path& xpkgsBase) {
141155
return cleaned;
142156
}
143157

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-
}
163158

164159
} // namespace mcpp::fallback

0 commit comments

Comments
 (0)