From d29c7956bc17ef106483c17ef5d256077463f571 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 22 Jul 2018 11:25:06 -0700 Subject: [PATCH 01/12] Upgrade ruamel.yaml compatibility version range and fix support for Python 3.7 (#38, #76). --- .gitignore | 1 + NEWS | 1 + setup.py | 2 +- test_requirements.txt | 6 +++--- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 768a6263..340aeed5 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ *.swp .cache .coverage +.pytest_cache .tox build dist diff --git a/NEWS b/NEWS index b29c11b0..7b07fe5f 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ 1.2.1.dev0 * Skip before/after backup hooks when only doing --prune, --check, --list, and/or --info. + * #38, #76: Upgrade ruamel.yaml compatibility version range and fix support for Python 3.7. 1.2.0 * #61: Support for Borg --list option via borgmatic command-line to list all archives. diff --git a/setup.py b/setup.py index 620b8e4d..a451f513 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ setup( ], install_requires=( 'pykwalify>=1.6.0', - 'ruamel.yaml<=0.15', + 'ruamel.yaml>0.15.0,<0.16.0', 'setuptools', ), tests_require=( diff --git a/test_requirements.txt b/test_requirements.txt index 6bf66e08..5335c20f 100644 --- a/test_requirements.txt +++ b/test_requirements.txt @@ -1,5 +1,5 @@ flexmock==0.10.2 -pykwalify==1.6.0 -pytest==2.9.1 +pykwalify==1.6.1 +pytest==3.6.3 pytest-cov==2.5.1 -ruamel.yaml==0.15.18 +ruamel.yaml>0.15.0,<0.16.0 From 64364b20ff34b7492910838b44e16b3191f27f33 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 22 Jul 2018 12:08:49 -0700 Subject: [PATCH 02/12] Skip non-"*.yaml" config filenames in /etc/borgmatic.d/ so as not to parse backup files, editor swap files, etc. (#77) --- NEWS | 2 ++ borgmatic/config/collect.py | 5 +++-- borgmatic/config/validate.py | 1 + borgmatic/tests/unit/config/test_collect.py | 15 +++++++++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 7b07fe5f..6ee552cc 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,8 @@ 1.2.1.dev0 * Skip before/after backup hooks when only doing --prune, --check, --list, and/or --info. * #38, #76: Upgrade ruamel.yaml compatibility version range and fix support for Python 3.7. + * #77: Skip non-"*.yaml" config filenames in /etc/borgmatic.d/ so as not to parse backup files, + editor swap files, etc. 1.2.0 * #61: Support for Borg --list option via borgmatic command-line to list all archives. diff --git a/borgmatic/config/collect.py b/borgmatic/config/collect.py index 02ccc9dc..47f0ef46 100644 --- a/borgmatic/config/collect.py +++ b/borgmatic/config/collect.py @@ -12,7 +12,8 @@ def collect_config_filenames(config_paths): ''' Given a sequence of config paths, both filenames and directories, resolve that to just an iterable of files. Accomplish this by listing any given directories looking for contained config - files. This is non-recursive, so any directories within the given directories are ignored. + files (ending with the ".yaml" extension). This is non-recursive, so any directories within the + given directories are ignored. Return paths even if they don't exist on disk, so the user can find out about missing configuration paths. However, skip a default config path if it's missing, so the user doesn't @@ -32,5 +33,5 @@ def collect_config_filenames(config_paths): for filename in os.listdir(path): full_filename = os.path.join(path, filename) - if not os.path.isdir(full_filename): + if full_filename.endswith('.yaml') and not os.path.isdir(full_filename): yield full_filename diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py index 795a07dd..bdbbbf78 100644 --- a/borgmatic/config/validate.py +++ b/borgmatic/config/validate.py @@ -10,6 +10,7 @@ from ruamel import yaml logger = logging.getLogger(__name__) + def schema_filename(): ''' Path to the installed YAML configuration schema file, used to validate and parse the diff --git a/borgmatic/tests/unit/config/test_collect.py b/borgmatic/tests/unit/config/test_collect.py index 65acbf8e..d573ae01 100644 --- a/borgmatic/tests/unit/config/test_collect.py +++ b/borgmatic/tests/unit/config/test_collect.py @@ -32,6 +32,21 @@ def test_collect_config_filenames_collects_files_from_given_directories_and_igno ) +def test_collect_config_filenames_collects_files_from_given_directories_and_ignores_non_yaml_filenames(): + config_paths = ('/etc/borgmatic.d',) + mock_path = flexmock(module.os.path) + mock_path.should_receive('exists').and_return(True) + mock_path.should_receive('isdir').with_args('/etc/borgmatic.d').and_return(True) + mock_path.should_receive('isdir').with_args('/etc/borgmatic.d/foo.yaml').and_return(False) + mock_path.should_receive('isdir').with_args('/etc/borgmatic.d/bar.yaml~').and_return(False) + mock_path.should_receive('isdir').with_args('/etc/borgmatic.d/baz.txt').and_return(False) + flexmock(module.os).should_receive('listdir').and_return(['foo.yaml', 'bar.yaml~', 'baz.txt']) + + config_filenames = tuple(module.collect_config_filenames(config_paths)) + + assert config_filenames == ('/etc/borgmatic.d/foo.yaml',) + + def test_collect_config_filenames_skips_etc_borgmatic_config_dot_yaml_if_it_does_not_exist(): config_paths = ('config.yaml', '/etc/borgmatic/config.yaml') mock_path = flexmock(module.os.path) From cf6ab60d2e4bf6dc23986d7f1079daa003445323 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 25 Jul 2018 01:34:05 +0000 Subject: [PATCH 03/12] Use XDG_CONFIG_HOME for user configuration directory, if set. (Thanks to floli.) (#71) Thanks! This will go out in the next release. --- NEWS | 2 ++ borgmatic/commands/borgmatic.py | 6 +++-- borgmatic/config/collect.py | 22 ++++++++++++++----- .../integration/commands/test_borgmatic.py | 22 +++++++++++++++++-- borgmatic/tests/unit/config/test_collect.py | 16 ++++++++++++++ 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 6ee552cc..18241c46 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 1.2.1.dev0 * Skip before/after backup hooks when only doing --prune, --check, --list, and/or --info. + * #71: Support for XDG_CONFIG_HOME environment variable for specifying alternate user ~/.config/ + path. * #38, #76: Upgrade ruamel.yaml compatibility version range and fix support for Python 3.7. * #77: Skip non-"*.yaml" config filenames in /etc/borgmatic.d/ so as not to parse backup files, editor swap files, etc. diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 99d5471b..ee225088 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -24,6 +24,8 @@ def parse_arguments(*arguments): Given command-line arguments with which this script was invoked, parse the arguments and return them as an ArgumentParser instance. ''' + config_paths = collect.get_default_config_paths() + parser = ArgumentParser( description= ''' @@ -36,8 +38,8 @@ def parse_arguments(*arguments): '-c', '--config', nargs='+', dest='config_paths', - default=collect.DEFAULT_CONFIG_PATHS, - help='Configuration filenames or directories, defaults to: {}'.format(' '.join(collect.DEFAULT_CONFIG_PATHS)), + default=config_paths, + help='Configuration filenames or directories, defaults to: {}'.format(' '.join(config_paths)), ) parser.add_argument( '--excludes', diff --git a/borgmatic/config/collect.py b/borgmatic/config/collect.py index 47f0ef46..aba9afd0 100644 --- a/borgmatic/config/collect.py +++ b/borgmatic/config/collect.py @@ -1,11 +1,21 @@ import os -DEFAULT_CONFIG_PATHS = [ - '/etc/borgmatic/config.yaml', - '/etc/borgmatic.d', - os.path.expanduser('~/.config/borgmatic/config.yaml'), -] +def get_default_config_paths(): + ''' + Based on the value of the XDG_CONFIG_HOME and HOME environment variables, return a list of + default configuration paths. This includes both system-wide configuration and configuration in + the current user's home directory. + ''' + user_config_directory = ( + os.getenv('XDG_CONFIG_HOME') or os.path.expandvars(os.path.join('$HOME', '.config')) + ) + + return [ + '/etc/borgmatic/config.yaml', + '/etc/borgmatic.d', + '%s/borgmatic/config.yaml' % user_config_directory, + ] def collect_config_filenames(config_paths): @@ -19,7 +29,7 @@ def collect_config_filenames(config_paths): configuration paths. However, skip a default config path if it's missing, so the user doesn't have to create a default config path unless they need it. ''' - real_default_config_paths = set(map(os.path.realpath, DEFAULT_CONFIG_PATHS)) + real_default_config_paths = set(map(os.path.realpath, get_default_config_paths())) for path in config_paths: exists = os.path.exists(path) diff --git a/borgmatic/tests/integration/commands/test_borgmatic.py b/borgmatic/tests/integration/commands/test_borgmatic.py index 1d6aa5c9..6207cdb5 100644 --- a/borgmatic/tests/integration/commands/test_borgmatic.py +++ b/borgmatic/tests/integration/commands/test_borgmatic.py @@ -7,14 +7,19 @@ from borgmatic.commands import borgmatic as module def test_parse_arguments_with_no_arguments_uses_defaults(): + config_paths = ['default'] + flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths) + parser = module.parse_arguments() - assert parser.config_paths == module.collect.DEFAULT_CONFIG_PATHS + assert parser.config_paths == config_paths assert parser.excludes_filename == None assert parser.verbosity is None def test_parse_arguments_with_path_arguments_overrides_defaults(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + parser = module.parse_arguments('--config', 'myconfig', '--excludes', 'myexcludes') assert parser.config_paths == ['myconfig'] @@ -23,6 +28,8 @@ def test_parse_arguments_with_path_arguments_overrides_defaults(): def test_parse_arguments_with_multiple_config_paths_parses_as_list(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + parser = module.parse_arguments('--config', 'myconfig', 'otherconfig') assert parser.config_paths == ['myconfig', 'otherconfig'] @@ -30,14 +37,19 @@ def test_parse_arguments_with_multiple_config_paths_parses_as_list(): def test_parse_arguments_with_verbosity_flag_overrides_default(): + config_paths = ['default'] + flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths) + parser = module.parse_arguments('--verbosity', '1') - assert parser.config_paths == module.collect.DEFAULT_CONFIG_PATHS + assert parser.config_paths == config_paths assert parser.excludes_filename == None assert parser.verbosity == 1 def test_parse_arguments_with_no_actions_defaults_to_all_actions_enabled(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + parser = module.parse_arguments() assert parser.prune is True @@ -46,6 +58,8 @@ def test_parse_arguments_with_no_actions_defaults_to_all_actions_enabled(): def test_parse_arguments_with_prune_action_leaves_other_actions_disabled(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + parser = module.parse_arguments('--prune') assert parser.prune is True @@ -54,6 +68,8 @@ def test_parse_arguments_with_prune_action_leaves_other_actions_disabled(): def test_parse_arguments_with_multiple_actions_leaves_other_action_disabled(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + parser = module.parse_arguments('--create', '--check') assert parser.prune is False @@ -62,5 +78,7 @@ def test_parse_arguments_with_multiple_actions_leaves_other_action_disabled(): def test_parse_arguments_with_invalid_arguments_exits(): + flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) + with pytest.raises(SystemExit): module.parse_arguments('--posix-me-harder') diff --git a/borgmatic/tests/unit/config/test_collect.py b/borgmatic/tests/unit/config/test_collect.py index d573ae01..72df916c 100644 --- a/borgmatic/tests/unit/config/test_collect.py +++ b/borgmatic/tests/unit/config/test_collect.py @@ -3,6 +3,22 @@ from flexmock import flexmock from borgmatic.config import collect as module +def test_get_default_config_paths_includes_absolute_user_config_path(): + flexmock(module.os, environ={'XDG_CONFIG_HOME': None, 'HOME': '/home/user'}) + + config_paths = module.get_default_config_paths() + + assert '/home/user/.config/borgmatic/config.yaml' in config_paths + + +def test_get_default_config_paths_prefers_xdg_config_home_for_user_config_path(): + flexmock(module.os, environ={'XDG_CONFIG_HOME': '/home/user/.etc', 'HOME': '/home/user'}) + + config_paths = module.get_default_config_paths() + + assert '/home/user/.etc/borgmatic/config.yaml' in config_paths + + def test_collect_config_filenames_collects_given_files(): config_paths = ('config.yaml', 'other.yaml') flexmock(module.os.path).should_receive('isdir').and_return(False) From 789bcd402abe6bc052e1b3e64d4a7338567a83e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=89VEIL?= Date: Sat, 28 Jul 2018 21:21:38 +0000 Subject: [PATCH 04/12] add support for `--list --json` (#74) --- borgmatic/borg/list.py | 7 +- borgmatic/commands/borgmatic.py | 136 +++++++++++------- .../integration/commands/test_borgmatic.py | 11 ++ borgmatic/tests/unit/borg/test_list.py | 13 +- borgmatic/tests/unit/commands/__init__.py | 0 .../tests/unit/commands/test_borgmatic.py | 38 +++++ 6 files changed, 148 insertions(+), 57 deletions(-) create mode 100644 borgmatic/tests/unit/commands/__init__.py create mode 100644 borgmatic/tests/unit/commands/test_borgmatic.py diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 989f8817..4bc24f7d 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -7,7 +7,7 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS logger = logging.getLogger(__name__) -def list_archives(verbosity, repository, storage_config, local_path='borg', remote_path=None): +def list_archives(verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False): ''' Given a verbosity flag, a local or remote repository path, and a storage config dict, list Borg archives in the repository. @@ -18,6 +18,7 @@ def list_archives(verbosity, repository, storage_config, local_path='borg', remo (local_path, 'list', repository) + (('--remote-path', remote_path) if remote_path else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + + (('--json',) if json else ()) + { VERBOSITY_SOME: ('--info',), VERBOSITY_LOTS: ('--debug',), @@ -25,4 +26,6 @@ def list_archives(verbosity, repository, storage_config, local_path='borg', remo ) logger.debug(' '.join(full_command)) - subprocess.check_call(full_command) + + output = subprocess.check_output(full_command) + return output.decode() if output is not None else None diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index ee225088..c8993ba1 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -1,5 +1,5 @@ - from argparse import ArgumentParser +import json import logging import os from subprocess import CalledProcessError @@ -76,6 +76,13 @@ def parse_arguments(*arguments): action='store_true', help='Display summary information on archives', ) + parser.add_argument( + '--json', + dest='json', + default=False, + action='store_true', + help='Output results from the --list option as json', + ) parser.add_argument( '-n', '--dry-run', dest='dry_run', @@ -90,6 +97,9 @@ def parse_arguments(*arguments): args = parser.parse_args(arguments) + if args.json and not args.list: + raise ValueError("The --json option can only be used with the --list option") + # If any of the action flags are explicitly requested, leave them as-is. Otherwise, assume # defaults: Mutate the given arguments to enable the default actions. if args.prune or args.create or args.check or args.list or args.info: @@ -121,59 +131,7 @@ def run_configuration(config_filename, args): # pragma: no cover if args.create: hook.execute_hook(hooks.get('before_backup'), config_filename, 'pre-backup') - for unexpanded_repository in location['repositories']: - repository = os.path.expanduser(unexpanded_repository) - dry_run_label = ' (dry run; not making any changes)' if args.dry_run else '' - if args.prune: - logger.info('{}: Pruning archives{}'.format(repository, dry_run_label)) - borg_prune.prune_archives( - args.verbosity, - args.dry_run, - repository, - storage, - retention, - local_path=local_path, - remote_path=remote_path, - ) - if args.create: - logger.info('{}: Creating archive{}'.format(repository, dry_run_label)) - borg_create.create_archive( - args.verbosity, - args.dry_run, - repository, - location, - storage, - local_path=local_path, - remote_path=remote_path, - ) - if args.check: - logger.info('{}: Running consistency checks'.format(repository)) - borg_check.check_archives( - args.verbosity, - repository, - storage, - consistency, - local_path=local_path, - remote_path=remote_path, - ) - if args.list: - logger.info('{}: Listing archives'.format(repository)) - borg_list.list_archives( - args.verbosity, - repository, - storage, - local_path=local_path, - remote_path=remote_path, - ) - if args.info: - logger.info('{}: Displaying summary info for archives'.format(repository)) - borg_info.display_archives_info( - args.verbosity, - repository, - storage, - local_path=local_path, - remote_path=remote_path, - ) + _run_commands(args, consistency, local_path, location, remote_path, retention, storage) if args.create: hook.execute_hook(hooks.get('after_backup'), config_filename, 'post-backup') @@ -182,6 +140,76 @@ def run_configuration(config_filename, args): # pragma: no cover raise +def _run_commands(args, consistency, local_path, location, remote_path, retention, storage): + json_results = [] + for unexpanded_repository in location['repositories']: + _run_commands_on_repository(args, consistency, json_results, local_path, location, remote_path, retention, + storage, unexpanded_repository) + if args.json: + sys.stdout.write(json.dumps(json_results)) + + +def _run_commands_on_repository(args, consistency, json_results, local_path, location, remote_path, retention, storage, + unexpanded_repository): # pragma: no cover + repository = os.path.expanduser(unexpanded_repository) + dry_run_label = ' (dry run; not making any changes)' if args.dry_run else '' + if args.prune: + logger.info('{}: Pruning archives{}'.format(repository, dry_run_label)) + borg_prune.prune_archives( + args.verbosity, + args.dry_run, + repository, + storage, + retention, + local_path=local_path, + remote_path=remote_path, + ) + if args.create: + logger.info('{}: Creating archive{}'.format(repository, dry_run_label)) + borg_create.create_archive( + args.verbosity, + args.dry_run, + repository, + location, + storage, + local_path=local_path, + remote_path=remote_path, + ) + if args.check: + logger.info('{}: Running consistency checks'.format(repository)) + borg_check.check_archives( + args.verbosity, + repository, + storage, + consistency, + local_path=local_path, + remote_path=remote_path, + ) + if args.list: + logger.info('{}: Listing archives'.format(repository)) + output = borg_list.list_archives( + args.verbosity, + repository, + storage, + local_path=local_path, + remote_path=remote_path, + json=args.json, + ) + if args.json: + json_results.append(json.loads(output)) + else: + sys.stdout.write(output) + if args.info: + logger.info('{}: Displaying summary info for archives'.format(repository)) + borg_info.display_archives_info( + args.verbosity, + repository, + storage, + local_path=local_path, + remote_path=remote_path, + ) + + def main(): # pragma: no cover try: configure_signals() diff --git a/borgmatic/tests/integration/commands/test_borgmatic.py b/borgmatic/tests/integration/commands/test_borgmatic.py index 6207cdb5..8cd1f76a 100644 --- a/borgmatic/tests/integration/commands/test_borgmatic.py +++ b/borgmatic/tests/integration/commands/test_borgmatic.py @@ -15,6 +15,7 @@ def test_parse_arguments_with_no_arguments_uses_defaults(): assert parser.config_paths == config_paths assert parser.excludes_filename == None assert parser.verbosity is None + assert parser.json is False def test_parse_arguments_with_path_arguments_overrides_defaults(): @@ -47,6 +48,11 @@ def test_parse_arguments_with_verbosity_flag_overrides_default(): assert parser.verbosity == 1 +def test_parse_arguments_with_json_flag_overrides_default(): + parser = module.parse_arguments('--list', '--json') + assert parser.json is True + + def test_parse_arguments_with_no_actions_defaults_to_all_actions_enabled(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) @@ -82,3 +88,8 @@ def test_parse_arguments_with_invalid_arguments_exits(): with pytest.raises(SystemExit): module.parse_arguments('--posix-me-harder') + + +def test_parse_arguments_with_json_flag_but_no_list_flag_raises_value_error(): + with pytest.raises(ValueError): + module.parse_arguments('--json') diff --git a/borgmatic/tests/unit/borg/test_list.py b/borgmatic/tests/unit/borg/test_list.py index 1c6a69f7..48bd095a 100644 --- a/borgmatic/tests/unit/borg/test_list.py +++ b/borgmatic/tests/unit/borg/test_list.py @@ -8,7 +8,7 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS def insert_subprocess_mock(check_call_command, **kwargs): subprocess = flexmock(module.subprocess) - subprocess.should_receive('check_call').with_args(check_call_command, **kwargs).once() + subprocess.should_receive('check_output').with_args(check_call_command, **kwargs).once() LIST_COMMAND = ('borg', 'list', 'repo') @@ -44,6 +44,17 @@ def test_list_archives_with_verbosity_lots_calls_borg_with_debug_parameter(): ) +def test_list_archives_with_json_calls_borg_with_json_parameter(): + insert_subprocess_mock(LIST_COMMAND + ('--json',)) + + module.list_archives( + verbosity=None, + repository='repo', + storage_config={}, + json=True, + ) + + def test_list_archives_with_local_path_calls_borg_via_local_path(): insert_subprocess_mock(('borg1',) + LIST_COMMAND[1:]) diff --git a/borgmatic/tests/unit/commands/__init__.py b/borgmatic/tests/unit/commands/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/borgmatic/tests/unit/commands/test_borgmatic.py b/borgmatic/tests/unit/commands/test_borgmatic.py new file mode 100644 index 00000000..cab0b0f3 --- /dev/null +++ b/borgmatic/tests/unit/commands/test_borgmatic.py @@ -0,0 +1,38 @@ +from borgmatic.commands import borgmatic +from flexmock import flexmock +import json +import pytest +import sys + + +def test__run_commands_handles_multiple_json_outputs_in_array(): + # THEN + (flexmock(borgmatic) + .should_receive("_run_commands_on_repository") + .times(3) + .replace_with(lambda args, consistency, json_results, local_path, location, remote_path, retention, storage, + unexpanded_repository: json_results.append({"whatever": unexpanded_repository})) + ) + + (flexmock(sys.stdout) + .should_call("write") + .with_args(json.dumps(json.loads(''' + [ + {"whatever": "fake_repo1"}, + {"whatever": "fake_repo2"}, + {"whatever": "fake_repo3"} + ] + '''))) + ) + + borgmatic._run_commands(args=flexmock(json=True), + consistency=None, + local_path=None, + location={"repositories": [ + "fake_repo1", + "fake_repo2", + "fake_repo3" + ]}, + remote_path=None, + retention=None, + storage=None) From d93da55ce9b7e1c5ed048b1f7265499f40045ef3 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 28 Jul 2018 15:02:17 -0700 Subject: [PATCH 05/12] Add code style guidelines to the documention, and reformat some code accordingly. --- NEWS | 3 + README.md | 14 ++++ borgmatic/commands/borgmatic.py | 14 ++-- .../tests/unit/commands/test_borgmatic.py | 76 +++++++++++-------- 4 files changed, 71 insertions(+), 36 deletions(-) diff --git a/NEWS b/NEWS index 18241c46..820f793a 100644 --- a/NEWS +++ b/NEWS @@ -2,9 +2,12 @@ * Skip before/after backup hooks when only doing --prune, --check, --list, and/or --info. * #71: Support for XDG_CONFIG_HOME environment variable for specifying alternate user ~/.config/ path. + * #74: Support for Borg --list --json options via borgmatic command-line to list all archives in + JSON format, ideal for programmatic consumption. * #38, #76: Upgrade ruamel.yaml compatibility version range and fix support for Python 3.7. * #77: Skip non-"*.yaml" config filenames in /etc/borgmatic.d/ so as not to parse backup files, editor swap files, etc. + * Add code style guidelines to the documention. 1.2.0 * #61: Support for Borg --list option via borgmatic command-line to list all archives. diff --git a/README.md b/README.md index d816ffd8..75d623a5 100644 --- a/README.md +++ b/README.md @@ -326,6 +326,20 @@ to discuss your idea. We also accept Pull Requests on GitHub, if that's more your thing. In general, contributions are very welcome. We don't bite! +### Code style + +Start with [PEP 8](https://www.python.org/dev/peps/pep-0008/). But then, apply +the following deviations from PEP 8: + + * For strings, prefer single quotes over double quotes. + * Limit all lines to a maximum of 100 characters. + * Use trailing commas within multiline values or argument lists. + * Within multiline constructs: + * Use standard four-space indentation. Don't align indentation with an opening + delimeter. + * Put opening and closing delimeters on lines separate from their contents. + + ### Development To get set up to hack on borgmatic, first clone master via HTTPS or SSH: diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index c8993ba1..3a22e4d4 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -98,7 +98,7 @@ def parse_arguments(*arguments): args = parser.parse_args(arguments) if args.json and not args.list: - raise ValueError("The --json option can only be used with the --list option") + raise ValueError('The --json option can only be used with the --list option') # If any of the action flags are explicitly requested, leave them as-is. Otherwise, assume # defaults: Mutate the given arguments to enable the default actions. @@ -143,14 +143,18 @@ def run_configuration(config_filename, args): # pragma: no cover def _run_commands(args, consistency, local_path, location, remote_path, retention, storage): json_results = [] for unexpanded_repository in location['repositories']: - _run_commands_on_repository(args, consistency, json_results, local_path, location, remote_path, retention, - storage, unexpanded_repository) + _run_commands_on_repository( + args, consistency, json_results, local_path, location, remote_path, retention, storage, + unexpanded_repository, + ) if args.json: sys.stdout.write(json.dumps(json_results)) -def _run_commands_on_repository(args, consistency, json_results, local_path, location, remote_path, retention, storage, - unexpanded_repository): # pragma: no cover +def _run_commands_on_repository( + args, consistency, json_results, local_path, location, remote_path, + retention, storage, unexpanded_repository, +): # pragma: no cover repository = os.path.expanduser(unexpanded_repository) dry_run_label = ' (dry run; not making any changes)' if args.dry_run else '' if args.prune: diff --git a/borgmatic/tests/unit/commands/test_borgmatic.py b/borgmatic/tests/unit/commands/test_borgmatic.py index cab0b0f3..f53728e2 100644 --- a/borgmatic/tests/unit/commands/test_borgmatic.py +++ b/borgmatic/tests/unit/commands/test_borgmatic.py @@ -1,38 +1,52 @@ -from borgmatic.commands import borgmatic -from flexmock import flexmock import json -import pytest import sys +from flexmock import flexmock +import pytest + +from borgmatic.commands import borgmatic + def test__run_commands_handles_multiple_json_outputs_in_array(): - # THEN - (flexmock(borgmatic) - .should_receive("_run_commands_on_repository") - .times(3) - .replace_with(lambda args, consistency, json_results, local_path, location, remote_path, retention, storage, - unexpanded_repository: json_results.append({"whatever": unexpanded_repository})) - ) + ( + flexmock(borgmatic) + .should_receive('_run_commands_on_repository') + .times(3) + .replace_with( + lambda args, consistency, json_results, local_path, location, remote_path, retention, + storage, + unexpanded_repository: json_results.append({"whatever": unexpanded_repository}) + ) + ) - (flexmock(sys.stdout) - .should_call("write") - .with_args(json.dumps(json.loads(''' - [ - {"whatever": "fake_repo1"}, - {"whatever": "fake_repo2"}, - {"whatever": "fake_repo3"} - ] - '''))) - ) + ( + flexmock(sys.stdout) + .should_call("write") + .with_args( + json.dumps( + json.loads( + ''' + [ + {"whatever": "fake_repo1"}, + {"whatever": "fake_repo2"}, + {"whatever": "fake_repo3"} + ] + ''', + ) + ) + ) + ) - borgmatic._run_commands(args=flexmock(json=True), - consistency=None, - local_path=None, - location={"repositories": [ - "fake_repo1", - "fake_repo2", - "fake_repo3" - ]}, - remote_path=None, - retention=None, - storage=None) + borgmatic._run_commands( + args=flexmock(json=True), + consistency=None, + local_path=None, + location={'repositories': [ + 'fake_repo1', + 'fake_repo2', + 'fake_repo3' + ]}, + remote_path=None, + retention=None, + storage=None, + ) From 9968a15ef8acdf5de6fd481c27a0c7982904e612 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 28 Jul 2018 15:21:19 -0700 Subject: [PATCH 06/12] Clarifying code style for multiline constructs. --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 75d623a5..ef822bc7 100644 --- a/README.md +++ b/README.md @@ -334,10 +334,10 @@ the following deviations from PEP 8: * For strings, prefer single quotes over double quotes. * Limit all lines to a maximum of 100 characters. * Use trailing commas within multiline values or argument lists. - * Within multiline constructs: - * Use standard four-space indentation. Don't align indentation with an opening - delimeter. - * Put opening and closing delimeters on lines separate from their contents. + * For multiline constructs, put opening and closing delimeters on lines + separate from their contents. + * Within multiline constructs, use standard four-space indentation. Don't align + indentation with an opening delimeter. ### Development From b714ffd48b2d77c8ec8ba60196d628f7ac200258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=89VEIL?= Date: Sun, 29 Jul 2018 03:17:45 +0000 Subject: [PATCH 07/12] add support for `--info --json` (#83) --- AUTHORS | 2 +- borgmatic/borg/info.py | 10 +++++++--- borgmatic/borg/list.py | 4 +++- borgmatic/commands/borgmatic.py | 16 +++++++++++++--- .../tests/integration/commands/test_borgmatic.py | 12 +++++++++++- borgmatic/tests/unit/borg/test_info.py | 13 ++++++++++++- 6 files changed, 47 insertions(+), 10 deletions(-) diff --git a/AUTHORS b/AUTHORS index 2bf82b39..32b105f5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -7,5 +7,5 @@ Michele Lazzeri: Custom archive names newtonne: Read encryption password from external file Robin `ypid` Schneider: Support additional options of Borg Scott Squires: Custom archive names -Thomas LÉVEIL: Support for a keep_minutely prune option +Thomas LÉVEIL: Support for a keep_minutely prune option. Support for the --json option Nick Whyte: Support prefix filtering for archive consistency checks diff --git a/borgmatic/borg/info.py b/borgmatic/borg/info.py index c55e3651..359da3f9 100644 --- a/borgmatic/borg/info.py +++ b/borgmatic/borg/info.py @@ -7,8 +7,9 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS logger = logging.getLogger(__name__) -def display_archives_info(verbosity, repository, storage_config, local_path='borg', - remote_path=None): +def display_archives_info( + verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False +): ''' Given a verbosity flag, a local or remote repository path, and a storage config dict, display summary information for Borg archives in the repository. @@ -19,6 +20,7 @@ def display_archives_info(verbosity, repository, storage_config, local_path='bor (local_path, 'info', repository) + (('--remote-path', remote_path) if remote_path else ()) + (('--lock-wait', str(lock_wait)) if lock_wait else ()) + + (('--json',) if json else ()) + { VERBOSITY_SOME: ('--info',), VERBOSITY_LOTS: ('--debug',), @@ -26,4 +28,6 @@ def display_archives_info(verbosity, repository, storage_config, local_path='bor ) logger.debug(' '.join(full_command)) - subprocess.check_call(full_command) + + output = subprocess.check_output(full_command) + return output.decode() if output is not None else None diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 4bc24f7d..fb11091e 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -7,7 +7,9 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS logger = logging.getLogger(__name__) -def list_archives(verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False): +def list_archives( + verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False +): ''' Given a verbosity flag, a local or remote repository path, and a storage config dict, list Borg archives in the repository. diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 3a22e4d4..d3e2d046 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -97,8 +97,13 @@ def parse_arguments(*arguments): args = parser.parse_args(arguments) - if args.json and not args.list: - raise ValueError('The --json option can only be used with the --list option') + if args.json and not (args.list or args.info): + raise ValueError('The --json option can only be used with the --list or --info options') + + if args.json and args.list and args.info: + raise ValueError( + 'With the --json option, options --list and --info cannot be used together' + ) # If any of the action flags are explicitly requested, leave them as-is. Otherwise, assume # defaults: Mutate the given arguments to enable the default actions. @@ -205,13 +210,18 @@ def _run_commands_on_repository( sys.stdout.write(output) if args.info: logger.info('{}: Displaying summary info for archives'.format(repository)) - borg_info.display_archives_info( + output = borg_info.display_archives_info( args.verbosity, repository, storage, local_path=local_path, remote_path=remote_path, + json=args.json, ) + if args.json: + json_results.append(json.loads(output)) + else: + sys.stdout.write(output) def main(): # pragma: no cover diff --git a/borgmatic/tests/integration/commands/test_borgmatic.py b/borgmatic/tests/integration/commands/test_borgmatic.py index 8cd1f76a..bd5f3e71 100644 --- a/borgmatic/tests/integration/commands/test_borgmatic.py +++ b/borgmatic/tests/integration/commands/test_borgmatic.py @@ -90,6 +90,16 @@ def test_parse_arguments_with_invalid_arguments_exits(): module.parse_arguments('--posix-me-harder') -def test_parse_arguments_with_json_flag_but_no_list_flag_raises_value_error(): +def test_parse_arguments_with_json_flag_with_list_or_info_flag_does_not_raise_any_error(): + module.parse_arguments('--list', '--json') + module.parse_arguments('--info', '--json') + + +def test_parse_arguments_with_json_flag_but_no_list_or_info_flag_raises_value_error(): with pytest.raises(ValueError): module.parse_arguments('--json') + + +def test_parse_arguments_with_json_flag_and_both_list_and_info_flag_raises_value_error(): + with pytest.raises(ValueError): + module.parse_arguments('--list', '--info', '--json') diff --git a/borgmatic/tests/unit/borg/test_info.py b/borgmatic/tests/unit/borg/test_info.py index 7a1cbbe7..43e881e2 100644 --- a/borgmatic/tests/unit/borg/test_info.py +++ b/borgmatic/tests/unit/borg/test_info.py @@ -8,7 +8,7 @@ from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS def insert_subprocess_mock(check_call_command, **kwargs): subprocess = flexmock(module.subprocess) - subprocess.should_receive('check_call').with_args(check_call_command, **kwargs).once() + subprocess.should_receive('check_output').with_args(check_call_command, **kwargs).once() INFO_COMMAND = ('borg', 'info', 'repo') @@ -44,6 +44,17 @@ def test_display_archives_info_with_verbosity_lots_calls_borg_with_debug_paramet ) +def test_display_archives_info_with_json_calls_borg_with_json_parameter(): + insert_subprocess_mock(INFO_COMMAND + ('--json',)) + + module.display_archives_info( + verbosity=None, + repository='repo', + storage_config={}, + json=True, + ) + + def test_display_archives_info_with_local_path_calls_borg_via_local_path(): insert_subprocess_mock(('borg1',) + INFO_COMMAND[1:]) From 282e9565c9166b0395d8953110ca073052a09e72 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 28 Jul 2018 20:24:19 -0700 Subject: [PATCH 08/12] Mentioning new --info --json option in NEWS. --- NEWS | 4 ++-- borgmatic/borg/info.py | 2 +- borgmatic/borg/list.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 820f793a..3cc473c9 100644 --- a/NEWS +++ b/NEWS @@ -2,8 +2,8 @@ * Skip before/after backup hooks when only doing --prune, --check, --list, and/or --info. * #71: Support for XDG_CONFIG_HOME environment variable for specifying alternate user ~/.config/ path. - * #74: Support for Borg --list --json options via borgmatic command-line to list all archives in - JSON format, ideal for programmatic consumption. + * #74, #83: Support for Borg --json option via borgmatic command-line to --list archives or show + archive --info in JSON format, ideal for programmatic consumption. * #38, #76: Upgrade ruamel.yaml compatibility version range and fix support for Python 3.7. * #77: Skip non-"*.yaml" config filenames in /etc/borgmatic.d/ so as not to parse backup files, editor swap files, etc. diff --git a/borgmatic/borg/info.py b/borgmatic/borg/info.py index 359da3f9..b7417a80 100644 --- a/borgmatic/borg/info.py +++ b/borgmatic/borg/info.py @@ -8,7 +8,7 @@ logger = logging.getLogger(__name__) def display_archives_info( - verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False + verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False ): ''' Given a verbosity flag, a local or remote repository path, and a storage config dict, diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index fb11091e..30427814 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -8,7 +8,7 @@ logger = logging.getLogger(__name__) def list_archives( - verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False + verbosity, repository, storage_config, local_path='borg', remote_path=None, json=False ): ''' Given a verbosity flag, a local or remote repository path, and a storage config dict, From 8e5b0bbf177ba0974cc3d49d70596c73f2771609 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 28 Jul 2018 20:27:18 -0700 Subject: [PATCH 09/12] Remove errant ctrl-F character from docs. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ef822bc7..bade4e8e 100644 --- a/README.md +++ b/README.md @@ -355,7 +355,7 @@ git clone ssh://git@projects.torsion.org:3022/witten/borgmatic.git ``` Then, install borgmatic -"[editable](https://pip.pypa.io/en/stable/reference/pip_install/#editable-installs)" +"[editable](https://pip.pypa.io/en/stable/reference/pip_install/#editable-installs)" so that you can easily run borgmatic commands while you're hacking on them to make sure your changes work. From 27f8a1df046bf9064764583eab97e1a55336766e Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 28 Jul 2018 20:29:55 -0700 Subject: [PATCH 10/12] Switch to non-raw link to sample cron job. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bade4e8e..ce60fc26 100644 --- a/README.md +++ b/README.md @@ -275,7 +275,7 @@ configure a job runner to invoke it periodically. ### cron If you're using cron, download the [sample cron -file](https://projects.torsion.org/witten/borgmatic/raw/master/sample/cron/borgmatic). +file](https://projects.torsion.org/witten/borgmatic/src/master/sample/cron/borgmatic). Then, from the directory where you downloaded it: ```bash From 3afa5ac76d5da3bfe34b46a57bb494d219fe1299 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 28 Jul 2018 22:22:25 -0700 Subject: [PATCH 11/12] Document hooks (#81). --- NEWS | 1 + README.md | 27 ++++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 3cc473c9..a00e8835 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ * #38, #76: Upgrade ruamel.yaml compatibility version range and fix support for Python 3.7. * #77: Skip non-"*.yaml" config filenames in /etc/borgmatic.d/ so as not to parse backup files, editor swap files, etc. + * #81: Document user-defined hooks run before/after backup, or on error. * Add code style guidelines to the documention. 1.2.0 diff --git a/README.md b/README.md index ce60fc26..ab95e016 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,31 @@ configuration paths on the command-line with borgmatic's `--config` option. See `borgmatic --help` for more information. +### Hooks + +If you find yourself performing prepraration tasks before your backup runs, or +cleanup work afterwards, borgmatic hooks may be of interest. They're simply +shell commands that borgmatic executes for you at various points, and they're +configured in the `hooks` section of your configuration file. + +For instance, you can specify `before_backup` hooks to dump a database to file +before backing it up, and specify `after_backup` hooks to delete the temporary +file afterwards. + +borgmatic hooks run once per configuration file. `before_backup` hooks run +prior to backups of all repositories. `after_backup` hooks run afterwards, but +not if an error occurs in previous hook or in the backups themselves. And +borgmatic runs `on_error` hooks if an error occurs. + +An important security note about hooks: borgmatic executes all hook commands +with the user permissions of borgmatic itself. So to prevent potential shell +injection or privilege escalation, do not forget to set secure permissions +(chmod 0700) on borgmatic configuration files and scripts invoked by hooks. + +See the sample generated configuration file mentioned above for specifics +about hook configuration syntax. + + ## Upgrading In general, all you should need to do to upgrade borgmatic is run the @@ -329,7 +354,7 @@ your thing. In general, contributions are very welcome. We don't bite! ### Code style Start with [PEP 8](https://www.python.org/dev/peps/pep-0008/). But then, apply -the following deviations from PEP 8: +the following deviations from it: * For strings, prefer single quotes over double quotes. * Limit all lines to a maximum of 100 characters. From 4644f613b25166fe39cc16e77797ecc1e402a210 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 28 Jul 2018 22:24:24 -0700 Subject: [PATCH 12/12] Fix typo in README. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ab95e016..3434da99 100644 --- a/README.md +++ b/README.md @@ -151,7 +151,7 @@ file afterwards. borgmatic hooks run once per configuration file. `before_backup` hooks run prior to backups of all repositories. `after_backup` hooks run afterwards, but -not if an error occurs in previous hook or in the backups themselves. And +not if an error occurs in a previous hook or in the backups themselves. And borgmatic runs `on_error` hooks if an error occurs. An important security note about hooks: borgmatic executes all hook commands