[PATCH b4 0/6] Keep the user's git config out of b4's scratch worktrees

[PATCH b4 0/6] Keep the user's git config out of b4's scratch worktrees

[Christian Brauner]

With submodule.recurse=true in the user's config b4 cannot apply
anything in a repo that carries submodules. The scratch worktree's
sparse checkout recurses, looks for the per-worktree submodule clones a
just-created worktree cannot have, and dies:

    fatal: not a git repository: ../../worktrees/b4-shazam-worktree/modules/ezgb
    fatal: could not reset submodule index

That takes review-branch creation, b4 shazam and the TUI's test applies
with it. The b4 repo itself is affected through its vendored
patatt/liblore/ezgb submodules.

Everything b4 does in those worktrees is thrown away when the call
returns, so they can run with overrides of their own: no submodule
recursion and no gpg signing. SCRATCH_GIT_OPTS carries both, since each
is inert where the other matters, and every scratch worktree switches
over to it: the shazam apply, the TUI's five hand-rolled test applies,
the fake-am staging worktree and the sent-tag worktree in b4 send. The
commits that outlive their worktree keep the user's signing config.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (6):
      git_run_command: look past -c overrides for the subcommand
      shazam: ignore the user's submodule.recurse in the scratch worktree
      review-tui: use the shared scratch-worktree overrides for test applies
      fake-am: use the scratch-worktree overrides in the staging worktree
      send: use the scratch-worktree overrides when tagging a sent series
      tests: exercise the scratch worktrees under submodule.recurse

 src/b4/__init__.py                 |  43 +++++++++--
 src/b4/ez.py                       |   8 +-
 src/b4/review_tui/_modals.py       |  44 +++++++----
 src/b4/review_tui/_tracking_app.py |  11 ++-
 src/tests/conftest.py              |  52 ++++++++++++-
 src/tests/test___init__.py         |  15 ++++
 src/tests/test_ez.py               |  45 +++++++++++-
 src/tests/test_tui_tracking.py     | 146 ++++++++++++++++++++++++++++++++++++-
 8 files changed, 332 insertions(+), 32 deletions(-)
---
base-commit: 99fe195c0aa5a88be5033f87cad22bc32d27cbfe
change-id: 20260729-work-b4-scratch-worktrees-f69cde824a95

[PATCH b4 1/6] git_run_command: look past -c overrides for the subcommand

[Christian Brauner]

git_run_command() counteracts a local log.abbrevCommit by inserting
--no-abbrev-commit after the subcommand, which it expects at args[0].
That holds only as long as no caller passes git-level options first.

The next patch adds a list of -c overrides that callers prefix to their
arguments. With it args[0] is '-c', the fixup stops firing and nothing
says so. b4 parses full shas out of git-log output in several places,
so a user with log.abbrevCommit=true would get short ones back.

Skip over leading -c key=value pairs when looking for the subcommand.
While we are here, stop inserting into the caller's list in place.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/__init__.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index f66d38d..8afbbab 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -3664,9 +3664,13 @@ def git_run_command(
                 gitdir = dotgit
             cmdargs += ['--git-dir', str(gitdir)]
 
-    # counteract some potential local settings
-    if args[0] == 'log':
-        args.insert(1, '--no-abbrev-commit')
+    # counteract some potential local settings; the subcommand is not
+    # necessarily args[0] because callers may prefix -c overrides
+    subcmd = 0
+    while subcmd + 1 < len(args) and args[subcmd] == '-c':
+        subcmd += 2
+    if subcmd < len(args) and args[subcmd] == 'log':
+        args = args[: subcmd + 1] + ['--no-abbrev-commit'] + args[subcmd + 1 :]
 
     cmdargs += args
 

-- 
2.53.0

[PATCH b4 2/6] shazam: ignore the user's submodule.recurse in the scratch worktree

[Christian Brauner]

With submodule.recurse=true in the user's config every apply through
git_fetch_am_into_repo() fails in a repo that carries submodules. That
covers b4 shazam and review-branch creation in the review TUI:

    Magic: Preparing a sparse worktree
    Error running checkout into sparse workdir
    fatal: not a git repository: ../../worktrees/b4-shazam-worktree/modules/ezgb
    fatal: could not reset submodule index

git gives every linked worktree its own submodule clones under
.git/worktrees/<name>/modules/ and a just-created worktree has none, so
the recursing sparse checkout dies looking for them. The b4 repo itself
is affected through its vendored patatt/liblore/ezgb submodules.

