Browse Source

Fix a runtime directory error from a conflict between "extra_borg_options" and special file detection (#999).

Dan Helfman 3 months ago
parent
commit
5d9c111910
5 changed files with 108 additions and 26 deletions
  1. 2 0
      NEWS
  2. 3 2
      borgmatic/borg/create.py
  3. 41 0
      borgmatic/borg/flags.py
  4. 30 24
      tests/unit/borg/test_create.py
  5. 32 0
      tests/unit/borg/test_flags.py

+ 2 - 0
NEWS

@@ -6,6 +6,8 @@
    databases are enabled.
    databases are enabled.
  * #998: Send the "encryption_passphrase" option to Borg via an anonymous pipe, which is more secure
  * #998: Send the "encryption_passphrase" option to Borg via an anonymous pipe, which is more secure
    than using an environment variable.
    than using an environment variable.
+ * #999: Fix a runtime directory error from a conflict between "extra_borg_options" and special file
+   detection.
  * #1001: For the ZFS, Btrfs, and LVM hooks, only make snapshots for root patterns that come from
  * #1001: For the ZFS, Btrfs, and LVM hooks, only make snapshots for root patterns that come from
    a borgmatic configuration option (e.g. "source_directories")—not from other hooks within
    a borgmatic configuration option (e.g. "source_directories")—not from other hooks within
    borgmatic.
    borgmatic.

+ 3 - 2
borgmatic/borg/create.py

@@ -140,9 +140,10 @@ def collect_special_file_paths(
     consume any database dumps and therefore borgmatic will hang when it tries to do so.
     consume any database dumps and therefore borgmatic will hang when it tries to do so.
     '''
     '''
     # Omit "--exclude-nodump" from the Borg dry run command, because that flag causes Borg to open
     # Omit "--exclude-nodump" from the Borg dry run command, because that flag causes Borg to open
-    # files including any named pipe we've created.
+    # files including any named pipe we've created. And omit "--filter" because that can break the
+    # paths output parsing below such that path lines no longer start with th expected "- ".
     paths_output = execute_command_and_capture_output(
     paths_output = execute_command_and_capture_output(
-        tuple(argument for argument in create_command if argument != '--exclude-nodump')
+        flags.omit_flag_and_value(flags.omit_flag(create_command, '--exclude-nodump'), '--filter')
         + ('--dry-run', '--list'),
         + ('--dry-run', '--list'),
         capture_stderr=True,
         capture_stderr=True,
         working_directory=working_directory,
         working_directory=working_directory,

+ 41 - 0
borgmatic/borg/flags.py

@@ -156,3 +156,44 @@ def warn_for_aggressive_archive_flags(json_command, json_output):
         logger.debug(f'Cannot parse JSON output from archive command: {error}')
         logger.debug(f'Cannot parse JSON output from archive command: {error}')
     except (TypeError, KeyError):
     except (TypeError, KeyError):
         logger.debug('Cannot parse JSON output from archive command: No "archives" key found')
         logger.debug('Cannot parse JSON output from archive command: No "archives" key found')
+
+
+def omit_flag(arguments, flag):
+    '''
+    Given a sequence of Borg command-line arguments, return them with the given (valueless) flag
+    omitted. For instance, if the flag is "--flag" and arguments is:
+
+        ('borg', 'create', '--flag', '--other-flag')
+
+    ... then return:
+
+        ('borg', 'create', '--other-flag')
+    '''
+    return tuple(argument for argument in arguments if argument != flag)
+
+
+def omit_flag_and_value(arguments, flag):
+    '''
+    Given a sequence of Borg command-line arguments, return them with the given flag and its
+    corresponding value omitted. For instance, if the flag is "--flag" and arguments is:
+
+        ('borg', 'create', '--flag', 'value', '--other-flag')
+
+    ... or:
+
+        ('borg', 'create', '--flag=value', '--other-flag')
+
+    ... then return:
+
+        ('borg', 'create', '--other-flag')
+    '''
+    # This works by zipping together a list of overlapping pairwise arguments. E.g., ('one', 'two',
+    # 'three', 'four') becomes ((None, 'one'), ('one, 'two'), ('two', 'three'), ('three', 'four')).
+    # This makes it easy to "look back" at the previous arguments so we can exclude both a flag and
+    # its value.
+    return tuple(
+        argument
+        for (previous_argument, argument) in zip((None,) + arguments, arguments)
+        if flag not in (previous_argument, argument)
+        if not argument.startswith(f'{flag}=')
+    )

+ 30 - 24
tests/unit/borg/test_create.py

@@ -185,6 +185,12 @@ def test_any_parent_directories_treats_unrelated_paths_as_non_match():
 
 
 
 
 def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_list():
 def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_list():
+    flexmock(module.flags).should_receive('omit_flag').replace_with(
+        lambda arguments, flag: arguments
+    )
+    flexmock(module.flags).should_receive('omit_flag_and_value').replace_with(
+        lambda arguments, flag: arguments
+    )
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         'Processing files ...\n- /foo\n+ /bar\n- /baz'
         'Processing files ...\n- /foo\n+ /bar\n- /baz'
@@ -204,6 +210,12 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_
 
 
 
 
 def test_collect_special_file_paths_skips_borgmatic_runtime_directory():
 def test_collect_special_file_paths_skips_borgmatic_runtime_directory():
+    flexmock(module.flags).should_receive('omit_flag').replace_with(
+        lambda arguments, flag: arguments
+    )
+    flexmock(module.flags).should_receive('omit_flag_and_value').replace_with(
+        lambda arguments, flag: arguments
+    )
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '+ /foo\n- /run/borgmatic/bar\n- /baz'
         '+ /foo\n- /run/borgmatic/bar\n- /baz'
@@ -231,6 +243,12 @@ def test_collect_special_file_paths_skips_borgmatic_runtime_directory():
 
 
 
 
 def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors():
 def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors():
+    flexmock(module.flags).should_receive('omit_flag').replace_with(
+        lambda arguments, flag: arguments
+    )
+    flexmock(module.flags).should_receive('omit_flag_and_value').replace_with(
+        lambda arguments, flag: arguments
+    )
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '+ /foo\n- /bar\n- /baz'
         '+ /foo\n- /bar\n- /baz'
@@ -251,6 +269,12 @@ def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_fro
 
 
 
 
 def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise():
 def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise():
+    flexmock(module.flags).should_receive('omit_flag').replace_with(
+        lambda arguments, flag: arguments
+    )
+    flexmock(module.flags).should_receive('omit_flag_and_value').replace_with(
+        lambda arguments, flag: arguments
+    )
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '+ /foo\n- /bar\n- /baz'
         '+ /foo\n- /bar\n- /baz'
@@ -270,6 +294,12 @@ def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory
 
 
 
 
 def test_collect_special_file_paths_excludes_non_special_files():
 def test_collect_special_file_paths_excludes_non_special_files():
+    flexmock(module.flags).should_receive('omit_flag').replace_with(
+        lambda arguments, flag: arguments
+    )
+    flexmock(module.flags).should_receive('omit_flag_and_value').replace_with(
+        lambda arguments, flag: arguments
+    )
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '+ /foo\n+ /bar\n+ /baz'
         '+ /foo\n+ /bar\n+ /baz'
@@ -290,30 +320,6 @@ def test_collect_special_file_paths_excludes_non_special_files():
     ) == ('/foo', '/baz')
     ) == ('/foo', '/baz')
 
 
 
 
-def test_collect_special_file_paths_omits_exclude_no_dump_flag_from_command():
-    flexmock(module.environment).should_receive('make_environment').and_return(None)
-    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
-        ('borg', 'create', '--dry-run', '--list'),
-        capture_stderr=True,
-        working_directory=None,
-        environment=None,
-        borg_local_path='borg',
-        borg_exit_codes=None,
-    ).and_return('Processing files ...\n- /foo\n+ /bar\n- /baz').once()
-    flexmock(module).should_receive('special_file').and_return(True)
-    flexmock(module.os.path).should_receive('exists').and_return(False)
-    flexmock(module).should_receive('any_parent_directories').never()
-
-    module.collect_special_file_paths(
-        dry_run=False,
-        create_command=('borg', 'create', '--exclude-nodump'),
-        config={},
-        local_path='borg',
-        working_directory=None,
-        borgmatic_runtime_directory='/run/borgmatic',
-    )
-
-
 DEFAULT_ARCHIVE_NAME = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}'  # noqa: FS003
 DEFAULT_ARCHIVE_NAME = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}'  # noqa: FS003
 REPO_ARCHIVE = (f'repo::{DEFAULT_ARCHIVE_NAME}',)
 REPO_ARCHIVE = (f'repo::{DEFAULT_ARCHIVE_NAME}',)
 
 

