Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jun 21, 2022, 09:29 UTC
Message-ID: <20220621092915.GD1689@szeder.dev>
In-Reply-To: <YrFwcL2dRS/v7xAw@coredump.intra.peff.net>

On Tue, Jun 21, 2022 at 03:17:04AM -0400, Jeff King wrote:

Show 19 quoted lines

> On Mon, Jun 20, 2022 at 06:20:07PM -0400, Derrick Stolee wrote:
> 
> > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> > index 4620f0ca7fa..c255a77e18a 100755
> > --- a/t/t5510-fetch.sh
> > +++ b/t/t5510-fetch.sh
> > @@ -853,7 +853,9 @@ test_configured_prune_type () {
> >  		then
> >  			new_cmdline=$cmdline_setup
> >  		else
> > -			new_cmdline=$(printf "%s" "$cmdline" | perl -pe 's[origin(?!/)]["'"$remote_url"'"]g')
> > +			new_cmdline=$(printf "%s" "$cmdline" |
> > +					sed -e "s~origin ~'$remote_url' ~g" \
> > +					    -e "s~ origin~ '$remote_url'~g")
> >  		fi
> 
> Doesn't this introduce a new problem if $remote_url contains a tilde?
> Unlikely, but I thought the point of the exercise was defending against
> funny paths.

And it doesn't replace a standalone "origin" word, either, which does occur a few times in the test script.

Show 5 quoted lines

> So perhaps something like:
> 
>   perl -e '
>     my ($cmdline, $url) = @ARGV;
>     $cmdline =~ s[origin(?!/)][quotemeta($url)]ge;

I don't like this "(?!/)" magic, because I haven't got the slightest idea of what it might do by merely looking at it, and these characters are not exactly easy to search for.

The good old "add a space prefix and suffix" trick can help to easily match the "origin" word even when it stands alone, but, alas, the result is still not as simple as I'd like with the \s and the string concatenation:

  perl -e '
    new_cmdline=$(perl -e '
            my ($cmdline, $url) = @ARGV;
            $cmdline =~ s[\sorigin\s][" " . quotemeta($url) . " "]ge;
            print $cmdline;
    ' -- " $cmdline " "$remote_url")

(The 'sed' variant looks good to me:

  printf " %s " "$cmdline" |sed "s~ origin ~ '$remote_url' ~g"

but then again it just trades one set of troublesome characters for an other.)

Show 8 quoted lines

>     print $cmdline;
>   ' -- "$cmdline" "$remote_url"
> 
> I don't mean to golf on this forever, but I wanted to show something
> concrete since you said you don't know perl well. I just think moving to
> sed introduces more opportunities for errors here, not fewer. :)
> 
> -Peff