use immutable data structure for the compression spec, fixes #2331

the bug was compr_args.update(compr_spec), helpers.py:2168 - that mutated
the compression spec dict (and not just some local one, but the compr spec
dict parsed from the commandline args).

so a change that was intended just for 1 chunk changed the desired
compression level on the archive scope.

I refactored the stuff to use a namedtuple (which is immutable, so such
effects can not happen again).

src/borg/archive.py

@@ -953,7 +953,7 @@ Utilization of max. archive size: {csize_max:.0%}
             item.chunks = chunks
         else:
             compress = self.compression_decider1.decide(path)
-            self.file_compression_logger.debug('%s -> compression %s', path, compress['name'])
+            self.file_compression_logger.debug('%s -> compression %s', path, compress.name)
             with backup_io('open'):
                 fh = Archive._open_rb(path)
             with os.fdopen(fh, 'rb') as fd:
@@ -1651,7 +1651,7 @@ class ArchiveRecreater:
         if self.recompress and not self.always_recompress and chunk_id in self.cache.chunks:
             # Check if this chunk is already compressed the way we want it
             old_chunk = self.key.decrypt(None, self.repository.get(chunk_id), decompress=False)
-            if Compressor.detect(old_chunk.data).name == compression_spec['name']:
+            if Compressor.detect(old_chunk.data).name == compression_spec.name:
                 # Stored chunk has the same compression we wanted
                 overwrite = False
         chunk_entry = self.cache.add_chunk(chunk_id, chunk, target.stats, overwrite=overwrite)

src/borg/archiver.py

@@ -37,7 +37,7 @@ from .constants import *  # NOQA
 from .crc32 import crc32
 from .helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR
 from .helpers import Error, NoManifestError, set_ec
-from .helpers import location_validator, archivename_validator, ChunkerParams, CompressionSpec
+from .helpers import location_validator, archivename_validator, ChunkerParams, CompressionSpec, ComprSpec
 from .helpers import PrefixSpec, SortBySpec, HUMAN_SORT_KEYS
 from .helpers import BaseFormatter, ItemFormatter, ArchiveFormatter
 from .helpers import format_time, format_timedelta, format_file_size, format_archive
