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: Junio C Hamano <gitster@pobox.com>
Date: Jun 21, 2022, 16:35 UTC
Message-ID: <xmqq1qviht4h.fsf@gitster.g>
In-Reply-To: <YrFwcL2dRS/v7xAw@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> And then further changes are easy:
>
>   - you could replace the ad-hoc "I hope single quotes are enough"
>     quoting of $url with a real regex

Yes, this is what I found a bit queasy about in the whole thing.

Show 10 quoted lines

>   - you can define $sq inside the perl script to avoid the gross '\''
>     quoting (or even avoid it entirely with quotemeta)
>
> So perhaps something like:
>
>   perl -e '
>     my ($cmdline, $url) = @ARGV;
>     $cmdline =~ s[origin(?!/)][quotemeta($url)]ge;
>     print $cmdline;
>   ' -- "$cmdline" "$remote_url"

Yup, a solution along that line was what I expected to see from those who write Perl when I saw the discussion yesterday.

FWIW, there are a few characters that won't be acceptable in the pathname for our source tree, including a single quote (I think some of our build procedures are loose in quoting, unlike parts of the Makefile that use $(FOO_SQ) for that) and a colon (testing the version of Git we just built needs the path of the build directory in GIT_EXEC_PATH, which is prepended to PATH at runtime during the test, but the colon means that the path is split into two there), so "I hope single quotes are enough" is not making things worse in practice, but quotemeta() is so simple---allowing us not having to worry about quoting is a very good thing.

Thanks.