[PATCH b4 0/2] A prep splat fix and rework of option handling

[PATCH b4 0/2] A prep splat fix and rework of option handling

[Rob Herring]

I wanted enroll a branch and run 'auto-to-cc' in one step which from the 
help looks valid, but was met with a splat. The same splat happens if 
one runs 'b4 prep -c' on a newly created branch with no commits (other 
than the cover letter). There's a couple of other options that might be 
useful to use on new branches. The 2nd patch makes this possible.

Alternatively, the help should be clearer on what options are valid 
together or not.

Another thing I noticed is that if we error out when creating a new 
branch after the branch is created, the branch remains afterwards. Not 
sure if this is intended in the name of minimizing modifications to git 
trees or just an oversight.

Signed-off-by: Rob Herring <robh@kernel.org>
---
Rob Herring (2):
      prep: Fix splat with --auto-to-cc when a branch has no commits
      prep: Allow configuration options when enrolling/creating branch

 b4/command.py | 13 +++++++------
 b4/ez.py      | 38 ++++++++++++++++++++++----------------
 2 files changed, 29 insertions(+), 22 deletions(-)
---
base-commit: c88e6a31442bc41e9b56df763ba8b30e64d18c93
change-id: 20230110-prep-opts-0ad63b62e507

Best regards,
-- 
Rob Herring <robh@kernel.org>

[PATCH b4 1/2] prep: Fix splat with --auto-to-cc when a branch has no commits

[Rob Herring]

Running 'b4 prep -c' on a branch with no commits will splat. Print a
friendly message instead.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 b4/ez.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/b4/ez.py b/b4/ez.py
index 36f935ebe993..1b7e390b7744 100644
--- a/b4/ez.py
+++ b/b4/ez.py
@@ -1819,7 +1819,12 @@ def auto_to_cc() -> None:
             logger.debug('added %s to seen', ltr.addr[1])
             extras.append(ltr)
 
-    tos, ccs, tag_msg, patches = get_prep_branch_as_patches()
+    try:
+        tos, ccs, tag_msg, patches = get_prep_branch_as_patches()
+    except RuntimeError:
+        logger.info('No commits in branch')
+        return
+
     logger.info('Collecting To/Cc addresses')
     # Go through the messages to make to/cc headers
     for commit, msg in patches:

-- 
2.39.0

[PATCH b4 2/2] prep: Allow configuration options when enrolling/creating branch

[Rob Herring]

I tried 'b4 prep -c -e v6.2-rc1' thinking I could populate the To/Cc list
and enroll a branch in one step, but got this:

Will collect To: addresses using get_maintainer.pl
Will collect Cc: addresses using get_maintainer.pl
Traceback (most recent call last):
  File "/home/rob/.local/bin/b4", line 33, in <module>
    sys.exit(load_entry_point('b4', 'console_scripts', 'b4')())
  File "/home/rob/proj/b4/b4/command.py", line 359, in cmd
    cmdargs.func(cmdargs)
  File "/home/rob/proj/b4/b4/command.py", line 76, in cmd_prep
    b4.ez.cmd_prep(cmdargs)
  File "/home/rob/proj/b4/b4/ez.py", line 1875, in cmd_prep
    return auto_to_cc()
  File "/home/rob/proj/b4/b4/ez.py", line 1797, in auto_to_cc
    tos, ccs, tag_msg, patches = get_prep_branch_as_patches()
  File "/home/rob/proj/b4/b4/ez.py", line 1113, in get_prep_branch_as_patches
    prefixes = tracking['series'].get('prefixes', list())
KeyError: 'series'

Auto populating the To/Cc list currently has to be run by itself. That's
an unnecessary restriction, and it is useful to do that as part of setting
up a new or existing branch. The same applies to '--force-revision',
'--set-prefixes', and '--edit-cover'.

A side effect is these options will now be silently ignored if given with
other sub-commands (e.g. --show-revision).

