diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index d288664b..e0b68bc5 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -43,44 +43,46 @@ def collect_patterns(config): directories, etc., but we'd like to collapse them all down to one common format (patterns) for ease of manipulation within borgmatic. ''' - return ( - tuple( - borgmatic.borg.pattern.Pattern(source_directory) - for source_directory in config.get('source_directories', ()) - ) - + tuple( - parse_pattern(pattern_line) - for pattern_line in config.get('patterns', ()) - if not pattern_line.lstrip().startswith('#') - ) - + tuple( - borgmatic.borg.pattern.Pattern( - exclude_line, - borgmatic.borg.pattern.Pattern_type.EXCLUDE, - borgmatic.borg.pattern.Pattern_style.FNMATCH, + try: + return ( + tuple( + borgmatic.borg.pattern.Pattern(source_directory) + for source_directory in config.get('source_directories', ()) ) - for exclude_line in config.get('exclude_patterns', ()) - ) - + tuple( - itertools.chain.from_iterable( - parse_pattern(pattern_line) + + tuple( + parse_pattern(pattern_line.strip()) + for pattern_line in config.get('patterns', ()) + if not pattern_line.lstrip().startswith('#') + ) + + tuple( + borgmatic.borg.pattern.Pattern( + exclude_line.strip(), + borgmatic.borg.pattern.Pattern_type.EXCLUDE, + borgmatic.borg.pattern.Pattern_style.FNMATCH, + ) + for exclude_line in config.get('exclude_patterns', ()) + ) + + tuple( + parse_pattern(pattern_line.strip()) for filename in config.get('patterns_from', ()) for pattern_line in open(filename).readlines() if not pattern_line.lstrip().startswith('#') ) - ) - + tuple( - itertools.chain.from_iterable( + + tuple( borgmatic.borg.pattern.Pattern( - exclude_line, + exclude_line.strip(), borgmatic.borg.pattern.Pattern_type.EXCLUDE, - borgmatic.borg.pattern.Pattern_type.FNMATCH, + borgmatic.borg.pattern.Pattern_style.FNMATCH, ) for filename in config.get('excludes_from', ()) for exclude_line in open(filename).readlines() + if not exclude_line.lstrip().startswith('#') ) ) - ) + except (FileNotFoundError, OSError) as error: + logger.debug(error) + + raise ValueError(f'Cannot read patterns_from/excludes_from file: {error.filename}') def expand_directory(directory, working_directory): diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 3fb3d9d5..02b3c5a5 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -337,8 +337,9 @@ def make_base_create_command( log_prefix=repository_path, patterns_file=patterns_file, ) + if '--patterns-from' not in create_flags: - create_flags.append(('--patterns-from', patterns_file.name)) + create_flags += ('--patterns-from', patterns_file.name) return (create_flags, create_positional_arguments, patterns_file) diff --git a/tests/unit/actions/test_create.py b/tests/unit/actions/test_create.py index 522cace0..97b93204 100644 --- a/tests/unit/actions/test_create.py +++ b/tests/unit/actions/test_create.py @@ -1,8 +1,104 @@ +import io +import sys + import pytest from flexmock import flexmock from borgmatic.actions import create as module -from borgmatic.borg.pattern import Pattern, Pattern_type +from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type + + +@pytest.mark.parametrize( + 'pattern_line,expected_pattern', + ( + ('R /foo', Pattern('/foo')), + ('P sh', Pattern('sh', Pattern_type.PATTERN_STYLE)), + ('+ /foo*', Pattern('/foo*', Pattern_type.INCLUDE)), + ('+ sh:/foo*', Pattern('/foo*', Pattern_type.INCLUDE, Pattern_style.SHELL)), + ), +) +def test_parse_pattern_transforms_pattern_line_to_instance(pattern_line, expected_pattern): + module.parse_pattern(pattern_line) == expected_pattern + + +def test_parse_pattern_with_invalid_pattern_line_errors(): + with pytest.raises(ValueError): + module.parse_pattern('/foo') + + +def test_collect_patterns_converts_source_directories(): + assert module.collect_patterns({'source_directories': ['/foo', '/bar']}) == ( + Pattern('/foo'), + Pattern('/bar'), + ) + + +def test_collect_patterns_parses_config_patterns(): + flexmock(module).should_receive('parse_pattern').with_args('R /foo').and_return(Pattern('/foo')) + flexmock(module).should_receive('parse_pattern').with_args('# comment').never() + flexmock(module).should_receive('parse_pattern').with_args('R /bar').and_return(Pattern('/bar')) + + assert module.collect_patterns({'patterns': ['R /foo', '# comment', 'R /bar']}) == ( + Pattern('/foo'), + Pattern('/bar'), + ) + + +def test_collect_patterns_converts_exclude_patterns(): + assert module.collect_patterns({'exclude_patterns': ['/foo', '/bar']}) == ( + Pattern('/foo', Pattern_type.EXCLUDE, Pattern_style.FNMATCH), + Pattern('/bar', Pattern_type.EXCLUDE, Pattern_style.FNMATCH), + ) + + +def test_collect_patterns_reads_config_patterns_from_file(): + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args('file1.txt').and_return(io.StringIO('R /foo')) + builtins.should_receive('open').with_args('file2.txt').and_return( + io.StringIO('R /bar\n# comment\nR /baz') + ) + flexmock(module).should_receive('parse_pattern').with_args('R /foo').and_return(Pattern('/foo')) + flexmock(module).should_receive('parse_pattern').with_args('# comment').never() + flexmock(module).should_receive('parse_pattern').with_args('R /bar').and_return(Pattern('/bar')) + flexmock(module).should_receive('parse_pattern').with_args('R /baz').and_return(Pattern('/baz')) + + assert module.collect_patterns({'patterns_from': ['file1.txt', 'file2.txt']}) == ( + Pattern('/foo'), + Pattern('/bar'), + Pattern('/baz'), + ) + + +def test_collect_patterns_errors_on_missing_config_patterns_from_file(): + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args('file1.txt').and_raise(FileNotFoundError) + flexmock(module).should_receive('parse_pattern').never() + + with pytest.raises(ValueError): + module.collect_patterns({'patterns_from': ['file1.txt', 'file2.txt']}) + + +def test_collect_patterns_reads_config_excludes_from_file(): + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args('file1.txt').and_return(io.StringIO('/foo')) + builtins.should_receive('open').with_args('file2.txt').and_return( + io.StringIO('/bar\n# comment\n/baz') + ) + + assert module.collect_patterns({'excludes_from': ['file1.txt', 'file2.txt']}) == ( + Pattern('/foo', Pattern_type.EXCLUDE, Pattern_style.FNMATCH), + Pattern('/bar', Pattern_type.EXCLUDE, Pattern_style.FNMATCH), + Pattern('/baz', Pattern_type.EXCLUDE, Pattern_style.FNMATCH), + ) + + +def test_collect_patterns_errors_on_missing_config_excludes_from_file(): + builtins = flexmock(sys.modules['builtins']) + builtins.should_receive('open').with_args('file1.txt').and_raise(OSError) + flexmock(module).should_receive('parse_pattern').never() + + with pytest.raises(ValueError): + module.collect_patterns({'excludes_from': ['file1.txt', 'file2.txt']}) def test_expand_directory_with_basic_path_passes_it_through(): @@ -275,6 +371,7 @@ def test_run_create_executes_and_calls_hooks_for_configured_repository(): flexmock(module.borgmatic.hooks.dispatch).should_receive( 'call_hooks_even_if_unconfigured' ).and_return({}) + flexmock(module).should_receive('collect_patterns').and_return(()) flexmock(module).should_receive('process_patterns').and_return([]) flexmock(module.os.path).should_receive('join').and_return('/run/borgmatic/bootstrap') create_arguments = flexmock( @@ -317,6 +414,7 @@ def test_run_create_runs_with_selected_repository(): flexmock(module.borgmatic.hooks.dispatch).should_receive( 'call_hooks_even_if_unconfigured' ).and_return({}) + flexmock(module).should_receive('collect_patterns').and_return(()) flexmock(module).should_receive('process_patterns').and_return([]) flexmock(module.os.path).should_receive('join').and_return('/run/borgmatic/bootstrap') create_arguments = flexmock( @@ -396,6 +494,7 @@ def test_run_create_produces_json(): flexmock(module.borgmatic.hooks.dispatch).should_receive( 'call_hooks_even_if_unconfigured' ).and_return({}) + flexmock(module).should_receive('collect_patterns').and_return(()) flexmock(module).should_receive('process_patterns').and_return([]) flexmock(module.os.path).should_receive('join').and_return('/run/borgmatic/bootstrap') create_arguments = flexmock( diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 32ed1482..2df4ebed 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -4,7 +4,7 @@ import pytest from flexmock import flexmock from borgmatic.borg import create as module -from borgmatic.borg.pattern import Pattern, Pattern_type +from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type from ..test_verbosity import insert_logging_mock @@ -418,38 +418,6 @@ def test_make_base_create_command_with_store_config_false_omits_config_files(): assert not pattern_file -def test_make_base_create_command_includes_exclude_patterns_in_borg_command(): - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) - flexmock(module).should_receive('write_patterns_file').and_return(flexmock(name='patterns')) - flexmock(module).should_receive('make_list_filter_flags').and_return('FOO') - flexmock(module.flags).should_receive('get_default_archive_name_format').and_return( - '{hostname}' - ) - flexmock(module.feature).should_receive('available').and_return(True) - flexmock(module).should_receive('make_exclude_flags').and_return(()) - flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( - (f'repo::{DEFAULT_ARCHIVE_NAME}',) - ) - - (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( - dry_run=False, - repository_path='repo', - config={ - 'source_directories': ['foo', 'bar'], - 'repositories': ['repo'], - 'exclude_patterns': ['exclude'], - }, - patterns=[Pattern('foo'), Pattern('bar')], - local_borg_version='1.2.3', - global_arguments=flexmock(log_json=False), - borgmatic_runtime_directory='/run/borgmatic', - ) - - assert create_flags == ('borg', 'create', '--patterns-from', 'patterns') - assert create_positional_arguments == REPO_ARCHIVE - assert pattern_file - - @pytest.mark.parametrize( 'option_name,option_value,feature_available,option_flags', ( @@ -675,8 +643,12 @@ def test_make_base_create_command_includes_list_flags_in_borg_command(): def test_make_base_create_command_with_stream_processes_ignores_read_special_false_and_excludes_special_files(): + patterns = [Pattern('foo'), Pattern('bar')] + patterns_file = flexmock(name='patterns') flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) - flexmock(module).should_receive('write_patterns_file').and_return(None) + flexmock(module).should_receive('write_patterns_file').with_args( + patterns, '/run/borgmatic', object + ).and_return(patterns_file) flexmock(module).should_receive('make_list_filter_flags').and_return('FOO') flexmock(module.flags).should_receive('get_default_archive_name_format').and_return( '{hostname}' @@ -689,7 +661,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('write_patterns_file').and_return(flexmock(name='patterns')) + flexmock(module).should_receive('write_patterns_file').with_args( + ( + Pattern( + '/dev/null', + Pattern_type.EXCLUDE, + Pattern_style.FNMATCH, + ), + ), + '/run/borgmatic', + 'repo', + patterns_file=patterns_file, + ).and_return(patterns_file).once() flexmock(module).should_receive('make_exclude_flags').and_return(()) (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( @@ -700,7 +683,7 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal 'repositories': ['repo'], 'read_special': False, }, - patterns=[Pattern('foo'), Pattern('bar')], + patterns=patterns, local_borg_version='1.2.3', global_arguments=flexmock(log_json=False), borgmatic_runtime_directory='/run/borgmatic', @@ -712,7 +695,58 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal assert pattern_file -def test_make_base_create_command_with_stream_processes_and_read_special_true_skip_special_files_excludes(): +def test_make_base_create_command_without_patterns_and_with_stream_processes_ignores_read_special_false_and_excludes_special_files(): + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) + flexmock(module).should_receive('write_patterns_file').with_args( + [], '/run/borgmatic', object + ).and_return(None) + flexmock(module).should_receive('make_list_filter_flags').and_return('FOO') + flexmock(module.flags).should_receive('get_default_archive_name_format').and_return( + '{hostname}' + ) + flexmock(module.feature).should_receive('available').and_return(True) + flexmock(module).should_receive('make_exclude_flags').and_return(()) + flexmock(module.flags).should_receive('make_repository_archive_flags').and_return( + (f'repo::{DEFAULT_ARCHIVE_NAME}',) + ) + 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('write_patterns_file').with_args( + ( + Pattern( + '/dev/null', + Pattern_type.EXCLUDE, + Pattern_style.FNMATCH, + ), + ), + '/run/borgmatic', + 'repo', + patterns_file=None, + ).and_return(flexmock(name='patterns')).once() + flexmock(module).should_receive('make_exclude_flags').and_return(()) + + (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command( + dry_run=False, + repository_path='repo', + config={ + 'source_directories': [], + 'repositories': ['repo'], + 'read_special': False, + }, + patterns=[], + local_borg_version='1.2.3', + global_arguments=flexmock(log_json=False), + borgmatic_runtime_directory='/run/borgmatic', + stream_processes=flexmock(), + ) + + assert create_flags == ('borg', 'create', '--read-special', '--patterns-from', 'patterns') + assert create_positional_arguments == REPO_ARCHIVE + assert pattern_file + + +def test_make_base_create_command_with_stream_processes_and_read_special_true_skips_special_files_excludes(): flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None) flexmock(module).should_receive('write_patterns_file').and_return(None) flexmock(module).should_receive('make_list_filter_flags').and_return('FOO')