Browse Source

Fix handling of "repository" and "data" consistency checks to prevent invalid Borg flags (#565).

Dan Helfman 2 years ago
parent
commit
6ddae20fa1
4 changed files with 20 additions and 28 deletions
  1. 3 0
      NEWS
  2. 8 7
      borgmatic/borg/check.py
  3. 1 1
      setup.py
  4. 8 20
      tests/unit/borg/test_check.py

+ 3 - 0
NEWS

@@ -1,3 +1,6 @@
+1.6.7.dev0
+ * #565: Fix handling of "repository" and "data" consistency checks to prevent invalid Borg flags.
+
 1.6.6
  * #559: Update documentation about configuring multiple consistency checks or multiple databases.
  * #560: Fix all database hooks to error when the requested database to restore isn't present in the

+ 8 - 7
borgmatic/borg/check.py

@@ -33,8 +33,6 @@ def parse_checks(consistency_config, only_checks=None):
 
     If no "checks" option is present in the config, return the DEFAULT_CHECKS. If a checks value
     has a name of "disabled", return an empty tuple, meaning that no checks should be run.
-
-    If the "data" check is present, then make sure the "archives" check is included as well.
     '''
     checks = only_checks or tuple(
         check_config['name']
@@ -48,9 +46,6 @@ def parse_checks(consistency_config, only_checks=None):
             )
         return ()
 
-    if 'data' in checks and 'archives' not in checks:
-        return checks + ('archives',)
-
     return checks
 
 
@@ -164,7 +159,7 @@ def make_check_flags(checks, check_last=None, prefix=None):
         ('--repository-only',)
 
     However, if both "repository" and "archives" are in checks, then omit them from the returned
-    flags because Borg does both checks by default.
+    flags because Borg does both checks by default. If "data" is in checks, that implies "archives".
 
     Additionally, if a check_last value is given and "archives" is in checks, then include a
     "--last" flag. And if a prefix value is given and "archives" is in checks, then include a
@@ -183,7 +178,13 @@ def make_check_flags(checks, check_last=None, prefix=None):
                 'Ignoring consistency prefix option, as "archives" is not in consistency checks'
             )
 
-    common_flags = last_flags + prefix_flags + (('--verify-data',) if 'data' in checks else ())
+    if 'data' in checks:
+        data_flags = ('--verify-data',)
+        checks += ('archives',)
+    else:
+        data_flags = ()
+
+    common_flags = last_flags + prefix_flags + data_flags
 
     if {'repository', 'archives'}.issubset(set(checks)):
         return common_flags

+ 1 - 1
setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.6.6'
+VERSION = '1.6.7.dev0'
 
 
 setup(

+ 8 - 20
tests/unit/borg/test_check.py

@@ -49,18 +49,6 @@ def test_parse_checks_with_disabled_returns_no_checks():
     assert checks == ()
 
 
-def test_parse_checks_with_data_check_also_injects_archives():
-    checks = module.parse_checks({'checks': [{'name': 'data'}]})
-
-    assert checks == ('data', 'archives')
-
-
-def test_parse_checks_with_data_check_passes_through_archives():
-    checks = module.parse_checks({'checks': [{'name': 'data'}, {'name': 'archives'}]})
-
-    assert checks == ('data', 'archives')
-
-
 def test_parse_checks_prefers_override_checks_to_configured_checks():
     checks = module.parse_checks(
         {'checks': [{'name': 'archives'}]}, only_checks=['repository', 'extract']
@@ -69,12 +57,6 @@ def test_parse_checks_prefers_override_checks_to_configured_checks():
     assert checks == ('repository', 'extract')
 
 
-def test_parse_checks_with_override_data_check_also_injects_archives():
-    checks = module.parse_checks({'checks': [{'name': 'extract'}]}, only_checks=['data'])
-
-    assert checks == ('data', 'archives')
-
-
 @pytest.mark.parametrize(
     'frequency,expected_result',
     (
@@ -217,10 +199,10 @@ def test_make_check_flags_with_archives_check_returns_flag():
     assert flags == ('--archives-only',)
 
 
-def test_make_check_flags_with_data_check_returns_flag():
+def test_make_check_flags_with_data_check_returns_flag_and_implies_archives():
     flags = module.make_check_flags(('data',))
 
-    assert flags == ('--verify-data',)
+    assert flags == ('--archives-only', '--verify-data',)
 
 
 def test_make_check_flags_with_extract_omits_extract_flag():
@@ -229,6 +211,12 @@ def test_make_check_flags_with_extract_omits_extract_flag():
     assert flags == ()
 
 
+def test_make_check_flags_with_repository_and_data_checks_does_not_return_repository_only():
+    flags = module.make_check_flags(('repository', 'data',))
+
+    assert flags == ('--verify-data',)
+
+
 def test_make_check_flags_with_default_checks_and_default_prefix_returns_default_flags():
     flags = module.make_check_flags(('repository', 'archives'), prefix=module.DEFAULT_PREFIX)