Skip to content

prune: remove the date-based archive filters (were accepted but never applied) - #10250

Merged
ThomasWaldmann merged 1 commit into
borgbackup:masterfrom
ThomasWaldmann:fix-prune-archive-filters
Aug 29, 2026
Merged

prune: remove the date-based archive filters (were accepted but never applied)#10250
ThomasWaldmann merged 1 commit into
borgbackup:masterfrom
ThomasWaldmann:fix-prune-archive-filters

Conversation

@ThomasWaldmann

@ThomasWaldmann ThomasWaldmann commented Aug 28, 2026

Copy link
Copy Markdown
Member

The date-based archive filters (--oldest/--newest/--older/--newer) were accepted by the prune parser but never applied: borg prune --keep 1 --older=1y happily pruned minutes-old archives (found via a docs-vs-code audit, runtime-verified). Prune inherited the options from the shared archive filters group (#7272) without anyone deciding prune should support date windows.

Reworked per the discussion below: instead of wiring the filters up, remove them from prune for now — --older is redundant with --keep-within, there are no convincing real-world use cases for --oldest/--newest (which anchor on endpoints that prune itself changes between runs), and blanket protection of old archives (--newer) is better served by a future --to option or by tags once #9936 lands. Prune now rejects the four options at parse time (exit code 2) instead of accepting and silently ignoring them.

The change is a single commit with a one-line diff: the four options are no longer added to prune's parser.

Tests: prune_cmd_test.py green; ruff clean. Note: the generated usage docs still list the options for prune; regeneration is left to the usual release-time build_usage/build_man run.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.48%. Comparing base (6299333) to head (9004bf4).
⚠️ Report is 37 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10250      +/-   ##
==========================================
+ Coverage   87.47%   87.48%   +0.01%     
==========================================
  Files         103      103              
  Lines       18534    18534              
  Branches     2843     2843              
==========================================
+ Hits        16213    16215       +2     
  Misses       1623     1623              
+ Partials      698      696       -2     

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

@ThomasWaldmann
ThomasWaldmann marked this pull request as draft August 28, 2026 22:24
@ThomasWaldmann

ThomasWaldmann commented Aug 29, 2026

Copy link
Copy Markdown
Member Author

Design note: does it even make sense to combine the archive filters with prune's --keep* rules?

Worth recording, because the combination was never really designed: the four options came into the shared define_archive_filters_group in #7272 (Jan 2023, date-based matching for archive listing), and prune inherited them only because its parser calls that shared group. Nobody ever decided "prune should support date windows" — which is presumably why they were never wired up. So the real choice here was: implement them, or reject them on prune.

The combination is semantically coherent and safe. With the restrict-the-candidate-set semantics this PR implements, a filter can only protect archives, never expose them: removing candidates can only improve a surviving archive's rank within --keep-last counting and within the period-based rules (fewer newer days/months means an archive that was kept stays kept). So filtered prune never deletes anything that unfiltered prune would have kept.

But the four filters are not equally meaningful with prune:

  • --newer=X ("never touch archives older than X") adds real expressive power — a blanket fence around deep history that no --keep* rule can express.
  • --older=X is essentially redundant: --older=1y --keep-daily=30--keep-within=1y --keep-daily=30, since keep-within already protects recent archives without counting them toward the other rules. Harmless, but a second spelling for the same policy.
  • --oldest/--newest are the shaky ones: they anchor on the matched set's own endpoints, and prune changes those endpoints, so with a repeatedly-run command the window slides between runs. There is a coherent-but-exotic use (walk through history compacting the oldest month per run), but mostly it is confusion potential on a destructive command.

Why wiring them up still seems right: the status quo was the worst option — the flags have been in prune's generated usage docs and man page since 2023, so someone running borg prune --keep=1 --older=1y believes their young archives are safe while borg prunes them. Both implementing the flags and erroring out fix that silent hazard; implementing matches what the docs have promised for two years and what delete/check already do with the same flags.

If narrowing is preferred, the defensible middle ground would be: accept --older/--newer (now-relative, well-behaved) and reject --oldest/--newest on prune with an error. Erroring on all four would just trade the fixed bug for a docs/CLI break with no safety gain, given the monotonicity property above.

🤖 ^ Evaluation generated by Claude Code

@ThomasWaldmann

Copy link
Copy Markdown
Member Author

@PhrozenByte @jdchristensen What do you think, shall we add/enable the archive filters?

@PhrozenByte

PhrozenByte commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

IMO: Drop them from prune. I agree with Claude that they can easily cause confusion.

  • --oldest/--newest/--older are all somewhat related to prune --from, although the latter also affects the --keep* options. In real-world scenarios, though, I rather expect users to want that.
  • --older is redundant with --keep-within (as Claude already pointed out).
  • I can't think of any real-world use cases for either --newest or --oldest. For --newest, I can at least think of a few theoretical use cases, but in practice one probably rather wants to use --from instead… For --oldest, not even that.
  • I agree with Claude's characterization of --oldest as being shaky, but not of --newest: --newest wouldn't change the anchor, because that anchor is always protected here.

I only consider --newer a reasonable candidate to keep, because it allows one to protect very old archives. If we want to keep that, I'd probably rather add a new --to CLI option (like prune --from X --to Y) which could use the exact same implementation as --newer. However, to protect old archives, I'd prefer using tags instead, but that would practically require implementing #9936 first ("practically", because we'd only want to tag the protected archives, not all other archives). So, if we prioritize #9936, I'd also drop --newer for prune.

Off topic: I don't like --oldest/--newest/--older/--newer in general; I'd rather see them replaced by date: patterns (see #8715).

@jdchristensen

Copy link
Copy Markdown
Contributor

I haven't thought about this carefully, but I would also lean towards dropping them. The prune options are already very complicated. My only thought is whether it would make sense to replace --keep-within with --older, so that the same option name is used for the same effect in different situations. (This is assuming that the semantics are the same. If not, different names is better.) And if this was done, then --newer could be kept as well.

ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this pull request Aug 29, 2026
--oldest/--newest/--older/--newer were accepted by the prune parser
(inherited via the shared archive filters group, borgbackup#7272), but never
applied - borg prune --keep 1 --older=1y silently pruned minutes-old
archives.

Per the PR discussion, prune does not support these filters for now
(--older is redundant with --keep-within, --oldest/--newest anchor on
endpoints that prune itself changes), so reject them at parse time
instead of wiring them up.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ThomasWaldmann ThomasWaldmann changed the title prune: fix --oldest/--newest/--older/--newer having no effect prune: remove the date-based archive filters (were accepted but never applied) Aug 29, 2026
@ThomasWaldmann

Copy link
Copy Markdown
Member Author

@PhrozenByte @jdchristensen Thanks for the feedback — reworked the PR accordingly: the four date-based filters are now removed from prune's parser (rejected at parse time with an error) instead of being wired up. Net diff is a one-line parser change plus a test asserting all four options are rejected. Whether --newer-style protection of old archives comes back later via --to, tags (#9936), or date: patterns (#8715) can be decided separately.

🤖 Generated with Claude Code

--oldest/--newest/--older/--newer were accepted by the prune parser
(inherited via the shared archive filters group, borgbackup#7272), but never
applied - borg prune --keep 1 --older=1y silently pruned minutes-old
archives.

Per the PR discussion, prune does not support these filters for now
(--older is redundant with --keep-within, --oldest/--newest anchor on
endpoints that prune itself changes), so reject them at parse time
instead of wiring them up.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ThomasWaldmann
ThomasWaldmann force-pushed the fix-prune-archive-filters branch from bffd019 to 9004bf4 Compare August 29, 2026 12:20
@ThomasWaldmann
ThomasWaldmann marked this pull request as ready for review August 29, 2026 12:22
@ThomasWaldmann
ThomasWaldmann merged commit 848a935 into borgbackup:master Aug 29, 2026
22 of 23 checks passed
@ThomasWaldmann
ThomasWaldmann deleted the fix-prune-archive-filters branch August 29, 2026 12:48
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.

3 participants