From 947dc77a5048bdea80d69a6657b8eed975840d92 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 12 Sep 2025 11:21:11 -0700 Subject: [PATCH 1/4] Always error and exit when the borgmatic runtime directory overlaps with the configured excludes (#1122). --- NEWS | 2 ++ borgmatic/borg/create.py | 27 ++++++++++++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index 5e601436..cb1b43ec 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ * #1114: Document systemd configuration changes for the ZFS filesystem hook. * #1118: Fix a bug in which Borg hangs during database backup when different filesystems are in use. + * #1122: To prevent the user from inadvertently excluding the "bootstrap" action's manifest, always + error and exit when the borgmatic runtime directory overlaps with the configured excludes. * #1125: Clarify documentation about ZFS, Btrfs, and LVM snapshotting when a separate filesystem is mounted in the source directory. (Spoiler: The separate filesystem doesn't get included in the snapshot.) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 2f0bf842..67e1d126 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -45,7 +45,7 @@ def any_parent_directories(path, candidate_parents): return False -def collect_special_file_paths( +def check_planned_backup_paths( dry_run, create_command, config, @@ -111,7 +111,6 @@ def collect_special_file_paths( return tuple( path for path in paths - if special_file(path, working_directory) if path not in paths_containing_runtime_directory ) @@ -228,6 +227,17 @@ def make_base_create_command( archive_name_format, local_borg_version, ) + working_directory = borgmatic.config.paths.get_working_directory(config) + + logger.debug('Checking file paths Borg plans to backup') + planned_backup_paths = check_planned_backup_paths( + dry_run, + create_flags + create_positional_arguments, + config, + local_path, + working_directory, + borgmatic_runtime_directory=borgmatic_runtime_directory, + ) # If database hooks are enabled (as indicated by streaming processes), exclude files that might # cause Borg to hang. But skip this if the user has explicitly set the "read_special" to True. @@ -235,16 +245,11 @@ def make_base_create_command( logger.warning( 'Ignoring configured "read_special" value of false, as true is needed for database hooks.', ) - working_directory = borgmatic.config.paths.get_working_directory(config) - logger.debug('Collecting special file paths') - special_file_paths = collect_special_file_paths( - dry_run, - create_flags + create_positional_arguments, - config, - local_path, - working_directory, - borgmatic_runtime_directory=borgmatic_runtime_directory, + special_file_paths = tuple( + path + for path in planned_backup_paths + if special_file(path, working_directory) ) if special_file_paths: From f60007545b41fa6d7e44c769666be9dcaeb7d0da Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 12 Sep 2025 12:54:38 -0700 Subject: [PATCH 2/4] Update automated tests (#1122). --- borgmatic/borg/create.py | 12 ++--- tests/unit/borg/test_create.py | 91 ++++++++++++++++++---------------- 2 files changed, 52 insertions(+), 51 deletions(-) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 67e1d126..ef80c7b8 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -108,11 +108,7 @@ def check_planned_backup_paths( f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes or patterns with excludes. Please ensure the runtime directory is not excluded.', ) - return tuple( - path - for path in paths - if path not in paths_containing_runtime_directory - ) + return tuple(path for path in paths if path not in paths_containing_runtime_directory) MAX_SPECIAL_FILE_PATHS_LENGTH = 1000 @@ -229,7 +225,7 @@ def make_base_create_command( ) working_directory = borgmatic.config.paths.get_working_directory(config) - logger.debug('Checking file paths Borg plans to backup') + logger.debug('Checking file paths Borg plans to include') planned_backup_paths = check_planned_backup_paths( dry_run, create_flags + create_positional_arguments, @@ -247,9 +243,7 @@ def make_base_create_command( ) special_file_paths = tuple( - path - for path in planned_backup_paths - if special_file(path, working_directory) + path for path in planned_backup_paths if special_file(path, working_directory) ) if special_file_paths: diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 08532317..7ddb99fe 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -59,7 +59,7 @@ def test_any_parent_directories_treats_unrelated_paths_as_non_match(): module.any_parent_directories('/foo/bar.txt', ('/usr', '/etc')) -def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_list(): +def test_check_planned_backup_paths_parses_borg_dry_run_file_list(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -70,11 +70,10 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_ flexmock(module).should_receive('execute_command_and_capture_output').and_return( 'Processing files ...\n- /foo\n+ /bar\n- /baz', ) - 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() - assert module.collect_special_file_paths( + assert module.check_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, @@ -84,7 +83,7 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_ ) == ('/foo', '/bar', '/baz') -def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): +def test_check_planned_backup_paths_skips_borgmatic_runtime_directory(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -95,7 +94,6 @@ def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /run/borgmatic/bar\n- /baz', ) - flexmock(module).should_receive('special_file').and_return(True) flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module).should_receive('any_parent_directories').with_args( '/foo', @@ -110,7 +108,7 @@ def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): ('/run/borgmatic',), ).and_return(False) - assert module.collect_special_file_paths( + assert module.check_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, @@ -120,7 +118,7 @@ def test_collect_special_file_paths_skips_borgmatic_runtime_directory(): ) == ('/foo', '/baz') -def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): +def test_check_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -131,12 +129,11 @@ def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_fro flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /bar\n- /baz', ) - flexmock(module).should_receive('special_file').and_return(True) flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module).should_receive('any_parent_directories').and_return(False) with pytest.raises(ValueError): - module.collect_special_file_paths( + module.check_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, @@ -146,7 +143,7 @@ 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_check_planned_backup_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, ) @@ -157,11 +154,10 @@ def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory flexmock(module).should_receive('execute_command_and_capture_output').and_return( '+ /foo\n- /bar\n- /baz', ) - flexmock(module).should_receive('special_file').and_return(True) flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module).should_receive('any_parent_directories').and_return(False) - assert module.collect_special_file_paths( + assert module.check_planned_backup_paths( dry_run=True, create_command=('borg', 'create'), config={}, @@ -171,33 +167,6 @@ def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory ) == ('/foo', '/bar', '/baz') -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).should_receive('execute_command_and_capture_output').and_return( - '+ /foo\n+ /bar\n+ /baz', - ) - flexmock(module).should_receive('special_file').and_return(True).and_return(False).and_return( - True, - ) - flexmock(module.os.path).should_receive('exists').and_return(False) - flexmock(module).should_receive('any_parent_directories').never() - - assert module.collect_special_file_paths( - dry_run=False, - create_command=('borg', 'create'), - config={}, - local_path=None, - working_directory=None, - borgmatic_runtime_directory='/run/borgmatic', - ) == ('/foo', '/baz') - - DEFAULT_ARCHIVE_NAME = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}' REPO_ARCHIVE = (f'repo::{DEFAULT_ARCHIVE_NAME}',) @@ -211,6 +180,7 @@ def test_make_base_create_produces_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -246,6 +216,7 @@ def test_make_base_create_command_includes_patterns_file_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -278,6 +249,7 @@ def test_make_base_create_command_with_store_config_false_omits_config_files(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -346,6 +318,7 @@ def test_make_base_create_command_includes_configuration_option_as_command_flag( flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -378,6 +351,7 @@ def test_make_base_create_command_includes_dry_run_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=True, @@ -410,6 +384,7 @@ def test_make_base_create_command_includes_comment_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -443,6 +418,7 @@ def test_make_base_create_command_includes_local_path_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -475,6 +451,7 @@ def test_make_base_create_command_includes_remote_path_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -507,6 +484,7 @@ def test_make_base_create_command_includes_log_json_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -539,6 +517,7 @@ def test_make_base_create_command_includes_list_flags_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -578,7 +557,18 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal ) flexmock(module.logger).should_receive('warning').twice() flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once() + flexmock(module).should_receive('check_planned_backup_paths').and_return( + ( + '/non/special', + '/dev/null', + ) + ) + flexmock(module).should_receive('special_file').with_args( + '/non/special', working_directory=None + ).and_return(False) + flexmock(module).should_receive('special_file').with_args( + '/dev/null', working_directory=None + ).and_return(True) flexmock(module.borgmatic.borg.pattern).should_receive('write_patterns_file').with_args( ( Pattern( @@ -630,7 +620,18 @@ def test_make_base_create_command_without_patterns_and_with_stream_processes_ign ) flexmock(module.logger).should_receive('warning').twice() flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once() + flexmock(module).should_receive('check_planned_backup_paths').and_return( + ( + '/non/special', + '/dev/null', + ) + ) + flexmock(module).should_receive('special_file').with_args( + '/non/special', working_directory=None + ).and_return(False) + flexmock(module).should_receive('special_file').with_args( + '/dev/null', working_directory=None + ).and_return(True) flexmock(module.borgmatic.borg.pattern).should_receive('write_patterns_file').with_args( ( Pattern( @@ -678,7 +679,7 @@ def test_make_base_create_command_with_stream_processes_and_read_special_true_sk (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) flexmock(module.logger).should_receive('warning').never() - flexmock(module).should_receive('collect_special_file_paths').never() + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -712,6 +713,7 @@ def test_make_base_create_command_includes_archive_name_format_in_borg_command() flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( ('repo::ARCHIVE_NAME',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -744,6 +746,7 @@ def test_make_base_create_command_includes_default_archive_name_format_in_borg_c flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( ('repo::{hostname}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -775,6 +778,7 @@ def test_make_base_create_command_includes_archive_name_format_with_placeholders flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (repository_archive_pattern,), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -807,6 +811,7 @@ def test_make_base_create_command_includes_repository_and_archive_name_format_wi flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (repository_archive_pattern,), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -835,6 +840,7 @@ def test_make_base_create_command_includes_archive_suffix_in_borg_command(): DEFAULT_ARCHIVE_NAME, ) flexmock(module.borgmatic.borg.flags).should_receive('make_exclude_flags').and_return(()) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -867,6 +873,7 @@ def test_make_base_create_command_includes_extra_borg_options_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) + flexmock(module).should_receive('check_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, From 5028fe9ff42ea98d6c777db371a3957149641530 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 12 Sep 2025 14:03:52 -0700 Subject: [PATCH 3/4] Bring doc string up to date and rename function for clarity. (#1122). --- borgmatic/borg/create.py | 23 ++++++------- tests/end-to-end/test_borgmatic.py | 2 +- tests/unit/borg/test_create.py | 54 +++++++++++++++--------------- 3 files changed, 38 insertions(+), 41 deletions(-) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index ef80c7b8..e55f8dcb 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -45,7 +45,7 @@ def any_parent_directories(path, candidate_parents): return False -def check_planned_backup_paths( +def validate_planned_backup_paths( dry_run, create_command, config, @@ -55,18 +55,15 @@ def check_planned_backup_paths( ): ''' Given a dry-run flag, a Borg create command as a tuple, a configuration dict, a local Borg path, - a working directory, and the borgmatic runtime directory, collect the paths for any special - files (character devices, block devices, and named pipes / FIFOs) that Borg would encounter - during a create. These are all paths that could cause Borg to hang if its --read-special flag is - used. + a working directory, and the borgmatic runtime directory, perform a "borg create --dry-run" to + determine whether Borg's planned paths to include in a backup look good. Specifically, if the + given runtime directory exists, validate that it will be included in a backup and hasn't been + excluded. - Skip looking for special files in the given borgmatic runtime directory, as borgmatic creates - its own special files there for database dumps and we don't want those omitted. - - Additionally, if the borgmatic runtime directory is not contained somewhere in the files Borg - plans to backup, that means the user must have excluded the runtime directory (e.g. via - "exclude_patterns" or similar). Therefore, raise, because this means Borg won't be able to - consume any database dumps and therefore borgmatic will hang when it tries to do so. + Raise ValueError if the runtime directory has been excluded via "exclude_patterns" or similar, + because any features that rely on the runtime directory getting backed up will break. For + instance, without the runtime directory, Borg can't consume any database dumps and borgmatic may + hang waiting for them to be consumed. ''' # Omit "--exclude-nodump" from the Borg dry run command, because that flag causes Borg to open # files including any named pipe we've created. And omit "--filter" because that can break the @@ -226,7 +223,7 @@ def make_base_create_command( working_directory = borgmatic.config.paths.get_working_directory(config) logger.debug('Checking file paths Borg plans to include') - planned_backup_paths = check_planned_backup_paths( + planned_backup_paths = validate_planned_backup_paths( dry_run, create_flags + create_positional_arguments, config, diff --git a/tests/end-to-end/test_borgmatic.py b/tests/end-to-end/test_borgmatic.py index e16fdb09..766c39a9 100644 --- a/tests/end-to-end/test_borgmatic.py +++ b/tests/end-to-end/test_borgmatic.py @@ -85,7 +85,7 @@ def test_borgmatic_command(generate_configuration): ) # Run borgmatic to generate a backup archive, and then list it to make sure it exists. - subprocess.check_call(f'borgmatic --config {config_path}'.split(' ')) + subprocess.check_call(f'borgmatic -v 2 --config {config_path}'.split(' ')) output = subprocess.check_output( f'borgmatic --config {config_path} list --json'.split(' '), ).decode(sys.stdout.encoding) diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 7ddb99fe..a956b458 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -59,7 +59,7 @@ def test_any_parent_directories_treats_unrelated_paths_as_non_match(): module.any_parent_directories('/foo/bar.txt', ('/usr', '/etc')) -def test_check_planned_backup_paths_parses_borg_dry_run_file_list(): +def test_validate_planned_backup_paths_parses_borg_dry_run_file_list(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -73,7 +73,7 @@ def test_check_planned_backup_paths_parses_borg_dry_run_file_list(): flexmock(module.os.path).should_receive('exists').and_return(False) flexmock(module).should_receive('any_parent_directories').never() - assert module.check_planned_backup_paths( + assert module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, @@ -83,7 +83,7 @@ def test_check_planned_backup_paths_parses_borg_dry_run_file_list(): ) == ('/foo', '/bar', '/baz') -def test_check_planned_backup_paths_skips_borgmatic_runtime_directory(): +def test_validate_planned_backup_paths_skips_borgmatic_runtime_directory(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -108,7 +108,7 @@ def test_check_planned_backup_paths_skips_borgmatic_runtime_directory(): ('/run/borgmatic',), ).and_return(False) - assert module.check_planned_backup_paths( + assert module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, @@ -118,7 +118,7 @@ def test_check_planned_backup_paths_skips_borgmatic_runtime_directory(): ) == ('/foo', '/baz') -def test_check_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): +def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -133,7 +133,7 @@ def test_check_planned_backup_paths_with_borgmatic_runtime_directory_missing_fro flexmock(module).should_receive('any_parent_directories').and_return(False) with pytest.raises(ValueError): - module.check_planned_backup_paths( + module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, @@ -143,7 +143,7 @@ def test_check_planned_backup_paths_with_borgmatic_runtime_directory_missing_fro ) -def test_check_planned_backup_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise(): +def test_validate_planned_backup_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, ) @@ -157,7 +157,7 @@ def test_check_planned_backup_paths_with_dry_run_and_borgmatic_runtime_directory flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module).should_receive('any_parent_directories').and_return(False) - assert module.check_planned_backup_paths( + assert module.validate_planned_backup_paths( dry_run=True, create_command=('borg', 'create'), config={}, @@ -180,7 +180,7 @@ def test_make_base_create_produces_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -216,7 +216,7 @@ def test_make_base_create_command_includes_patterns_file_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -249,7 +249,7 @@ def test_make_base_create_command_with_store_config_false_omits_config_files(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -318,7 +318,7 @@ def test_make_base_create_command_includes_configuration_option_as_command_flag( flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -351,7 +351,7 @@ def test_make_base_create_command_includes_dry_run_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=True, @@ -384,7 +384,7 @@ def test_make_base_create_command_includes_comment_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -418,7 +418,7 @@ def test_make_base_create_command_includes_local_path_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -451,7 +451,7 @@ def test_make_base_create_command_includes_remote_path_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -484,7 +484,7 @@ def test_make_base_create_command_includes_log_json_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -517,7 +517,7 @@ def test_make_base_create_command_includes_list_flags_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -557,7 +557,7 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal ) flexmock(module.logger).should_receive('warning').twice() flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('check_planned_backup_paths').and_return( + flexmock(module).should_receive('validate_planned_backup_paths').and_return( ( '/non/special', '/dev/null', @@ -620,7 +620,7 @@ def test_make_base_create_command_without_patterns_and_with_stream_processes_ign ) flexmock(module.logger).should_receive('warning').twice() flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('check_planned_backup_paths').and_return( + flexmock(module).should_receive('validate_planned_backup_paths').and_return( ( '/non/special', '/dev/null', @@ -679,7 +679,7 @@ def test_make_base_create_command_with_stream_processes_and_read_special_true_sk (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) flexmock(module.logger).should_receive('warning').never() - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -713,7 +713,7 @@ def test_make_base_create_command_includes_archive_name_format_in_borg_command() flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( ('repo::ARCHIVE_NAME',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -746,7 +746,7 @@ def test_make_base_create_command_includes_default_archive_name_format_in_borg_c flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( ('repo::{hostname}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -778,7 +778,7 @@ def test_make_base_create_command_includes_archive_name_format_with_placeholders flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (repository_archive_pattern,), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -811,7 +811,7 @@ def test_make_base_create_command_includes_repository_and_archive_name_format_wi flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (repository_archive_pattern,), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -840,7 +840,7 @@ def test_make_base_create_command_includes_archive_suffix_in_borg_command(): DEFAULT_ARCHIVE_NAME, ) flexmock(module.borgmatic.borg.flags).should_receive('make_exclude_flags').and_return(()) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, @@ -873,7 +873,7 @@ def test_make_base_create_command_includes_extra_borg_options_in_borg_command(): flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( (f'repo::{DEFAULT_ARCHIVE_NAME}',), ) - flexmock(module).should_receive('check_planned_backup_paths').and_return(()) + flexmock(module).should_receive('validate_planned_backup_paths').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( dry_run=False, From c4c40af8124acb275074c2ed5f978fd553a66055 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 12 Sep 2025 22:17:44 -0700 Subject: [PATCH 4/4] Account for the case where "store_config_files: false" and the runtime directory never gets added to patterns to begin with (#1122). --- borgmatic/borg/create.py | 37 +++++++++++----- tests/unit/borg/test_create.py | 81 +++++++++++++++++++++++++++------- 2 files changed, 90 insertions(+), 28 deletions(-) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index e55f8dcb..2d7d58d5 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -49,6 +49,7 @@ def validate_planned_backup_paths( dry_run, create_command, config, + patterns, local_path, working_directory, borgmatic_runtime_directory, @@ -91,21 +92,32 @@ def validate_planned_backup_paths( if path_line and path_line.startswith(('- ', '+ ')) ) - # These are the subset of those files that contain the borgmatic runtime directory. - paths_containing_runtime_directory = {} + # These are the subset of output paths contained within the borgmatic runtime directory. + paths_inside_runtime_directory = { + path for path in paths if any_parent_directories(path, (borgmatic_runtime_directory,)) + } - if os.path.exists(borgmatic_runtime_directory): - paths_containing_runtime_directory = { - path for path in paths if any_parent_directories(path, (borgmatic_runtime_directory,)) - } + # If the runtime directory isn't present in the source patterns, then we shouldn't expect it to + # be in the paths output from the Borg dry run. + runtime_directory_present_in_patterns = any( + pattern + for pattern in patterns + if any_parent_directories(pattern.path, (borgmatic_runtime_directory,)) + if pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT + ) - # If no paths to backup contain the runtime directory, it must've been excluded. - if not paths_containing_runtime_directory and not dry_run: - raise ValueError( - f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes or patterns with excludes. Please ensure the runtime directory is not excluded.', - ) + # If no paths to backup are inside the runtime directory, it must've been excluded. + if ( + not paths_inside_runtime_directory + and runtime_directory_present_in_patterns + and not dry_run + and os.path.exists(borgmatic_runtime_directory) + ): + raise ValueError( + f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes or patterns with excludes. Please ensure the runtime directory is not excluded.', + ) - return tuple(path for path in paths if path not in paths_containing_runtime_directory) + return tuple(path for path in paths if path not in paths_inside_runtime_directory) MAX_SPECIAL_FILE_PATHS_LENGTH = 1000 @@ -227,6 +239,7 @@ def make_base_create_command( dry_run, create_flags + create_positional_arguments, config, + patterns, local_path, working_directory, borgmatic_runtime_directory=borgmatic_runtime_directory, diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index a956b458..b200a1c8 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -71,12 +71,17 @@ def test_validate_planned_backup_paths_parses_borg_dry_run_file_list(): 'Processing files ...\n- /foo\n+ /bar\n- /baz', ) flexmock(module.os.path).should_receive('exists').and_return(False) - flexmock(module).should_receive('any_parent_directories').never() + flexmock(module).should_receive('any_parent_directories').and_return(False) assert module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern('/bar'), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', @@ -95,23 +100,21 @@ def test_validate_planned_backup_paths_skips_borgmatic_runtime_directory(): '+ /foo\n- /run/borgmatic/bar\n- /baz', ) flexmock(module.os.path).should_receive('exists').and_return(True) - flexmock(module).should_receive('any_parent_directories').with_args( - '/foo', - ('/run/borgmatic',), - ).and_return(False) - flexmock(module).should_receive('any_parent_directories').with_args( - '/run/borgmatic/bar', - ('/run/borgmatic',), - ).and_return(True) - flexmock(module).should_receive('any_parent_directories').with_args( - '/baz', - ('/run/borgmatic',), - ).and_return(False) + flexmock(module).should_receive('any_parent_directories').replace_with( + lambda path, _: path == '/run/borgmatic/bar' + ) assert module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern( + '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT + ), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', @@ -130,19 +133,58 @@ def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_ '+ /foo\n- /bar\n- /baz', ) flexmock(module.os.path).should_receive('exists').and_return(True) - flexmock(module).should_receive('any_parent_directories').and_return(False) + flexmock(module).should_receive('any_parent_directories').replace_with( + lambda path, _: path == '/run/borgmatic/bar' + ) with pytest.raises(ValueError): module.validate_planned_backup_paths( dry_run=False, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern( + '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT + ), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', ) +def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_patterns_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).should_receive('execute_command_and_capture_output').and_return( + '+ /foo\n- /run/borgmatic/bar\n- /baz', + ) + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module).should_receive('any_parent_directories').replace_with( + lambda path, _: path == '/run/borgmatic/bar' + ) + + assert module.validate_planned_backup_paths( + dry_run=False, + create_command=('borg', 'create'), + config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), + local_path=None, + working_directory=None, + borgmatic_runtime_directory='/run/borgmatic', + ) == ('/foo', '/baz') + + def test_validate_planned_backup_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, @@ -152,7 +194,7 @@ def test_validate_planned_backup_paths_with_dry_run_and_borgmatic_runtime_direct ) flexmock(module.environment).should_receive('make_environment').and_return(None) flexmock(module).should_receive('execute_command_and_capture_output').and_return( - '+ /foo\n- /bar\n- /baz', + '+ /foo\n- /run/borgmatic/bar\n- /baz', ) flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module).should_receive('any_parent_directories').and_return(False) @@ -161,10 +203,17 @@ def test_validate_planned_backup_paths_with_dry_run_and_borgmatic_runtime_direct dry_run=True, create_command=('borg', 'create'), config={}, + patterns=( + module.borgmatic.borg.pattern.Pattern('/foo'), + module.borgmatic.borg.pattern.Pattern( + '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT + ), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), local_path=None, working_directory=None, borgmatic_runtime_directory='/run/borgmatic', - ) == ('/foo', '/bar', '/baz') + ) == ('/foo', '/run/borgmatic/bar', '/baz') DEFAULT_ARCHIVE_NAME = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}'