Merge pull request #2508 from enkore/f/proper-common-options

Support common options on the main command

docs/man_intro.rst

@@ -14,7 +14,7 @@ deduplicating and encrypting backup tool
 SYNOPSIS
 --------
 
-borg <command> [options] [arguments]
+borg [common options] <command> [options] [arguments]
 
 DESCRIPTION
 -----------

docs/usage.rst

@@ -19,7 +19,7 @@ In case you are interested in more details (like formulas), please see
 :ref:`json_output`.
 
 Common options
-++++++++++++++
+~~~~~~~~~~~~~~
 
 All |project_name| commands share these options:
 

setup.py

@@ -259,7 +259,7 @@ class build_usage(Command):
                               "command_": command.replace(' ', '_'),
                               "underline": '-' * len('borg ' + command)}
                     doc.write(".. _borg_{command_}:\n\n".format(**params))
-                    doc.write("borg {command}\n{underline}\n::\n\n    borg {command}".format(**params))
+                    doc.write("borg {command}\n{underline}\n::\n\n    borg [common options] {command}".format(**params))
                     self.write_usage(parser, doc)
                     epilog = parser.epilog
                     parser.epilog = None
@@ -402,10 +402,10 @@ class build_man(Command):
             if is_intermediary:
                 subparsers = [action for action in parser._actions if 'SubParsersAction' in str(action.__class__)][0]
                 for subcommand in subparsers.choices:
-                    write('| borg', command, subcommand, '...')
+                    write('| borg', '[common options]', command, subcommand, '...')
                     self.see_also.setdefault(command, []).append('%s-%s' % (command, subcommand))
             else:
-                write('borg', command, end='')
+                write('borg', '[common options]', command, end='')
                 self.write_usage(write, parser)
             write('\n')
 

src/borg/archiver.py

@@ -1870,6 +1870,132 @@ class Archiver:
                     print(warning, file=sys.stderr)
         return args
 
+    class CommonOptions:
+        """
+        Support class to allow specifying common options directly after the top-level command.
+
+        Normally options can only be specified on the parser defining them, which means
+        that generally speaking *all* options go after all sub-commands. This is annoying
+        for common options in scripts, e.g. --remote-path or logging options.
+
+        This class allows adding the same set of options to both the top-level parser
+        and the final sub-command parsers (but not intermediary sub-commands, at least for now).
+
+        It does so by giving every option's target name ("dest") a suffix indicating its level
+        -- no two options in the parser hierarchy can have the same target --
+        then, after parsing the command line, multiple definitions are resolved.
+
+        Defaults are handled by only setting them on the top-level parser and setting
+        a sentinel object in all sub-parsers, which then allows to discern which parser
+        supplied the option.
+        """
+
+        def __init__(self, define_common_options, suffix_precedence):
+            """
+            *define_common_options* should be a callable taking one argument, which
+            will be a argparse.Parser.add_argument-like function.
+
+            *define_common_options* will be called multiple times, and should call
+            the passed function to define common options exactly the same way each time.
+
+            *suffix_precedence* should be a tuple of the suffixes that will be used.
+            It is ordered from lowest precedence to highest precedence:
+            An option specified on the parser belonging to index 0 is overridden if the
+            same option is specified on any parser with a higher index.
+            """
+            self.define_common_options = define_common_options
+            self.suffix_precedence = suffix_precedence
+
+            # Maps suffixes to sets of target names.
+            # E.g. common_options["_subcommand"] = {..., "log_level", ...}
+            self.common_options = dict()
+            # Set of options with the 'append' action.
+            self.append_options = set()
+            # This is the sentinel object that replaces all default values in parsers
+            # below the top-level parser.
+            self.default_sentinel = object()
+
+        def add_common_group(self, parser, suffix, provide_defaults=False):
+            """
+            Add common options to *parser*.
+
+            *provide_defaults* must only be True exactly once in a parser hierarchy,
+            at the top level, and False on all lower levels. The default is chosen
+            accordingly.
+
+            *suffix* indicates the suffix to use internally. It also indicates
+            which precedence the *parser* has for common options. See *suffix_precedence*
+            of __init__.
+            """
+            assert suffix in self.suffix_precedence
+
+            def add_argument(*args, **kwargs):
+                if 'dest' in kwargs:
+                    kwargs.setdefault('action', 'store')
+                    assert kwargs['action'] in ('help', 'store_const', 'store_true', 'store_false', 'store', 'append')
+                    is_append = kwargs['action'] == 'append'
+                    if is_append:
+                        self.append_options.add(kwargs['dest'])
+                        assert kwargs['default'] == [], 'The default is explicitly constructed as an empty list in resolve()'
+                    else:
+                        self.common_options.setdefault(suffix, set()).add(kwargs['dest'])
+                    kwargs['dest'] += suffix
+                    if not provide_defaults:
+                        # Interpolate help now, in case the %(default)d (or so) is mentioned,
+                        # to avoid producing incorrect help output.
+                        # Assumption: Interpolated output can safely be interpolated again,
+                        # which should always be the case.
+                        # Note: We control all inputs.
+                        kwargs['help'] = kwargs['help'] % kwargs
+                        if not is_append:
+                            kwargs['default'] = self.default_sentinel
+
+                common_group.add_argument(*args, **kwargs)
+
+            common_group = parser.add_argument_group('Common options')
+            self.define_common_options(add_argument)
+
+        def resolve(self, args: argparse.Namespace):  # Namespace has "in" but otherwise is not like a dict.
+            """
+            Resolve the multiple definitions of each common option to the final value.
+            """
+            for suffix in self.suffix_precedence:
+                # From highest level to lowest level, so the "most-specific" option wins, e.g.
+                # "borg --debug create --info" shall result in --info being effective.
+                for dest in self.common_options.get(suffix, []):
+                    # map_from is this suffix' option name, e.g. log_level_subcommand
+                    # map_to is the target name, e.g. log_level
+                    map_from = dest + suffix
+                    map_to = dest
+                    # Retrieve value; depending on the action it may not exist, but usually does
+                    # (store_const/store_true/store_false), either because the action implied a default
+                    # or a default is explicitly supplied.
+                    # Note that defaults on lower levels are replaced with default_sentinel.
+                    # Only the top level has defaults.
+                    value = getattr(args, map_from, self.default_sentinel)
+                    if value is not self.default_sentinel:
+                        # value was indeed specified on this level. Transfer value to target,
+                        # and un-clobber the args (for tidiness - you *cannot* use the suffixed
+                        # names for other purposes, obviously).
+                        setattr(args, map_to, value)
+                    try:
+                        delattr(args, map_from)
+                    except AttributeError:
+                        pass
+
+            # Options with an "append" action need some special treatment. Instead of
+            # overriding values, all specified values are merged together.
+            for dest in self.append_options:
+                option_value = []
+                for suffix in self.suffix_precedence:
+                    # Find values of this suffix, if any, and add them to the final list
+                    extend_from = dest + suffix
+                    if extend_from in args:
+                        values = getattr(args, extend_from)
+                        delattr(args, extend_from)
+                        option_value.extend(values)
+                setattr(args, dest, option_value)
+
     def build_parser(self):
         def process_epilog(epilog):
             epilog = textwrap.dedent(epilog).splitlines()
