Re: [PATCH v4 8/8] t5520: check reflog action in fast-forward merge

2015-05-21 Thread Junio C Hamano
Paul Tan  writes:

> On Mon, May 18, 2015 at 11:20 PM, Johannes Schindelin
> ...
>>> + sed "s/$_x05[0-9a-f]*/OBJID/g" reflog.actual >reflog.fuzzy &&
>>
>> Actually, let's use "s/^[0-9a-f]*/OBJID/" instead: you only want to
>> replace the first few characters.
>
> Did you mean "s/^$_x05[0-9a-f]*/OBJID/"? (with "$_x05" expanding to
> '[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' from test-lib.sh). If not,
> then it would match even if there was no SHA1 hash.
>
> But yes, without the "^" there will very likely be false positives.
> Thanks for catching.

I think the suggestion was more about "do we guarantee that there
would always be at least five?"  It might happen to be the case with
our current default, but these tests do not fundamentally rely on
that default staying the same.  s/^[0-9a-f][0-9a-f]*/OBJECTNAME/g is
probably the right balance between cautiousness (you do not want to
match an empty string) and future-proofing (you do not want to rely
on us having at least five).

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 8/8] t5520: check reflog action in fast-forward merge

2015-05-21 Thread Paul Tan
Hi,

On Mon, May 18, 2015 at 11:20 PM, Johannes Schindelin
 wrote:
> Hi Paul,
>
> On 2015-05-18 15:32, Paul Tan wrote:
>
>> @@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' '
>>   git checkout copy &&
>>   test "$(cat file)" = file &&
>>   git pull &&
>> - test "$(cat file)" = updated
>> + test "$(cat file)" = updated &&
>> + git reflog -1 >reflog.actual &&
>> + sed "s/$_x05[0-9a-f]*/OBJID/g" reflog.actual >reflog.fuzzy &&
>
> Actually, let's use "s/^[0-9a-f]*/OBJID/" instead: you only want to replace 
> the first few characters.

Did you mean "s/^$_x05[0-9a-f]*/OBJID/"? (with "$_x05" expanding to
'[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' from test-lib.sh). If not,
then it would match even if there was no SHA1 hash.

But yes, without the "^" there will very likely be false positives.
Thanks for catching.

>> @@ -106,7 +109,11 @@ test_expect_success 'the default remote . should
>> not break explicit pull' '
>>   git reset --hard HEAD^ &&
>>   test "$(cat file)" = file &&
>>   git pull . second &&
>> - test "$(cat file)" = modified
>> + test "$(cat file)" = modified &&
>> + git reflog -1 >reflog.actual &&
>> + sed "s/$_x05[0-9a-f]*/OBJID/g" reflog.actual >reflog.fuzzy &&
>
> Same here.

Regards,
Paul
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html