Re: [PATCH v4 2/2] interpret-trailers: add option for in-place editing

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 18, 2016, 21:11 UTC
Message-ID: <CAPig+cRRdca7PfkqppY2X7KSFpHX0yH19fxRL+w_=u9vg7NV9A@mail.gmail.com>
In-Reply-To: <xmqqio2vki0i.fsf@gitster.mtv.corp.google.com>

On Thu, Jan 14, 2016 at 3:45 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Tobias Klauser <tklauser@distanz.ch> writes:
>> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
>> @@ -326,6 +326,46 @@ test_expect_success 'with complex patch, args and --trim-empty' '
>> +test_expect_success POSIXPERM,SANITY "in-place editing doesn't clobber original file on error" '

I think POSIXPERM is all you need for this case; SANITY doesn't buy you anything, if I understand correctly.

Show 18 quoted lines

>> +     cat basic_message >message &&
>> +     chmod -r message &&
>> +     test_must_fail git interpret-trailers --trailer "Reviewed-by: Alice" --in-place message &&
>> +     chmod +r message &&
>> +     test_cmp message basic_message
>> +'
>
> If for some reason interpret-trailers fails to fail, this would
> leave an unreadable 'message' in the trash directory.  Maybe no
> other tests that come after this one want to be able to read the
> contents of the file right now, but this is an accident waiting to
> happen:
>
>         cat basic_message >message &&
> +       test_when_finished "chmod +r message" &&
>         chmod -r message &&
>         test_must_fail ... &&
>         chmod +r message &&

Don't forget to remove this (now unnecessary) "chmod +r" once you've added the 'test_when_finished "chmod +r"'.

>         test_cmp ...