prune: remove the date-based archive filters (were accepted but never applied) - #10250
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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. |
|
Design note: does it even make sense to combine the archive filters with prune's Worth recording, because the combination was never really designed: the four options came into the shared 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 But the four filters are not equally meaningful with prune:
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 If narrowing is preferred, the defensible middle ground would be: accept 🤖 ^ Evaluation generated by Claude Code |
|
@PhrozenByte @jdchristensen What do you think, shall we add/enable the archive filters? |
|
IMO: Drop them from
I only consider Off topic: I don't like |
|
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 |
--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>
|
@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 🤖 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>
bffd019 to
9004bf4
Compare
The date-based archive filters (
--oldest/--newest/--older/--newer) were accepted by the prune parser but never applied:borg prune --keep 1 --older=1yhappily 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 —
--olderis 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--tooption 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.pygreen; ruff clean. Note: the generated usage docs still list the options for prune; regeneration is left to the usual release-timebuild_usage/build_manrun.🤖 Generated with Claude Code