Re: [PATCH b4 v2] ez: allow cleaning multiple branches at once

[Konstantin Ryabitsev <konstantin@linuxfoundation.org>]

On Tue, 03 Feb 2026 10:54:01 +0100, Antonin Godard <antonin.godard@bootlin.com> wrote:
> Allow "b4 prep --cleanup" to cleanup multiple branches in one go. If an
> error occurs (branch not known/empty branch/currently checked out), just
> return to continue cleaning up branches. Give the user multiple options:
> "y" to cleanup, "n" to skip, "q" to abort the cleanup, "s" to show
> the branch log with diffs, and "?" to print help about these options.
> 
> Passing nothing to --cleanup still shows the available branches to
> cleanup.

Thanks for sending this in! I'm not against the feature, but there are 
several places where I think this can be improved.

> diff --git a/src/b4/ez.py b/src/b4/ez.py
> index 298e382..7af8e14 100644
> --- a/src/b4/ez.py
> +++ b/src/b4/ez.py
> @@ -26,6 +26,7 @@ import io
>  import tarfile
>  import hashlib
>  import urllib.parse
> +import subprocess

This import is only needed for the single subprocess.run() call in the
"s" handler. Consider using b4.git_run_command() instead, which would
eliminate the need for this import entirely.

> @@ -2518,47 +2519,40 @@ def get_prep_managed_branches(gitdir: Optional[str] = None) -> List[str]:
>      return mybranches
>  
>  
> -def cleanup(param: str) -> None:
> -    if param == '_show':
> -        # Show all b4-tracked branches
> -        mybranches = get_prep_managed_branches(None)
> -        if not len(mybranches):
> -            logger.info('No b4-tracked branches found')
> -            sys.exit(0)
> +def _cleanup_branch(branch: str) -> None:
>  
> -        logger.info('Please specify branch:')
> -        for branch in mybranches:
> -            logger.info(' %s', branch)
> +    if not b4.git_branch_exists(None, branch):
> +        logger.error('ERROR: Not a known branch: %s', branch)
>          return
>  
> -    mybranch = param
> -    if not b4.git_branch_exists(None, mybranch):
> -        logger.critical('Not a known branch: %s', mybranch)
> -        sys.exit(1)
> -    is_prep_branch(mustbe=True, usebranch=mybranch)
> -    base_commit, start_commit, end_commit = get_series_range(usebranch=mybranch)
> +    if not is_prep_branch(usebranch=branch):

is_prep_branch() without mustbe=True returns False silently. The user
gets no indication of why this branch was skipped. Consider adding an
error message here, e.g.:
    if not is_prep_branch(usebranch=branch):
        logger.error('ERROR: %s is not a prep-managed branch', branch)
        return

> @@ -2573,7 +2567,32 @@ def cleanup(param: str) -> None:
>              tags.append((tagname, base_commit, tag_commit, revision, cover))
>      logger.info('---')
>      try:
> -        input('Press Enter to confirm or Ctrl-C to abort')
> +        resp = None
> +        while resp is None:
> +            resp = input('Proceed? [y/s/q/N/?] ')
> +            if resp == "?":
> +                logger.info(textwrap.dedent(
> +                    """
> +                    Possible answers:
> +                    y: cleanup the branch
> +                    s: show branch log
> +                    q or Ctrl-C: abort cleanup
> +                    n (default): do not cleanup this branch
> +                    ?: show this help message
> +                    """))
> +                resp = None
> +            elif resp in ("show", "s"):
> +                start_commit = get_info(branch)["start-commit"]
> +                end_commit = get_info(branch)["end-commit"]

start_commit and end_commit are already computed at the top of this
function via get_series_range(). Re-fetching them from get_info() is
wasteful (get_info computes recipients, prereqs, preflight checks, etc.)
and also shadows the outer local variables. If the user presses "s" then
"y", the archiving code below will use these reassigned values.

Just use the already-available locals directly, e.g.:
    b4.git_run_command(None, ['log', '-p',
                              f'{start_commit}~..{end_commit}'])

> +                subprocess.run(["git", "--paginate", "log", "-p",
> +                                f"{start_commit}~..{end_commit}"])

Same as above, this should be using git_run_command().

> @@ -2636,6 +2655,23 @@ def cleanup(param: str) -> None:
>      logger.info('Wrote: %s', tarpath)
>  
>  
> +def cleanup(branches: list[str]) -> None:

Small nitpick: the lowercase list[str] type annotation requires Python 3.9+.
The rest of this file uses List from typing (already imported). Use List[str]
for consistency.

Cheers,

-- 
KR