From c46f2b8508e1e23ec8bb0f27aacbe6c999ff47ad Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 25 Aug 2022 11:55:34 -0700 Subject: [PATCH] Fix conflict between "patterns" and "source_directories" (#574), make "source_directories" optional (#542). --- NEWS | 9 +++++++++ borgmatic/borg/create.py | 17 ++++++++++------- borgmatic/config/schema.yaml | 8 ++++---- borgmatic/config/validate.py | 2 +- setup.py | 2 +- tests/unit/borg/test_create.py | 19 ++++++++++++++----- 6 files changed, 39 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 96902f51f..2771197c2 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,12 @@ +1.7.1.dev0 + * #542: Make the "source_directories" option optional. This is useful for "check"-only setups or + using "patterns" exclusively. + * #574: Fix for potential data loss (data not getting backed up) when the "patterns" option was + used with "source_directories" (or the "~/.borgmatic" path existed, which got injected into + "source_directories" implicitly). The fix is for borgmatic to convert "source_directories" into + patterns whenever "patterns" is used, working around a potential Borg bug: + https://github.com/borgbackup/borg/issues/6994 + 1.7.0 * #463: Add "before_actions" and "after_actions" command hooks that run before/after all the actions for each repository. These new hooks are a good place to run per-repository steps like diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 04d7ec1e0..f9cf49f6f 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -98,16 +98,19 @@ def deduplicate_directories(directory_devices): return tuple(sorted(deduplicated)) -def write_pattern_file(patterns=None): +def write_pattern_file(patterns=None, sources=None): ''' - Given a sequence of patterns, write them to a named temporary file and return it. Return None - if no patterns are provided. + Given a sequence of patterns and an optional sequence of source directories, write them to a + named temporary file (with the source directories as additional roots) and return the file. + Return None if no patterns are provided. ''' if not patterns: return None pattern_file = tempfile.NamedTemporaryFile('w') - pattern_file.write('\n'.join(patterns)) + pattern_file.write( + '\n'.join(tuple(patterns) + tuple(f'R {source}' for source in (sources or []))) + ) pattern_file.flush() return pattern_file @@ -216,7 +219,7 @@ def create_archive( sources = deduplicate_directories( map_directories_to_devices( expand_directories( - location_config['source_directories'] + location_config.get('source_directories', []) + borgmatic_source_directories(location_config.get('borgmatic_source_directory')) ) ) @@ -226,7 +229,7 @@ def create_archive( working_directory = os.path.expanduser(location_config.get('working_directory')) except TypeError: working_directory = None - pattern_file = write_pattern_file(location_config.get('patterns')) + pattern_file = write_pattern_file(location_config.get('patterns'), sources) exclude_file = write_pattern_file( expand_home_directories(location_config.get('exclude_patterns')) ) @@ -299,7 +302,7 @@ def create_archive( + (('--json',) if json else ()) + (tuple(extra_borg_options.split(' ')) if extra_borg_options else ()) + flags.make_repository_archive_flags(repository, archive_name_format, local_borg_version) - + sources + + (sources if not pattern_file else ()) ) if json: diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index 17290629e..e1fe9e3fa 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -11,7 +11,6 @@ properties: https://borgbackup.readthedocs.io/en/stable/usage/create.html for details. required: - - source_directories - repositories additionalProperties: false properties: @@ -20,8 +19,8 @@ properties: items: type: string description: | - List of source directories to backup (required). Globs and - tildes are expanded. Do not backslash spaces in path names. + List of source directories to backup. Globs and tildes are + expanded. Do not backslash spaces in path names. example: - /home - /etc @@ -123,7 +122,8 @@ properties: backups. Globs are expanded. (Tildes are not.) See the output of "borg help patterns" for more details. Quote any value if it contains leading punctuation, so it parses - correctly. + correctly. Note that only one of "patterns" and + "source_directories" may be used. example: - 'R /' - '- /home/*/.cache' diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index 76f2a0f88..3a2807f9c 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -72,7 +72,7 @@ def apply_logical_validation(config_filename, parsed_configuration): raise Validation_error( config_filename, ( - 'Unknown repository in the consistency section\'s check_repositories: {}'.format( + 'Unknown repository in the "consistency" section\'s "check_repositories": {}'.format( repository ), ), diff --git a/setup.py b/setup.py index 14a545f05..151ba613b 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.7.0' +VERSION = '1.7.1.dev0' setup( diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 35fcd4da8..b2e851170 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -109,11 +109,20 @@ def test_deduplicate_directories_removes_child_paths_on_the_same_filesystem( assert module.deduplicate_directories(directories) == expected_directories -def test_write_pattern_file_does_not_raise(): - temporary_file = flexmock(name='filename', write=lambda mode: None, flush=lambda: None) +def test_write_pattern_file_writes_pattern_lines(): + temporary_file = flexmock(name='filename', flush=lambda: None) + temporary_file.should_receive('write').with_args('R /foo\n+ /foo/bar') flexmock(module.tempfile).should_receive('NamedTemporaryFile').and_return(temporary_file) - module.write_pattern_file(['exclude']) + module.write_pattern_file(['R /foo', '+ /foo/bar']) + + +def test_write_pattern_file_with_sources_writes_sources_as_roots(): + temporary_file = flexmock(name='filename', flush=lambda: None) + temporary_file.should_receive('write').with_args('R /foo\n+ /foo/bar\nR /baz\nR /quux') + flexmock(module.tempfile).should_receive('NamedTemporaryFile').and_return(temporary_file) + + module.write_pattern_file(['R /foo', '+ /foo/bar'], sources=['/baz', '/quux']) def test_write_pattern_file_with_empty_exclude_patterns_does_not_raise(): @@ -357,7 +366,7 @@ def test_create_archive_calls_borg_with_environment(): ) -def test_create_archive_with_patterns_calls_borg_with_patterns(): +def test_create_archive_with_patterns_calls_borg_with_patterns_including_converted_source_directories(): pattern_flags = ('--patterns-from', 'patterns') flexmock(module).should_receive('borgmatic_source_directories').and_return([]) flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar')) @@ -377,7 +386,7 @@ def test_create_archive_with_patterns_calls_borg_with_patterns(): ) flexmock(module.environment).should_receive('make_environment') flexmock(module).should_receive('execute_command').with_args( - ('borg', 'create') + pattern_flags + REPO_ARCHIVE_WITH_PATHS, + ('borg', 'create') + pattern_flags + (f'repo::{DEFAULT_ARCHIVE_NAME}',), output_log_level=logging.INFO, output_file=None, borg_local_path='borg',