@@ -1881,58 +2007,67 @@ class Archiver:
                 epilog = [line for line in epilog if not line.startswith('.. man')]
             return '\n'.join(epilog)
 
-        common_parser = argparse.ArgumentParser(add_help=False, prog=self.prog)
+        def define_common_options(add_common_option):
+            add_common_option('-h', '--help', action='help', help='show this help message and exit')
+            add_common_option('--critical', dest='log_level',
+                              action='store_const', const='critical', default='warning',
+                              help='work on log level CRITICAL')
+            add_common_option('--error', dest='log_level',
+                              action='store_const', const='error', default='warning',
+                              help='work on log level ERROR')
+            add_common_option('--warning', dest='log_level',
+                              action='store_const', const='warning', default='warning',
+                              help='work on log level WARNING (default)')
+            add_common_option('--info', '-v', '--verbose', dest='log_level',
+                              action='store_const', const='info', default='warning',
+                              help='work on log level INFO')
+            add_common_option('--debug', dest='log_level',
+                              action='store_const', const='debug', default='warning',
+                              help='enable debug output, work on log level DEBUG')
+            add_common_option('--debug-topic', dest='debug_topics',
+                              action='append', metavar='TOPIC', default=[],
+                              help='enable TOPIC debugging (can be specified multiple times). '
+                                   'The logger path is borg.debug.<TOPIC> if TOPIC is not fully qualified.')
+            add_common_option('-p', '--progress', dest='progress', action='store_true',
+                              help='show progress information')
+            add_common_option('--log-json', dest='log_json', action='store_true',
+                              help='Output one JSON object per log line instead of formatted text.')
+            add_common_option('--lock-wait', dest='lock_wait', type=int, metavar='N', default=1,
+                              help='wait for the lock, but max. N seconds (default: %(default)d).')
+            add_common_option('--show-version', dest='show_version', action='store_true', default=False,
+                              help='show/log the borg version')
+            add_common_option('--show-rc', dest='show_rc', action='store_true', default=False,
+                              help='show/log the return code (rc)')
+            add_common_option('--no-files-cache', dest='cache_files', action='store_false',
+                              help='do not load/update the file metadata cache used to detect unchanged files')
+            add_common_option('--umask', dest='umask', type=lambda s: int(s, 8), default=UMASK_DEFAULT, metavar='M',
+                              help='set umask to M (local and remote, default: %(default)04o)')
+            add_common_option('--remote-path', dest='remote_path', metavar='PATH',
+                              help='use PATH as borg executable on the remote (default: "borg")')
+            add_common_option('--remote-ratelimit', dest='remote_ratelimit', type=int, metavar='rate',
+                              help='set remote network upload rate limit in kiByte/s (default: 0=unlimited)')
+            add_common_option('--consider-part-files', dest='consider_part_files',
+                              action='store_true', default=False,
+                              help='treat part files like normal files (e.g. to list/extract them)')
 