Signed-off-by: Rob Herring <robh@kernel.org>
---
 b4/command.py | 13 +++++++------
 b4/ez.py      | 31 ++++++++++++++++---------------
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/b4/command.py b/b4/command.py
index ccae05d24488..b4a02cef7fc7 100644
--- a/b4/command.py
+++ b/b4/command.py
@@ -257,23 +257,24 @@ def setup_parser() -> argparse.ArgumentParser:
 
     # b4 prep
     sp_prep = subparsers.add_parser('prep', help='Work on patch series to submit for mailing list review')
-    spp_g = sp_prep.add_mutually_exclusive_group()
-    spp_g.add_argument('-c', '--auto-to-cc', action='store_true', default=False,
+    sp_prep.add_argument('-c', '--auto-to-cc', action='store_true', default=False,
                        help='Automatically populate cover letter trailers with To and Cc addresses')
+    sp_prep.add_argument('--force-revision', metavar='N', type=int,
+                       help='Force revision to be this number instead')
+    sp_prep.add_argument('--set-prefixes', metavar='PREFIX', nargs='+',
+                       help='Extra prefixes to add to [PATCH] (e.g.: RFC mydrv)')
+
+    spp_g = sp_prep.add_mutually_exclusive_group()
     spp_g.add_argument('-p', '--format-patch', metavar='OUTPUT_DIR',
                        help='Output prep-tracked commits as patches')
     spp_g.add_argument('--edit-cover', action='store_true', default=False,
                        help='Edit the cover letter in your defined $EDITOR (or core.editor)')
     spp_g.add_argument('--show-revision', action='store_true', default=False,
                        help='Show current series revision number')
-    spp_g.add_argument('--force-revision', metavar='N', type=int,
-                       help='Force revision to be this number instead')
     spp_g.add_argument('--compare-to', metavar='vN',
                        help='Display a range-diff to previously sent revision N')
     spp_g.add_argument('--manual-reroll', dest='reroll', default=None, metavar='COVER_MSGID',
                        help='Mark current revision as sent and reroll (requires cover letter msgid)')
-    spp_g.add_argument('--set-prefixes', metavar='PREFIX', nargs='+',
-                       help='Extra prefixes to add to [PATCH] (e.g.: RFC mydrv)')
     spp_g.add_argument('--show-info', action='store_true', default=False,
                        help='Show current series info in a column-parseable format')
 
diff --git a/b4/ez.py b/b4/ez.py
index 1b7e390b7744..35c3e9c07ff6 100644
--- a/b4/ez.py
+++ b/b4/ez.py
@@ -480,7 +480,7 @@ def start_new_series(cmdargs: argparse.Namespace) -> None:
         if revision is None:
             revision = 1
         prefixes = list()
-        if cmdargs.set_prefixes and len(prefixes[0].strip()):
+        if cmdargs.set_prefixes:
             prefixes = list(cmdargs.set_prefixes)
         else:
             config = b4.get_main_config()
@@ -1881,12 +1881,6 @@ def cmd_prep(cmdargs: argparse.Namespace) -> None:
         logger.critical('          Stash or commit them first.')
         sys.exit(1)
 
-    if cmdargs.edit_cover:
-        return edit_cover()
-
-    if cmdargs.auto_to_cc:
-        return auto_to_cc()
-
     if cmdargs.reroll:
         msgid = cmdargs.reroll
         msgs = b4.get_pi_thread_by_msgid(msgid, onlymsgids={msgid}, nocache=True)
@@ -1913,23 +1907,30 @@ def cmd_prep(cmdargs: argparse.Namespace) -> None:
     if cmdargs.show_info:
         return show_info()
 
-    if cmdargs.force_revision:
-        return force_revision(cmdargs.force_revision)
-
     if cmdargs.format_patch:
         return format_patch(cmdargs.format_patch)
 
     if cmdargs.compare_to:
         return compare(cmdargs.compare_to)
 
+    if cmdargs.enroll_base or cmdargs.new_series_name:
+        if is_prep_branch():
+            logger.critical('CRITICAL: This appears to already be a b4-prep managed branch.')
+            sys.exit(1)
+
+        start_new_series(cmdargs)
+
+    if cmdargs.force_revision:
+        force_revision(cmdargs.force_revision)
+
     if cmdargs.set_prefixes:
-        return set_prefixes(cmdargs.set_prefixes)
+        set_prefixes(cmdargs.set_prefixes)
 
-    if is_prep_branch():
-        logger.critical('CRITICAL: This appears to already be a b4-prep managed branch.')
-        sys.exit(1)
+    if cmdargs.auto_to_cc:
+        auto_to_cc()
 
-    return start_new_series(cmdargs)
+    if cmdargs.edit_cover:
+        return edit_cover()
 
 
 def cmd_trailers(cmdargs: argparse.Namespace) -> None:

-- 
2.39.0

Re: [PATCH b4 0/2] A prep splat fix and rework of option handling

[Rob Herring]

On Tue, Jan 10, 2023 at 3:46 PM Rob Herring <robh@kernel.org> wrote:
>
> I wanted enroll a branch and run 'auto-to-cc' in one step which from the
> help looks valid, but was met with a splat. The same splat happens if
> one runs 'b4 prep -c' on a newly created branch with no commits (other
> than the cover letter). There's a couple of other options that might be
> useful to use on new branches. The 2nd patch makes this possible.
>
> Alternatively, the help should be clearer on what options are valid
> together or not.
>
> Another thing I noticed is that if we error out when creating a new
> branch after the branch is created, the branch remains afterwards. Not
> sure if this is intended in the name of minimizing modifications to git
> trees or just an oversight.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> Rob Herring (2):
>       prep: Fix splat with --auto-to-cc when a branch has no commits
>       prep: Allow configuration options when enrolling/creating branch

I ran into a few more issues sending this. The first was b4 is
configured to use the web endpoint by default. Is there some way to
override that other than editing .b4-config? I didn't see one, so I
just edited the config and sent this.

The second issue is quite critical as users' work could be lost! After
I sent this, my (uncommitted).b4-config changes were gone! This is due
to the reroll to v2 and it seems git-filter-repo wipes out uncommitted
changes. I guess the '--force' option disables all checking, so we
need to do some of our own.

Rob

Re: [PATCH b4 0/2] A prep splat fix and rework of option handling

[Konstantin Ryabitsev]

On Tue, Jan 10, 2023 at 04:15:34PM -0600, Rob Herring wrote:
> I ran into a few more issues sending this. The first was b4 is
> configured to use the web endpoint by default. Is there some way to
> override that other than editing .b4-config? I didn't see one, so I
> just edited the config and sent this.

Ah, yes, I didn't think about this. You *can* override it by setting
b4.send-endpoint-web = no in your local git config, but we should change the
default so that we always use SMTP when we find it, and only fall back to the
web endpoint when SMTP is not configured.

> The second issue is quite critical as users' work could be lost! After
> I sent this, my (uncommitted).b4-config changes were gone! This is due
> to the reroll to v2 and it seems git-filter-repo wipes out uncommitted
> changes. I guess the '--force' option disables all checking, so we
> need to do some of our own.

This is actually because we create a detached head for the purposes of tagging
it. This needs to be improved so we don't modify the current worktree, I just
didn't research how to do it.

For now, I added the check for the state of the tree to make sure that we
refuse to operate if it's not clean.

Thank you for the report.

-K

Re: [PATCH b4 0/2] A prep splat fix and rework of option handling

[Konstantin Ryabitsev]

On Tue, 10 Jan 2023 15:46:07 -0600, Rob Herring wrote:
> I wanted enroll a branch and run 'auto-to-cc' in one step which from the
> help looks valid, but was met with a splat. The same splat happens if
> one runs 'b4 prep -c' on a newly created branch with no commits (other
> than the cover letter). There's a couple of other options that might be
> useful to use on new branches. The 2nd patch makes this possible.
> 
> Alternatively, the help should be clearer on what options are valid
> together or not.
> 
> [...]

Applied, thanks!

[1/2] prep: Fix splat with --auto-to-cc when a branch has no commits
      commit: d45b65977fc76558d9d69631a7047ccd33cce6ea
[2/2] prep: Allow configuration options when enrolling/creating branch
      commit: 5b2056d4002a1d71dc64c56ee65585c5d8739c32

Best regards,
-- 
Konstantin Ryabitsev <konstantin@linuxfoundation.org>