Everything b4 does in that worktree is thrown away when the call
returns, so the user's checkout conveniences have no business running
there. Let's add SCRATCH_GIT_OPTS with the two overrides a git command
in a scratch worktree needs, no submodule recursion and no gpg signing,
the same override the TUI's test applies already pass by hand. The
sparse-checkout and the checkout get it.

The fetch out of the worktree takes nothing. It runs in the user's own
repo rather than in the scratch worktree, so the user's config is the
right one there, and a recursing fetch does not trip over the missing
clones anyway.

The apply itself keeps the user's config. Its commits are the real
series and not scratch throwaways, so signing stays in force. The
replay-on-full-worktree fallback needs nothing. Neither git-am nor
git-sparse-checkout consults submodule.recurse (checked on git 2.53).

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/__init__.py | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index 8afbbab..1a883b4 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -132,6 +132,24 @@ DEVSIG_HDR = 'X-Developer-Signature'
 LOREADDR = 'https://lore.kernel.org'
 LINKADDR = 'https://patch.msgid.link'
 
+# Overrides for git commands b4 runs inside its own scratch worktrees. A fresh
+# linked worktree has no per-worktree submodule clones, so a checkout or reset
+# obeying submodule.recurse=true dies with "fatal: not a git repository:
+# .../worktrees/<wt>/modules/<name>"; and unattended commit-creating commands
+# must never gpg-sign -- signing hangs on a pinentry prompt no terminal will
+# answer. Each override is inert where the other matters, so the one list
+# serves every scratch-worktree git command. Commits that outlive the worktree
+# are the exception and keep the user's signing config: the real apply in
+# git_fetch_am_into_repo. Fetching *out of* a
+# worktree takes nothing -- that one runs in the user's repo, not in the
+# scratch, so the user's config still governs it.
+SCRATCH_GIT_OPTS: List[str] = [
+    '-c',
+    'submodule.recurse=false',
+    '-c',
+    'commit.gpgsign=false',
+]
+
 DEFAULT_CONFIG: ConfigDictT = {
     'midmask': LOREADDR + '/all/%s',
     'searchmask': LOREADDR + '/all/?x=m&q=%s',
@@ -5972,19 +5990,24 @@ def git_fetch_am_into_repo(
     try:
         logger.info('Magic: Preparing a sparse worktree')
         ecode, out = git_run_command(
-            gwt, ['sparse-checkout', 'set'], logstderr=True, rundir=gwt
+            gwt,
+            [*SCRATCH_GIT_OPTS, 'sparse-checkout', 'set'],
+            logstderr=True,
+            rundir=gwt,
         )
         if ecode > 0:
             logger.critical('Error running sparse-checkout set')
             logger.critical(out)
             raise RuntimeError
         ecode, out = git_run_command(
-            gwt, ['checkout', '-f'], logstderr=True, rundir=gwt
+            gwt, [*SCRATCH_GIT_OPTS, 'checkout', '-f'], logstderr=True, rundir=gwt
         )
         if ecode > 0:
             logger.critical('Error running checkout into sparse workdir')
             logger.critical(out)
             raise RuntimeError
+        # No SCRATCH_GIT_OPTS on the apply: its commits are the real series,
+        # not scratch throwaways, so the user's signing config stays in force.
         amargs = ['am']
         if am_flags:
             amargs.extend(am_flags)

-- 
2.53.0

[PATCH b4 3/6] review-tui: use the shared scratch-worktree overrides for test applies

[Christian Brauner]

The test-apply paths build the same sparse scratch worktree in five
places, the four modal am probes and the take flow's cherry-pick probe.
Each of them passes -c commit.gpgsign=false on the apply by hand and
nothing at all on the checkout.

That checkout is the bug just fixed in git_fetch_am_into_repo(). With
submodule.recurse=true the probe's checkout -f dies in any repo that
carries submodules, so every test apply reported a failure that had
nothing to do with the patches.

Let's switch all five sites to SCRATCH_GIT_OPTS. gpgsign means nothing
to a checkout and recursion means nothing to am and cherry-pick, so the
one list serves every command and the applies keep behaving as before.
The per-site "# No signing" comments go with it.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/review_tui/_modals.py       | 44 ++++++++++++++++++++++++--------------
 src/b4/review_tui/_tracking_app.py | 11 ++++++----
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/src/b4/review_tui/_modals.py b/src/b4/review_tui/_modals.py
index 89253bc..1d8ea4f 100644
--- a/src/b4/review_tui/_modals.py
+++ b/src/b4/review_tui/_modals.py
@@ -1075,15 +1075,18 @@ class TakeConfirmScreen(ModalScreen[bool]):
             # Test apply in a temporary sparse worktree
             try:
                 with b4.git_temp_worktree(topdir, resolved_base) as gwt:
-                    ecode, out = b4.git_run_command(gwt, ['sparse-checkout', 'set'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'sparse-checkout', 'set']
+                    )
                     if ecode > 0:
                         return False, 'failed to set up worktree'
-                    ecode, out = b4.git_run_command(gwt, ['checkout', '-f'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'checkout', '-f']
+                    )
                     if ecode > 0:
                         return False, 'failed to checkout base'
-                    # No signing: gpg would prompt while the TUI owns the tty.
                     ecode, out = b4.git_run_command(
-                        gwt, ['-c', 'commit.gpgsign=false', 'am'], stdin=ambytes
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'am'], stdin=ambytes
                     )
                     if ecode > 0:
                         for line in out.splitlines():
@@ -2069,15 +2072,18 @@ class RebaseScreen(ModalScreen[bool]):
         with _quiet_worker():
             try:
                 with b4.git_temp_worktree(topdir, branch) as gwt:
-                    ecode, out = b4.git_run_command(gwt, ['sparse-checkout', 'set'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'sparse-checkout', 'set']
+                    )
                     if ecode > 0:
                         return False, 'failed to set up worktree'
-                    ecode, out = b4.git_run_command(gwt, ['checkout', '-f'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'checkout', '-f']
+                    )
                     if ecode > 0:
                         return False, 'failed to checkout base'
-                    # No signing: gpg would prompt while the TUI owns the tty.
                     ecode, out = b4.git_run_command(
-                        gwt, ['-c', 'commit.gpgsign=false', 'am'], stdin=ambytes
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'am'], stdin=ambytes
                     )
                     if ecode > 0:
                         for line in out.splitlines():
