Re: [PATCH] fixup! use perl instead of sed

From: Junio C Hamano <gitster@pobox.com>
Date: May 9, 2017, 22:38 UTC
Message-ID: <xmqqd1bh1wqy.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACBZZX5DF6sTTYSUpYnvas925N-XsoUGrM-bYbAN_67g8-w2GQ@mail.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 14 quoted lines

> On Tue, May 9, 2017 at 6:45 PM, Jonathan Tan <jonathantanmy@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.

Show 6 quoted lines

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