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

Ævar Arnfjörð Bjarmason [off-list ref] writes:

On Tue, May 9, 2017 at 6:45 PM, Jonathan Tan [off-list ref] wrote:
...

quoted

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

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.

On Wed, May 10, 2017 at 12:38 AM, Junio C Hamano [off-list ref] wrote:

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

On Tue, May 9, 2017 at 6:45 PM, Jonathan Tan [off-list ref] wrote:
...

quoted

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

quoted

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 :)