-        common_group = common_parser.add_argument_group('Common options')
-        common_group.add_argument('-h', '--help', action='help', help='show this help message and exit')
-        common_group.add_argument('--critical', dest='log_level',
-                                  action='store_const', const='critical', default='warning',
-                                  help='work on log level CRITICAL')
-        common_group.add_argument('--error', dest='log_level',
-                                  action='store_const', const='error', default='warning',
-                                  help='work on log level ERROR')
-        common_group.add_argument('--warning', dest='log_level',
-                                  action='store_const', const='warning', default='warning',
-                                  help='work on log level WARNING (default)')
-        common_group.add_argument('--info', '-v', '--verbose', dest='log_level',
-                                  action='store_const', const='info', default='warning',
-                                  help='work on log level INFO')
-        common_group.add_argument('--debug', dest='log_level',
-                                  action='store_const', const='debug', default='warning',
-                                  help='enable debug output, work on log level DEBUG')
-        common_group.add_argument('--debug-topic', dest='debug_topics',
-                                  action='append', metavar='TOPIC', default=[],
-                                  help='enable TOPIC debugging (can be specified multiple times). '
-                                       'The logger path is borg.debug.<TOPIC> if TOPIC is not fully qualified.')
-        common_group.add_argument('-p', '--progress', dest='progress', action='store_true',
-                                  help='show progress information')
-        common_group.add_argument('--log-json', dest='log_json', action='store_true',
-                                  help='Output one JSON object per log line instead of formatted text.')
-        common_group.add_argument('--lock-wait', dest='lock_wait', type=int, metavar='N', default=1,
-                                  help='wait for the lock, but max. N seconds (default: %(default)d).')
-        common_group.add_argument('--show-version', dest='show_version', action='store_true', default=False,
-                                  help='show/log the borg version')
-        common_group.add_argument('--show-rc', dest='show_rc', action='store_true', default=False,
-                                  help='show/log the return code (rc)')
-        common_group.add_argument('--no-files-cache', dest='cache_files', action='store_false',
-                                  help='do not load/update the file metadata cache used to detect unchanged files')
-        common_group.add_argument('--umask', dest='umask', type=lambda s: int(s, 8), default=UMASK_DEFAULT, metavar='M',
-                                  help='set umask to M (local and remote, default: %(default)04o)')
-        common_group.add_argument('--remote-path', dest='remote_path', metavar='PATH',
-                                  help='use PATH as borg executable on the remote (default: "borg")')
-        common_group.add_argument('--remote-ratelimit', dest='remote_ratelimit', type=int, metavar='rate',
-                                  help='set remote network upload rate limit in kiByte/s (default: 0=unlimited)')
-        common_group.add_argument('--consider-part-files', dest='consider_part_files',
-                                  action='store_true', default=False,
-                                  help='treat part files like normal files (e.g. to list/extract them)')
-
-        parser = argparse.ArgumentParser(prog=self.prog, description='Borg - Deduplicated Backups')
+        parser = argparse.ArgumentParser(prog=self.prog, description='Borg - Deduplicated Backups',
+                                         add_help=False)
+        parser.common_options = self.CommonOptions(define_common_options,
+                                                   suffix_precedence=('_maincommand', '_midcommand', '_subcommand'))
         parser.add_argument('-V', '--version', action='version', version='%(prog)s ' + __version__,
                             help='show version number and exit')
-        subparsers = parser.add_subparsers(title='required arguments', metavar='<command>')
+        parser.common_options.add_common_group(parser, '_maincommand', provide_defaults=True)
 
+        common_parser = argparse.ArgumentParser(add_help=False, prog=self.prog)
         # some empty defaults for all subparsers
         common_parser.set_defaults(paths=[], patterns=[])
