On Wed, May 10, 2017 at 12:38 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <ava...@gmail.com> writes:
>
>> On Tue, May 9, 2017 at 6:45 PM, Jonathan Tan <jonathanta...@google.com> 
>> wrote:
>> ...
>>>         # Tweak the push output to make the push option outside the cert
>>>         # different, then replay it on a fresh dst, checking that ff is not
>>>         # deleted.
>>> -       sed -i "s/\\([^ ]\\)bar/\\1baz/" push &&
>>> +       perl -pe "s/([^ ])bar/\\1baz/" push >push.tweak &&
>>>         prepare_dst &&
>>>         git -C dst config receive.certnonceseed sekrit &&
>>>         git -C dst config receive.advertisepushoptions 1 &&
>>> -       git receive-pack dst <push >out &&
>>> +       git receive-pack dst <push.tweak >out &&
>>
>> The test should have a PERL prerequisite now, that's missing.
>
> For a single-liner like this, our stance has always been that t/
> scripts can assume _some_ version of Perl interpreter available for
> use, cf. t/README where it lists prerequisites and explains them:
>
>      - PERL
>
>        Git wasn't compiled with NO_PERL=YesPlease.
>
>        Even without the PERL prerequisite, tests can assume there is a
>        usable perl interpreter at $PERL_PATH, though it need not be
>        particularly modern.
>
> So unless "receive-pack" that is being tested here requires Perl at
> runtime, we do not want PERL prerequisite for this test.

Oops, sorry about that.

>> Also using \1 will likely be deprecated in future versions of perl at
>> some point:
>>
>>     $ echo hifoo | perl -wpe "s/([^ ])bar/\\1baz/"
>>     \1 better written as $1 at -e line 1.
>>     hifoo
>
> Very good advice from a Perl expert; thanks.

No problem. Hopefully my noisy advice will at least lead to fixing a
bug in perl if there is one :)

Reply via email to