Skip to content

Commit d0745ed

Browse files
check: address review feedback for problem/repair summary
Require explicit repaired=, note the manifest only after rebuild, always report repair counts under --repair, and do not count defect-chunk deletion as a repair. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 809862a commit d0745ed

1 file changed

Lines changed: 24 additions & 17 deletions

File tree

src/borg/archive.py

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1908,7 +1908,12 @@ def __init__(self):
19081908
# longer matches the packs.
19091909
self.chunks_modified = False
19101910

1911-
def _note_problem(self, *, repaired=False):
1911+
def _note_problem(self, *, repaired):
1912+
"""Record a problem found during the check.
1913+
1914+
Callers must always pass repaired= explicitly: True only when the problem was
1915+
actually fixed (not merely detected, and not when --repair only discarded data).
1916+
"""
19121917
self.error_found = True
19131918
self.problems_found += 1
19141919
if repaired and self.repair:
@@ -1982,11 +1987,11 @@ def check(
19821987
logger.error("Repository manifest is corrupted: %s", exc)
19831988
rebuild_manifest = True
19841989
if rebuild_manifest:
1985-
if self.repair:
1986-
self._note_problem(repaired=True)
1987-
else:
1988-
self._note_problem()
1990+
# Rebuild first; only then can we say whether the problem was repaired.
1991+
# Without --repair, rebuild_manifest() only builds an in-memory manifest for the
1992+
# rest of this check run; finish() writes it only when self.repair is set.
19891993
self.manifest = self.rebuild_manifest()
1994+
self._note_problem(repaired=self.repair)
19901995
# On Ctrl-C, skip any scan not yet started; a scan already running stops at its own boundary.
19911996
if find_lost_archives and not sig_int:
19921997
self.rebuild_archives_directory()
@@ -2010,7 +2015,8 @@ def check(
20102015
logger.info("Archive consistency check interrupted, no problems found so far.")
20112016
raise Error("Got Ctrl-C / SIGINT.")
20122017
if self.error_found:
2013-
if self.repair and self.repairs_done:
2018+
if self.repair:
2019+
# Always report the repair count in --repair mode, including 0 repaired.
20142020
logger.error(
20152021
"Archive consistency check complete, %d problem(s) found, %d repaired.",
20162022
self.problems_found,
@@ -2081,9 +2087,9 @@ def verify_data(self):
20812087
if isinstance(err, IntegrityErrorBase):
20822088
defect_chunks.append(chunk_id)
20832089
if not self.repair:
2084-
self._note_problem()
2090+
self._note_problem(repaired=False)
20852091
else:
2086-
self._note_problem()
2092+
self._note_problem(repaired=False)
20872093
else:
20882094
try:
20892095
# we must decompress, so it'll call assert_id() in there.
@@ -2097,7 +2103,7 @@ def verify_data(self):
20972103
logger.error("chunk %s, integrity error: %s", bin_to_hex(chunk_id), integrity_error)
20982104
defect_chunks.append(chunk_id)
20992105
if not self.repair:
2100-
self._note_problem()
2106+
self._note_problem(repaired=False)
21012107
pi.finish()
21022108
if defect_chunks:
21032109
if self.repair:
@@ -2126,7 +2132,8 @@ def verify_data(self):
21262132
self.chunks_modified = True
21272133
# drop it from our own index too, so rebuild_archives reports the file it belongs to.
21282134
del self.chunks[defect_chunk]
2129-
self._note_problem(repaired=True)
2135+
# Removing a defect chunk is not a repair: referenced data is still lost.
2136+
self._note_problem(repaired=False)
21302137
else:
21312138
logger.warning("chunk %s not deleted, did not consistently fail.", bin_to_hex(defect_chunk))
21322139
else:
@@ -2191,7 +2198,7 @@ def valid_archive(obj):
21912198
meta = self.repo_objs.parse_meta(chunk_id, cdata, ro_type=ROBJ_DONTCARE)
21922199
except IntegrityErrorBase as exc:
21932200
logger.error("Skipping corrupted chunk: %s", exc)
2194-
self._note_problem()
2201+
self._note_problem(repaired=False)
21952202
continue
21962203
if meta["type"] != ROBJ_ARCHIVE_META:
21972204
continue
@@ -2201,7 +2208,7 @@ def valid_archive(obj):
22012208
meta, data = self.repo_objs.parse(chunk_id, cdata, ro_type=ROBJ_DONTCARE)
22022209
except IntegrityErrorBase as exc:
22032210
logger.error("Skipping corrupted chunk: %s", exc)
2204-
self._note_problem()
2211+
self._note_problem(repaired=False)
22052212
continue
22062213
if meta["type"] != ROBJ_ARCHIVE_META:
22072214
continue # should never happen
@@ -2228,7 +2235,7 @@ def valid_archive(obj):
22282235
self._note_problem(repaired=True)
22292236
else:
22302237
logger.warning(f"Would create archives directory entry for {name} {archive_id_hex}.")
2231-
self._note_problem()
2238+
self._note_problem(repaired=False)
22322239

22332240
pi.finish()
22342241
if sig_int:
@@ -2296,7 +2303,7 @@ def verify_file_chunks(archive_name, item):
22962303
)
22972304
)
22982305
record_missing_chunk(archive_name, item.path, chunk_id, size)
2299-
self._note_problem()
2306+
self._note_problem(repaired=False)
23002307
offset += size
23012308
if "size" in item:
23022309
item_size = item.size
@@ -2348,7 +2355,7 @@ def missing_chunk_detector(chunk_id):
23482355
def report(msg, chunk_id, chunk_no):
23492356
cid = bin_to_hex(chunk_id)
23502357
msg += " [chunk: %06d_%s]" % (chunk_no, cid) # see "debug dump-archive-items"
2351-
self._note_problem()
2358+
self._note_problem(repaired=False)
23522359
logger.error(msg)
23532360

23542361
def list_keys_safe(keys):
@@ -2453,7 +2460,7 @@ def valid_item(obj):
24532460
self._note_problem(repaired=True)
24542461
else:
24552462
logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.")
2456-
self._note_problem()
2463+
self._note_problem(repaired=False)
24572464
continue
24582465
cdata = self.repository.get(archive_id)
24592466
try:
@@ -2466,7 +2473,7 @@ def valid_item(obj):
24662473
self._note_problem(repaired=True)
24672474
else:
24682475
logger.error(f"Would delete broken archive {info.name} {archive_id_hex}.")
2469-
self._note_problem()
2476+
self._note_problem(repaired=False)
24702477
continue
24712478
archive = self.key.unpack_archive(data)
24722479
archive = ArchiveItem(internal_dict=archive)

0 commit comments

Comments
 (0)