All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Aleksander Boruch-Gruszecki <aleksander.boruchgruszecki@gmail.com>
Cc: Duy Nguyen <pclouds@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Stefan Beller <stefanbeller@googlemail.com>
Subject: Re: [PATCH v2] merge-file: correctly open files when in a subdir
Date: Wed, 11 Feb 2015 10:21:56 -0800	[thread overview]
Message-ID: <xmqqd25gux4r.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAPHKiG6M9_fOjpx8Pt8UTpUcrS+tmqL3YcT11WyJJu8m6nkJ4A@mail.gmail.com> (Aleksander Boruch-Gruszecki's message of "Wed, 11 Feb 2015 10:58:51 +0100")

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

>>> @@ -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.

  reply	other threads:[~2015-02-11 18:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-10 20:23 [PATCH v2] merge-file: correctly open files when in a subdir Aleksander Boruch-Gruszecki
2015-02-10 20:57 ` Junio C Hamano
2015-02-11  9:58   ` Aleksander Boruch-Gruszecki
2015-02-11 18:21     ` Junio C Hamano [this message]
2015-02-11 19:02       ` Junio C Hamano
2015-02-11 21:20         ` Aleksander Boruch-Gruszecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqd25gux4r.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=aleksander.boruchgruszecki@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    --cc=stefanbeller@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.