+        parser.common_options.add_common_group(common_parser, '_subcommand')
+
+        mid_common_parser = argparse.ArgumentParser(add_help=False, prog=self.prog)
+        mid_common_parser.set_defaults(paths=[], patterns=[])
+        parser.common_options.add_common_group(mid_common_parser, '_midcommand')
+
+        subparsers = parser.add_subparsers(title='required arguments', metavar='<command>')
 
         serve_epilog = process_epilog("""
         This command starts a repository server process. This command is usually not used manually.
@@ -1953,7 +2088,7 @@ class Archiver:
         This command initializes an empty repository. A repository is a filesystem
         directory containing the deduplicated data from zero or more archives.
 
-        Encryption can be enabled at repository init time.
+        Encryption can be enabled at repository init time. It cannot be changed later.
 
         It is not recommended to work without encryption. Repository encryption protects
         you e.g. against the case that an attacker has access to your backup repository.
@@ -2008,8 +2143,8 @@ class Archiver:
 
         `authenticated` mode uses no encryption, but authenticates repository contents
         through the same keyed BLAKE2b-256 hash as the other blake2 modes (it uses it
-        as chunk ID hash). The key is stored like repokey.
-        This mode is new and not compatible with borg 1.0.x.
+        as the chunk ID hash). The key is stored like repokey.
+        This mode is new and *not* compatible with borg 1.0.x.
 
         `none` mode uses no encryption and no authentication. It uses sha256 as chunk
         ID hash. Not recommended, rather consider using an authenticated or
@@ -2035,7 +2170,7 @@ class Archiver:
                                help='repository to create')
         subparser.add_argument('-e', '--encryption', dest='encryption', required=True,
                                choices=('none', 'keyfile', 'repokey', 'keyfile-blake2', 'repokey-blake2', 'authenticated'),
-                               help='select encryption key mode')
+                               help='select encryption key mode **(required)**')
         subparser.add_argument('-a', '--append-only', dest='append_only', action='store_true',
                                help='create an append-only mode repository')
 
@@ -2113,7 +2248,7 @@ class Archiver:
                                help='work slower, but using less space')
         self.add_archives_filters_args(subparser)
 
-        subparser = subparsers.add_parser('key', parents=[common_parser], add_help=False,
+        subparser = subparsers.add_parser('key', parents=[mid_common_parser], add_help=False,
                                           description="Manage a keyfile or repokey of a repository",
                                           epilog="",
                                           formatter_class=argparse.RawDescriptionHelpFormatter,
@@ -3113,7 +3248,7 @@ class Archiver:
         in case you ever run into some severe malfunction. Use them only if you know
         what you are doing or if a trusted developer tells you what to do.""")
 
