forall: fix crash with no command

When callback= is used, optparse does not automatically initialize
The destination when a dest= is not specified.  Refine the test to
allow dest= options when callback= is used even when it seems like
it is otherwise redundant.

Bug: b/436611422
Change-Id: I5185f95cb857ca6d37357cac77fb117a83db9c0c
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/509861
Tested-by: Mike Frysinger <vapier@google.com>
Commit-Queue: Mike Frysinger <vapier@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>

2 files changed, 8 insertions, 2 deletions

diff --git a/subcmds/forall.py b/subcmds/forall.py
index 4bae46af..57a41fc1 100644
--- a/subcmds/forall.py
+++ b/subcmds/forall.py
@@ -133,7 +133,7 @@ without iterating through the remaining projects.
 
     @staticmethod
     def _cmd_option(option, _opt_str, _value, parser):
-        setattr(parser.values, option.dest or "command", list(parser.rargs))
+        setattr(parser.values, option.dest, list(parser.rargs))
         while parser.rargs:
             del parser.rargs[0]
 
@@ -161,6 +161,7 @@ without iterating through the remaining projects.
         p.add_option(
             "-c",
             "--command",
+            dest="command",
             help="command (and arguments) to execute",
             action="callback",
             callback=self._cmd_option,
diff --git a/tests/test_subcmds.py b/tests/test_subcmds.py
index 2d680fb7..0683f1dd 100644
--- a/tests/test_subcmds.py
+++ b/tests/test_subcmds.py
@@ -94,7 +94,12 @@ class AllCommands(unittest.TestCase):
         """Block redundant dest= arguments."""
 
         def _check_dest(opt):
-            if opt.dest is None or not opt._long_opts:
+            """Check the dest= setting."""
+            # If the destination is not set, nothing to check.
+            # If long options are not set, then there's no implicit destination.
+            # If callback is used, then a destination might be needed because
+            # optparse cannot assume a value is always stored.
+            if opt.dest is None or not opt._long_opts or opt.callback:
                 return
 
             long = opt._long_opts[0]