diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index 30f8dca5..d288664b 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -1,5 +1,3 @@ -import collections -import enum import glob import itertools import logging @@ -135,7 +133,7 @@ def expand_patterns(patterns, working_directory=None, skip_paths=None): for expanded_path in expand_directory(pattern.path, working_directory) ) if pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT - and pattern.path not in skip_paths + and pattern.path not in (skip_paths or ()) else (pattern,) ) for pattern in patterns @@ -188,11 +186,11 @@ def deduplicate_patterns(patterns): even hangs, e.g. when a database hook is using a named pipe for streaming database dumps to Borg. ''' - deduplicated = [] + deduplicated = {} # Use just the keys as an ordered set. for pattern in patterns: if pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT: - deduplicated.append(pattern) + deduplicated[pattern] = None continue parents = pathlib.PurePath(pattern.path).parents @@ -203,17 +201,17 @@ def deduplicate_patterns(patterns): if other_pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT: continue - for parent in parents: - if ( - pathlib.PurePath(other_pattern.path) == parent - and pattern.device is not None - and other_pattern.device == pattern.device - ): - break + if any( + pathlib.PurePath(other_pattern.path) == parent + and pattern.device is not None + and other_pattern.device == pattern.device + for parent in parents + ): + break else: - deduplicated.append(pattern) + deduplicated[pattern] = None - return tuple(deduplicated) + return tuple(deduplicated.keys()) def process_patterns(patterns, working_directory, skip_expand_paths=None): diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index e979d63f..183c08e8 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -144,8 +144,8 @@ properties: type: string description: | Read include/exclude patterns from one or more separate named files, - one pattern per line. See the output of "borg help patterns" for more - details. + one pattern per line. See the output of "borg help patterns" for + more details. example: - /etc/borgmatic/patterns exclude_patterns: diff --git a/borgmatic/hooks/data_source/snapshot.py b/borgmatic/hooks/data_source/snapshot.py index c6423539..2ee08a8f 100644 --- a/borgmatic/hooks/data_source/snapshot.py +++ b/borgmatic/hooks/data_source/snapshot.py @@ -1,8 +1,5 @@ import pathlib -import borgmatic.borg.pattern - - IS_A_HOOK = False diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 8e327785..327024c9 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -1,8 +1,8 @@ import pytest from flexmock import flexmock -from borgmatic.borg.pattern import Pattern from borgmatic.actions import check as module +from borgmatic.borg.pattern import Pattern def test_parse_checks_returns_them_as_tuple(): diff --git a/tests/unit/actions/test_create.py b/tests/unit/actions/test_create.py index 664296b3..522cace0 100644 --- a/tests/unit/actions/test_create.py +++ b/tests/unit/actions/test_create.py @@ -2,6 +2,7 @@ import pytest from flexmock import flexmock from borgmatic.actions import create as module +from borgmatic.borg.pattern import Pattern, Pattern_type def test_expand_directory_with_basic_path_passes_it_through(): @@ -53,7 +54,7 @@ def test_expand_directory_with_slashdot_hack_globs_working_directory_and_strips_ assert paths == ['./foo', './food'] -def test_expand_directories_flattens_expanded_directories(): +def test_expand_patterns_flattens_expanded_directories(): flexmock(module).should_receive('expand_directory').with_args('~/foo', None).and_return( ['/root/foo'] ) @@ -61,167 +62,205 @@ def test_expand_directories_flattens_expanded_directories(): ['bar', 'barf'] ) - paths = module.expand_directories(('~/foo', 'bar*')) + paths = module.expand_patterns((Pattern('~/foo'), Pattern('bar*'))) - assert paths == ('/root/foo', 'bar', 'barf') + assert paths == (Pattern('/root/foo'), Pattern('bar'), Pattern('barf')) -def test_expand_directories_with_working_directory_passes_it_through(): +def test_expand_patterns_with_working_directory_passes_it_through(): flexmock(module).should_receive('expand_directory').with_args('foo', '/working/dir').and_return( ['/working/dir/foo'] ) - paths = module.expand_directories(('foo',), working_directory='/working/dir') + patterns = module.expand_patterns((Pattern('foo'),), working_directory='/working/dir') - assert paths == ('/working/dir/foo',) + assert patterns == (Pattern('/working/dir/foo'),) -def test_expand_directories_considers_none_as_no_directories(): - paths = module.expand_directories(None, None) +def test_expand_patterns_does_not_expand_skip_paths(): + flexmock(module).should_receive('expand_directory').with_args('/foo', None).and_return(['/foo']) + flexmock(module).should_receive('expand_directory').with_args('/bar*', None).never() - assert paths == () + patterns = module.expand_patterns((Pattern('/foo'), Pattern('/bar*')), skip_paths=('/bar*',)) + + assert patterns == (Pattern('/foo'), Pattern('/bar*')) -def test_map_directories_to_devices_gives_device_id_per_path(): +def test_expand_patterns_considers_none_as_no_patterns(): + assert module.expand_patterns(None) == () + + +def test_expand_patterns_only_considers_root_patterns(): + flexmock(module).should_receive('expand_directory').with_args('~/foo', None).and_return( + ['/root/foo'] + ) + flexmock(module).should_receive('expand_directory').with_args('bar*', None).never() + + paths = module.expand_patterns((Pattern('~/foo'), Pattern('bar*', Pattern_type.INCLUDE))) + + assert paths == (Pattern('/root/foo'), Pattern('bar*', Pattern_type.INCLUDE)) + + +def test_device_map_patterns_gives_device_id_per_path(): flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module.os).should_receive('stat').with_args('/foo').and_return(flexmock(st_dev=55)) flexmock(module.os).should_receive('stat').with_args('/bar').and_return(flexmock(st_dev=66)) - device_map = module.map_directories_to_devices(('/foo', '/bar')) + device_map = module.device_map_patterns((Pattern('/foo'), Pattern('/bar'))) - assert device_map == { - '/foo': 55, - '/bar': 66, - } + assert device_map == ( + Pattern('/foo', device=55), + Pattern('/bar', device=66), + ) -def test_map_directories_to_devices_with_missing_path_does_not_error(): +def test_device_map_patterns_with_missing_path_does_not_error(): flexmock(module.os.path).should_receive('exists').and_return(True).and_return(False) flexmock(module.os).should_receive('stat').with_args('/foo').and_return(flexmock(st_dev=55)) flexmock(module.os).should_receive('stat').with_args('/bar').never() - device_map = module.map_directories_to_devices(('/foo', '/bar')) + device_map = module.device_map_patterns((Pattern('/foo'), Pattern('/bar'))) - assert device_map == { - '/foo': 55, - '/bar': None, - } + assert device_map == ( + Pattern('/foo', device=55), + Pattern('/bar'), + ) -def test_map_directories_to_devices_uses_working_directory_to_construct_path(): +def test_device_map_patterns_uses_working_directory_to_construct_path(): flexmock(module.os.path).should_receive('exists').and_return(True) flexmock(module.os).should_receive('stat').with_args('/foo').and_return(flexmock(st_dev=55)) flexmock(module.os).should_receive('stat').with_args('/working/dir/bar').and_return( flexmock(st_dev=66) ) - device_map = module.map_directories_to_devices( - ('/foo', 'bar'), working_directory='/working/dir' + device_map = module.device_map_patterns( + (Pattern('/foo'), Pattern('bar')), working_directory='/working/dir' ) - assert device_map == { - '/foo': 55, - 'bar': 66, - } + assert device_map == ( + Pattern('/foo', device=55), + Pattern('bar', device=66), + ) + + +def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it(): + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.os).should_receive('stat').with_args('/foo').and_return(flexmock(st_dev=55)) + flexmock(module.os).should_receive('stat').with_args('/bar').and_return(flexmock(st_dev=100)) + + device_map = module.device_map_patterns((Pattern('/foo'), Pattern('/bar', device=66))) + + assert device_map == ( + Pattern('/foo', device=55), + Pattern('/bar', device=66), + ) @pytest.mark.parametrize( - 'directories,additional_directories,expected_directories', + 'patterns,expected_patterns', ( - ({'/': 1, '/root': 1}, {}, ['/']), - ({'/': 1, '/root/': 1}, {}, ['/']), - ({'/': 1, '/root': 2}, {}, ['/', '/root']), - ({'/root': 1, '/': 1}, {}, ['/']), - ({'/root': 1, '/root/foo': 1}, {}, ['/root']), - ({'/root/': 1, '/root/foo': 1}, {}, ['/root/']), - ({'/root': 1, '/root/foo/': 1}, {}, ['/root']), - ({'/root': 1, '/root/foo': 2}, {}, ['/root', '/root/foo']), - ({'/root/foo': 1, '/root': 1}, {}, ['/root']), - ({'/root': None, '/root/foo': None}, {}, ['/root', '/root/foo']), - ({'/root': 1, '/etc': 1, '/root/foo/bar': 1}, {}, ['/etc', '/root']), - ({'/root': 1, '/root/foo': 1, '/root/foo/bar': 1}, {}, ['/root']), - ({'/dup': 1, '/dup': 1}, {}, ['/dup']), - ({'/foo': 1, '/bar': 1}, {}, ['/bar', '/foo']), - ({'/foo': 1, '/bar': 2}, {}, ['/bar', '/foo']), - ({'/root/foo': 1}, {'/root': 1}, []), - ({'/root/foo': 1}, {'/root': 2}, ['/root/foo']), - ({'/root/foo': 1}, {}, ['/root/foo']), + ((Pattern('/', device=1), Pattern('/root', device=1)), (Pattern('/', device=1),)), + ((Pattern('/', device=1), Pattern('/root/', device=1)), (Pattern('/', device=1),)), + ( + (Pattern('/', device=1), Pattern('/root', device=2)), + (Pattern('/', device=1), Pattern('/root', device=2)), + ), + ((Pattern('/root', device=1), Pattern('/', device=1)), (Pattern('/', device=1),)), + ( + (Pattern('/root', device=1), Pattern('/root/foo', device=1)), + (Pattern('/root', device=1),), + ), + ( + (Pattern('/root/', device=1), Pattern('/root/foo', device=1)), + (Pattern('/root/', device=1),), + ), + ( + (Pattern('/root', device=1), Pattern('/root/foo/', device=1)), + (Pattern('/root', device=1),), + ), + ( + (Pattern('/root', device=1), Pattern('/root/foo', device=2)), + (Pattern('/root', device=1), Pattern('/root/foo', device=2)), + ), + ( + (Pattern('/root/foo', device=1), Pattern('/root', device=1)), + (Pattern('/root', device=1),), + ), + ( + (Pattern('/root', device=None), Pattern('/root/foo', device=None)), + (Pattern('/root'), Pattern('/root/foo')), + ), + ( + ( + Pattern('/root', device=1), + Pattern('/etc', device=1), + Pattern('/root/foo/bar', device=1), + ), + (Pattern('/root', device=1), Pattern('/etc', device=1)), + ), + ( + ( + Pattern('/root', device=1), + Pattern('/root/foo', device=1), + Pattern('/root/foo/bar', device=1), + ), + (Pattern('/root', device=1),), + ), + ((Pattern('/dup', device=1), Pattern('/dup', device=1)), (Pattern('/dup', device=1),)), + ( + (Pattern('/foo', device=1), Pattern('/bar', device=1)), + (Pattern('/foo', device=1), Pattern('/bar', device=1)), + ), + ( + (Pattern('/foo', device=1), Pattern('/bar', device=2)), + (Pattern('/foo', device=1), Pattern('/bar', device=2)), + ), + ((Pattern('/root/foo', device=1),), (Pattern('/root/foo', device=1),)), + ( + (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)), + (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)), + ), ), ) -def test_deduplicate_directories_removes_child_paths_on_the_same_filesystem( - directories, additional_directories, expected_directories -): - assert ( - module.deduplicate_directories(directories, additional_directories) == expected_directories +def test_deduplicate_patterns_omits_child_paths_on_the_same_filesystem(patterns, expected_patterns): + assert module.deduplicate_patterns(patterns) == expected_patterns + + +def test_process_patterns_includes_patterns(): + flexmock(module).should_receive('deduplicate_patterns').and_return( + (Pattern('foo'), Pattern('bar')) ) - - -def test_pattern_root_directories_deals_with_none_patterns(): - assert module.pattern_root_directories(patterns=None) == [] - - -def test_pattern_root_directories_parses_roots_and_ignores_others(): - assert module.pattern_root_directories( - ['R /root', '+ /root/foo', '- /root/foo/bar', 'R /baz'] - ) == ['/root', '/baz'] - - -def test_process_source_directories_includes_source_directories(): - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( - '/working' - ) - flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar')) - flexmock(module).should_receive('map_directories_to_devices').and_return({}) - flexmock(module).should_receive('expand_directories').with_args( - ('foo', 'bar'), working_directory='/working' + flexmock(module).should_receive('device_map_patterns').and_return({}) + flexmock(module).should_receive('expand_patterns').with_args( + (Pattern('foo'), Pattern('bar')), + working_directory='/working', + skip_paths=set(), ).and_return(()).once() - flexmock(module).should_receive('pattern_root_directories').and_return(()) - flexmock(module).should_receive('expand_directories').with_args( - (), working_directory='/working' - ).and_return(()) - assert module.process_source_directories( - config={'source_directories': ['foo', 'bar']}, - ) == ('foo', 'bar') + assert module.process_patterns( + (Pattern('foo'), Pattern('bar')), + working_directory='/working', + ) == [Pattern('foo'), Pattern('bar')] -def test_process_source_directories_prefers_source_directory_argument_to_config(): - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( - '/working' +def test_process_patterns_skips_expand_for_requested_paths(): + skip_paths = {flexmock()} + flexmock(module).should_receive('deduplicate_patterns').and_return( + (Pattern('foo'), Pattern('bar')) ) - flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar')) - flexmock(module).should_receive('map_directories_to_devices').and_return({}) - flexmock(module).should_receive('expand_directories').with_args( - ('foo', 'bar'), working_directory='/working' + flexmock(module).should_receive('device_map_patterns').and_return({}) + flexmock(module).should_receive('expand_patterns').with_args( + (Pattern('foo'), Pattern('bar')), + working_directory='/working', + skip_paths=skip_paths, ).and_return(()).once() - flexmock(module).should_receive('pattern_root_directories').and_return(()) - flexmock(module).should_receive('expand_directories').with_args( - (), working_directory='/working' - ).and_return(()) - assert module.process_source_directories( - config={'source_directories': ['nope']}, - source_directories=['foo', 'bar'], - ) == ('foo', 'bar') - - -def test_process_source_directories_skips_expand_for_requested_paths(): - flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( - '/working' - ) - flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar')) - flexmock(module).should_receive('map_directories_to_devices').and_return({}) - flexmock(module).should_receive('expand_directories').with_args( - ('bar',), working_directory='/working' - ).and_return(()).once() - flexmock(module).should_receive('pattern_root_directories').and_return(()) - flexmock(module).should_receive('expand_directories').with_args( - (), working_directory='/working' - ).and_return(()) - - assert module.process_source_directories( - config={'source_directories': ['foo', 'bar']}, skip_expand_paths=('foo',) - ) == ('foo', 'bar') + assert module.process_patterns( + (Pattern('foo'), Pattern('bar')), + working_directory='/working', + skip_expand_paths=skip_paths, + ) == [Pattern('foo'), Pattern('bar')] def test_run_create_executes_and_calls_hooks_for_configured_repository(): @@ -236,7 +275,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('process_source_directories').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( repository=None, @@ -278,7 +317,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('process_source_directories').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( repository=flexmock(), @@ -357,7 +396,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('process_source_directories').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( repository=flexmock(), diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 8343bcbe..32ed1482 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -1,5 +1,4 @@ import logging -import sys import pytest from flexmock import flexmock diff --git a/tests/unit/hooks/data_source/test_btrfs.py b/tests/unit/hooks/data_source/test_btrfs.py index c7c43732..45d3d1ad 100644 --- a/tests/unit/hooks/data_source/test_btrfs.py +++ b/tests/unit/hooks/data_source/test_btrfs.py @@ -1,7 +1,7 @@ import pytest from flexmock import flexmock -from borgmatic.borg.pattern import Pattern, Pattern_type, Pattern_style +from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type from borgmatic.hooks.data_source import btrfs as module diff --git a/tests/unit/hooks/data_source/test_sqlite.py b/tests/unit/hooks/data_source/test_sqlite.py index 7a942c88..f7acce8d 100644 --- a/tests/unit/hooks/data_source/test_sqlite.py +++ b/tests/unit/hooks/data_source/test_sqlite.py @@ -2,7 +2,6 @@ import logging from flexmock import flexmock -from borgmatic.borg.pattern import Pattern from borgmatic.hooks.data_source import sqlite as module