diff --git a/NEWS b/NEWS index 60fee340..2a3df2c2 100644 --- a/NEWS +++ b/NEWS @@ -39,6 +39,7 @@ * SECURITY: Prevent shell injection attacks via constant interpolation in command hooks. (This was already implemented for deprecated "before_*"/"after_*" command hooks.) * Fix for an error in the "key import" action when importing a key from stdin. + * Update the "list" action to support the "--json" flag when the "--archive" flag is also used. * Promote the ZFS, LVM, and Btrfs hooks from beta features to stable. 2.0.13 diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 026b98fd..e6390598 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -528,7 +528,8 @@ def compare_spot_check_hashes( tuple( shlex.quote(part) for part in shlex.split(spot_check_config.get('xxh64sum_command', 'xxh64sum')) - ) + hash_paths, + ) + + hash_paths, working_directory=working_directory, ) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 22eccc76..308a6f31 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -56,7 +56,7 @@ def make_list_command( + flags.make_flags('remote-path', remote_path) + flags.make_flags('umask', config.get('umask')) + ('--log-json',) - + (('--json-lines',) if list_arguments.json else ()) + + flags.make_flags('json-lines', list_arguments.json) + flags.make_flags('lock-wait', config.get('lock_wait')) + flags.make_flags('format', list_arguments.format or config.get('file_list_format')) + flags.make_flags_from_arguments(list_arguments, excludes=MAKE_FLAGS_EXCLUDES) @@ -198,11 +198,6 @@ def list_archive( f"The --{name.replace('_', '-')} flag on the list action is ignored when using the --archive flag.", ) - if list_arguments.json: - raise ValueError( - 'The --json flag on the list action is not supported when using the --archive/--find flags.', - ) - borg_exit_codes = config.get('borg_exit_codes') # If there are any paths to find (and there's not a single archive already selected), start by diff --git a/borgmatic/execute.py b/borgmatic/execute.py index af3a9709..0e9d639b 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -196,14 +196,14 @@ def handle_log_record(log_record, last_lines): return log_record -def log_outputs( +def log_outputs( # noqa: PLR0912 processes, exclude_stdouts, output_log_level, borg_local_path, borg_exit_codes, capture_stderr=False, -): # noqa: PLR0912 +): ''' Given a sequence of subprocess.Popen() instances for multiple processes, log the outputs (stderr and stdout). Use the requested output log level for stdout, but always log stderr to the ERROR diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 6e04158d..df6cfc03 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -850,13 +850,11 @@ def test_collect_spot_check_archive_paths_excludes_directories_and_pipes(): flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory', ).and_return('/home/user/.borgmatic') - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ( - 'f etc/path', - 'p var/pipe', - 'f etc/other', - 'd etc/dir', - ), + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'type': 'f', 'path': 'etc/path'}, + {'type': 'p', 'path': 'var/pipe'}, + {'type': 'f', 'path': 'etc/other'}, + {'type': 'd', 'path': 'etc/dir'}, ) assert module.collect_spot_check_archive_paths( @@ -875,11 +873,9 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_dir flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory', ).and_return('/root/.borgmatic') - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ( - 'f etc/path', - 'f borgmatic/some/thing', - ), + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'type': 'f', 'path': 'etc/path'}, + {'type': 'f', 'path': 'borgmatic/some/thing'}, ) assert module.collect_spot_check_archive_paths( @@ -898,11 +894,9 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_source_dire flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory', ).and_return('/root/.borgmatic') - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ( - 'f etc/path', - 'f root/.borgmatic/some/thing', - ), + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'type': 'f', 'path': 'etc/path'}, + {'type': 'f', 'path': 'root/.borgmatic/some/thing'}, ) assert module.collect_spot_check_archive_paths( @@ -921,11 +915,9 @@ def test_collect_spot_check_archive_paths_excludes_file_in_borgmatic_runtime_dir flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory', ).and_return('/root.borgmatic') - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ( - 'f etc/path', - 'f run/user/0/borgmatic/some/thing', - ), + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'type': 'f', 'path': 'etc/path'}, + {'type': 'f', 'path': 'run/user/0/borgmatic/some/thing'}, ) assert module.collect_spot_check_archive_paths( @@ -1009,8 +1001,9 @@ def test_compare_spot_check_hashes_returns_paths_having_failing_hashes(): 'hash1 /foo', 'hash2 /bar', ) - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo', 'nothash2 bar'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'nothash2', 'path': 'bar'}, ) assert module.compare_spot_check_hashes( @@ -1051,8 +1044,9 @@ def test_compare_spot_check_hashes_returns_relative_paths_having_failing_hashes( 'hash1 foo', 'hash2 bar', ) - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo', 'nothash2 bar'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'nothash2', 'path': 'bar'}, ) assert module.compare_spot_check_hashes( @@ -1093,8 +1087,9 @@ def test_compare_spot_check_hashes_handles_data_sample_percentage_above_100(): 'hash1 /foo', 'hash2 /bar', ) - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['nothash1 foo', 'nothash2 bar'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'nothash1', 'path': 'foo'}, + {'xxh64': 'nothash2', 'path': 'bar'}, ) assert module.compare_spot_check_hashes( @@ -1135,8 +1130,9 @@ def test_compare_spot_check_hashes_uses_xxh64sum_command_option(): ('/usr/local/bin/xxhsum', '-H64', '/foo', '/bar'), working_directory=None, ).and_yield('hash1 /foo', 'hash2 /bar') - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo', 'nothash2 bar'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'nothash2', 'path': 'bar'}, ) assert module.compare_spot_check_hashes( @@ -1174,8 +1170,8 @@ def test_compare_spot_check_hashes_considers_path_missing_from_archive_as_not_ma 'hash1 /foo', 'hash2 /bar', ) - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'} ) assert module.compare_spot_check_hashes( @@ -1210,8 +1206,9 @@ def test_compare_spot_check_hashes_considers_symlink_path_as_not_matching(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output', ).with_args(('xxh64sum', '/foo'), working_directory=None).and_yield('hash1 /foo') - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo', 'hash2 bar'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'hash2', 'path': 'bar'}, ) assert module.compare_spot_check_hashes( @@ -1246,8 +1243,9 @@ def test_compare_spot_check_hashes_considers_non_existent_path_as_not_matching() flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output', ).with_args(('xxh64sum', '/foo'), working_directory=None).and_yield('hash1 /foo') - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo', 'hash2 bar'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'hash2', 'path': 'bar'}, ) assert module.compare_spot_check_hashes( @@ -1291,9 +1289,13 @@ def test_compare_spot_check_hashes_with_too_many_paths_feeds_them_to_commands_in 'hash3 /baz', 'hash4 /quux', ) - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo', 'hash2 bar'], - ).and_return(['hash3 baz', 'nothash4 quux']) + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'hash2', 'path': 'bar'}, + ).and_yield( + {'xxh64': 'hash3', 'path': 'baz'}, + {'xxh64': 'nothash4', 'path': 'quux'}, + ) assert module.compare_spot_check_hashes( repository={'path': 'repo'}, @@ -1334,8 +1336,9 @@ def test_compare_spot_check_hashes_uses_working_directory_to_access_source_paths 'hash1 foo', 'hash2 bar', ) - flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( - ['hash1 foo', 'nothash2 bar'], + flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_yield( + {'xxh64': 'hash1', 'path': 'foo'}, + {'xxh64': 'nothash2', 'path': 'bar'}, ) assert module.compare_spot_check_hashes( diff --git a/tests/unit/borg/test_list.py b/tests/unit/borg/test_list.py index 0be0c4e1..fbb3a3a7 100644 --- a/tests/unit/borg/test_list.py +++ b/tests/unit/borg/test_list.py @@ -26,10 +26,10 @@ def test_make_list_command_includes_log_info(): assert command == ('borg', 'list', '--info', '--log-json', 'repo') -def test_make_list_command_includes_json_but_not_info(): +def test_make_list_command_includes_json_lines_but_not_info(): insert_logging_mock(logging.INFO) flexmock(module.flags).should_receive('make_flags').and_return(()) - flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(('--json',)) + flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(('--json-lines',)) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) command = module.make_list_command( @@ -40,7 +40,7 @@ def test_make_list_command_includes_json_but_not_info(): global_arguments=flexmock(), ) - assert command == ('borg', 'list', '--log-json', '--json', 'repo') + assert command == ('borg', 'list', '--log-json', '--json-lines', 'repo') def test_make_list_command_includes_log_debug(): @@ -332,7 +332,7 @@ def test_make_find_paths_adds_globs_to_path_fragments(): def test_capture_archive_listing_does_not_raise(): 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_and_capture_output').and_yield('') + flexmock(module).should_receive('execute_command_and_capture_output').and_yield('{}', '{}') flexmock(module).should_receive('make_list_command') module.capture_archive_listing( @@ -393,24 +393,6 @@ def test_list_archive_calls_borg_with_flags(): ) -def test_list_archive_with_archive_and_json_errors(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER - flexmock(module.logger).answer = lambda message: None - list_arguments = argparse.Namespace(archive='archive', paths=None, json=True, find_paths=None) - - flexmock(module.feature).should_receive('available').and_return(False) - - with pytest.raises(ValueError): - module.list_archive( - repository_path='repo', - config={}, - local_borg_version='1.2.3', - list_arguments=list_arguments, - global_arguments=flexmock(), - ) - - def test_list_archive_calls_borg_with_local_path(): flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index f0115894..2b1b2b31 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -104,11 +104,10 @@ def test_output_buffers_for_process_returns_stderr_only_when_stdout_excluded(): ) -def test_borg_log_line_to_record_parses_line(): - flexmock(module).should_receive('log_line_to_record').never() - line = '{"levelname": "INFO", "time": 12345, "message": "All done", "name": "borg.something"}' +def test_borg_json_log_line_to_record_parses_log_message_line(): + line = '{"type": "log_message", "levelname": "INFO", "time": 12345, "message": "All done", "name": "borg.something"}' - record = module.borg_log_line_to_record(line, module.logging.INFO) + record = module.borg_json_log_line_to_record(line, module.logging.INFO) assert record.levelno == module.logging.INFO assert record.created == 12345 @@ -117,24 +116,35 @@ def test_borg_log_line_to_record_parses_line(): assert record.name == 'borg.something' -def test_borg_log_line_to_record_with_invalid_json_falls_back_to_raw_line(): - record = flexmock() +def test_borg_json_log_line_to_record_parses_file_status_line(): + flexmock(module.time).should_receive('time').and_return(12345) + line = '{"type": "file_status", "status": "-", "path": "/foo/bar"}' + + record = module.borg_json_log_line_to_record(line, module.logging.INFO) + + assert record.levelno == module.logging.INFO + assert record.created == 12345 + assert record.msg == '- /foo/bar' + assert record.levelname == 'INFO' + assert record.name == 'borg.file_status' + + +def test_borg_json_log_line_to_record_handles_invalid_json(): line = '{invalid' - flexmock(module).should_receive('log_line_to_record').with_args( - line, module.logging.INFO - ).and_return(record).once() - assert module.borg_log_line_to_record(line, module.logging.INFO) == record + assert module.borg_json_log_line_to_record(line, module.logging.INFO) is None -def test_borg_log_line_to_record_with_non_dict_json_falls_back_to_raw_line(): - record = flexmock() +def test_borg_json_log_line_to_record_handles_non_dict_json(): line = '[]' - flexmock(module).should_receive('log_line_to_record').with_args( - line, module.logging.INFO - ).and_return(record).once() - assert module.borg_log_line_to_record(line, module.logging.INFO) == record + assert module.borg_json_log_line_to_record(line, module.logging.INFO) is None + + +def test_borg_json_log_line_to_record_handles_json_dict_without_type(): + line = '{"status": "-", "path": "/foo/bar"}' + + assert module.borg_json_log_line_to_record(line, module.logging.INFO) is None def test_log_line_to_record_makes_log_record(): @@ -149,14 +159,14 @@ def test_log_line_to_record_makes_log_record(): def test_parse_log_line_with_borg_command_parses_borg_log_line(): record = flexmock() - flexmock(module).should_receive('borg_log_line_to_record').and_return(record).once() + flexmock(module).should_receive('borg_json_log_line_to_record').and_return(record).once() flexmock(module).should_receive('log_line_to_record').never() assert ( module.parse_log_line( 'All done', module.logging.INFO, - came_from_stderr=False, + elevate_stderr=False, borg_local_path='borg', command=['borg', 'do-stuff'], ) @@ -166,14 +176,14 @@ def test_parse_log_line_with_borg_command_parses_borg_log_line(): def test_parse_log_line_without_borg_command_parses_plain_log_line(): record = flexmock() - flexmock(module).should_receive('borg_log_line_to_record').never() + flexmock(module).should_receive('borg_json_log_line_to_record').never() flexmock(module).should_receive('log_line_to_record').and_return(record).once() assert ( module.parse_log_line( 'All done', module.logging.INFO, - came_from_stderr=False, + elevate_stderr=False, borg_local_path='borg', command=['totally-not-borg', 'do-stuff'], ) @@ -181,9 +191,9 @@ def test_parse_log_line_without_borg_command_parses_plain_log_line(): ) -def test_parse_log_line_with_came_from_stderr_makes_error_record(): +def test_parse_log_line_with_elevate_stderr_makes_error_record(): record = flexmock() - flexmock(module).should_receive('borg_log_line_to_record').never() + flexmock(module).should_receive('borg_json_log_line_to_record').never() flexmock(module).should_receive('log_line_to_record').with_args( 'All done', module.logging.ERROR ).and_return(record).once() @@ -192,7 +202,7 @@ def test_parse_log_line_with_came_from_stderr_makes_error_record(): module.parse_log_line( 'All done', module.logging.INFO, - came_from_stderr=True, + elevate_stderr=True, borg_local_path='borg', command=['totally-not-borg', 'do-stuff'], ) @@ -200,9 +210,9 @@ def test_parse_log_line_with_came_from_stderr_makes_error_record(): ) -def test_parse_log_line_with_came_from_stderr_and_warning_prefix_makes_warning_record(): +def test_parse_log_line_with_elevate_stderr_and_warning_prefix_makes_warning_record(): record = flexmock() - flexmock(module).should_receive('borg_log_line_to_record').never() + flexmock(module).should_receive('borg_json_log_line_to_record').never() flexmock(module).should_receive('log_line_to_record').with_args( 'warning: All done', module.logging.WARNING ).and_return(record).once() @@ -211,7 +221,7 @@ def test_parse_log_line_with_came_from_stderr_and_warning_prefix_makes_warning_r module.parse_log_line( 'warning: All done', module.logging.INFO, - came_from_stderr=True, + elevate_stderr=True, borg_local_path='borg', command=['totally-not-borg', 'do-stuff'], )