* Re: [PATCH] merge-file: fix BUG when --object-id is used in a worktree
2026-03-10 11:46 [PATCH] merge-file: fix BUG when --object-id is used in a worktree Mathias Rav
@ 2026-03-10 12:35 ` Karthik Nayak
2026-03-10 12:49 ` Patrick Steinhardt
2026-03-10 13:34 ` [PATCH] " Kristoffer Haugsbakk
2 siblings, 0 replies; 13+ messages in thread
From: Karthik Nayak @ 2026-03-10 12:35 UTC (permalink / raw)
To: Mathias Rav, git
Cc: Phillip Wood, John Cai, Ævar Arnfjörð Bjarmason,
Junio C Hamano, brian m. carlson, Patrick Steinhardt
[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]
"Mathias Rav" <m@git.strova.dk> writes:
> The `--object-id` option was added in commit e1068f0ad4
> ("merge-file: add an option to process object IDs", 2023-11-01)
> together with a call to setup_git_directory() to avoid crashing
> when run outside a repository.
>
> However, the call to setup_git_directory() is redundant when run inside
> a repository, as merge-file runs with RUN_SETUP_GENTLY, so the
> repository has already been set up. The redundant call is harmless when
> worktrees are not used, but when run inside a worktree, the
> repo_set_gitdir() function ends up being called twice.
>
> Calling repo_set_gitdir() used to be silently accepted, but commit
> 2816b748e5 ("odb: handle changing a repository's commondir", 2025-11-19)
> changed this to a BUG in repository.c with the error message:
> "cannot reinitialize an already-initialized object directory".
>
> Guard the call to setup_git_directory() behind a repo pointer check,
> to ensure that we continue to give the correct "not a git repo" error
> whilst avoiding the BUG when running inside a worktree.
>
Well explained. Additionally you may want to clarify that you're talking
about a linked worktree. Since the dir initialized by 'git-init(1)' or
'git-clone(1)' is also a worktree (AKA main worktree).
> Signed-off-by: Mathias Rav <m@git.strova.dk>
> ---
> builtin/merge-file.c | 4 ++--
> t/t6403-merge-file.sh | 9 +++++++++
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge-file.c b/builtin/merge-file.c
> index 46775d0c79..a8768c6e0c 100644
> --- a/builtin/merge-file.c
> +++ b/builtin/merge-file.c
> @@ -60,7 +60,7 @@ static int diff_algorithm_cb(const struct option *opt,
> int cmd_merge_file(int argc,
> const char **argv,
> const char *prefix,
> - struct repository *repo UNUSED)
> + struct repository *repo)
> {
> const char *names[3] = { 0 };
> mmfile_t mmfs[3] = { 0 };
> @@ -110,7 +110,7 @@ int cmd_merge_file(int argc,
> return error_errno("failed to redirect stderr to /dev/null");
> }
>
> - if (object_id)
> + if (object_id && !repo)
> setup_git_directory();
>
Nit: but would be nice to also add a comment here.
> for (i = 0; i < 3; i++) {
> diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
> index 06ab4d7aed..60cc43775f 100755
> --- a/t/t6403-merge-file.sh
> +++ b/t/t6403-merge-file.sh
> @@ -506,6 +506,15 @@ test_expect_success '--object-id fails without repository' '
> grep "not a git repository" err
> '
>
> +test_expect_success 'run inside worktree with --object-id' '
> + empty="$(test_oid empty_blob)" &&
> + git worktree add work &&
> + (cd work && git merge-file --object-id $empty $empty $empty) >actual &&
> + git worktree remove work &&
> + git merge-file --object-id $empty $empty $empty >expected &&
> + test_cmp actual expected
> +'
> +
> test_expect_success 'merging C files with "myers" diff algorithm creates some spurious conflicts' '
> cat >expect.c <<-\EOF &&
> int g(size_t u)
> --
> 2.53.0
Looks good otherwise.
- Karthik
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] merge-file: fix BUG when --object-id is used in a worktree
2026-03-10 11:46 [PATCH] merge-file: fix BUG when --object-id is used in a worktree Mathias Rav
2026-03-10 12:35 ` Karthik Nayak
@ 2026-03-10 12:49 ` Patrick Steinhardt
2026-03-10 20:01 ` Junio C Hamano
2026-03-10 13:34 ` [PATCH] " Kristoffer Haugsbakk
2 siblings, 1 reply; 13+ messages in thread
From: Patrick Steinhardt @ 2026-03-10 12:49 UTC (permalink / raw)
To: Mathias Rav
Cc: git, Phillip Wood, John Cai,
Ævar Arnfjörð Bjarmason, Junio C Hamano,
brian m. carlson
On Tue, Mar 10, 2026 at 11:46:01AM +0000, Mathias Rav wrote:
Which commit is this patch based on? It doesn't apply in its current
form on top of "master" since at least 8600b4ec9e (merge-file: honor
merge.conflictStyle outside of a repository, 2026-02-07). Please rebase
the patch.
> diff --git a/builtin/merge-file.c b/builtin/merge-file.c
> index 46775d0c79..a8768c6e0c 100644
> --- a/builtin/merge-file.c
> +++ b/builtin/merge-file.c
> @@ -110,7 +110,7 @@ int cmd_merge_file(int argc,
> return error_errno("failed to redirect stderr to /dev/null");
> }
>
> - if (object_id)
> + if (object_id && !repo)
> setup_git_directory();
>
> for (i = 0; i < 3; i++) {
Okay, makes sense. Makes me wonder whether we have other cases of the
same error class.
> diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
> index 06ab4d7aed..60cc43775f 100755
> --- a/t/t6403-merge-file.sh
> +++ b/t/t6403-merge-file.sh
> @@ -506,6 +506,15 @@ test_expect_success '--object-id fails without repository' '
> grep "not a git repository" err
> '
>
> +test_expect_success 'run inside worktree with --object-id' '
> + empty="$(test_oid empty_blob)" &&
> + git worktree add work &&
> + (cd work && git merge-file --object-id $empty $empty $empty) >actual &&
This can be written without a subshell by saying `git -C work
merge-file`.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] merge-file: fix BUG when --object-id is used in a worktree
2026-03-10 12:49 ` Patrick Steinhardt
@ 2026-03-10 20:01 ` Junio C Hamano
2026-03-11 6:44 ` [PATCH v2] " Mathias Rav
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2026-03-10 20:01 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Mathias Rav, git, Phillip Wood, John Cai,
Ævar Arnfjörð Bjarmason, brian m. carlson
Patrick Steinhardt <ps@pks.im> writes:
> On Tue, Mar 10, 2026 at 11:46:01AM +0000, Mathias Rav wrote:
>
> Which commit is this patch based on? It doesn't apply in its current
> form on top of "master" since at least 8600b4ec9e (merge-file: honor
> merge.conflictStyle outside of a repository, 2026-02-07). Please rebase
> the patch.
This applies cleanly relative to v2.53.0. Generally, it is a
recommended practice to fork from the latest stable release in many
projects, so I do not mind it too much.
>> diff --git a/builtin/merge-file.c b/builtin/merge-file.c
>> index 46775d0c79..a8768c6e0c 100644
>> --- a/builtin/merge-file.c
>> +++ b/builtin/merge-file.c
>> @@ -110,7 +110,7 @@ int cmd_merge_file(int argc,
>> return error_errno("failed to redirect stderr to /dev/null");
>> }
>>
>> - if (object_id)
>> + if (object_id && !repo)
>> setup_git_directory();
Good. I suspect we would want to check !repo first, though. The
idea here is
(1) We know we need to support running "git merge-file" outside a
repository, so the git potty may have called us with !repo (aka
RUN_SETUP_GENTLY);
(2) But we do need to make sure we have a repository in some case.
We happen to have only one case, i.e., when "--object-id"
option is in use, but that condition may grow in the future.
So in the longer run, we'd better prepared for the "has the user
gave us the object_id option?" part to grow over time, which would
mean
if (!repo &&
(object_id || another_option || yet_another ...))
setup_git_directory();
would be easier to handle.
>> for (i = 0; i < 3; i++) {
>
> Okay, makes sense. Makes me wonder whether we have other cases of the
> same error class.
While I agree that the second call to setup_git_directory() that
triggered this patch is pointless, I also wish that the function
were more robust. I wonder if there is a clean way to make it
idempotent.
>> diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
>> index 06ab4d7aed..60cc43775f 100755
>> --- a/t/t6403-merge-file.sh
>> +++ b/t/t6403-merge-file.sh
>> @@ -506,6 +506,15 @@ test_expect_success '--object-id fails without repository' '
>> grep "not a git repository" err
>> '
>>
>> +test_expect_success 'run inside worktree with --object-id' '
>> + empty="$(test_oid empty_blob)" &&
>> + git worktree add work &&
>> + (cd work && git merge-file --object-id $empty $empty $empty) >actual &&
>
> This can be written without a subshell by saying `git -C work
> merge-file`.
Yup, that would make the test even better.
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-10 20:01 ` Junio C Hamano
@ 2026-03-11 6:44 ` Mathias Rav
2026-03-11 7:18 ` Kristoffer Haugsbakk
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Mathias Rav @ 2026-03-11 6:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Phillip Wood, John Cai, Ævar Arnfjörð Bjarmason,
brian m. carlson, Patrick Steinhardt, git
The `--object-id` option was added in commit e1068f0ad4
(merge-file: add an option to process object IDs, 2023-11-01)
together with a call to setup_git_directory() to avoid crashing
when run outside a repository.
However, the call to setup_git_directory() is redundant when run inside
a repository, as merge-file runs with RUN_SETUP_GENTLY, so the
repository has already been set up. The redundant call is harmless
when linked worktrees are not used, but in a linked worktree,
the repo_set_gitdir() function ends up being called twice.
Calling repo_set_gitdir() used to be silently accepted, but commit
2816b748e5 (odb: handle changing a repository's commondir, 2025-11-19)
changed this to a BUG in repository.c with the error message:
"cannot reinitialize an already-initialized object directory".
Guard the redundant call to setup_git_directory() behind a repo pointer
check, to ensure that we continue to give the correct "not a git repo"
error whilst avoiding the BUG when running in a linked worktree.
Signed-off-by: Mathias Rav <m@git.strova.dk>
---
Thanks Karthik, Patrick, Kristoffer and Junio for your feedback.
I've incorporated the sum of it all in this PATCH v2:
- Check !repo before object_id and add a comment
- Use term "linked worktree" instead of just "worktree" throughout
- Use git -C instead of a subshell in test
- Remove gitk's quotes from the commit references in the commit message
As for the quotes in the commit references, I use gitk's "Copy commit
reference" daily and am personally used to the quotes. Since
SubmittingPatches seems to give equal preference to --pretty=reference and
"Copy commit reference" I didn't think that the quotes were a problem.
(I wonder how controversial it would be to remove the quotes in gitk.)
builtin/merge-file.c | 5 +++--
t/t6403-merge-file.sh | 9 +++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 46775d0c79..cc8fda3b5b 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -60,7 +60,7 @@ static int diff_algorithm_cb(const struct option *opt,
int cmd_merge_file(int argc,
const char **argv,
const char *prefix,
- struct repository *repo UNUSED)
+ struct repository *repo)
{
const char *names[3] = { 0 };
mmfile_t mmfs[3] = { 0 };
@@ -110,7 +110,8 @@ int cmd_merge_file(int argc,
return error_errno("failed to redirect stderr to /dev/null");
}
- if (object_id)
+ if (!repo && object_id)
+ /* emit the correct "not a git repo" error in this case */
setup_git_directory();
for (i = 0; i < 3; i++) {
diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
index 06ab4d7aed..ed7eec8f93 100755
--- a/t/t6403-merge-file.sh
+++ b/t/t6403-merge-file.sh
@@ -506,6 +506,15 @@ test_expect_success '--object-id fails without repository' '
grep "not a git repository" err
'
+test_expect_success 'run in a linked worktree with --object-id' '
+ empty="$(test_oid empty_blob)" &&
+ git worktree add work &&
+ git -C work merge-file --object-id $empty $empty $empty >actual &&
+ git worktree remove work &&
+ git merge-file --object-id $empty $empty $empty >expected &&
+ test_cmp actual expected
+'
+
test_expect_success 'merging C files with "myers" diff algorithm creates some spurious conflicts' '
cat >expect.c <<-\EOF &&
int g(size_t u)
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-11 6:44 ` [PATCH v2] " Mathias Rav
@ 2026-03-11 7:18 ` Kristoffer Haugsbakk
2026-03-11 11:14 ` Kristoffer Haugsbakk
2026-03-18 19:16 ` Mathias Rav
2 siblings, 0 replies; 13+ messages in thread
From: Kristoffer Haugsbakk @ 2026-03-11 7:18 UTC (permalink / raw)
To: Mathias Rav, Junio C Hamano
Cc: Phillip Wood, John Cai, Ævar Arnfjörð Bjarmason,
brian m. carlson, Patrick Steinhardt, git, Karthik Nayak
On Wed, Mar 11, 2026, at 07:44, Mathias Rav wrote:
>[snip]
> Signed-off-by: Mathias Rav <m@git.strova.dk>
> ---
> Thanks Karthik, Patrick, Kristoffer and Junio for your feedback.
> I've incorporated the sum of it all in this PATCH v2:
>
> - Check !repo before object_id and add a comment
> - Use term "linked worktree" instead of just "worktree" throughout
> - Use git -C instead of a subshell in test
> - Remove gitk's quotes from the commit references in the commit message
>
> As for the quotes in the commit references, I use gitk's "Copy commit
> reference" daily and am personally used to the quotes. Since
> SubmittingPatches seems to give equal preference to --pretty=reference and
> "Copy commit reference" I didn't think that the quotes were a problem.
> (I wonder how controversial it would be to remove the quotes in gitk.)
Thanks for this explanation. I was ignorant of this Gitk fact.
>[snip]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-11 6:44 ` [PATCH v2] " Mathias Rav
2026-03-11 7:18 ` Kristoffer Haugsbakk
@ 2026-03-11 11:14 ` Kristoffer Haugsbakk
2026-03-11 17:26 ` Junio C Hamano
2026-03-18 19:16 ` Mathias Rav
2 siblings, 1 reply; 13+ messages in thread
From: Kristoffer Haugsbakk @ 2026-03-11 11:14 UTC (permalink / raw)
To: Mathias Rav, Junio C Hamano
Cc: Phillip Wood, John Cai, Ævar Arnfjörð Bjarmason,
brian m. carlson, Patrick Steinhardt, git, Karthik Nayak
On Wed, Mar 11, 2026, at 07:44, Mathias Rav wrote:
>[snip]
> As for the quotes in the commit references, I use gitk's "Copy commit
> reference" daily and am personally used to the quotes. Since
> SubmittingPatches seems to give equal preference to --pretty=reference and
> "Copy commit reference" I didn't think that the quotes were a problem.
> (I wonder how controversial it would be to remove the quotes in gitk.)
Turns out that this has been attempted least once before:
https://lore.kernel.org/git/1472230741-5161-1-git-send-email-dev+git@drbeat.li/
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-11 11:14 ` Kristoffer Haugsbakk
@ 2026-03-11 17:26 ` Junio C Hamano
2026-03-11 20:16 ` Kristoffer Haugsbakk
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2026-03-11 17:26 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: Mathias Rav, Phillip Wood, John Cai,
Ævar Arnfjörð Bjarmason, brian m. carlson,
Patrick Steinhardt, git, Karthik Nayak
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> On Wed, Mar 11, 2026, at 07:44, Mathias Rav wrote:
>>[snip]
>> As for the quotes in the commit references, I use gitk's "Copy commit
>> reference" daily and am personally used to the quotes. Since
>> SubmittingPatches seems to give equal preference to --pretty=reference and
>> "Copy commit reference" I didn't think that the quotes were a problem.
>
>> (I wonder how controversial it would be to remove the quotes in gitk.)
>
> Turns out that this has been attempted least once before:
>
> https://lore.kernel.org/git/1472230741-5161-1-git-send-email-dev+git@drbeat.li/
True.
Perhaps something like this patch makes it clear that what the gitk
command gives and what --pretty=reference gives are not identical,
to avoid confusion like this? I dunno.
Documentation/SubmittingPatches | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git c/Documentation/SubmittingPatches w/Documentation/SubmittingPatches
index e270ccbe85..fad0b41af0 100644
--- c/Documentation/SubmittingPatches
+++ w/Documentation/SubmittingPatches
@@ -335,7 +335,7 @@ date)", like this:
....
The "Copy commit reference" command of gitk can be used to obtain this
-format (with the subject enclosed in a pair of double-quotes), or this
+format (but with the subject enclosed in an extra pair of double-quotes), or this
invocation of `git show`:
....
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-11 17:26 ` Junio C Hamano
@ 2026-03-11 20:16 ` Kristoffer Haugsbakk
2026-03-18 19:40 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: Kristoffer Haugsbakk @ 2026-03-11 20:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Mathias Rav, Phillip Wood, John Cai,
Ævar Arnfjörð Bjarmason, brian m. carlson,
Patrick Steinhardt, git, Karthik Nayak
On Wed, Mar 11, 2026, at 18:26, Junio C Hamano wrote:
>>>[snip]
>>
>> Turns out that this has been attempted least once before:
>>
>> https://lore.kernel.org/git/1472230741-5161-1-git-send-email-dev+git@drbeat.li/
>
> True.
>
> Perhaps something like this patch makes it clear that what the gitk
> command gives and what --pretty=reference gives are not identical,
> to avoid confusion like this? I dunno.
>
>
> Documentation/SubmittingPatches | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git c/Documentation/SubmittingPatches w/Documentation/SubmittingPatches
> index e270ccbe85..fad0b41af0 100644
> --- c/Documentation/SubmittingPatches
> +++ w/Documentation/SubmittingPatches
> @@ -335,7 +335,7 @@ date)", like this:
> ....
>
> The "Copy commit reference" command of gitk can be used to obtain this
> -format (with the subject enclosed in a pair of double-quotes), or this
And now I see for the first time that the doc points out the difference
already... I’m really paying attention it turns out.
> +format (but with the subject enclosed in an extra pair of double-quotes), or this
I think replacing “with” with “but” is good. But why “extra” pairs? It’s
just a pair of double quotes.
> invocation of `git show`:
>
> ....
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-11 20:16 ` Kristoffer Haugsbakk
@ 2026-03-18 19:40 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2026-03-18 19:40 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: Mathias Rav, Phillip Wood, John Cai,
Ævar Arnfjörð Bjarmason, brian m. carlson,
Patrick Steinhardt, git, Karthik Nayak
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>> diff --git c/Documentation/SubmittingPatches w/Documentation/SubmittingPatches
>> index e270ccbe85..fad0b41af0 100644
>> --- c/Documentation/SubmittingPatches
>> +++ w/Documentation/SubmittingPatches
>> @@ -335,7 +335,7 @@ date)", like this:
>> ....
>>
>> The "Copy commit reference" command of gitk can be used to obtain this
>> -format (with the subject enclosed in a pair of double-quotes), or this
>
> And now I see for the first time that the doc points out the difference
> already... I’m really paying attention it turns out.
>
>> +format (but with the subject enclosed in an extra pair of double-quotes), or this
>
> I think replacing “with” with “but” is good. But why “extra” pairs? It’s
> just a pair of double quotes.
Because they can exist but they do not have to be there to be
understandable?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-11 6:44 ` [PATCH v2] " Mathias Rav
2026-03-11 7:18 ` Kristoffer Haugsbakk
2026-03-11 11:14 ` Kristoffer Haugsbakk
@ 2026-03-18 19:16 ` Mathias Rav
2026-03-18 19:45 ` Junio C Hamano
2 siblings, 1 reply; 13+ messages in thread
From: Mathias Rav @ 2026-03-18 19:16 UTC (permalink / raw)
To: git
Cc: Phillip Wood, John Cai, Ævar Arnfjörð Bjarmason,
brian m. carlson, Patrick Steinhardt, Junio C Hamano,
Karthik Nayak, Kristoffer Haugsbakk
On Wed, Mar 11, 2026, at 6:44 AM, Mathias Rav wrote:
> Thanks Karthik, Patrick, Kristoffer and Junio for your feedback.
> I've incorporated the sum of it all in this PATCH v2:
>
> - Check !repo before object_id and add a comment
> - Use term "linked worktree" instead of just "worktree" throughout
> - Use git -C instead of a subshell in test
> - Remove gitk's quotes from the commit references in the commit message
I'm unsure of the process from here for a small bugfix like this.
I believe I followed the SubmittingPatches document
by sending a PATCH v2 with To: Junio, Cc: list and others.
Do I need to do anything else to see this patch eventually land in Junio's tree?
Cheers,
Mathias
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] merge-file: fix BUG when --object-id is used in a worktree
2026-03-18 19:16 ` Mathias Rav
@ 2026-03-18 19:45 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2026-03-18 19:45 UTC (permalink / raw)
To: Mathias Rav
Cc: git, Phillip Wood, John Cai,
Ævar Arnfjörð Bjarmason, brian m. carlson,
Patrick Steinhardt, Karthik Nayak, Kristoffer Haugsbakk
"Mathias Rav" <m@git.strova.dk> writes:
> On Wed, Mar 11, 2026, at 6:44 AM, Mathias Rav wrote:
>> Thanks Karthik, Patrick, Kristoffer and Junio for your feedback.
>> I've incorporated the sum of it all in this PATCH v2:
>>
>> - Check !repo before object_id and add a comment
>> - Use term "linked worktree" instead of just "worktree" throughout
>> - Use git -C instead of a subshell in test
>> - Remove gitk's quotes from the commit references in the commit message
>
> I'm unsure of the process from here for a small bugfix like this.
> I believe I followed the SubmittingPatches document
> by sending a PATCH v2 with To: Junio, Cc: list and others.
> Do I need to do anything else to see this patch eventually land in Junio's tree?
Pinging like this was absolutely the right thing to do in this case,
as the patch fell through the cracks. Will apply (assuming that
everybody involved in the review is now happy with this iteration).
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] merge-file: fix BUG when --object-id is used in a worktree
2026-03-10 11:46 [PATCH] merge-file: fix BUG when --object-id is used in a worktree Mathias Rav
2026-03-10 12:35 ` Karthik Nayak
2026-03-10 12:49 ` Patrick Steinhardt
@ 2026-03-10 13:34 ` Kristoffer Haugsbakk
2 siblings, 0 replies; 13+ messages in thread
From: Kristoffer Haugsbakk @ 2026-03-10 13:34 UTC (permalink / raw)
To: Mathias Rav, git
Cc: Phillip Wood, John Cai, Ævar Arnfjörð Bjarmason,
Junio C Hamano, brian m. carlson, Patrick Steinhardt
On Tue, Mar 10, 2026, at 12:46, Mathias Rav wrote:
> The `--object-id` option was added in commit e1068f0ad4
> ("merge-file: add an option to process object IDs", 2023-11-01)
Using `git show -s --pretty=reference <commit>` for mentioning commits
is recommended (SubmittingPatches).
> together with a call to setup_git_directory() to avoid crashing
> when run outside a repository.
>
> However, the call to setup_git_directory() is redundant when run inside
> a repository, as merge-file runs with RUN_SETUP_GENTLY, so the
> repository has already been set up. The redundant call is harmless when
> worktrees are not used, but when run inside a worktree, the
> repo_set_gitdir() function ends up being called twice.
>
> Calling repo_set_gitdir() used to be silently accepted, but commit
> 2816b748e5 ("odb: handle changing a repository's commondir", 2025-11-19)
> changed this to a BUG in repository.c with the error message:
> "cannot reinitialize an already-initialized object directory".
>
> Guard the call to setup_git_directory() behind a repo pointer check,
> to ensure that we continue to give the correct "not a git repo" error
> whilst avoiding the BUG when running inside a worktree.
>
> Signed-off-by: Mathias Rav <m@git.strova.dk>
> ---
> builtin/merge-file.c | 4 ++--
> t/t6403-merge-file.sh | 9 +++++++++
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge-file.c b/builtin/merge-file.c
> index 46775d0c79..a8768c6e0c 100644
> --- a/builtin/merge-file.c
> +++ b/builtin/merge-file.c
> @@ -60,7 +60,7 @@ static int diff_algorithm_cb(const struct option *opt,
> int cmd_merge_file(int argc,
> const char **argv,
> const char *prefix,
> - struct repository *repo UNUSED)
> + struct repository *repo)
> {
> const char *names[3] = { 0 };
> mmfile_t mmfs[3] = { 0 };
> @@ -110,7 +110,7 @@ int cmd_merge_file(int argc,
> return error_errno("failed to redirect stderr to /dev/null");
> }
>
> - if (object_id)
> + if (object_id && !repo)
> setup_git_directory();
>
> for (i = 0; i < 3; i++) {
> diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
> index 06ab4d7aed..60cc43775f 100755
> --- a/t/t6403-merge-file.sh
> +++ b/t/t6403-merge-file.sh
> @@ -506,6 +506,15 @@ test_expect_success '--object-id fails without
> repository' '
> grep "not a git repository" err
> '
>
> +test_expect_success 'run inside worktree with --object-id' '
> + empty="$(test_oid empty_blob)" &&
> + git worktree add work &&
> + (cd work && git merge-file --object-id $empty $empty $empty) >actual
> &&
> + git worktree remove work &&
> + git merge-file --object-id $empty $empty $empty >expected &&
> + test_cmp actual expected
> +'
> +
> test_expect_success 'merging C files with "myers" diff algorithm
> creates some spurious conflicts' '
> cat >expect.c <<-\EOF &&
> int g(size_t u)
> --
> 2.53.0
^ permalink raw reply [flat|nested] 13+ messages in thread