-        subparser = subparsers.add_parser('debug', parents=[common_parser], add_help=False,
+        subparser = subparsers.add_parser('debug', parents=[mid_common_parser], add_help=False,
                                           description='debugging command (not intended for normal use)',
                                           epilog=debug_epilog,
                                           formatter_class=argparse.RawDescriptionHelpFormatter,
@@ -3254,7 +3389,7 @@ class Archiver:
 
         benchmark_epilog = process_epilog("These commands do various benchmarks.")
 
-        subparser = subparsers.add_parser('benchmark', parents=[common_parser], add_help=False,
+        subparser = subparsers.add_parser('benchmark', parents=[mid_common_parser], add_help=False,
                                           description='benchmark command',
                                           epilog=benchmark_epilog,
                                           formatter_class=argparse.RawDescriptionHelpFormatter,
@@ -3358,6 +3493,7 @@ class Archiver:
             args = self.preprocess_args(args)
         parser = self.build_parser()
         args = parser.parse_args(args or ['-h'])
+        parser.common_options.resolve(args)
         # This works around http://bugs.python.org/issue9351
         func = getattr(args, 'func', None) or getattr(args, 'fallback_func')
         if func == self.do_create and not args.paths:

src/borg/testsuite/archiver.py

@@ -1,3 +1,4 @@
+import argparse
 import errno
 import json
 import logging
@@ -1680,6 +1681,12 @@ class ArchiverTestCase(ArchiverTestCaseBase):
         assert log_message['name'].startswith('borg.')
         assert isinstance(log_message['message'], str)
 
+    def test_common_options(self):
+        self.create_test_files()
+        self.cmd('init', '--encryption=repokey', self.repository_location)
+        log = self.cmd('--debug', 'create', self.repository_location + '::test', 'input')
+        assert 'security: read previous_location' in log
+
     def _get_sizes(self, compression, compressible, size=10000):
         if compressible:
             contents = b'X' * size
@@ -2989,3 +2996,116 @@ class TestBuildFilter:
         assert not filter(Item(path='shallow/'))  # can this even happen? paths are normalized...
         assert filter(Item(path='deep enough/file'))
         assert filter(Item(path='something/dir/file'))
+
+
+class TestCommonOptions:
+    @staticmethod
+    def define_common_options(add_common_option):
+        add_common_option('-h', '--help', action='help', help='show this help message and exit')
+        add_common_option('--critical', dest='log_level', help='foo',
+                          action='store_const', const='critical', default='warning')
+        add_common_option('--error', dest='log_level', help='foo',
+                          action='store_const', const='error', default='warning')
+        add_common_option('--append', dest='append', help='foo',
+                          action='append', metavar='TOPIC', default=[])
+        add_common_option('-p', '--progress', dest='progress', action='store_true', help='foo')
+        add_common_option('--lock-wait', dest='lock_wait', type=int, metavar='N', default=1,
+                          help='(default: %(default)d).')
+        add_common_option('--no-files-cache', dest='no_files_cache', action='store_false', help='foo')
+
+    @pytest.fixture
+    def basic_parser(self):
+        parser = argparse.ArgumentParser(prog='test', description='test parser', add_help=False)
+        parser.common_options = Archiver.CommonOptions(self.define_common_options,
+                                                       suffix_precedence=('_level0', '_level1'))
+        return parser
+
+    @pytest.fixture
+    def subparsers(self, basic_parser):
+        return basic_parser.add_subparsers(title='required arguments', metavar='<command>')
+
+    @pytest.fixture
+    def parser(self, basic_parser):
+        basic_parser.common_options.add_common_group(basic_parser, '_level0', provide_defaults=True)
+        return basic_parser
+
+    @pytest.fixture
+    def common_parser(self, parser):
+        common_parser = argparse.ArgumentParser(add_help=False, prog='test')
+        parser.common_options.add_common_group(common_parser, '_level1')
+        return common_parser
+
+    @pytest.fixture
+    def parse_vars_from_line(self, parser, subparsers, common_parser):
+        subparser = subparsers.add_parser('subcommand', parents=[common_parser], add_help=False,
+                                          description='foo', epilog='bar', help='baz',
+                                          formatter_class=argparse.RawDescriptionHelpFormatter)
+        subparser.set_defaults(func=1234)
+        subparser.add_argument('--append-only', dest='append_only', action='store_true')
+
+        def parse_vars_from_line(*line):
+            print(line)
+            args = parser.parse_args(line)
+            parser.common_options.resolve(args)
+            return vars(args)
+
+        return parse_vars_from_line
+
+    def test_simple(self, parse_vars_from_line):
+        assert parse_vars_from_line('--error') == {
+            'no_files_cache': True,
+            'append': [],
+            'lock_wait': 1,
+            'log_level': 'error',
+            'progress': False
+        }
+
+        assert parse_vars_from_line('--error', 'subcommand', '--critical') == {
+            'no_files_cache': True,
+            'append': [],
+            'lock_wait': 1,
+            'log_level': 'critical',
+            'progress': False,
+            'append_only': False,
+            'func': 1234,
+        }
+
+        with pytest.raises(SystemExit):
+            parse_vars_from_line('--append-only', 'subcommand')
+
+        assert parse_vars_from_line('--append=foo', '--append', 'bar', 'subcommand', '--append', 'baz') == {
+            'no_files_cache': True,
+            'append': ['foo', 'bar', 'baz'],
+            'lock_wait': 1,
+            'log_level': 'warning',
+            'progress': False,
+            'append_only': False,
+            'func': 1234,
+        }
+
+    @pytest.mark.parametrize('position', ('before', 'after', 'both'))
+    @pytest.mark.parametrize('flag,args_key,args_value', (
+        ('-p', 'progress', True),
+        ('--lock-wait=3', 'lock_wait', 3),
+        ('--no-files-cache', 'no_files_cache', False),
+    ))
+    def test_flag_position_independence(self, parse_vars_from_line, position, flag, args_key, args_value):
+        line = []
+        if position in ('before', 'both'):
+            line.append(flag)
+        line.append('subcommand')
+        if position in ('after', 'both'):
+            line.append(flag)
+
+        result = {
+            'no_files_cache': True,
+            'append': [],
+            'lock_wait': 1,
+            'log_level': 'warning',
+            'progress': False,
+            'append_only': False,
+            'func': 1234,
+        }
+        result[args_key] = args_value
+
+        assert parse_vars_from_line(*line) == result