@@ -2342,15 +2348,18 @@ class TargetBranchScreen(ModalScreen[Optional[str]]):
         with _quiet_worker():
             try:
                 with b4.git_temp_worktree(topdir, branch) as gwt:
-                    ecode, out = b4.git_run_command(gwt, ['sparse-checkout', 'set'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'sparse-checkout', 'set']
+                    )
                     if ecode > 0:
                         return False, 'failed to set up worktree'
-                    ecode, out = b4.git_run_command(gwt, ['checkout', '-f'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'checkout', '-f']
+                    )
                     if ecode > 0:
                         return False, 'failed to checkout base'
-                    # No signing: gpg would prompt while the TUI owns the tty.
                     ecode, out = b4.git_run_command(
-                        gwt, ['-c', 'commit.gpgsign=false', 'am'], stdin=ambytes
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'am'], stdin=ambytes
                     )
                     if ecode > 0:
                         for line in out.splitlines():
@@ -3059,15 +3068,18 @@ class BaseSelectionScreen(ModalScreen[Optional[str]]):
         with _quiet_worker():
             try:
                 with b4.git_temp_worktree(topdir, base) as gwt:
-                    ecode, out = b4.git_run_command(gwt, ['sparse-checkout', 'set'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'sparse-checkout', 'set']
+                    )
                     if ecode > 0:
                         return False, 'failed to set up worktree'
-                    ecode, out = b4.git_run_command(gwt, ['checkout', '-f'])
+                    ecode, out = b4.git_run_command(
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'checkout', '-f']
+                    )
                     if ecode > 0:
                         return False, 'failed to checkout base'
-                    # No signing: gpg would prompt while the TUI owns the tty.
                     ecode, out = b4.git_run_command(
-                        gwt, ['-c', 'commit.gpgsign=false', 'am'], stdin=ambytes
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'am'], stdin=ambytes
                     )
                     if ecode > 0:
                         # Extract just the "Patch failed" line
diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index 2b29dc1..efdfeaf 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -3370,11 +3370,15 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
             with b4.git_temp_worktree(topdir, target_head) as gwt:
                 # Set up sparse checkout for minimal disk usage
                 ecode, out = b4.git_run_command(
-                    gwt, ['sparse-checkout', 'set'], logstderr=True
+                    gwt,
+                    [*b4.SCRATCH_GIT_OPTS, 'sparse-checkout', 'set'],
+                    logstderr=True,
                 )
                 if ecode != 0:
                     logger.warning('Could not set up sparse checkout: %s', out.strip())
