[PATCH b4 1/3] review-tui: honor shazam-merge-flags in take->merge
Christian Brauner <[email protected]> Mon, 22 Jun 2026 14:20:43 +0200
| Newsgroups | org.kernel.linux.tools |
|---|---|
| Message-ID | <[email protected]> |
The review TUI's take->merge path had drifted from "b4 shazam": it never read b4.shazam-merge-flags, so --log, --stat and --gpg-sign were silently dropped; only the take dialog's "add Link:" and "add Signed-off-by:" checkboxes had any effect. It also ran b4's own editor and then `git merge --no-edit` through the stdio-capturing git_run_command, so git could never open an editor -- which is the only way `git merge --log` makes its shortlog visible. Make it behave like shazam: - Parse b4.shazam-merge-flags via a new _shazam_merge_flags() helper. Signed-off-by is stripped from the flags because the take dialog's checkbox already adds it to the body (and `git merge --signoff` does not dedup), but --log/--stat/--gpg-sign now reach git merge. - Pass origin= to git_fetch_am_into_repo so FETCH_HEAD is labelled "patches from <link>", matching the shortlog header shazam produces. - Write the cover with -F and let `git merge --edit` open the editor, run directly with an inherited tty under suspend() like _suspend_to_shell, so the appended --log shortlog is visible and editable. The pytest path keeps --no-edit through the captured runner since tests have no tty. Signed-off-by: Christian Brauner (Amutable) <[email protected]> --- src/b4/review_tui/_tracking_app.py | 80 ++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 20 deletions(-) diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py index fc1e1a6..959670a 100644 --- a/src/b4/review_tui/_tracking_app.py +++ b/src/b4/review_tui/_tracking_app.py @@ -15,7 +15,10 @@ import json import os import pathlib import re +import shlex import sqlite3 +import subprocess +import sys from string import Template from typing import Any, Dict, List, Literal, Optional, Tuple @@ -134,6 +137,27 @@ _ACTIONABLE_STATUSES: frozenset[str] = frozenset( ) +def _shazam_merge_flags(config: Dict[str, Any]) -> List[str]: + """Extra ``git merge`` flags for the take->merge path from config. + + Mirrors the ``b4 shazam`` CLI's handling of ``b4.shazam-merge-flags`` + (e.g. ``--log``, ``--stat``, ``--gpg-sign``) so a merge taken from the + review TUI carries the same options. + + Signed-off-by is deliberately dropped here: in the review TUI the merge + commit's SoB is controlled by the take dialog's "add Signed-off-by" + checkbox, which edits the merge message body directly. ``git merge + --signoff`` does not dedup against an existing trailer, so leaving it in + would append a second Signed-off-by line. + """ + raw = str(config.get('shazam-merge-flags', '--signoff')) + if not raw: + return [] + sp = shlex.shlex(raw, posix=True) + sp.whitespace_split = True + return [f for f in sp if f not in ('-s', '--signoff', '--no-signoff')] + + def _resolve_worktree_am_conflict(topdir: str, cex: 'b4.AmConflictError') -> bool: """Handle an AmConflictError by dropping the user into a shell. @@ -2633,21 +2657,11 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): else: body = stripped + '\n\n' + sob + '\n' - # Open editor - try: - edited = b4.edit_in_editor(body.encode(), filehint='MERGE_MSG') - except Exception as ex: - logger.critical('Editor error: %s', ex) - _wait_for_enter() - return - merge_msg = edited.decode(errors='replace').strip() - if not merge_msg: - logger.info('Empty merge message, aborting') - _wait_for_enter() - return - # Apply trailer-amended patches in a sparse worktree and fetch # into FETCH_HEAD, so individual commits carry their trailers. + # The merge message is opened for editing below by git-merge --edit + # (mirroring "b4 shazam"), so the --log shortlog git appends is + # visible and editable there. base_commit = t_series.get('base-commit', '') if not base_commit: # Fall back to target branch HEAD @@ -2660,7 +2674,11 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): try: b4.git_fetch_am_into_repo( - topdir, ambytes, at_base=base_commit, am_flags=['-3'] + topdir, + ambytes, + at_base=base_commit, + origin=t_series.get('link', ''), + am_flags=['-3'], ) except b4.AmConflictError as cex: if not _resolve_worktree_am_conflict(topdir, cex): @@ -2693,12 +2711,34 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): return mmf = os.path.join(gitdir.strip(), 'b4-merge-msg') with open(mmf, 'w') as fh: - fh.write(merge_msg) + fh.write(body) - # Merge FETCH_HEAD (trailer-amended patches) instead of the - # review branch directly, so each commit carries its trailers. - gitargs = ['merge', '--no-ff', '--no-edit', '-F', mmf, 'FETCH_HEAD'] - ecode, out = b4.git_run_command(topdir, gitargs, logstderr=True) + # Merge FETCH_HEAD (trailer-amended patches) instead of the review + # branch directly, so each commit carries its trailers. Mirror + # "b4 shazam": git-merge builds the final message from -F + the flags + # and opens the editor (--edit), so b4.shazam-merge-flags such as + # --log/--stat/--gpg-sign take effect and the appended shortlog is + # visible. Signed-off-by is left out of the flags -- it is already in + # the body via the take dialog's checkbox (see _shazam_merge_flags). + mergeflags = _shazam_merge_flags(config) + out = '' + if hasattr(sys, '_running_in_pytest'): + # Tests have no tty for an interactive editor; run the merge + # non-interactively through the captured runner, like the CLI does. + mergeargs = ( + ['merge', '--no-ff', '-F', mmf, '--no-edit', 'FETCH_HEAD'] + + mergeflags + ) + ecode, out = b4.git_run_command(topdir, mergeargs, logstderr=True) + else: + # Run git directly with an inherited tty (under the caller's + # suspend()) so git can open the editor, like _suspend_to_shell. + mergeargs = ( + ['git', '-C', topdir, 'merge', '--no-ff', '-F', mmf, '--edit', + 'FETCH_HEAD'] + + mergeflags + ) + ecode = subprocess.run(mergeargs).returncode # Clean up message file try: @@ -2707,7 +2747,7 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): pass if ecode != 0: - logger.critical('Merge failed: %s', out.strip()) + logger.critical('Merge failed%s', f': {out.strip()}' if out.strip() else '') logger.critical('Aborting merge...') b4.git_run_command(topdir, ['merge', '--abort'], logstderr=True) b4.git_run_command(topdir, ['checkout', prev_branch], logstderr=True) -- 2.53.0