+ 32 - 0
tests/unit/borg/test_flags.py

@@ -285,3 +285,35 @@ def test_warn_for_aggressive_archive_flags_with_glob_archives_and_json_missing_a
     flexmock(module.logger).should_receive('warning').never()
     flexmock(module.logger).should_receive('warning').never()
 
 
     module.warn_for_aggressive_archive_flags(('borg', '--glob-archives', 'foo*'), '{}')
     module.warn_for_aggressive_archive_flags(('borg', '--glob-archives', 'foo*'), '{}')
+
+
+def test_omit_flag_removes_flag_from_arguments():
+    module.omit_flag(('borg', 'create', '--flag', '--other'), '--flag') == (
+        'borg',
+        'create',
+        '--other',
+    )
+
+
+def test_omit_flag_without_flag_present_passes_through_arguments():
+    module.omit_flag(('borg', 'create', '--other'), '--flag') == ('borg', 'create', '--other')
+
+
+def test_omit_flag_and_value_removes_flag_and_value_from_arguments():
+    module.omit_flag(('borg', 'create', '--flag', 'value', '--other'), '--flag') == (
+        'borg',
+        'create',
+        '--other',
+    )
+
+
+def test_omit_flag_and_value_with_equals_sign_removes_flag_and_value_from_arguments():
+    module.omit_flag(('borg', 'create', '--flag=value', '--other'), '--flag') == (
+        'borg',
+        'create',
+        '--other',
+    )
+
+
+def test_omit_flag_and_value_without_flag_present_passes_through_arguments():
+    module.omit_flag(('borg', 'create', '--other'), '--flag') == ('borg', 'create', '--other')