-                ecode, out = b4.git_run_command(gwt, ['checkout', '-f'], logstderr=True)
+                ecode, out = b4.git_run_command(
+                    gwt, [*b4.SCRATCH_GIT_OPTS, 'checkout', '-f'], logstderr=True
+                )
                 if ecode != 0:
                     logger.warning(
                         'Could not checkout sparse worktree: %s', out.strip()
@@ -3382,8 +3386,7 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
 
                 # Try cherry-picking the commits
                 gitargs = [
-                    '-c',
-                    'commit.gpgsign=false',
+                    *b4.SCRATCH_GIT_OPTS,
                     'cherry-pick',
                     f'{base_commit}..{series_tip}',
                 ]

-- 
2.53.0

[PATCH b4 4/6] fake-am: use the scratch-worktree overrides in the staging worktree

[Christian Brauner]

make_fake_am_range() has a throwaway worktree of its own. It stages the
blobs the series expects to find, commits that tree, resets onto it and
ams the patches on top to get a commit range b4 can diff against. b4
diff goes through it, and so do the review TUI's range-diff and its
three-way merge prep.

The reset recurses like any other. A series that touches a submodule
puts a gitlink into the synthesized tree, and the reset then dies in
the same fresh worktree with no submodule clones. The am is worse.
Those commits exist to compute a diff range and nothing ever references
them, but git still reaches for the user's signing key, and with a card
that is not plugged in the whole range fails.

Both get SCRATCH_GIT_OPTS.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/__init__.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index 1a883b4..d418cee 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -1449,13 +1449,15 @@ class LoreSeries:
                 return None, None
             start_commit = out.strip()
             logger.debug('start_commit=%s', start_commit)
-            git_run_command(dfn, ['reset', '--hard', start_commit])
+            git_run_command(dfn, [*SCRATCH_GIT_OPTS, 'reset', '--hard', start_commit])
 
             ifh = io.BytesIO()
             save_git_am_mbox(msgs, ifh)
             ambytes = ifh.getvalue()
 
-            ecode, out = git_run_command(dfn, ['am'], stdin=ambytes, logstderr=True)
+            ecode, out = git_run_command(
+                dfn, [*SCRATCH_GIT_OPTS, 'am'], stdin=ambytes, logstderr=True
+            )
             if ecode > 0:
                 logger.critical('ERROR: Could not fake-am version v%s', self.revision)
                 return None, None

-- 
2.53.0

[PATCH b4 5/6] send: use the scratch-worktree overrides when tagging a sent series

[Christian Brauner]

When the cover lives in a commit, which is the default cover strategy,
reroll() builds the sent tag out of a scratch worktree: sparse
checkout, cherry-pick the series onto the base, fetch the result back.
That checkout is the one fixed in git_fetch_am_into_repo() and it dies
the same way with submodule.recurse=true.

Nothing tells the user. The whole block sits under a try that logs
"Error tagging the revision" and carries on, so b4 send finishes
normally and the sent tag for the revision is simply never created.

The cherry-pick keeps the user's signing config. Its commits are what
the tag points at, so they outlive the worktree.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/__init__.py | 2 +-
 src/b4/ez.py       | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index d418cee..bc4a797 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -140,7 +140,7 @@ LINKADDR = 'https://patch.msgid.link'
 # answer. Each override is inert where the other matters, so the one list
 # serves every scratch-worktree git command. Commits that outlive the worktree
 # are the exception and keep the user's signing config: the real apply in
-# git_fetch_am_into_repo. Fetching *out of* a
+# git_fetch_am_into_repo, the cherry-pick in ez.reroll(). Fetching *out of* a
 # worktree takes nothing -- that one runs in the user's repo, not in the
 # scratch, so the user's config still governs it.
 SCRATCH_GIT_OPTS: List[str] = [
diff --git a/src/b4/ez.py b/src/b4/ez.py
index 268d783..ec922e4 100644
--- a/src/b4/ez.py
+++ b/src/b4/ez.py
@@ -3302,19 +3302,23 @@ def reroll(
                 with b4.git_temp_worktree(topdir, base_commit) as gwt:
                     logger.debug('Preparing a sparse worktree')
                     ecode, out = b4.git_run_command(
-                        gwt, ['sparse-checkout', 'set'], logstderr=True
+                        gwt,
+                        [*b4.SCRATCH_GIT_OPTS, 'sparse-checkout', 'set'],
+                        logstderr=True,
                     )
                     if ecode > 0:
                         logger.critical('Error running sparse-checkout set')
                         logger.critical(out)
                         raise RuntimeError
                     ecode, out = b4.git_run_command(
-                        gwt, ['checkout', '-f'], logstderr=True
+                        gwt, [*b4.SCRATCH_GIT_OPTS, 'checkout', '-f'], logstderr=True
                     )
                     if ecode > 0:
                         logger.critical('Error running checkout into sparse workdir')
                         logger.critical(out)
                         raise RuntimeError
+                    # No SCRATCH_GIT_OPTS: the sent tag points at these commits,
+                    # so the user's signing config stays in force.
                     gitargs = ['cherry-pick', f'{start_commit}..{end_commit}']
                     ecode, out = b4.git_run_command(gwt, gitargs, logstderr=True)
                     if ecode > 0:

-- 
2.53.0

[PATCH b4 6/6] tests: exercise the scratch worktrees under submodule.recurse

[Christian Brauner]

An active but unpopulated submodule reproduces the recursing-checkout
fatal: a gitlink and .gitmodules in the tree plus a configured url, and
no clone anywhere. That is what every fresh linked worktree sees. The
fixture is built with plumbing so nothing is signed and master can be
advanced in place, and submodule.recurse goes on only once it is done.

git_fetch_am_into_repo() runs end to end that way, and so do the four
modal test-apply probes, the take flow's cherry-pick probe and
reroll()'s sent-tag worktree, which has to come back with the tag it
was asked for.

The fake-am test drives a series that bumps a gitlink, because that is
what puts a submodule into the synthesized tree, and poisons gpg on top
of it, so the reset and the am are covered at once.

The git_run_command() fixup gets its own test. With log.abbrevCommit=true
and a -c override in front of the subcommand the sha has to come back in
full.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/tests/conftest.py          |  52 ++++++++++++++-
 src/tests/test___init__.py     |  15 +++++
 src/tests/test_ez.py           |  45 ++++++++++++-
 src/tests/test_tui_tracking.py | 146 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 255 insertions(+), 3 deletions(-)

diff --git a/src/tests/conftest.py b/src/tests/conftest.py
index 48b2573..aa6dbc8 100644
--- a/src/tests/conftest.py
+++ b/src/tests/conftest.py
@@ -2,7 +2,7 @@ import copy
 import os
 import pathlib
 import sys
-from typing import Generator
+from typing import Callable, Generator
 
 import pytest
 
@@ -49,6 +49,56 @@ def sampledir(request: pytest.FixtureRequest) -> str:
     return os.path.join(request.path.parent, 'samples')
 
 
+@pytest.fixture(scope='function')
+def add_unpopulated_submodule() -> Callable[..., str]:
+    """Factory: advance master with an active but unpopulated submodule.
+
+    A gitlink and .gitmodules in the tree plus a configured url is all it
+    takes: no clone exists anywhere, which is exactly what any fresh linked
+    worktree sees, and a git command recursing into submodules there dies with
+    "fatal: not a git repository: .../worktrees/<wt>/modules/<name>". Built
+    with plumbing so nothing is signed and master can be advanced in place.
+    Callers flip submodule.recurse on only once their own fixtures are built --
+    this setup would trip over it too. Returns the sha the gitlink points at.
+    """
+
+    def _add(gitdir: str, name: str = 'sub') -> str:
+        ecode, head = b4.git_run_command(gitdir, ['rev-parse', 'master'])
+        assert ecode == 0
+        gitlink = head.strip()
+        gm = pathlib.Path(gitdir) / '.gitmodules'
+        gm.write_text(f'[submodule "{name}"]\n\tpath = {name}\n\turl = ./{name}\n')
+        ecode, _ = b4.git_run_command(gitdir, ['add', '.gitmodules'])
+        assert ecode == 0
+        ecode, _ = b4.git_run_command(
+            gitdir,
+            ['update-index', '--add', '--cacheinfo', f'160000,{gitlink},{name}'],
+        )
+        assert ecode == 0
+        ecode, tree = b4.git_run_command(gitdir, ['write-tree'])
+        assert ecode == 0
+        ecode, commit = b4.git_run_command(
+            gitdir,
+            ['commit-tree', tree.strip(), '-p', 'master'],
+            stdin=f'add {name} gitlink\n'.encode(),
+        )
+        assert ecode == 0
+        ecode, _ = b4.git_run_command(
+            gitdir, ['update-ref', 'refs/heads/master', commit.strip()]
+        )
+        assert ecode == 0
+        ecode, _ = b4.git_run_command(gitdir, ['reset', '--hard', 'master'])
+        assert ecode == 0
+        ecode, entry = b4.git_run_command(gitdir, ['ls-tree', 'master', name])
+        assert ecode == 0 and entry.startswith('160000 commit'), (
+            f'gitlink did not land in master: {entry!r}'
+        )
+        b4.git_set_config(gitdir, f'submodule.{name}.url', f'./{name}')
+        return gitlink
+
+    return _add
+
+
 @pytest.fixture(scope='function')
 def gitdir(
     request: pytest.FixtureRequest, tmp_path: pathlib.Path
diff --git a/src/tests/test___init__.py b/src/tests/test___init__.py
index e66f993..b02ed12 100644
--- a/src/tests/test___init__.py
+++ b/src/tests/test___init__.py
@@ -1219,3 +1219,18 @@ def test_edit_in_editor_normalizes_crlf(
     monkeypatch.setenv('GIT_EDITOR', 'true')
     out = b4.edit_in_editor(b'line one\r\nline two\r\rlast\r\n', filehint='reply.eml')
     assert out == b'line one\nline two\n\nlast\n'
+
+
+def test_git_run_command_log_fixup_looks_past_option_prefix(gitdir: str) -> None:
+    """``--no-abbrev-commit`` must survive a leading ``-c`` override.
+
+    git_run_command counteracts log.abbrevCommit by injecting the flag after
+    the subcommand -- which it can only find by skipping the ``-c key=value``
+    pairs callers prefix (see SCRATCH_GIT_OPTS).
+    """
+    b4.git_set_config(gitdir, 'log.abbrevCommit', 'true')
+    ecode, out = b4.git_run_command(gitdir, ['-c', 'gc.auto=0', 'log', '-1'])
+    assert ecode == 0
+    assert out.startswith('commit '), out
+    sha = out.split('\n', 1)[0].split()[1]
+    assert len(sha) == 40, f'log abbreviated the sha despite the fixup: {sha}'
diff --git a/src/tests/test_ez.py b/src/tests/test_ez.py
index 192b000..97a3bd2 100644
--- a/src/tests/test_ez.py
+++ b/src/tests/test_ez.py
@@ -1,7 +1,7 @@
 import logging
 import os
 from email.message import EmailMessage
-from typing import Any, Dict, Generator, List, Optional, Set, Tuple, cast
+from typing import Any, Callable, Dict, Generator, List, Optional, Set, Tuple, cast
 from unittest.mock import patch
 
 import pytest
@@ -1253,3 +1253,46 @@ def test_claim_branch_description_restamps_series(prepdir: str) -> None:
     # Authorship preserved; committer re-stamped to the current identity.
     assert all(a == orig_email for a, _c in post)
     assert all(c == 'changed@example.com' for _a, c in post)
+
+
+def test_reroll_ignores_submodule_recurse(
+    gitdir: str, add_unpopulated_submodule: Callable[..., str]
+) -> None:
+    """reroll()'s sent-tag worktree is one of b4's scratch worktrees too.
+
+    Under prep-cover-strategy=commit the series is cherry-picked into a fresh
+    sparse worktree whose ``checkout -f`` dies on the user's submodule.recurse
+    -- and the sent tag goes with it, swallowed as "Error tagging the
+    revision".
+    """
+    add_unpopulated_submodule(gitdir)
+
+    b4.MAIN_CONFIG.update({'prep-cover-strategy': 'commit'})
+    parser = b4.command.setup_parser()
+    cmdargs = parser.parse_args(
+        ['--no-stdin', '--no-interactive', '--offline-mode', 'prep', '-n', 'pytest']
+    )
+    b4.ez.cmd_prep(cmdargs)
+
+    with open(os.path.join(gitdir, 'file1.txt'), 'a') as fh:
+        fh.write('reroll tweak\n')
+    b4.git_run_command(gitdir, ['add', 'file1.txt'])
+    ecode, _ = b4.git_run_command(gitdir, ['commit', '-m', 'reroll: tweak file1'])
+    assert ecode == 0
+
+    _cover, tracking = b4.ez.load_cover(strip_comments=True)
+    tagname, _rev = b4.ez.get_sent_tagname(
+        tracking['series']['change-id'], b4.ez.SENT_TAG_PREFIX, 1
+    )
+
+    # Only now flip the config the bug needs, so the fixture setup above never
+    # ran under recursion itself.
+    b4.git_set_config(gitdir, 'submodule.recurse', 'true')
+
+    mybranch = b4.git_get_current_branch()
+    assert mybranch is not None
+    b4.ez.reroll(mybranch, 'sent tag message\n', 'reroll-recurse@test.local')
+
+    assert b4.git_revparse_tag(None, tagname), (
+        f'reroll did not tag {tagname}: the scratch worktree recursed'
+    )
diff --git a/src/tests/test_tui_tracking.py b/src/tests/test_tui_tracking.py
index b4db46b..993c640 100644
--- a/src/tests/test_tui_tracking.py
+++ b/src/tests/test_tui_tracking.py
@@ -15,7 +15,7 @@ import datetime
 import email.message
 import os
 import pathlib
-from typing import Any, Dict, List, Optional, Tuple
+from typing import Any, Callable, Dict, List, Optional, Tuple
 from unittest.mock import patch
 
 import pytest
@@ -3010,6 +3010,114 @@ class TestCrossWorktreeFetch:
         assert blob.strip() == 'cross-worktree marker'
 
 
+def _gitlink_bump_series(gitdir: str, name: str = 'sub') -> b4.LoreSeries:
+    """A 1-patch series that bumps the submodule gitlink.
+
+    The patch carries an ``index <old>..<new> 160000`` line, so the fake-am
+    tree synthesized from it holds a gitlink of its own -- which is what makes
+    make_fake_am_range's ``reset --hard`` recurse.
+    """
+    ecode, newlink = b4.git_run_command(gitdir, ['rev-parse', 'master'])
+    assert ecode == 0
+    ecode, _ = b4.git_run_command(
+        gitdir,
+        ['update-index', '--cacheinfo', f'160000,{newlink.strip()},{name}'],
+    )
+    assert ecode == 0
+    ecode, tree = b4.git_run_command(gitdir, ['write-tree'])
+    assert ecode == 0
+    ecode, commit = b4.git_run_command(
+        gitdir,
+        ['commit-tree', tree.strip(), '-p', 'master'],
+        stdin=f'bump {name}\n'.encode(),
+    )
+    assert ecode == 0
+    ecode, mbox = b4.git_run_command(
+        gitdir, ['format-patch', '-1', '--stdout', commit.strip()], decode=False
+    )
+    assert ecode == 0
+    assert b'160000' in mbox, 'patch does not carry a gitlink index line'
+    ecode, _ = b4.git_run_command(gitdir, ['reset', '--hard', 'master'])
+    assert ecode == 0
+    msgs = b4.mailsplit_bytes(mbox)
+    for idx, msg in enumerate(msgs):
+        if not msg['Message-Id']:
+            msg['Message-Id'] = f'<gitlink-bump-{idx}@test.local>'
+    return b4.review._get_lore_series(msgs)
+
+
+class TestScratchWorktreeSubmodules:
+    """b4's scratch worktrees must ignore the user's submodule.recurse.
+
+    A fresh linked worktree has no per-worktree submodule clones, so a
+    recursing checkout or reset dies with "fatal: not a git repository:
+    .../worktrees/<wt>/modules/<name>" before anything is applied. In a repo
+    carrying submodules (the b4 repo itself among them) that killed every
+    review-branch creation and shazam apply.
+    """
+
+    def test_fetch_am_ignores_submodule_recurse(
+        self, gitdir: str, add_unpopulated_submodule: Callable[..., str]
+    ) -> None:
+        add_unpopulated_submodule(gitdir)
+
+        # An am-able patch on top of the submodule-bearing master.
+        patchfile = pathlib.Path(gitdir) / 'sub_recurse_patch.txt'
+        patchfile.write_text('submodule recurse marker\n')
+        ecode, _ = b4.git_run_command(gitdir, ['add', 'sub_recurse_patch.txt'])
+        assert ecode == 0
+        ecode, tree = b4.git_run_command(gitdir, ['write-tree'])
+        assert ecode == 0
+        ecode, commit = b4.git_run_command(
+            gitdir,
+            ['commit-tree', tree.strip(), '-p', 'master'],
+            stdin=b'add sub_recurse_patch\n',
+        )
+        assert ecode == 0
+        ecode, mbox = b4.git_run_command(
+            gitdir, ['format-patch', '-1', '--stdout', commit.strip()], decode=False
+        )
+        assert ecode == 0
+        ecode, _ = b4.git_run_command(gitdir, ['reset', '--hard', 'master'])
+        assert ecode == 0
+
+        # Only now flip the config the bug needs, so the fixture setup above
+        # never ran under recursion itself.
+        b4.git_set_config(gitdir, 'submodule.recurse', 'true')
+
+        b4.git_fetch_am_into_repo(gitdir, mbox, at_base='master', am_flags=['-3'])
+
+        ecode, blob = b4.git_run_command(
+            gitdir, ['show', 'FETCH_HEAD:sub_recurse_patch.txt']
+        )
+        assert ecode == 0, 'scratch-worktree apply failed under submodule.recurse'
+        assert blob.strip() == 'submodule recurse marker'
+
+    def test_fake_am_range_ignores_scratch_worktree_config(
+        self,
+        gitdir: str,
+        monkeypatch: pytest.MonkeyPatch,
+        add_unpopulated_submodule: Callable[..., str],
+    ) -> None:
+        """The fake-am staging worktree is a scratch worktree too.
+
+        Its ``reset --hard`` recurses once the synthesized tree carries a
+        gitlink, and its ``git am`` builds throwaway commits nothing will ever
+        ship -- so it must not reach for the user's signing key either.
+        """
+        add_unpopulated_submodule(gitdir)
+        lser = _gitlink_bump_series(gitdir)
+        b4.git_set_config(gitdir, 'submodule.recurse', 'true')
+        _poison_gpg_signing(monkeypatch)
+
+        start, end = lser.make_fake_am_range(gitdir=gitdir, at_base='master')
+
+        assert start, 'fake-am range died in the scratch worktree'
+        assert end
+        assert b4.git_commit_exists(gitdir, start)
+        assert b4.git_commit_exists(gitdir, end)
+
+
 class TestMergeTakeSkipRouting:
     """Take routing when patches are skipped (bug 6d1d35c).
 
@@ -4596,3 +4704,39 @@ class TestTestApplyNoSigning:
         screen = TakeConfirmScreen('linear', 'master', branch)
         ok, detail = screen._test_take()
         assert ok, f'take test-apply tried to sign: {detail}'
+
+
+class TestTestApplySubmoduleRecurse:
+    """The same test-applies must survive the user's submodule.recurse.
+
+    Their scratch worktree is as fresh as any other, so the sparse
+    ``checkout -f`` hits the same missing-submodule-clone fatal and every
+    probe reports a failure that has nothing to do with the patches.
+    """
+
+    @pytest.mark.parametrize(
+        'screen_cls',
+        [RebaseScreen, TargetBranchScreen, BaseSelectionScreen],
+        ids=lambda c: c.__name__,
+    )
+    def test_static_test_apply_ignores_submodule_recurse(
+        self,
+        gitdir: str,
+        screen_cls: Any,
+        add_unpopulated_submodule: Callable[..., str],
+    ) -> None:
+        ambytes = _one_patch_mbox(gitdir)
+        add_unpopulated_submodule(gitdir)
+        b4.git_set_config(gitdir, 'submodule.recurse', 'true')
+        ok, detail = screen_cls._test_apply_at(ambytes, 'master')
+        assert ok, f'{screen_cls.__name__} test-apply recursed: {detail}'
+
+    def test_take_confirm_test_apply_ignores_submodule_recurse(
+        self, gitdir: str, add_unpopulated_submodule: Callable[..., str]
+    ) -> None:
+        branch = _create_review_branch(gitdir, 'recurse-take', with_patch=True)
+        add_unpopulated_submodule(gitdir)
+        b4.git_set_config(gitdir, 'submodule.recurse', 'true')
+        screen = TakeConfirmScreen('linear', 'master', branch)
+        ok, detail = screen._test_take()
+        assert ok, f'take test-apply recursed: {detail}'

-- 
2.53.0

Re: [PATCH b4 0/6] Keep the user's git config out of b4's scratch worktrees

[Konstantin Ryabitsev]

On Wed, 29 Jul 2026 12:44:27 +0200, Christian Brauner wrote:
> Keep the user's git config out of b4's scratch worktrees
> 
> With submodule.recurse=true in the user's config b4 cannot apply
> anything in a repo that carries submodules. The scratch worktree's
> sparse checkout recurses, looks for the per-worktree submodule clones a
> just-created worktree cannot have, and dies:
> 
> [...]

Applied, thanks!

[1/6] git_run_command: look past -c overrides for the subcommand
      commit: 97e09203a66f7849500cd0258d83864c15535461
[2/6] shazam: ignore the user's submodule.recurse in the scratch worktree
      commit: ea34dcdac92304fa422ee90ab9b541931d7e527e
[3/6] review-tui: use the shared scratch-worktree overrides for test applies
      commit: c9a21b8111ddd79485a4e8d787ff43158ee49aff
[4/6] fake-am: use the scratch-worktree overrides in the staging worktree
      commit: cecec5d1844fdbcc4cfa90e56b10f6358eb4948d
[5/6] send: use the scratch-worktree overrides when tagging a sent series
      commit: 0abeddbb46c2f86b052de4e31816912246bd9d23
[6/6] tests: exercise the scratch worktrees under submodule.recurse
      commit: 6991ac96a05cd410c1734a0cd15307481f11860b

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