Merge pull request #2383 from enkore/f/recreaterecompressargsthingsthatareugly

recreate: add --recompress flag, avoid weirdo use of args.compression

docs/usage.rst

@@ -463,7 +463,7 @@ Examples
     $ borg create /mnt/backup::archive /some/files --compression lz4
     # Then compress it - this might take longer, but the backup has already completed, so no inconsistencies
     # from a long-running backup job.
-    $ borg recreate /mnt/backup::archive --compression zlib,9
+    $ borg recreate /mnt/backup::archive --recompress --compression zlib,9
 
     # Remove unwanted files from all archives in a repository
     $ borg recreate /mnt/backup -e /home/icke/Pictures/drunk_photos

src/borg/archive.py

@@ -1557,7 +1557,7 @@ class ArchiveRecreater:
 
     def __init__(self, repository, manifest, key, cache, matcher,
                  exclude_caches=False, exclude_if_present=None, keep_exclude_tags=False,
-                 chunker_params=None, compression=None, always_recompress=False,
+                 chunker_params=None, compression=None, recompress=False, always_recompress=False,
                  dry_run=False, stats=False, progress=False, file_status_printer=None,
                  checkpoint_interval=1800):
         self.repository = repository
@@ -1574,7 +1574,7 @@ class ArchiveRecreater:
         if self.rechunkify:
             logger.debug('Rechunking archives to %s', chunker_params)
         self.chunker_params = chunker_params or CHUNKER_PARAMS
-        self.recompress = bool(compression)
+        self.recompress = recompress
         self.always_recompress = always_recompress
         self.compression = compression or CompressionSpec('none')
         self.seen_chunks = set()

src/borg/archiver.py

@@ -109,14 +109,7 @@ def with_repository(fake=False, invert_fake=False, create=False, lock=True, excl
             with repository:
                 if manifest or cache:
                     kwargs['manifest'], kwargs['key'] = Manifest.load(repository)
-                    # do_recreate uses args.compression is None as in band signalling for "don't recompress",
-                    # note that it does not look at key.compressor. In this case the default compressor applies
-                    # to new chunks.
-                    #
-                    # We can't use a check like `'compression' in args` (an argparse.Namespace speciality),
-                    # since the compression attribute is set. So we need to see whether it's set to something
-                    # true-ish, like a CompressionSpec instance.
-                    if getattr(args, 'compression', False):
+                    if 'compression' in args:
                         kwargs['key'].compressor = args.compression.compressor
                 if cache:
                     with Cache(repository, kwargs['key'], kwargs['manifest'],
@@ -1331,11 +1324,13 @@ class Archiver:
         matcher, include_patterns = self.build_matcher(args.patterns, args.paths)
         self.output_list = args.output_list
         self.output_filter = args.output_filter
+        recompress = args.recompress != 'never'
+        always_recompress = args.recompress == 'always'
 
         recreater = ArchiveRecreater(repository, manifest, key, cache, matcher,
                                      exclude_caches=args.exclude_caches, exclude_if_present=args.exclude_if_present,
                                      keep_exclude_tags=args.keep_exclude_tags, chunker_params=args.chunker_params,
-                                     compression=args.compression, always_recompress=args.always_recompress,
+                                     compression=args.compression, recompress=recompress, always_recompress=always_recompress,
                                      progress=args.progress, stats=args.stats,
                                      file_status_printer=self.print_file_status,
                                      checkpoint_interval=args.checkpoint_interval,
@@ -2930,7 +2925,7 @@ class Archiver:
         Note that all paths in an archive are relative, therefore absolute patterns/paths
         will *not* match (--exclude, --exclude-from, PATHs).
 
-        --compression: all chunks seen will be stored using the given method.
+        --recompress allows to change the compression of existing data in archives.
         Due to how Borg stores compressed size information this might display
         incorrect information for archives that were not recreated at the same time.
         There is no risk of data loss by this.
@@ -3017,12 +3012,15 @@ class Archiver:
                                    help='manually specify the archive creation date/time (UTC, yyyy-mm-ddThh:mm:ss format). '
                                         'alternatively, give a reference file/directory.')
         archive_group.add_argument('-C', '--compression', dest='compression',
-                                   type=CompressionSpec, default=None, metavar='COMPRESSION',
+                                   type=CompressionSpec, default=CompressionSpec('lz4'), metavar='COMPRESSION',
                                    help='select compression algorithm, see the output of the '
                                         '"borg help compression" command for details.')
-        archive_group.add_argument('--always-recompress', dest='always_recompress', action='store_true',
-                                   help='always recompress chunks, don\'t skip chunks already compressed with the same '
-                                        'algorithm.')
+        archive_group.add_argument('--recompress', dest='recompress', nargs='?', default='never', const='if-different',
+                                   choices=('never', 'if-different', 'always'),
+                                   help='recompress data chunks according to --compression if "if-different". '
+                                        'When "always", chunks that are already compressed that way are not skipped, '
+                                        'but compressed again. Only the algorithm is considered for "if-different", '
+                                        'not the compression level (if any).')
         archive_group.add_argument('--chunker-params', dest='chunker_params',
                                    type=ChunkerParams, default=CHUNKER_PARAMS,
                                    metavar='PARAMS',

src/borg/testsuite/archiver.py

@@ -2049,7 +2049,7 @@ class ArchiverTestCase(ArchiverTestCaseBase):
                              '--format', '{size} {csize} {sha256}')
         size, csize, sha256_before = file_list.split(' ')
         assert int(csize) >= int(size)  # >= due to metadata overhead
-        self.cmd('recreate', self.repository_location, '-C', 'lz4')
+        self.cmd('recreate', self.repository_location, '-C', 'lz4', '--recompress')
         self.check_cache()
         file_list = self.cmd('list', self.repository_location + '::test', 'input/compressible',
                              '--format', '{size} {csize} {sha256}')