Re: [PATCH] merge-file: fix BUG when --object-id is used in a worktree
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 10, 2026, 20:01 UTC
- Message-ID
- <xmqqh5qntpvy.fsf@gitster.g>
- In-Reply-To
- <abATPiRUczb8fe4t@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 6 quoted lines
> 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.
Show 11 quoted lines
>> 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.
Show 15 quoted lines
>> 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.