@@ -2394,7 +2394,7 @@ class Archiver:
                                    help='specify the chunker parameters (CHUNK_MIN_EXP, CHUNK_MAX_EXP, '
                                         'HASH_MASK_BITS, HASH_WINDOW_SIZE). default: %d,%d,%d,%d' % CHUNKER_PARAMS)
         archive_group.add_argument('-C', '--compression', dest='compression',
-                                   type=CompressionSpec, default=dict(name='lz4'), metavar='COMPRESSION',
+                                   type=CompressionSpec, default=ComprSpec(name='lz4', spec=None), metavar='COMPRESSION',
                                    help='select compression algorithm, see the output of the '
                                         '"borg help compression" command for details.')
         archive_group.add_argument('--compression-from', dest='compression_files',

src/borg/helpers.py

@@ -705,6 +705,9 @@ def ChunkerParams(s):
     return int(chunk_min), int(chunk_max), int(chunk_mask), int(window_size)
 
 
+ComprSpec = namedtuple('ComprSpec', ('name', 'spec'))
+
+
 def CompressionSpec(s):
     values = s.split(',')
     count = len(values)
@@ -713,7 +716,7 @@ def CompressionSpec(s):
     # --compression algo[,level]
     name = values[0]
     if name in ('none', 'lz4', ):
-        return dict(name=name)
+        return ComprSpec(name=name, spec=None)
     if name in ('zlib', 'lzma', ):
         if count < 2:
             level = 6  # default compression level in py stdlib
@@ -723,13 +726,13 @@ def CompressionSpec(s):
                 raise ValueError
         else:
             raise ValueError
-        return dict(name=name, level=level)
+        return ComprSpec(name=name, spec=level)
     if name == 'auto':
         if 2 <= count <= 3:
             compression = ','.join(values[1:])
         else:
             raise ValueError
-        return dict(name=name, spec=CompressionSpec(compression))
+        return ComprSpec(name=name, spec=CompressionSpec(compression))
     raise ValueError
 
 
@@ -2147,7 +2150,7 @@ class CompressionDecider2:
         # if we compress the data here to decide, we can even update the chunk data
         # and modify the metadata as desired.
         compr_spec = chunk.meta.get('compress', self.compression)
-        if compr_spec['name'] == 'auto':
+        if compr_spec.name == 'auto':
             # we did not decide yet, use heuristic:
             compr_spec, chunk = self.heuristic_lz4(compr_spec, chunk)
         return compr_spec, chunk
@@ -2160,14 +2163,14 @@ class CompressionDecider2:
         data_len = len(data)
         cdata_len = len(cdata)
         if cdata_len < data_len:
-            compr_spec = compr_args['spec']
+            compr_spec = compr_args.spec
         else:
             # uncompressible - we could have a special "uncompressible compressor"
             # that marks such data as uncompressible via compression-type metadata.
             compr_spec = CompressionSpec('none')
-        compr_args.update(compr_spec)
         self.logger.debug("len(data) == %d, len(lz4(data)) == %d, choosing %s", data_len, cdata_len, compr_spec)
-        return compr_args, Chunk(data, **meta)
+        meta['compress'] = compr_spec
+        return compr_spec, Chunk(data, **meta)
 
 
 class ErrorIgnoringTextIOWrapper(io.TextIOWrapper):

src/borg/key.py

@@ -153,7 +153,7 @@ class KeyBase:
 
     def compress(self, chunk):
         compr_args, chunk = self.compression_decider2.decide(chunk)
-        compressor = Compressor(**compr_args)
+        compressor = Compressor(name=compr_args.name, level=compr_args.spec)
         meta, data = chunk
         data = compressor.compress(data)
         return Chunk(data, **meta)

src/borg/testsuite/helpers.py

@@ -24,7 +24,7 @@ from ..helpers import StableDict, int_to_bigint, bigint_to_int, bin_to_hex
 from ..helpers import parse_timestamp, ChunkIteratorFileWrapper, ChunkerParams, Chunk
 from ..helpers import ProgressIndicatorPercent, ProgressIndicatorEndless
 from ..helpers import load_exclude_file, load_pattern_file
-from ..helpers import CompressionSpec, CompressionDecider1, CompressionDecider2
+from ..helpers import CompressionSpec, ComprSpec, CompressionDecider1, CompressionDecider2
 from ..helpers import parse_pattern, PatternMatcher, RegexPattern, PathPrefixPattern, FnmatchPattern, ShellPattern
 from ..helpers import swidth_slice
 from ..helpers import chunkit
@@ -671,16 +671,16 @@ def test_pattern_matcher():
 def test_compression_specs():
     with pytest.raises(ValueError):
         CompressionSpec('')
-    assert CompressionSpec('none') == dict(name='none')
-    assert CompressionSpec('lz4') == dict(name='lz4')
-    assert CompressionSpec('zlib') == dict(name='zlib', level=6)
-    assert CompressionSpec('zlib,0') == dict(name='zlib', level=0)
-    assert CompressionSpec('zlib,9') == dict(name='zlib', level=9)
+    assert CompressionSpec('none') == ComprSpec(name='none', spec=None)
+    assert CompressionSpec('lz4') == ComprSpec(name='lz4', spec=None)
+    assert CompressionSpec('zlib') == ComprSpec(name='zlib', spec=6)
+    assert CompressionSpec('zlib,0') == ComprSpec(name='zlib', spec=0)
+    assert CompressionSpec('zlib,9') == ComprSpec(name='zlib', spec=9)
     with pytest.raises(ValueError):
         CompressionSpec('zlib,9,invalid')
-    assert CompressionSpec('lzma') == dict(name='lzma', level=6)
-    assert CompressionSpec('lzma,0') == dict(name='lzma', level=0)
-    assert CompressionSpec('lzma,9') == dict(name='lzma', level=9)
+    assert CompressionSpec('lzma') == ComprSpec(name='lzma', spec=6)
+    assert CompressionSpec('lzma,0') == ComprSpec(name='lzma', spec=0)
+    assert CompressionSpec('lzma,9') == ComprSpec(name='lzma', spec=9)
     with pytest.raises(ValueError):
         CompressionSpec('lzma,9,invalid')
     with pytest.raises(ValueError):
@@ -1202,14 +1202,14 @@ none:*.zip
 """.splitlines()
 
     cd = CompressionDecider1(default, [])  # no conf, always use default
-    assert cd.decide('/srv/vm_disks/linux')['name'] == 'zlib'
-    assert cd.decide('test.zip')['name'] == 'zlib'
-    assert cd.decide('test')['name'] == 'zlib'
+    assert cd.decide('/srv/vm_disks/linux').name == 'zlib'
+    assert cd.decide('test.zip').name == 'zlib'
+    assert cd.decide('test').name == 'zlib'
 
     cd = CompressionDecider1(default, [conf, ])
-    assert cd.decide('/srv/vm_disks/linux')['name'] == 'lz4'
-    assert cd.decide('test.zip')['name'] == 'none'
-    assert cd.decide('test')['name'] == 'zlib'  # no match in conf, use default
+    assert cd.decide('/srv/vm_disks/linux').name == 'lz4'
+    assert cd.decide('test.zip').name == 'none'
+    assert cd.decide('test').name == 'zlib'  # no match in conf, use default
 
 
 def test_compression_decider2():
@@ -1217,9 +1217,9 @@ def test_compression_decider2():
 
     cd = CompressionDecider2(default)
     compr_spec, chunk = cd.decide(Chunk(None))
-    assert compr_spec['name'] == 'zlib'
+    assert compr_spec.name == 'zlib'
     compr_spec, chunk = cd.decide(Chunk(None, compress=CompressionSpec('lzma')))
-    assert compr_spec['name'] == 'lzma'
+    assert compr_spec.name == 'lzma'
 
 
 def test_format_line():