Skip to content

for-each-repo: work correctly in a worktree #2056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
derrickstolee wants to merge 4 commits into from
+77 −24
Closed

for-each-repo: work correctly in a worktree #2056

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e9d4f3
for-each-repo: test outside of repo context
derrickstolee Feb 24, 2026
2439866
run-command: extract sanitize_repo_env helper
derrickstolee Mar 2, 2026
e759b35
for-each-repo: work correctly in a worktree
derrickstolee Feb 24, 2026
8b1f083
for-each-repo: simplify passing of parameters
derrickstolee Mar 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions t/t0068-for-each-repo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

Copy link
Copy Markdown

@gitgitgadget gitgitgadget Bot Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeff King wrote on the Git mailing list (how to reply to this email):

On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:

>  test_description='git for-each-repo builtin'
>  
> +# We need to test running 'git for-each-repo' outside of a repo context.
> +TEST_NO_CREATE_REPO=1
> +
>  . ./test-lib.sh

Interesting. I was going to point out that this won't do what you want
by itself, because Git will keep walking out of the trash directory and
may find the containing repository.

But it looks like this should be enough due to 614c3d8f2e (test-lib: set
GIT_CEILING_DIRECTORIES to protect the surrounding repository,
2021-08-29). Supporting this case wasn't the intent of that patch, but I
don't see any reason why it should not work reliably.

-Peff

Copy link
Copy Markdown

@gitgitgadget gitgitgadget Bot Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Jeff King <peff@peff.net> writes:

> On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:
>
>>  test_description='git for-each-repo builtin'
>>  
>> +# We need to test running 'git for-each-repo' outside of a repo context.
>> +TEST_NO_CREATE_REPO=1
>> +
>>  . ./test-lib.sh
>
> Interesting. I was going to point out that this won't do what you want
> by itself, because Git will keep walking out of the trash directory and
> may find the containing repository.
>
> But it looks like this should be enough due to 614c3d8f2e (test-lib: set
> GIT_CEILING_DIRECTORIES to protect the surrounding repository,
> 2021-08-29). Supporting this case wasn't the intent of that patch, but I
> don't see any reason why it should not work reliably.

I am surprised that use of GIT_CEILING_DIRECTORIES was not done
until 2021, actually.  The reason the configuration variable was
invented for is exactly to avoid discovery processes going upward
and ending up in a repository different from what we mean to work
with.

Copy link
Copy Markdown

@gitgitgadget gitgitgadget Bot Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Derrick Stolee wrote on the Git mailing list (how to reply to this email):

On 3/2/2026 1:31 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
>> On Mon, Mar 02, 2026 at 03:36:42PM +0000, Derrick Stolee via GitGitGadget wrote:
>>
>>>  test_description='git for-each-repo builtin'
>>>  
>>> +# We need to test running 'git for-each-repo' outside of a repo context.
>>> +TEST_NO_CREATE_REPO=1
>>> +
>>>  . ./test-lib.sh
>>
>> Interesting. I was going to point out that this won't do what you want
>> by itself, because Git will keep walking out of the trash directory and
>> may find the containing repository.
>>
>> But it looks like this should be enough due to 614c3d8f2e (test-lib: set
>> GIT_CEILING_DIRECTORIES to protect the surrounding repository,
>> 2021-08-29). Supporting this case wasn't the intent of that patch, but I
>> don't see any reason why it should not work reliably.
> 
> I am surprised that use of GIT_CEILING_DIRECTORIES was not done
> until 2021, actually.  The reason the configuration variable was
> invented for is exactly to avoid discovery processes going upward
> and ending up in a repository different from what we mean to work
> with.
 
I didn't know about these historical details. All I know is that I
wrote these changes on top of the buggy patch in [1] and confirmed that
it failed with a segfault. Thanks for confirming the reason that this
works!

[1] https://lore.kernel.org/git/86cd83f65b30aab3233e27b3e5c4f03041e68766.1771903950.git.gitgitgadget@gmail.com/

Thanks,
-Stolee

test_description='git for-each-repo builtin'

# We need to test running 'git for-each-repo' outside of a repo context.
TEST_NO_CREATE_REPO=1

. ./test-lib.sh

test_expect_success 'run based on configured value' '
Expand All @@ -10,9 +13,10 @@ test_expect_success 'run based on configured value' '
git init three &&
git init ~/four &&
git -C two commit --allow-empty -m "DID NOT RUN" &&
git config run.key "$TRASH_DIRECTORY/one" &&
git config --add run.key "$TRASH_DIRECTORY/three" &&
git config --add run.key "~/four" &&
git config --global run.key "$TRASH_DIRECTORY/one" &&
git config --global --add run.key "$TRASH_DIRECTORY/three" &&
git config --global --add run.key "~/four" &&

git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
git -C one log -1 --pretty=format:%s >message &&
grep ran message &&
Expand All @@ -22,6 +26,7 @@ test_expect_success 'run based on configured value' '
grep ran message &&
git -C ~/four log -1 --pretty=format:%s >message &&
grep ran message &&

git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
git -C one log -1 --pretty=format:%s >message &&
grep again message &&
Expand All @@ -46,7 +51,7 @@ test_expect_success 'error on bad config keys' '
'

test_expect_success 'error on NULL value for config keys' '
cat >>.git/config <<-\EOF &&
cat >>.gitconfig <<-\EOF &&
[empty]
key
EOF
Expand All @@ -59,8 +64,8 @@ test_expect_success 'error on NULL value for config keys' '
'

test_expect_success '--keep-going' '
git config keep.going non-existing &&
git config --add keep.going . &&
git config --global keep.going non-existing &&
git config --global --add keep.going one &&

test_must_fail git for-each-repo --config=keep.going \
-- branch >out 2>err &&
Expand All @@ -70,7 +75,7 @@ test_expect_success '--keep-going' '
test_must_fail git for-each-repo --config=keep.going --keep-going \
-- branch >out 2>err &&
test_grep "cannot change to .*non-existing" err &&
git branch >expect &&
git -C one branch >expect &&
test_cmp expect out
'

Expand Down