From b8642439cded4aa53c34dc4710ce202ce964fe3a Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 22 Dec 2025 15:46:28 -0800 Subject: [PATCH 1/7] Fix for the database "restore" action restoring more databases than the "--database" flag specifies (#1208). --- NEWS | 4 ++++ borgmatic/actions/restore.py | 4 ++++ borgmatic/hooks/data_source/bootstrap.py | 4 ++++ borgmatic/hooks/data_source/btrfs.py | 4 ++++ borgmatic/hooks/data_source/dump.py | 2 +- borgmatic/hooks/data_source/lvm.py | 4 ++++ borgmatic/hooks/data_source/mariadb.py | 18 +++++++++++++++--- borgmatic/hooks/data_source/mongodb.py | 18 +++++++++++++++--- borgmatic/hooks/data_source/mysql.py | 18 +++++++++++++++--- borgmatic/hooks/data_source/postgresql.py | 18 +++++++++++++++--- borgmatic/hooks/data_source/sqlite.py | 18 +++++++++++++++--- borgmatic/hooks/data_source/zfs.py | 4 ++++ pyproject.toml | 2 +- tests/unit/actions/test_restore.py | 16 ++++++++++++++++ tests/unit/hooks/data_source/test_dump.py | 2 +- 15 files changed, 118 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 185b4483..4b26ba95 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +2.0.14.dev0 + * #1208: Fix for the database "restore" action restoring more databases than the "--database" flag + specifies. + 2.0.13 * #1054: Allow the Btrfs hook to create and delete snapshots even when running as a non-root user. See the documentation for more information: diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 40ba95ca..8d0e3c7c 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -200,6 +200,10 @@ def restore_single_dump( borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE, borgmatic_runtime_directory, data_source['name'], + data_source.get('hostname'), + data_source.get('port'), + data_source.get('container'), + data_source.get('label'), )[hook_name.split('_databases', 1)[0]] destination_path = ( diff --git a/borgmatic/hooks/data_source/bootstrap.py b/borgmatic/hooks/data_source/bootstrap.py index c1add4fe..6765d33c 100644 --- a/borgmatic/hooks/data_source/bootstrap.py +++ b/borgmatic/hooks/data_source/bootstrap.py @@ -116,6 +116,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Restores are implemented via the separate, purpose-specific "bootstrap" action rather than the diff --git a/borgmatic/hooks/data_source/btrfs.py b/borgmatic/hooks/data_source/btrfs.py index cab8beb9..9f938c75 100644 --- a/borgmatic/hooks/data_source/btrfs.py +++ b/borgmatic/hooks/data_source/btrfs.py @@ -412,6 +412,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Restores aren't implemented, because stored files can be extracted directly with "extract". diff --git a/borgmatic/hooks/data_source/dump.py b/borgmatic/hooks/data_source/dump.py index a585548b..8eaae4f2 100644 --- a/borgmatic/hooks/data_source/dump.py +++ b/borgmatic/hooks/data_source/dump.py @@ -124,6 +124,6 @@ def convert_glob_patterns_to_borg_pattern(patterns): # longer than the pattern. E.g., a pattern of "borgmatic/*/foo_databases/test" should also match # paths like "borgmatic/*/foo_databases/test/toc.dat" return 're:' + '|'.join( - fnmatch.translate(pattern.lstrip('/')).replace('\\z', '').replace('\\Z', '') + fnmatch.translate(pattern.lstrip('/')).replace('\\z', '').replace('\\Z', '') + '$' for pattern in patterns ) diff --git a/borgmatic/hooks/data_source/lvm.py b/borgmatic/hooks/data_source/lvm.py index 3cdc2cdc..a51834af 100644 --- a/borgmatic/hooks/data_source/lvm.py +++ b/borgmatic/hooks/data_source/lvm.py @@ -452,6 +452,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Restores aren't implemented, because stored files can be extracted directly with "extract". diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index 625acb6d..190a0d7c 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -410,6 +410,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, @@ -419,16 +423,24 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename(make_dump_path('borgmatic'), name, label='*'), + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_runtime_directory), name, - label='*', + hostname, + port, + container, + label, ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_source_directory), name, - label='*', + hostname, + port, + container, + label, ), ) diff --git a/borgmatic/hooks/data_source/mongodb.py b/borgmatic/hooks/data_source/mongodb.py index c4dceed5..63311ac4 100644 --- a/borgmatic/hooks/data_source/mongodb.py +++ b/borgmatic/hooks/data_source/mongodb.py @@ -212,6 +212,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, @@ -221,16 +225,24 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename(make_dump_path('borgmatic'), name, label='*'), + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_runtime_directory), name, - label='*', + hostname, + port, + container, + label, ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_source_directory), name, - label='*', + hostname, + port, + container, + label, ), ) diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index a7932495..ddabd41b 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -341,6 +341,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, @@ -350,16 +354,24 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename(make_dump_path('borgmatic'), name, label='*'), + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, name, hostname, port, container, label + ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_runtime_directory), name, - label='*', + hostname, + port, + container, + label, ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_source_directory), name, - label='*', + hostname, + port, + container, + label, ), ) diff --git a/borgmatic/hooks/data_source/postgresql.py b/borgmatic/hooks/data_source/postgresql.py index 6c746417..af2cc987 100644 --- a/borgmatic/hooks/data_source/postgresql.py +++ b/borgmatic/hooks/data_source/postgresql.py @@ -302,6 +302,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, @@ -311,16 +315,24 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename(make_dump_path('borgmatic'), name, label='*'), + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_runtime_directory), name, - label='*', + hostname, + port, + container, + label, ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_source_directory), name, - label='*', + hostname, + port, + container, + label, ), ) diff --git a/borgmatic/hooks/data_source/sqlite.py b/borgmatic/hooks/data_source/sqlite.py index a31a8abf..43c9bd07 100644 --- a/borgmatic/hooks/data_source/sqlite.py +++ b/borgmatic/hooks/data_source/sqlite.py @@ -144,6 +144,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Given a sequence of configurations dicts, a configuration dict, the borgmatic runtime directory, @@ -153,16 +157,24 @@ def make_data_source_dump_patterns( borgmatic_source_directory = borgmatic.config.paths.get_borgmatic_source_directory(config) return ( - dump.make_data_source_dump_filename(make_dump_path('borgmatic'), name, label='*'), + dump.make_data_source_dump_filename( + make_dump_path('borgmatic'), name, hostname, port, container, label + ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_runtime_directory), name, - label='*', + hostname, + port, + container, + label, ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_source_directory), name, - label='*', + hostname, + port, + container, + label, ), ) diff --git a/borgmatic/hooks/data_source/zfs.py b/borgmatic/hooks/data_source/zfs.py index 77b3ac9d..7b285849 100644 --- a/borgmatic/hooks/data_source/zfs.py +++ b/borgmatic/hooks/data_source/zfs.py @@ -453,6 +453,10 @@ def make_data_source_dump_patterns( config, borgmatic_runtime_directory, name=None, + hostname=None, + port=None, + container=None, + label=None, ): # pragma: no cover ''' Restores aren't implemented, because stored files can be extracted directly with "extract". diff --git a/pyproject.toml b/pyproject.toml index 8a7eb600..2a0a0ff9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "2.0.13" +version = "2.0.14.dev0" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ] diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index d167b254..00fc474d 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -315,6 +315,10 @@ def test_restore_single_dump_extracts_and_restores_single_file_dump(): object, object, object, + object, + object, + object, + object, ).and_return({'postgresql': flexmock()}) flexmock(module.tempfile).should_receive('mkdtemp').never() flexmock(module.borgmatic.hooks.data_source.dump).should_receive( @@ -359,6 +363,10 @@ def test_restore_single_dump_extracts_and_restores_directory_dump(): object, object, object, + object, + object, + object, + object, ).and_return({'postgresql': flexmock()}) flexmock(module.tempfile).should_receive('mkdtemp').once().and_return( '/run/user/0/borgmatic/tmp1234', @@ -405,6 +413,10 @@ def test_restore_single_dump_with_directory_dump_error_cleans_up_temporary_direc object, object, object, + object, + object, + object, + object, ).and_return({'postgresql': flexmock()}) flexmock(module.tempfile).should_receive('mkdtemp').once().and_return( '/run/user/0/borgmatic/tmp1234', @@ -452,6 +464,10 @@ def test_restore_single_dump_with_directory_dump_and_dry_run_skips_directory_mov object, object, object, + object, + object, + object, + object, ).and_return({'postgresql': flexmock()}) flexmock(module.tempfile).should_receive('mkdtemp').once().and_return('/run/borgmatic/tmp1234') flexmock(module.borgmatic.hooks.data_source.dump).should_receive( diff --git a/tests/unit/hooks/data_source/test_dump.py b/tests/unit/hooks/data_source/test_dump.py index 2ea45b63..0fb30506 100644 --- a/tests/unit/hooks/data_source/test_dump.py +++ b/tests/unit/hooks/data_source/test_dump.py @@ -158,5 +158,5 @@ def test_remove_data_source_dumps_without_dump_path_present_skips_removal(): def test_convert_glob_patterns_to_borg_pattern_makes_multipart_regular_expression(): assert ( module.convert_glob_patterns_to_borg_pattern(('/etc/foo/bar', '/bar/*/baz')) - == 're:(?s:etc/foo/bar)|(?s:bar/.*/baz)' + == 're:(?s:etc/foo/bar)$|(?s:bar/.*/baz)$' ) From ea25d969cb733929616aedb4364f5a39a40c1086 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 22 Dec 2025 16:02:53 -0800 Subject: [PATCH 2/7] Fix typo in MySQL hook (#1208). --- borgmatic/hooks/data_source/mysql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index ddabd41b..ab412312 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -355,7 +355,7 @@ def make_data_source_dump_patterns( return ( dump.make_data_source_dump_filename( - make_dump_path('borgmatic'), name, name, hostname, port, container, label + make_dump_path('borgmatic'), name, hostname, port, container, label ), dump.make_data_source_dump_filename( make_dump_path(borgmatic_runtime_directory), From a3750d1e060ac1886c5fab5875b93f7c76e3b3e0 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 23 Dec 2025 20:28:15 -0800 Subject: [PATCH 3/7] Fix an error when restoring multiple directory-format database dumps at once (#1212). --- NEWS | 3 +++ borgmatic/actions/restore.py | 18 +++++++++--------- pyproject.toml | 2 +- tests/unit/actions/test_restore.py | 1 + 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 185b4483..c9562283 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +2.0.14.dev0 + * #1212: Fix an error when restoring multiple directory-format database dumps at once. + 2.0.13 * #1054: Allow the Btrfs hook to create and delete snapshots even when running as a non-root user. See the documentation for more information: diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 40ba95ca..0503f77f 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -129,14 +129,14 @@ def get_configured_data_source(config, restore_dump): def strip_path_prefix_from_extracted_dump_destination( - destination_path, + extract_path, borgmatic_runtime_directory, ): ''' Directory-format dump files get extracted into a temporary directory containing a path prefix - that depends how the files were stored in the archive. So, given the destination path where the - dump was extracted and the borgmatic runtime directory, move the dump files such that the - restore doesn't have to deal with that varying path prefix. + that depends how the files were stored in the archive. So, given the path where the dump was + extracted and the borgmatic runtime directory, move the dump files such that the restore doesn't + have to deal with that varying path prefix. For instance, if the dump was extracted to: @@ -150,16 +150,16 @@ def strip_path_prefix_from_extracted_dump_destination( /run/user/0/borgmatic/postgresql_databases/test/... ''' - for subdirectory_path, _, _ in os.walk(destination_path): + for subdirectory_path, _, _ in os.walk(extract_path): databases_directory = os.path.basename(subdirectory_path) if not databases_directory.endswith('_databases'): continue - shutil.move( - subdirectory_path, - os.path.join(borgmatic_runtime_directory, databases_directory), - ) + destination_path = os.path.join(borgmatic_runtime_directory, databases_directory) + shutil.rmtree(destination_path, ignore_errors=True) + shutil.move(subdirectory_path, destination_path) + break diff --git a/pyproject.toml b/pyproject.toml index 8a7eb600..2a0a0ff9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "2.0.13" +version = "2.0.14.dev0" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ] diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index d167b254..fa64e9c4 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -295,6 +295,7 @@ def test_strip_path_prefix_from_extracted_dump_destination_renames_first_matchin ], ) + flexmock(module.shutil).should_receive('rmtree') flexmock(module.shutil).should_receive('move').with_args( '/foo/bar/postgresql_databases', '/run/user/0/borgmatic/postgresql_databases', From 00e22846623268ac8f6277d9e06e7ef430e5e81f Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 24 Dec 2025 12:56:52 -0800 Subject: [PATCH 4/7] When creating patterns for a "borg extract" during a restore, match directory format dumps too. Also make dump restore ordering to be deterministic (#1208). --- NEWS | 4 +- borgmatic/actions/restore.py | 66 ++++++++++++++--------------- borgmatic/hooks/data_source/dump.py | 28 +++++++++--- 3 files changed, 57 insertions(+), 41 deletions(-) diff --git a/NEWS b/NEWS index 994c97b4..7262e1bd 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,6 @@ 2.0.14.dev0 - * #1208: Fix for the database "restore" action restoring more databases than the "--database" flag - specifies. + * #1208: Fix for the "restore" action incorrectly extracting more database dumps than the + "--database" flag specifies. * #1212: Fix an error when restoring multiple directory-format database dumps at once. 2.0.13 diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 92d4612a..086a7531 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -273,7 +273,7 @@ def collect_dumps_from_archive( borgmatic runtime directory, query the archive for the names of data sources dumps it contains and return them as a set of Dump instances. ''' - dumps_from_archive = set() + dumps_from_archive = {} # Use a dict as an ordered set. # There is (at most) one dump metadata file per data source hook. Load each. for dumps_metadata_path in borgmatic.borg.list.capture_archive_listing( @@ -302,33 +302,30 @@ def collect_dumps_from_archive( if not dumps_metadata_path: continue - dumps_from_archive.update( - set( - borgmatic.hooks.data_source.dump.parse_data_source_dumps_metadata( - borgmatic.borg.extract.extract_archive( - global_arguments.dry_run, - repository, - archive, - [dumps_metadata_path], - config, - local_borg_version, - global_arguments, - local_path=local_path, - remote_path=remote_path, - extract_to_stdout=True, - ) - .stdout.read() - .decode(), - dumps_metadata_path, - ) + for dump in borgmatic.hooks.data_source.dump.parse_data_source_dumps_metadata( + borgmatic.borg.extract.extract_archive( + global_arguments.dry_run, + repository, + archive, + [dumps_metadata_path], + config, + local_borg_version, + global_arguments, + local_path=local_path, + remote_path=remote_path, + extract_to_stdout=True, ) - ) + .stdout.read() + .decode(), + dumps_metadata_path, + ): + dumps_from_archive[dump] = None # If we've successfully loaded any dumps metadata, we're done. if dumps_from_archive: logger.debug('Collecting database dumps from archive data source dumps metadata files') - return dumps_from_archive + return tuple(dumps_from_archive.keys()) # No dumps metadata files were found, so for backwards compatibility, fall back to parsing the # paths of dumps found in the archive to get their respective dump metadata. @@ -392,11 +389,11 @@ def collect_dumps_from_archive( except (ValueError, TypeError): port = None - dumps_from_archive.add( + dumps_from_archive[ Dump( hook_name, data_source_name, None if hostname == 'localhost' else hostname, port ) - ) + ] = None # We've successfully parsed the dump path, so need to probe any further. break @@ -405,13 +402,13 @@ def collect_dumps_from_archive( f'Ignoring invalid data source dump path "{dump_path}" in archive {archive}', ) - return dumps_from_archive + return tuple(dumps_from_archive.keys()) def get_dumps_to_restore(restore_arguments, dumps_from_archive): ''' Given restore arguments as an argparse.Namespace instance indicating which dumps to restore and - a set of Dump instances representing the dumps found in an archive, return a set of specific + a tuple of Dump instances representing the dumps found in an archive, return a tuple of specific Dump instances from the archive to restore. As part of this, replace any Dump having a data source name of "all" with multiple named Dump instances as appropriate. @@ -456,11 +453,12 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): } ) missing_dumps = set() - dumps_to_restore = set() + dumps_to_restore = {} # Use a dict as an ordered set. # If there's a requested "all" dump, add every dump from the archive to the dumps to restore. if any(dump for dump in requested_dumps if dump.data_source_name == 'all'): - dumps_to_restore.update(dumps_from_archive) + for dump in dumps_from_archive: + dumps_to_restore[dump] = None # If any archive dump matches a requested dump, add the archive dump to the dumps to restore. for requested_dump in requested_dumps: @@ -476,7 +474,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): if len(matching_dumps) == 0: missing_dumps.add(requested_dump) elif len(matching_dumps) == 1: - dumps_to_restore.add(matching_dumps[0]) + dumps_to_restore[matching_dumps[0]] = None else: raise ValueError( f'Cannot restore data source {render_dump_metadata(requested_dump)} because there are multiple matching dumps in the archive. Try adding flags to disambiguate.', @@ -491,20 +489,20 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): f"Cannot restore data source dump{'s' if len(missing_dumps) > 1 else ''} {rendered_dumps} missing from archive", ) - return dumps_to_restore + return tuple(dumps_to_restore.keys()) def ensure_requested_dumps_restored(dumps_to_restore, dumps_actually_restored): ''' - Given a set of requested dumps to restore and a set of dumps actually restored, raise ValueError - if any requested dumps to restore weren't restored, indicating that they were missing from the - configuration. + Given a tuple of requested dumps to restore and a set of dumps actually restored, raise + ValueError if any requested dumps to restore weren't restored, indicating that they were missing + from the configuration. ''' if not dumps_actually_restored: raise ValueError('No data source dumps were found to restore') missing_dumps = sorted( - dumps_to_restore - dumps_actually_restored, + set(dumps_to_restore) - dumps_actually_restored, key=lambda dump: dump.data_source_name, ) diff --git a/borgmatic/hooks/data_source/dump.py b/borgmatic/hooks/data_source/dump.py index 8eaae4f2..e26ca5a7 100644 --- a/borgmatic/hooks/data_source/dump.py +++ b/borgmatic/hooks/data_source/dump.py @@ -2,6 +2,7 @@ import fnmatch import json import logging import os +import re import shutil import borgmatic.actions.restore @@ -114,16 +115,33 @@ def remove_data_source_dumps(dump_path, data_source_type_name, dry_run): shutil.rmtree(dump_path) +END_OF_STRING_PATTTERN = re.compile(r'\\z', flags=re.IGNORECASE) + + def convert_glob_patterns_to_borg_pattern(patterns): ''' Convert a sequence of shell glob patterns like "/etc/*", "/tmp/*" to the corresponding Borg regular expression archive pattern as a single string like "re:etc/.*|tmp/.*". ''' - # Remove the "\z" or "\Z" generated by fnmatch.translate() because we don't want the pattern to - # match only at the end of a path, as directory format dumps require extracting files with paths - # longer than the pattern. E.g., a pattern of "borgmatic/*/foo_databases/test" should also match - # paths like "borgmatic/*/foo_databases/test/toc.dat" + # This deserves some explanation. Here's what this code is doing for each shell glob pattern: + # + # * Strip off the leading slash, since Borg doesn't store leading slashes in archives. + # * Use fnmatch.translate() to convert the pattern to a Borg pattern. + # * Remove the "\z" or \"Z" end-of-string special character generated by fnmatch.translate(), + # because Borg doesn't like it. Replace it with a "$" end-of-string special character + # instead. And yes, this is using a regular expression to modify a regular expression. + # * Do the above for each of: + # * The plain pattern. This supports the use case of a standard database where its name is a + # filename in the path. Example: borgmatic/foo_databases/hostname/test + # * The pattern with "/*" tacked onto the end of it. This supports the use case of a + # directory-format database where its name is a directory name in the path. Example: + # borgmatic/foo_databases/hostname/test/toc.dat + # + # Join the resulting transformed patterns together with "|" and return them as a string. return 're:' + '|'.join( - fnmatch.translate(pattern.lstrip('/')).replace('\\z', '').replace('\\Z', '') + '$' + re.sub(END_OF_STRING_PATTTERN, '$', fnmatch.translate(stripped)) + + '|' + + re.sub(END_OF_STRING_PATTTERN, '$', fnmatch.translate(stripped + '/*')) for pattern in patterns + for stripped in (pattern.lstrip('/'),) ) From 047a374514603c4f0c6edf4c737387d11f4cf5ab Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 25 Dec 2025 13:56:01 -0800 Subject: [PATCH 5/7] Fix existing automated tests (#1208). --- borgmatic/actions/restore.py | 13 ++-- tests/unit/actions/test_restore.py | 84 ++++++++++------------- tests/unit/hooks/data_source/test_dump.py | 4 +- 3 files changed, 45 insertions(+), 56 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 086a7531..856ae766 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -271,7 +271,7 @@ def collect_dumps_from_archive( Given a local or remote repository path, a resolved archive name, a configuration dict, the local Borg version, global arguments an argparse.Namespace, local and remote Borg paths, and the borgmatic runtime directory, query the archive for the names of data sources dumps it contains - and return them as a set of Dump instances. + and return them as a tuple of Dump instances. ''' dumps_from_archive = {} # Use a dict as an ordered set. @@ -415,7 +415,8 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): Raise ValueError if any of the requested data source names cannot be found in the archive or if there are multiple archive dump matches for a given requested dump. ''' - requested_dumps = ( + requested_dumps = tuple( + # Use a dict comprehension as an ordered set. { Dump( hook_name=( @@ -432,16 +433,16 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): port=restore_arguments.original_port, label=restore_arguments.original_label or UNSPECIFIED, container=restore_arguments.original_container or UNSPECIFIED, - ) + ): None for name in restore_arguments.data_sources or (UNSPECIFIED,) - } + }.keys() if restore_arguments.hook or restore_arguments.data_sources or restore_arguments.original_hostname or restore_arguments.original_port or restore_arguments.original_label or restore_arguments.original_container - else { + else ( Dump( hook_name=UNSPECIFIED, data_source_name='all', @@ -450,7 +451,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): label=UNSPECIFIED, container=UNSPECIFIED, ), - } + ) ) missing_dumps = set() dumps_to_restore = {} # Use a dict as an ordered set. diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index d15e9b87..93f91797 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -542,7 +542,7 @@ def test_collect_dumps_from_archive_with_dumps_metadata_parses_it(): borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_dumps == set(dumps_metadata) + assert archive_dumps == tuple(dumps_metadata) def test_collect_dumps_from_archive_with_empty_dumps_metadata_path_falls_back_to_parsing_archive_paths(): @@ -580,11 +580,11 @@ def test_collect_dumps_from_archive_with_empty_dumps_metadata_path_falls_back_to borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_dumps == { + assert archive_dumps == ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar', 'host', 1234), module.Dump('mysql_databases', 'quux'), - } + ) def test_collect_dumps_from_archive_without_dumps_metadata_falls_back_to_parsing_archive_paths(): @@ -622,11 +622,11 @@ def test_collect_dumps_from_archive_without_dumps_metadata_falls_back_to_parsing borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_dumps == { + assert archive_dumps == ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar', 'host', 1234), module.Dump('mysql_databases', 'quux'), - } + ) def test_collect_dumps_from_archive_parses_archive_paths_with_different_base_directories(): @@ -665,12 +665,12 @@ def test_collect_dumps_from_archive_parses_archive_paths_with_different_base_dir borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_dumps == { + assert archive_dumps == ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), module.Dump('postgresql_databases', 'baz'), module.Dump('mysql_databases', 'quux'), - } + ) def test_collect_dumps_from_archive_parses_directory_format_archive_paths(): @@ -707,9 +707,7 @@ def test_collect_dumps_from_archive_parses_directory_format_archive_paths(): borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_dumps == { - module.Dump('postgresql_databases', 'foo'), - } + assert archive_dumps == (module.Dump('postgresql_databases', 'foo'),) def test_collect_dumps_from_archive_skips_bad_archive_paths_or_bad_path_components(): @@ -749,18 +747,18 @@ def test_collect_dumps_from_archive_skips_bad_archive_paths_or_bad_path_componen borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_dumps == { + assert archive_dumps == ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), - } + ) def test_get_dumps_to_restore_gets_requested_dumps_found_in_archive(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), module.Dump('postgresql_databases', 'baz'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -793,16 +791,14 @@ def test_get_dumps_to_restore_gets_requested_dumps_found_in_archive(): original_container=None, ), dumps_from_archive=dumps_from_archive, - ) == { + ) == ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), - } + ) def test_get_dumps_to_restore_raises_for_requested_dumps_missing_from_archive(): - dumps_from_archive = { - module.Dump('postgresql_databases', 'foo'), - } + dumps_from_archive = (module.Dump('postgresql_databases', 'foo'),) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -832,10 +828,10 @@ def test_get_dumps_to_restore_raises_for_requested_dumps_missing_from_archive(): def test_get_dumps_to_restore_without_requested_dumps_finds_all_archive_dumps(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) assert ( @@ -855,10 +851,10 @@ def test_get_dumps_to_restore_without_requested_dumps_finds_all_archive_dumps(): def test_get_dumps_to_restore_with_all_in_requested_dumps_finds_all_archive_dumps(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -898,10 +894,10 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_finds_all_archive_dump def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_requested_dumps_omits_duplicates(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -974,10 +970,10 @@ def test_get_dumps_to_restore_raises_for_multiple_matching_dumps_in_archive(): original_label=None, original_container=None, ), - dumps_from_archive={ + dumps_from_archive=( module.Dump('postgresql_databases', 'foo'), module.Dump('mariadb_databases', 'foo'), - }, + ), ) assert 'Try adding flags to disambiguate.' in str(exc_info.value) @@ -1006,17 +1002,17 @@ def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_du original_label=None, original_container=None, ), - dumps_from_archive={module.Dump('postgresql_databases', 'foo')}, + dumps_from_archive=(module.Dump('postgresql_databases', 'foo'),), ) assert 'dump test missing from archive' in str(exc_info.value) def test_get_dumps_to_restore_with_requested_hook_name_filters_dumps_found_in_archive(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('mariadb_databases', 'foo'), module.Dump('postgresql_databases', 'foo'), module.Dump('sqlite_databases', 'bar'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -1039,17 +1035,15 @@ def test_get_dumps_to_restore_with_requested_hook_name_filters_dumps_found_in_ar original_container=None, ), dumps_from_archive=dumps_from_archive, - ) == { - module.Dump('postgresql_databases', 'foo'), - } + ) == (module.Dump('postgresql_databases', 'foo'),) def test_get_dumps_to_restore_with_requested_shortened_hook_name_filters_dumps_found_in_archive(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('mariadb_databases', 'foo'), module.Dump('postgresql_databases', 'foo'), module.Dump('sqlite_databases', 'bar'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -1072,17 +1066,15 @@ def test_get_dumps_to_restore_with_requested_shortened_hook_name_filters_dumps_f original_container=None, ), dumps_from_archive=dumps_from_archive, - ) == { - module.Dump('postgresql_databases', 'foo'), - } + ) == (module.Dump('postgresql_databases', 'foo'),) def test_get_dumps_to_restore_with_requested_hostname_filters_dumps_found_in_archive(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'foo', 'host'), module.Dump('postgresql_databases', 'bar'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -1105,17 +1097,15 @@ def test_get_dumps_to_restore_with_requested_hostname_filters_dumps_found_in_arc original_container=None, ), dumps_from_archive=dumps_from_archive, - ) == { - module.Dump('postgresql_databases', 'foo', 'host'), - } + ) == (module.Dump('postgresql_databases', 'foo', 'host'),) def test_get_dumps_to_restore_with_requested_port_filters_dumps_found_in_archive(): - dumps_from_archive = { + dumps_from_archive = ( module.Dump('postgresql_databases', 'foo', 'host'), module.Dump('postgresql_databases', 'foo', 'host', 1234), module.Dump('postgresql_databases', 'bar'), - } + ) flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump( @@ -1139,9 +1129,7 @@ def test_get_dumps_to_restore_with_requested_port_filters_dumps_found_in_archive original_container=None, ), dumps_from_archive=dumps_from_archive, - ) == { - module.Dump('postgresql_databases', 'foo', 'host', 1234), - } + ) == (module.Dump('postgresql_databases', 'foo', 'host', 1234),) def test_ensure_requested_dumps_restored_with_all_dumps_restored_does_not_raise(): diff --git a/tests/unit/hooks/data_source/test_dump.py b/tests/unit/hooks/data_source/test_dump.py index 0fb30506..a00606e3 100644 --- a/tests/unit/hooks/data_source/test_dump.py +++ b/tests/unit/hooks/data_source/test_dump.py @@ -157,6 +157,6 @@ def test_remove_data_source_dumps_without_dump_path_present_skips_removal(): def test_convert_glob_patterns_to_borg_pattern_makes_multipart_regular_expression(): assert ( - module.convert_glob_patterns_to_borg_pattern(('/etc/foo/bar', '/bar/*/baz')) - == 're:(?s:etc/foo/bar)$|(?s:bar/.*/baz)$' + module.convert_glob_patterns_to_borg_pattern(('/etc/foo/bar', '/bar/baz/quux')) + == 're:(?s:etc/foo/bar)$|(?s:etc/foo/bar/.*)$|(?s:bar/baz/quux)$|(?s:bar/baz/quux/.*)$' ) From 802ca5aef305ae79afc0aa94ae32424101d7350a Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 26 Dec 2025 09:46:39 -0800 Subject: [PATCH 6/7] Fix an error when running the "spot" check or "extract" action with the "progress" option or "--progress" flag (#1210). --- NEWS | 2 + borgmatic/actions/check.py | 5 +-- borgmatic/actions/config/bootstrap.py | 2 - borgmatic/actions/restore.py | 2 +- borgmatic/borg/extract.py | 27 ++++++------- tests/unit/actions/test_check.py | 4 +- tests/unit/actions/test_restore.py | 4 +- tests/unit/borg/test_extract.py | 56 +++++++++++++++++++-------- 8 files changed, 59 insertions(+), 43 deletions(-) diff --git a/NEWS b/NEWS index c9562283..15caa986 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ 2.0.14.dev0 + * #1210: Fix an error when running the "spot" check or "extract" action with the "progress" option + or "--progress" flag. * #1212: Fix an error when restoring multiple directory-format database dumps at once. 2.0.13 diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 7186e3da..631feaca 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -380,10 +380,7 @@ def collect_spot_check_source_paths( dry_run=True, repository_path=repository['path'], # Omit "progress" because it interferes with "list_details". - config=dict( - {option: value for option, value in config.items() if option != 'progress'}, - list_details=True, - ), + config=dict(config, progress=False, list_details=True), patterns=borgmatic.actions.pattern.process_patterns( borgmatic.actions.pattern.collect_patterns(config) + tuple( diff --git a/borgmatic/actions/config/bootstrap.py b/borgmatic/actions/config/bootstrap.py index 20558a0c..5e035a11 100644 --- a/borgmatic/actions/config/bootstrap.py +++ b/borgmatic/actions/config/bootstrap.py @@ -127,8 +127,6 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version): bootstrap_arguments.repository, archive_name, [config_path.lstrip(os.path.sep) for config_path in manifest_config_paths], - # Only add progress here and not the extract_archive() call above, because progress - # conflicts with extract_to_stdout. dict(config, progress=bootstrap_arguments.progress or False), local_borg_version, global_arguments, diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 0503f77f..f34c5446 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -496,7 +496,7 @@ def ensure_requested_dumps_restored(dumps_to_restore, dumps_actually_restored): if any requested dumps to restore weren't restored, indicating that they were missing from the configuration. ''' - if not dumps_actually_restored: + if not dumps_to_restore: raise ValueError('No data source dumps were found to restore') missing_dumps = sorted( diff --git a/borgmatic/borg/extract.py b/borgmatic/borg/extract.py index 3004a47e..c4113762 100644 --- a/borgmatic/borg/extract.py +++ b/borgmatic/borg/extract.py @@ -97,9 +97,6 @@ def extract_archive( lock_wait = config.get('lock_wait', None) extra_borg_options = config.get('extra_borg_options', {}).get('extract', '') - if config.get('progress') and extract_to_stdout: - raise ValueError('progress and extract to stdout cannot both be set') - if feature.available(feature.Feature.NUMERIC_IDS, local_borg_version): numeric_ids_flags = ('--numeric-ids',) if config.get('numeric_ids') else () else: @@ -133,7 +130,7 @@ def extract_archive( + (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--dry-run',) if dry_run else ()) + (('--strip-components', str(strip_components)) if strip_components else ()) - + (('--progress',) if config.get('progress') else ()) + + (('--progress',) if config.get('progress') and not extract_to_stdout else ()) + (('--stdout',) if extract_to_stdout else ()) + (tuple(shlex.split(extra_borg_options)) if extra_borg_options else ()) + flags.make_repository_archive_flags( @@ -152,6 +149,17 @@ def extract_archive( os.path.join(working_directory or '', destination_path) if destination_path else None ) + if extract_to_stdout: + return execute_command( + full_command, + output_file=subprocess.PIPE, + run_to_completion=False, + environment=environment.make_environment(config), + working_directory=full_destination_path, + borg_local_path=local_path, + borg_exit_codes=borg_exit_codes, + ) + # The progress output isn't compatible with captured and logged output, as progress messes with # the terminal directly. if config.get('progress'): @@ -165,17 +173,6 @@ def extract_archive( ) return None - if extract_to_stdout: - return execute_command( - full_command, - output_file=subprocess.PIPE, - run_to_completion=False, - environment=environment.make_environment(config), - working_directory=full_destination_path, - borg_local_path=local_path, - borg_exit_codes=borg_exit_codes, - ) - # Don't give Borg local path so as to error on warnings, as "borg extract" only gives a warning # if the restore paths don't exist in the archive. execute_command( diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index d262a5da..3420848b 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -651,7 +651,7 @@ def test_collect_spot_check_source_paths_omits_progress_from_create_dry_run_comm flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args( dry_run=True, repository_path='repo', - config={'working_directory': '/', 'list_details': True}, + config={'working_directory': '/', 'progress': False, 'list_details': True}, patterns=[Pattern('foo'), Pattern('bar')], local_borg_version=object, global_arguments=object, @@ -941,7 +941,7 @@ def test_collect_spot_check_source_paths_uses_working_directory(): flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args( dry_run=True, repository_path='repo', - config={'working_directory': '/working/dir', 'list_details': True}, + config={'working_directory': '/working/dir', 'progress': False, 'list_details': True}, patterns=[Pattern('foo'), Pattern('bar')], local_borg_version=object, global_arguments=object, diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index fa64e9c4..081725ad 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -1144,8 +1144,8 @@ def test_ensure_requested_dumps_restored_with_all_dumps_restored_does_not_raise( def test_ensure_requested_dumps_restored_with_no_dumps_raises(): with pytest.raises(ValueError): module.ensure_requested_dumps_restored( - dumps_to_restore={}, - dumps_actually_restored={}, + dumps_to_restore=set(), + dumps_actually_restored=set(), ) diff --git a/tests/unit/borg/test_extract.py b/tests/unit/borg/test_extract.py index e293e687..de0fa5c7 100644 --- a/tests/unit/borg/test_extract.py +++ b/tests/unit/borg/test_extract.py @@ -654,23 +654,7 @@ def test_extract_archive_calls_borg_with_progress_flag(): ) -def test_extract_archive_with_progress_and_extract_to_stdout_raises(): - flexmock(module).should_receive('execute_command').never() - - with pytest.raises(ValueError): - module.extract_archive( - dry_run=False, - repository='repo', - archive='archive', - paths=None, - config={'progress': True}, - local_borg_version='1.2.3', - global_arguments=flexmock(), - extract_to_stdout=True, - ) - - -def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process(): +def test_extract_archive_calls_borg_with_extract_to_stdout_returns_process(): flexmock(module.os.path).should_receive('abspath').and_return('repo') process = flexmock() flexmock(module.environment).should_receive('make_environment') @@ -708,6 +692,44 @@ def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process(): ) +def test_extract_archive_with_progress_and_extract_to_stdout_ignores_progress(): + flexmock(module.os.path).should_receive('abspath').and_return('repo') + process = flexmock() + flexmock(module.environment).should_receive('make_environment') + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'extract', '--stdout', 'repo::archive'), + output_file=module.subprocess.PIPE, + run_to_completion=False, + environment=None, + working_directory=None, + borg_local_path='borg', + borg_exit_codes=None, + ).and_return(process).once() + flexmock(module.feature).should_receive('available').and_return(True) + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) + flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( + ('repo::archive',), + ) + flexmock(module.borgmatic.config.validate).should_receive( + 'normalize_repository_path', + ).and_return('repo') + + assert ( + module.extract_archive( + dry_run=False, + repository='repo', + archive='archive', + paths=None, + config={'progress': True}, + local_borg_version='1.2.3', + global_arguments=flexmock(), + extract_to_stdout=True, + ) + == process + ) + + def test_extract_archive_skips_abspath_for_remote_repository(): flexmock(module.os.path).should_receive('abspath').never() flexmock(module.environment).should_receive('make_environment') From ec8168af46d50a607b007d1c15f2e31d84fd68db Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 26 Dec 2025 22:53:21 -0800 Subject: [PATCH 7/7] Fix an error about the runtime directory getting excluded by tweaking its logic and lowering the error to a warning (#1211). --- NEWS | 2 + borgmatic/borg/create.py | 32 +++++++--- tests/unit/borg/test_create.py | 107 ++++++++++++++++++++++----------- 3 files changed, 97 insertions(+), 44 deletions(-) diff --git a/NEWS b/NEWS index c5177846..4661d927 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ "--database" flag specifies. * #1210: Fix an error when running the "spot" check or "extract" action with the "progress" option or "--progress" flag. + * #1211: Fix an error about the runtime directory getting excluded by tweaking its logic and + lowering the error to a warning. * #1212: Fix an error when restoring multiple directory-format database dumps at once. 2.0.13 diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 4220bb21..54a09a83 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -96,22 +96,36 @@ def validate_planned_backup_paths( if path_line and path_line.startswith(('- ', '+ ')) ) + include_pattern_paths = { + pattern.path + for pattern in patterns + if pattern.type == borgmatic.borg.pattern.Pattern_type.INCLUDE + } runtime_directory_root_patterns = tuple( pattern for pattern in patterns if any_parent_directories(pattern.path, (borgmatic_runtime_directory,)) if pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT + # Skip root patterns that have corresponding include patterns, because those will "punch + # through" any subsequent excludes. + if pattern.path not in include_pattern_paths ) - if not dry_run and os.path.exists(borgmatic_runtime_directory): - # If there are any root patterns in the runtime directory that are missing from the paths - # Borg is planning to backup, then they must've gotten excluded, e.g. by user-configured - # excludes. Error accordingly. - for pattern in runtime_directory_root_patterns: - if not any(any_parent_directories(path, (pattern.path,)) for path in paths): - 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 all root patterns in the runtime directory are missing from the paths Borg is planning to + # backup, then they must've gotten excluded, e.g. by user-configured excludes. Warn accordingly. + if ( + not dry_run + and os.path.exists(borgmatic_runtime_directory) + and runtime_directory_root_patterns + and not any( + any_parent_directories(path, (pattern.path,)) + for pattern in runtime_directory_root_patterns + for path in paths + ) + ): + logger.warning( + f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes (or the snapshotted source directories are empty). Please ensure the runtime directory is not excluded.' + ) # Return the subset of output paths *not* contained within the borgmatic runtime directory. The # intent is that any downstream checks using these paths should skip runtime paths that diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index b8f4c4b8..ba693c77 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -121,7 +121,7 @@ def test_validate_planned_backup_paths_skips_borgmatic_runtime_directory(): ) == ('/foo', '/baz') -def test_validate_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_warns(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -136,26 +136,26 @@ def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_ flexmock(module).should_receive('any_parent_directories').replace_with( lambda path, candidates: any(path.startswith(parent) for parent in candidates) ) + flexmock(module.logger).should_receive('warning').once() - 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'), + 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 ), - local_path=None, - working_directory=None, - borgmatic_runtime_directory='/run/borgmatic', - ) + module.borgmatic.borg.pattern.Pattern('/baz'), + ), + local_path=None, + working_directory=None, + borgmatic_runtime_directory='/run/borgmatic', + ) == ('/foo', '/bar', '/baz') -def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_partially_excluded_from_paths_output_errors(): +def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_partially_excluded_from_paths_output_does_not_warn(): flexmock(module.flags).should_receive('omit_flag').replace_with( lambda arguments, flag: arguments, ) @@ -172,26 +172,63 @@ def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_partiall flexmock(module).should_receive('any_parent_directories').replace_with( lambda path, candidates: any(path.startswith(parent) for parent in candidates) ) + flexmock(module.logger).should_receive('warning').never() - 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'), - module.borgmatic.borg.pattern.Pattern( - '/run/borgmatic/quux', module.borgmatic.borg.pattern.Pattern_type.ROOT - ), + 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 ), - local_path=None, - working_directory=None, - borgmatic_runtime_directory='/run/borgmatic', - ) + module.borgmatic.borg.pattern.Pattern('/baz'), + module.borgmatic.borg.pattern.Pattern( + '/run/borgmatic/quux', module.borgmatic.borg.pattern.Pattern_type.ROOT + ), + ), + local_path=None, + working_directory=None, + borgmatic_runtime_directory='/run/borgmatic', + ) == ('/foo', '/baz') + + +def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_with_corresponding_include_and_missing_from_paths_output_does_not_warn(): + 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.os.path).should_receive('exists').and_return(True) + flexmock(module).should_receive('any_parent_directories').replace_with( + lambda path, candidates: any(path.startswith(parent) for parent in candidates) + ) + flexmock(module.logger).should_receive('warning').never() + + 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( + '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.INCLUDE + ), + module.borgmatic.borg.pattern.Pattern('/baz'), + ), + local_path=None, + working_directory=None, + borgmatic_runtime_directory='/run/borgmatic', + ) == ('/foo', '/bar', '/baz') def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_patterns_does_not_raise():