Skip to content

check: support custom archive formatting, #9411#9910

Open
mr-raj12 wants to merge 3 commits into
borgbackup:masterfrom
mr-raj12:check-format-9411
Open

check: support custom archive formatting, #9411#9910
mr-raj12 wants to merge 3 commits into
borgbackup:masterfrom
mr-raj12:check-format-9411

Conversation

@mr-raj12

@mr-raj12 mr-raj12 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #9411.

repo-list and prune both let you format the archive part of their output. check had it hardcoded, so the format env vars did nothing there.

This adds --format and BORG_CHECK_FORMAT, same pattern as those two. Default is {archive} {time} {id}, which stays close to the old output.

The format gets checked in do_check(), before the repository check starts. That check can run for hours on a big repo and it would be annoying to only find out about a typo afterwards.

Keys like {comment} need the archive metadata. When it cannot be loaded, formatting falls back to the plain description from the archive directory entry and the checks report the problem as usual. The fallback formats the timestamp the same way the formatter would, so a damaged archive lines up with an intact one.

--format only affects the archives check, so passing it together with --repository-only now raises the same contradiction error as --verify-data and friends. A set BORG_CHECK_FORMAT does not, since an env var sitting in a shell profile is a default rather than a request.

One behaviour change worth noting: the default {time} now prints as Sun, 2026-02-22 17:01:01 +0000. The old hardcoded line printed 2026-02-22 17:01:01.359832+00:00.

No changelog entry yet.

Two pre-existing issues found on the way

Both are on master already, not from this PR.

repo-list dies with an uncaught Archive.DoesNotExist if an archive metadata block is missing, because its default format resolves {username}/{hostname}/{comment}/{tags} and ArchiveFormatter.get_meta never catches it. prune hits the same thing with a custom format.

A lost archive shows up as archive-does-not-exist with a 1970 timestamp, which looks like a bug in borg. The name is simply gone: the archives directory entry is stored with empty content, so name and timestamp only exist inside the missing metadata block.

Can file both separately if that is useful.

Add --format and BORG_CHECK_FORMAT to control the archive part of the
"Analyzing archive ..." output, like repo-list and prune already do.
Archives with damaged or missing metadata fall back to the plain description.
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.22%. Comparing base (7da670c) to head (38ee6ea).
⚠️ Report is 5 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9910      +/-   ##
==========================================
+ Coverage   85.13%   85.22%   +0.09%     
==========================================
  Files          93       93              
  Lines       15899    15921      +22     
  Branches     2428     2430       +2     
==========================================
+ Hits        13535    13569      +34     
+ Misses       1654     1648       -6     
+ Partials      710      704       -6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@ThomasWaldmann

Copy link
Copy Markdown
Member

@mr-raj12 I am in google meet now.

@ThomasWaldmann

Copy link
Copy Markdown
Member

Review

Overview

Adds --format and BORG_CHECK_FORMAT to borg check, replacing the hardcoded Analyzing archive <name> <ts> <id> line in the archives check with an ArchiveFormatter-driven one, mirroring the repo-list/prune pattern. Format keys are validated in do_check() before the potentially hours-long repository check, and archives with missing/corrupt metadata fall back to the always-available directory-entry values.

Verification done: ran the full check_cmd_test.py module on the PR head — 19 passed, 2 skipped, including the 4 new tests — and manually probed edge cases against a scratch repo.

What's correct (verified, not just read)

  • Early validation works and is necessary. --format '{archive'Command error: Invalid format string: expected '}' before end of string (rc 4); {nosuchkey} → clean Invalid format keys error. This is actually better than the status quo: repo-list --format '{nosuchkey}' today dies with rc 2 and a traceback.
  • The key set is exact. KEY_DESCRIPTIONS ∪ FIXED_KEYS matches precisely what ArchiveFormatter.get_item_data can supply (13 archive keys + the NL/TAB/… aids) — no false accepts or rejects.
  • No perf regression by default. The default {archive} {time} {id} resolves entirely from the archive directory entry (used_call_keys is empty), so no extra metadata fetch per archive. Only metadata keys like {comment} cost a read — and check loads that same block right afterwards anyway.
  • The fallback exception set is right. Archive.__init__DoesNotExist (directory entry gone), _load_metaRepository.ObjectNotFound (block missing) / IntegrityErrorBase (block corrupt). All three paths land in the fallback; the missing-metadata test exercises it.
  • No UnboundLocalError trap. format is assigned under if not args.repo_only: and consumed under the same short-circuited condition.
  • Tests correctly handle the FORK_DEFAULT split (borg Errors propagate in non-fork mode), and --format-overrides-env-var is covered.

Issues / suggestions

  1. Inconsistent timestamp style between fallback and normal output (archive.py, fallback in rebuild_archives). The fallback prints info.ts.astimezone() (2026-02-22 17:01:01.359832+00:00) while the formatter prints OutputTimestamp (Sun, 2026-02-22 17:01:01 +0000). Using OutputTimestamp(info.ts) in the fallback would keep damaged and intact archives visually aligned.

  2. --repository-only --format X is silently ignored (verified). Other archives-check-only options (--verify-data, --first, …) raise CommandError when combined with --repository-only; consider adding args.format to that contradiction check in do_check. (Only the explicit CLI flag — a set BORG_CHECK_FORMAT env var must not error there.)

  3. Validation logic wants to live on the formatter. The Formatter().parse + key-set check in do_check is generic; as a classmethod like ArchiveFormatter.validate_format(format) it would keep key knowledge next to the class and let repo-list/prune fix their rc-2-traceback behavior by reusing it. Fine as a follow-up.

  4. Minor signature style (ArchiveChecker.check): format as a required keyword-only param sandwiched between defaulted ones is legal but reads oddly; moving it up or giving it None + resolving in one place would be tidier. Only one caller exists, so no compat concern.

  5. Test gap (small): no test for a malformed format string ({archiveInvalid format string); verified manually that it works, but a one-liner test would pin it.

  6. Changelog entry missing — already noted in the PR description; the {time} default-output change (Sun, 2026-02-22 17:01:01 +0000 vs old microsecond format) deserves a line there since scripts may grep this output.

Risks

Low. The behavior change is cosmetic (log-line formatting), the default stays close to the old output, error paths degrade gracefully, and the feature is opt-in beyond that. The two pre-existing bugs described in the PR text (uncaught DoesNotExist in repo-list's default format; archive-does-not-exist + 1970 timestamp for lost archives) are real and worth filing as separate issues as offered — they are not introduced or worsened by this PR.

Verdict: solid, well-tested contribution; points 1–2 are both two-line changes and worth doing here.

🤖 Review generated with Claude Code

@mr-raj12
mr-raj12 marked this pull request as ready for review July 17, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

borg2: check does not support custom output formatting

2 participants