git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: rsbecker@nexbridge.com, 'Junio C Hamano' <gitster@pobox.com>
Cc: git@vger.kernel.org, avarab@gmail.com
Subject: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)
Date: Mon, 20 Jun 2022 18:20:07 -0400	[thread overview]
Message-ID: <484a330e-0902-6e1b-8189-63c72dcea494@github.com> (raw)
In-Reply-To: <495bd957-43dc-f252-657d-2969bb7ad5f3@github.com>

On 6/20/2022 4:00 PM, 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]

Here is a v2 that updates the sed call based on the feedback so far.

I'm basically switching to sed because I don't know perl and the
discussion about how to fix this replacement command is completely over
my head. That makes me think that using a simpler find-and-replace tool
like sed would be good here.

Thanks,
-Stolee

--- >8 ---

From 7fae22e0ad09af00de5a294f61dfd29cb349feeb Mon Sep 17 00:00:00 2001
From: Derrick Stolee <derrickstolee@github.com>
Date: Mon, 20 Jun 2022 15:52:09 -0400
Subject: [PATCH v2] 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..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
 
 		if test "$fetch_prune_tags" = 'true' ||
-- 
2.36.1.220.g1fae7daf425



  parent reply	other threads:[~2022-06-20 22:20 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-20 18:04 Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1 rsbecker
2022-06-20 18:46 ` Derrick Stolee
2022-06-20 18:59   ` rsbecker
2022-06-20 20:00     ` Derrick Stolee
2022-06-20 20:30       ` Ævar Arnfjörð Bjarmason
2022-06-20 20:43         ` Junio C Hamano
2022-06-20 21:24           ` rsbecker
2022-06-20 21:33           ` Ævar Arnfjörð Bjarmason
2022-06-20 20:34       ` SZEDER Gábor
2022-06-20 22:17       ` rsbecker
2022-06-20 22:20       ` Derrick Stolee [this message]
2022-06-21  5:28         ` [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1) Johannes Sixt
2022-06-21  7:17         ` Jeff King
2022-06-21  9:29           ` SZEDER Gábor
2022-06-21 10:07             ` Jeff King
2022-06-21 16:35           ` Junio C Hamano
2022-06-21 20:27             ` Junio C Hamano
2022-06-21 21:13               ` Derrick Stolee
2022-06-21 21:36                 ` Junio C Hamano
2022-06-21 22:34                   ` [PATCH 00/10] t5510: fix the quoting mess Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 01/10] fetch tests: remove redundant test_unconfig() Ævar Arnfjörð Bjarmason
2022-06-22  5:52                       ` Junio C Hamano
2022-06-21 22:34                     ` [PATCH 02/10] fetch tests: use named, not positional parameters Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 03/10] fetch tests: use "local", &&-chaining, style etc Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 04/10] fetch tests: add a helper to avoid boilerplate Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 05/10] fetch tests: pass "mode" parameter first, pave way for "$@" Ævar Arnfjörð Bjarmason
2022-06-22  6:01                       ` Junio C Hamano
2022-06-21 22:34                     ` [PATCH 06/10] fetch tests: pass a list, not a string of arguments Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 07/10] fetch tests: remove lazy variable setup Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 08/10] fetch tests: remove shelling out for previously "lazy" variables Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 09/10] fetch tests: stop implicitly adding refspecs Ævar Arnfjörð Bjarmason
2022-06-21 22:34                     ` [PATCH 10/10] fetch tests: fix needless and buggy re-quoting Ævar Arnfjörð Bjarmason
2022-06-22  6:12                       ` Junio C Hamano
2022-06-22 11:25                         ` Derrick Stolee
2022-06-22 15:21                           ` Ævar Arnfjörð Bjarmason
2022-06-22 15:44                           ` Junio C Hamano
2022-06-21 13:51 ` Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1 rsbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=484a330e-0902-6e1b-8189-63c72dcea494@github.com \
    --to=derrickstolee@github.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rsbecker@nexbridge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).