Re: [PATCH v2] merge-file: correctly open files when in a subdir

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 11, 2015, 18:21 UTC
Message-ID: <xmqqd25gux4r.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAPHKiG6M9_fOjpx8Pt8UTpUcrS+tmqL3YcT11WyJJu8m6nkJ4A@mail.gmail.com>

Aleksander Boruch-Gruszecki <aleksander.boruchgruszecki@gmail.com> writes:

Show 16 quoted lines

>>> @@ -72,6 +72,12 @@ test_expect_success 'works in subdirectory' '
>>>      ( cd dir && git merge-file a.txt o.txt b.txt )
>>>  '
>>>
>>> +mkdir -p dir/deep
>>> +cp new1.txt orig.txt new2.txt dir/deep
>>> +test_expect_success 'accounts for subdirectory when writing' '
>>> +    (cd dir && git merge-file deep/new1.txt deep/orig.txt deep/new2.txt)
>>> +'
>>
>> Interesting.  Makes us wonder why the one before this new one you
>> added did not catch the issue, doesn't it?
>
> The test before the one added by me does work because merge-file
> tries to open "a.txt" for writing in repo root directory, which will create
> a file if it does not exist.

Ahh, this existing test

>>>      ( cd dir && git merge-file a.txt o.txt b.txt )

implicitly expects that dir/a.txt is written, but the broken implementation writes to a.txt (i.e. outside dir). But the test only checks the exit code from the command without making sure that dir/a.txt is written, it does not notice the breakage.

Thanks, that makes sense and it also makes sense that checking the resulting content in dir/a.txt would make sense. Then we many not need to add a new dir/deep/* test---after all they are checking the same thing.