borg delete --force --force to delete severely corrupted archives, fixes #1975 #2184
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2184 +/- ##
==========================================
- Coverage 83.82% 83.65% -0.18%
==========================================
Files 20 20
Lines 7203 7219 +16
Branches 1234 1237 +3
==========================================
+ Hits 6038 6039 +1
- Misses 837 850 +13
- Partials 328 330 +2
Continue to review full report at Codecov.
|
src/borg/archiver.py
Outdated
| @@ -2384,6 +2403,9 @@ def process_epilog(epilog): | |||
| subparser.add_argument('--force', dest='forced', | |||
| action='store_true', default=False, | |||
| help='force deletion of corrupted archives') | |||
| subparser.add_argument('--zap', dest='zap', | |||
| action='store_true', default=False, | |||
| help='force manifest removal of corrupted archives') | |||
There was a problem hiding this comment.
What's the difference between --force and --zap?
There was a problem hiding this comment.
--zap does much less (like the external fix script you made for this ticket).
There was a problem hiding this comment.
Hm, ok. I see. Yes. Wouldn't be straightforward to detect. Perhaps this should be more like a "--force --force" option, so that it's clear that this should only be used if it's too corrupt to work with even "--force"?
There was a problem hiding this comment.
ok, sounds good and argparse supports it.
ThomasWaldmann
changed the title
src/borg/archiver.py
Outdated
| else: | ||
| deleted = True | ||
| logger.info('Deleted {} ({}/{}).'.format(archive_name, i, len(archive_names))) | ||
| if deleted and yes('Are you sure? [y/N] '): |
There was a problem hiding this comment.
thinking about removing the yes().
the forced == 1 does not ask either and if users add another --force, they already said that they mean it.
|
I can has test case? ;) |
src/borg/archiver.py
Outdated
| logger.info('Deleted {} ({}/{}).'.format(archive_name, i, len(archive_names))) | ||
| if deleted: | ||
| manifest.write() | ||
| repository.append_only = True # avoid issues in compaction by not doing it |
There was a problem hiding this comment.
hmm, does not work with remote repo.
|
btw, i also tried to add a test for the single --force delete, but it needs some different corruption code that cleanly removes an archive chunk from repo. |
|
Suggestion: Don't corrupt the low-level structures for the tests, it is expected that only check--repair can fix it and that --force² might fail after the commit. Instead, write some invalid data via the Repository API, that way it's easier to test. |
|
fixed the test, collapsed changesets. anything else? |
|
merge? this one first, there is also another one that adds a test for single-force also, that one second. |
No description provided.