Fix "Unrecognized argument" error when the same value is with different command-line flags (#881).

NEWS

@@ -6,6 +6,7 @@
    of files at once.
  * #874: Add the configured repository label as "repository_label" to the interpolated variables
    passed to before/after command hooks.
+ * #881: Fix "Unrecognized argument" error when the same value is with different command-line flags.
  * In the "spot" check, don't try to hash symlinked directories.
 
 1.8.11

borgmatic/commands/arguments.py

@@ -113,6 +113,54 @@ def parse_and_record_action_arguments(
     return tuple(argument for argument in remaining if argument != action_name)
 
 
+def argument_is_flag(argument):
+    '''
+    Return True if the given argument looks like a flag, e.g. '--some-flag', as opposed to a
+    non-flag value.
+    '''
+    return isinstance(argument, str) and argument.startswith('--')
+
+
+def group_arguments_with_values(arguments):
+    '''
+    Given a sequence of arguments, return a sequence of tuples where each one contains either a
+    single argument (such as for a stand-alone flag) or a flag argument and its corresponding value.
+
+    For instance, given the following arguments sequence as input:
+
+      ('--foo', '--bar', '33', '--baz')
+
+    ... return the following output:
+
+      (('--foo',), ('--bar', '33'), ('--baz',))
+    '''
+    grouped_arguments = []
+    index = 0
+
+    while index < len(arguments):
+        this_argument = arguments[index]
+
+        try:
+            next_argument = arguments[index + 1]
+        except IndexError:
+            grouped_arguments.append((this_argument,))
+            break
+
+        if (
+            argument_is_flag(this_argument)
+            and not argument_is_flag(next_argument)
+            and next_argument not in ACTION_ALIASES
+        ):
+            grouped_arguments.append((this_argument, next_argument))
+            index += 2
+            continue
+
+        grouped_arguments.append((this_argument,))
+        index += 1
+
+    return tuple(grouped_arguments)
+
+
 def get_unparsable_arguments(remaining_action_arguments):
     '''
     Given a sequence of argument tuples (one per action parser that parsed arguments), determine the
@@ -121,12 +169,21 @@ def get_unparsable_arguments(remaining_action_arguments):
     if not remaining_action_arguments:
         return ()
 
+    grouped_action_arguments = tuple(
+        group_arguments_with_values(action_arguments)
+        for action_arguments in remaining_action_arguments
+    )
+
     return tuple(
-        argument
-        for argument in dict.fromkeys(
-            itertools.chain.from_iterable(remaining_action_arguments)
-        ).keys()
-        if all(argument in action_arguments for action_arguments in remaining_action_arguments)
+        itertools.chain.from_iterable(
+            argument_group
+            for argument_group in dict.fromkeys(
+                itertools.chain.from_iterable(grouped_action_arguments)
+            ).keys()
+            if all(
+                argument_group in action_arguments for action_arguments in grouped_action_arguments
+            )
+        )
     )
 
 

borgmatic/logger.py

@@ -89,7 +89,7 @@ class Multi_stream_handler(logging.Handler):
 
 
 class Console_no_color_formatter(logging.Formatter):
-    def format(self, record):
+    def format(self, record):  # pragma: no cover
         return record.msg
 
 

tests/unit/commands/test_arguments.py

@@ -130,36 +130,175 @@ def test_parse_and_record_action_arguments_with_borg_action_consumes_arguments_a
     assert borg_parsed_arguments.options == ('list',)
 
 
+@pytest.mark.parametrize(
+    'argument, expected',
+    [
+        ('--foo', True),
+        ('foo', False),
+        (33, False),
+    ],
+)
+def test_argument_is_flag_only_for_string_starting_with_double_dash(argument, expected):
+    assert module.argument_is_flag(argument) == expected
+
+
 @pytest.mark.parametrize(
     'arguments, expected',
     [
-        # A global flag remaining from each parsed action.
+        # Ending with a valueless flag.
         (
+            ('--foo', '--bar', 33, '--baz'),
             (
-                ('--latest', 'archive', 'prune', 'extract', 'list', '--test-flag'),
-                ('--latest', 'archive', 'check', 'extract', 'list', '--test-flag'),
-                ('prune', 'check', 'list', '--test-flag'),
-                ('prune', 'check', 'extract', '--test-flag'),
+                ('--foo',),
+                ('--bar', 33),
+                ('--baz',),
             ),
-            ('--test-flag',),
         ),
-        # No global flags remaining.
+        # Ending with a flag and its corresponding value.
+        (
+            ('--foo', '--bar', 33, '--baz', '--quux', 'thing'),
+            (('--foo',), ('--bar', 33), ('--baz',), ('--quux', 'thing')),
+        ),
+        # Starting with an action name.
         (
+            ('check', '--foo', '--bar', 33, '--baz'),
             (
-                ('--latest', 'archive', 'prune', 'extract', 'list'),
-                ('--latest', 'archive', 'check', 'extract', 'list'),
+                ('check',),
+                ('--foo',),
+                ('--bar', 33),
+                ('--baz',),
+            ),
+        ),
+        # Action name that one could mistake for a flag value.
+        (('--progress', 'list'), (('--progress',), ('list',))),
+        # No arguments.
+        ((), ()),
+    ],
+)
+def test_group_arguments_with_values_returns_flags_with_corresponding_values(arguments, expected):
+    flexmock(module).should_receive('argument_is_flag').with_args('--foo').and_return(True)
+    flexmock(module).should_receive('argument_is_flag').with_args('--bar').and_return(True)
+    flexmock(module).should_receive('argument_is_flag').with_args('--baz').and_return(True)
+    flexmock(module).should_receive('argument_is_flag').with_args('--quux').and_return(True)
+    flexmock(module).should_receive('argument_is_flag').with_args('--progress').and_return(True)
+    flexmock(module).should_receive('argument_is_flag').with_args(33).and_return(False)
+    flexmock(module).should_receive('argument_is_flag').with_args('thing').and_return(False)
+    flexmock(module).should_receive('argument_is_flag').with_args('check').and_return(False)
+    flexmock(module).should_receive('argument_is_flag').with_args('list').and_return(False)
+
+    assert module.group_arguments_with_values(arguments) == expected
+
+
+@pytest.mark.parametrize(
+    'arguments, grouped_arguments, expected',
+    [
+        # An unparsable flag remaining from each parsed action.
+        (
+            (
+                ('--latest', 'archive', 'prune', 'extract', 'list', '--flag'),
+                ('--latest', 'archive', 'check', 'extract', 'list', '--flag'),
+                ('prune', 'check', 'list', '--flag'),
+                ('prune', 'check', 'extract', '--flag'),
+            ),
+            (
+                (
+                    ('--latest',),
+                    ('archive',),
+                    ('prune',),
+                    ('extract',),
+                    ('list',),
+                    ('--flag',),
+                ),
+                (
+                    ('--latest',),
+                    ('archive',),
+                    ('check',),
+                    ('extract',),
+                    ('list',),
+                    ('--flag',),
+                ),
+                (('prune',), ('check',), ('list',), ('--flag',)),
+                (('prune',), ('check',), ('extract',), ('--flag',)),
+            ),
+            ('--flag',),
+        ),
+        # No unparsable flags remaining.
+        (
+            (
+                ('--archive', 'archive', 'prune', 'extract', 'list'),
+                ('--archive', 'archive', 'check', 'extract', 'list'),
                 ('prune', 'check', 'list'),
                 ('prune', 'check', 'extract'),
             ),
+            (
+                (
+                    (
+                        '--archive',
+                        'archive',
+                    ),
+                    ('prune',),
+                    ('extract',),
+                    ('list',),
+                ),
+                (
+                    (
+                        '--archive',
+                        'archive',
+                    ),
+                    ('check',),
+                    ('extract',),
+                    ('list',),
+                ),
+                (('prune',), ('check',), ('list',)),
+                (('prune',), ('check',), ('extract',)),
+            ),
+            (),
+        ),
+        # No unparsable flags remaining, but some values in common.
+        (
+            (
+                ('--verbosity', '5', 'archive', 'prune', 'extract', 'list'),
+                ('--last', '5', 'archive', 'check', 'extract', 'list'),
+                ('prune', 'check', 'list', '--last', '5'),
+                ('prune', 'check', '--verbosity', '5', 'extract'),
+            ),
+            (
+                (('--verbosity', '5'), ('archive',), ('prune',), ('extract',), ('list',)),
+                (
+                    (
+                        '--last',
+                        '5',
+                    ),
+                    ('archive',),
+                    ('check',),
+                    ('extract',),
+                    ('list',),
+                ),
+                (('prune',), ('check',), ('list',), ('--last', '5')),
+                (
+                    ('prune',),
+                    ('check',),
+                    (
+                        '--verbosity',
+                        '5',
+                    ),
+                    ('extract',),
+                ),
+            ),
             (),
         ),
         # No flags.
-        ((), ()),
+        ((), (), ()),
     ],
 )
 def test_get_unparsable_arguments_returns_remaining_arguments_that_no_action_can_parse(
-    arguments, expected
+    arguments, grouped_arguments, expected
 ):
+    for action_arguments, grouped_action_arguments in zip(arguments, grouped_arguments):
+        flexmock(module).should_receive('group_arguments_with_values').with_args(
+            action_arguments
+        ).and_return(grouped_action_arguments)
+
     assert module.get_unparsable_arguments(arguments) == expected