Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1

>
>
> On Mon, Jun 20 2022, Derrick Stolee wrote:
>
> > On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:
> >> On June 20, 2022 2:46 PM, Derrick Stolee wrote:
> >
> >>> The issue is this line (some tabs removed):
> >>>
> >>>  new_cmdline=$(printf "%s" "$cmdline" | perl -pe
> >>> 's[origin(?!/)]["'"$remote_url"'"]g')
> >>>
> >>> At this point, $remote_url contains the file path including the @ symbol. However,
> >>> this perl invocation is dropping everything starting at the @ to the next slash.
> >>>
> >>> I'm not sure of a better way to accomplish what is trying to be done here (replace
> >>> 'origin' with that specific url) without maybe causing other issues.
> >>>
> >>> This line was introduced by e1790f9245f (fetch tests: fetch <url> <spec> as well as
> >>> fetch [<remote>], 2018-02-09).
> >>
> >> How about using sed instead of perl for this?
> >
> > I wasn't sure if using sed would create a different kind of replacement
> > problem, but using single-quotes seems to get around that kind of issue.
> >
> > Please see the patch below. I'm currently running CI in a GGG PR [1]
> >
> > [1] https://github.com/gitgitgadget/git/pull/1267
> >
> > Thanks,
> > -Stolee
> >
> >
> > --- >8 ---
> >
> > From 1df4fc66d4a62adc7087d7d22c8d78842b4e9b4d Mon Sep 17 00:00:00 2001
> > From: Derrick Stolee <derrickstolee@github.com>
> > Date: Mon, 20 Jun 2022 15:52:09 -0400
> > Subject: [PATCH] t5510: replace 'origin' with URL more carefully
> >
> > The many test_configured_prune tests in t5510-fetch.sh test many
> > combinations of --prune, --prune-tags, and using 'origin' or an explicit
> > URL. Some machinery was introduced in e1790f9245f (fetch tests: fetch
> > <url> <spec> as well as fetch [<remote>], 2018-02-09) to replace
> > 'origin' with this explicit URL. This URL is a "file:///" URL for the
> > root of the $TRASH_DIRECTORY.
> >
> > However, if the current build tree has an '@' symbol, the replacement
> > using perl fails. It drops the '@' as well as anything else in that
> > directory name.
> >
> > You can verify this locally by cloning git.git into a "victim@03"
> > directory and running the test script.
> >
> > To resolve this issue, replace the perl invocation with two sed
> > commands. These two are used to ensure that we match exactly on the
> > whole word 'origin'. We can guarantee that the word boundaries are
> > spaces in our tests. The reason to use exact words is that sometimes a
> > refspec is supplied, such as "+refs/heads/*:refs/remotes/origin/*" which
> > would cause an incorrect replacement. The two commands are used because
> > there is not a clear POSIX way to match on word boundaries without
> > getting extremely pedantic about what possible characters we could have
> > at the boundaries.
> >
> > Reported-by: Randall Becker <rsbecker@nexbridge.com>
> > Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> > ---
> >  t/t5510-fetch.sh | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> > index 4620f0ca7fa..8ca3aa5e931 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 "s~origin ~'$remote_url' ~g" | \
> > +                                     sed "s~ origin~ '$remote_url'~g")
> >               fi
> >
> >               if test "$fetch_prune_tags" = 'true' ||
>
> Thanks for looking at this. Checking this out again this whole quoting
> mess is a bit of a ... mess, I wonder if there's some better way to
> avoid this. Anyway:
>
> So, is this functionally the same as:
>
>         diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
>         index 4620f0ca7fa..9cd8b36f835 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" |
>         +                                     perl -pe 's[origin ]["'"$remote_url"'" ]g' |
>         +                                     perl -pe 's[ origin][ "'"$remote_url"'"]g')
>                         fi
>
>                         if test "$fetch_prune_tags" = 'true' ||
>
> ?
>
> I don't mind the migration to "sed", but doing so to fix a bug makes
> this especially hard to analyze. I.e. you've gotten rid of the (?!/), I
> haven't re-looked at this enough to see if/how that's important.
>
> I just came up with the above as a quick hack, but for any proper
> migration to sed can't we do this in one command?
>
> In any case you never need "| \" in your scripts, just end the line with
> "|".