{"thread":{"id":"58041","subject":"Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","startedAt":"2022-06-20T18:04:40Z","lastAt":"2022-06-22T15:44:53Z","messageCount":37,"participants":["rsbecker@nexbridge.com","Derrick Stolee","SZEDER Gábor","Ævar Arnfjörð Bjarmason","Junio C Hamano","Johannes Sixt","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"457567","messageId":"00a401d884d0$32885890$979909b0$@nexbridge.com","threadId":"58041","inReplyTo":null,"subject":"Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-06-20T18:04:28Z","receivedAt":"2022-06-20T18:04:40Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"Subtests t5510.69, 71, 97, 99, 125, 127, 147, 149, 151, 153, 155, 157, 159, 161, 163, 165, 167, 170 failed in rc1 on the NonStop ia64 platform. These were working in rc0, but after analysis, I do not think that is relevant. Here is what we see for subtest 69:\n\nexpecting success of 5510.69 'link prune fetch.prune=unset remote.origin.prune=unset fetch.pruneTags=unset remote.origin.pruneTags=unset --prune origin refs/tags/*:refs/tags/*; branch:kept tag:pruned':\n...\n git fetch \"file:///home/jenkins/.jenkins/workspace/Git_Pipeline@2/t/trash directory.t5510-fetch/.\" \"+refs/heads/*:refs/remotes/origin/*\" &&\n...\n git$git_fetch_c fetch --prune \"file:///home/jenkins/.jenkins/workspace/Git_Pipeline/t/trash directory.t5510-fetch/.\" refs/tags/*:refs/tags/* &&\n\nWhat seems to be happening is that the test script is computing the wrong parent directory in the second case, cutting the @2. We use Jenkins for our builds and occasional multiple builds run. The @2 is Jenkins way of running in another place. This seems to be confusing the script parsing of the parent, causing the issue. When I rebuilt and test in the primary directory, the failures disappeared because no @2.\n\nt5562 also seems to have the same issue.\n\n\n\n\n"},{"id":"457568","messageId":"8d2a0a36-1d2f-c723-db1e-8978e5d03d1d@github.com","threadId":"58041","inReplyTo":"00a401d884d0$32885890$979909b0$@nexbridge.com","subject":"Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-20T18:46:18Z","receivedAt":"2022-06-20T18:46:25Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/20/22 2:04 PM, rsbecker@nexbridge.com wrote:\n> Subtests t5510.69, 71, 97, 99, 125, 127, 147, 149, 151, 153, 155, 157, 159, 161, 163, 165, 167, 170 failed in rc1 on the NonStop ia64 platform. These were working in rc0, but after analysis, I do not think that is relevant. Here is what we see for subtest 69:\n> \n> expecting success of 5510.69 'link prune fetch.prune=unset remote.origin.prune=unset fetch.pruneTags=unset remote.origin.pruneTags=unset --prune origin refs/tags/*:refs/tags/*; branch:kept tag:pruned':\n> ...\n> git fetch \"file:///home/jenkins/.jenkins/workspace/Git_Pipeline@2/t/trash directory.t5510-fetch/.\" \"+refs/heads/*:refs/remotes/origin/*\" &&\n> ...\n> git$git_fetch_c fetch --prune \"file:///home/jenkins/.jenkins/workspace/Git_Pipeline/t/trash directory.t5510-fetch/.\" refs/tags/*:refs/tags/* &&\n> \n> What seems to be happening is that the test script is computing the wrong parent directory in the second case, cutting the @2. We use Jenkins for our builds and occasional multiple builds run. The @2 is Jenkins way of running in another place. This seems to be confusing the script parsing of the parent, causing the issue. When I rebuilt and test in the primary directory, the failures disappeared because no @2.\n\nYes, the script is incorrectly computing the remote URL. This\nis not a bug in Git, but a bug in the test script.\n\nThe issue is this line (some tabs removed):\n\n new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n\nAt this point, $remote_url contains the file path including\nthe @ symbol. However, this perl invocation is dropping\neverything starting at the @ to the next slash.\n\nI'm not sure of a better way to accomplish what is trying to\nbe done here (replace 'origin' with that specific url) without\nmaybe causing other issues.\n\nThis line was introduced by e1790f9245f (fetch tests: fetch\n as well as fetch [], 2018-02-09).\n\nThanks,\n-Stolee\n"},{"id":"457569","messageId":"00b501d884d7$d8ed1200$8ac73600$@nexbridge.com","threadId":"58041","inReplyTo":"8d2a0a36-1d2f-c723-db1e-8978e5d03d1d@github.com","subject":"RE: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-06-20T18:59:14Z","receivedAt":"2022-06-20T18:59:25Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n>On 6/20/22 2:04 PM, rsbecker@nexbridge.com wrote:\n>> Subtests t5510.69, 71, 97, 99, 125, 127, 147, 149, 151, 153, 155, 157, 159, 161, 163,\n>165, 167, 170 failed in rc1 on the NonStop ia64 platform. These were working in rc0,\n>but after analysis, I do not think that is relevant. Here is what we see for subtest\n>69:\n>>\n>> expecting success of 5510.69 'link prune fetch.prune=unset\n>remote.origin.prune=unset fetch.pruneTags=unset\n>remote.origin.pruneTags=unset --prune origin refs/tags/*:refs/tags/*;\n>branch:kept tag:pruned':\n>> ...\n>> git fetch\n>> \"file:///home/jenkins/.jenkins/workspace/Git_Pipeline@2/t/trash\n>directory.t5510-fetch/.\" \"+refs/heads/*:refs/remotes/origin/*\" && ...\n>> git$git_fetch_c fetch --prune\n>> \"file:///home/jenkins/.jenkins/workspace/Git_Pipeline/t/trash\n>> directory.t5510-fetch/.\" refs/tags/*:refs/tags/* &&\n>>\n>> What seems to be happening is that the test script is computing the wrong\n>parent directory in the second case, cutting the @2. We use Jenkins for our builds\n>and occasional multiple builds run. The @2 is Jenkins way of running in another\n>place. This seems to be confusing the script parsing of the parent, causing the\n>issue. When I rebuilt and test in the primary directory, the failures disappeared\n>because no @2.\n>\n>Yes, the script is incorrectly computing the remote URL. This is not a bug in Git, but\n>a bug in the test script.\n>\n>The issue is this line (some tabs removed):\n>\n> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>'s[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>\n>At this point, $remote_url contains the file path including the @ symbol. However,\n>this perl invocation is dropping everything starting at the @ to the next slash.\n>\n>I'm not sure of a better way to accomplish what is trying to be done here (replace\n>'origin' with that specific url) without maybe causing other issues.\n>\n>This line was introduced by e1790f9245f (fetch tests: fetch as well as\n>fetch [], 2018-02-09).\n\nHow about using sed instead of perl for this?\n\n"},{"id":"457571","messageId":"495bd957-43dc-f252-657d-2969bb7ad5f3@github.com","threadId":"58041","inReplyTo":"00b501d884d7$d8ed1200$8ac73600$@nexbridge.com","subject":"Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-20T20:00:03Z","receivedAt":"2022-06-20T20:00:08Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:\n> On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n\n>> The issue is this line (some tabs removed):\n>>\n>> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>> 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>>\n>> At this point, $remote_url contains the file path including the @ symbol. However,\n>> this perl invocation is dropping everything starting at the @ to the next slash.\n>>\n>> I'm not sure of a better way to accomplish what is trying to be done here (replace\n>> 'origin' with that specific url) without maybe causing other issues.\n>>\n>> This line was introduced by e1790f9245f (fetch tests: fetch as well as\n>> fetch [], 2018-02-09).\n> \n> How about using sed instead of perl for this?\n\nI wasn't sure if using sed would create a different kind of replacement\nproblem, but using single-quotes seems to get around that kind of issue.\n\nPlease see the patch below. I'm currently running CI in a GGG PR [1]\n\n[1] https://github.com/gitgitgadget/git/pull/1267\n\nThanks,\n-Stolee\n\n\n--- >8 ---\n\nFrom 1df4fc66d4a62adc7087d7d22c8d78842b4e9b4d Mon Sep 17 00:00:00 2001\nFrom: Derrick Stolee \nDate: Mon, 20 Jun 2022 15:52:09 -0400\nSubject: [PATCH] t5510: replace 'origin' with URL more carefully\n\nThe many test_configured_prune tests in t5510-fetch.sh test many\ncombinations of --prune, --prune-tags, and using 'origin' or an explicit\nURL. Some machinery was introduced in e1790f9245f (fetch tests: fetch\n as well as fetch [], 2018-02-09) to replace\n'origin' with this explicit URL. This URL is a \"file:///\" URL for the\nroot of the $TRASH_DIRECTORY.\n\nHowever, if the current build tree has an '@' symbol, the replacement\nusing perl fails. It drops the '@' as well as anything else in that\ndirectory name.\n\nYou can verify this locally by cloning git.git into a \"victim@03\"\ndirectory and running the test script.\n\nTo resolve this issue, replace the perl invocation with two sed\ncommands. These two are used to ensure that we match exactly on the\nwhole word 'origin'. We can guarantee that the word boundaries are\nspaces in our tests. The reason to use exact words is that sometimes a\nrefspec is supplied, such as \"+refs/heads/*:refs/remotes/origin/*\" which\nwould cause an incorrect replacement. The two commands are used because\nthere is not a clear POSIX way to match on word boundaries without\ngetting extremely pedantic about what possible characters we could have\nat the boundaries.\n\nReported-by: Randall Becker \nSigned-off-by: Derrick Stolee \n---\n t/t5510-fetch.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 4620f0ca7fa..8ca3aa5e931 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -853,7 +853,9 @@ test_configured_prune_type () {\n \t\tthen\n \t\t\tnew_cmdline=$cmdline_setup\n \t\telse\n-\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n+\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | \\\n+\t\t\t\t\tsed \"s~origin ~'$remote_url' ~g\" | \\\n+\t\t\t\t\tsed \"s~ origin~ '$remote_url'~g\")\n \t\tfi\n \n \t\tif test \"$fetch_prune_tags\" = 'true' ||\n-- \n2.36.1.vfs.0.0\n\n\n \n"},{"id":"457573","messageId":"20220620203405.GC1689@szeder.dev","threadId":"58041","inReplyTo":"495bd957-43dc-f252-657d-2969bb7ad5f3@github.com","subject":"Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2022-06-20T20:34:05Z","receivedAt":"2022-06-20T20:34:14Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jun 20, 2022 at 04:00:03PM -0400, Derrick Stolee wrote:\n> On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:\n> > On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n> \n> >> The issue is this line (some tabs removed):\n> >>\n> >> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n> >> 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> >>\n> >> At this point, $remote_url contains the file path including the @ symbol. However,\n> >> this perl invocation is dropping everything starting at the @ to the next slash.\n> >>\n> >> I'm not sure of a better way to accomplish what is trying to be done here (replace\n> >> 'origin' with that specific url) without maybe causing other issues.\n> >>\n> >> This line was introduced by e1790f9245f (fetch tests: fetch as well as\n> >> fetch [], 2018-02-09).\n> > \n> > How about using sed instead of perl for this?\n> \n> I wasn't sure if using sed would create a different kind of replacement\n> problem, but using single-quotes seems to get around that kind of issue.\n\nIn a 'sed s/regexp/replacement/' command the replacement part has a\nfew characters with special meaning, and if those characters happen to\nappear in the path of the trash directory, then:\n\n $ ./t5510-fetch.sh --root='/tmp/foo\\1/'\n ok 64 - name prune fetch.prune=unset remote.origin.prune=unset fetch.pruneTags=unset remote.origin.pruneTags=unset --no-prune; branch:kept tag:kept\n sed: -e expression #1, char 62: invalid reference \\1 on `s' command's RHS\n sed: -e expression #1, char 62: invalid reference \\1 on `s' command's RHS\n ok 65 - link prune fetch.prune=unset remote.origin.prune=unset fetch.pruneTags=unset remote.origin.pruneTags=unset --no-prune; branch:kept tag:kept\n ok 66 - name prune fetch.prune=unset remote.origin.prune=unset fetch.pruneTags=unset remote.origin.pruneTags=unset --prune; branch:pruned tag:kept\n sed: -e expression #1, char 62: invalid reference \\1 on `s' command's RHS\n sed: -e expression #1, char 62: invalid reference \\1 on `s' command's RHS\n not ok 67 - link prune fetch.prune=unset remote.origin.prune=unset fetch.pruneTags=unset remote.origin.pruneTags=unset --prune; branch:pruned tag:kept\n [...]\n # failed 28 among 183 test(s)\n\n> Please see the patch below. I'm currently running CI in a GGG PR [1]\n> \n> [1] https://github.com/gitgitgadget/git/pull/1267\n> \n> Thanks,\n> -Stolee\n> \n> \n> --- >8 ---\n> \n> >From 1df4fc66d4a62adc7087d7d22c8d78842b4e9b4d Mon Sep 17 00:00:00 2001\n> From: Derrick Stolee \n> Date: Mon, 20 Jun 2022 15:52:09 -0400\n> Subject: [PATCH] t5510: replace 'origin' with URL more carefully\n> \n> The many test_configured_prune tests in t5510-fetch.sh test many\n> combinations of --prune, --prune-tags, and using 'origin' or an explicit\n> URL. Some machinery was introduced in e1790f9245f (fetch tests: fetch\n> as well as fetch [], 2018-02-09) to replace\n> 'origin' with this explicit URL. This URL is a \"file:///\" URL for the\n> root of the $TRASH_DIRECTORY.\n> \n> However, if the current build tree has an '@' symbol, the replacement\n> using perl fails. It drops the '@' as well as anything else in that\n> directory name.\n> \n> You can verify this locally by cloning git.git into a \"victim@03\"\n> directory and running the test script.\n> \n> To resolve this issue, replace the perl invocation with two sed\n> commands. These two are used to ensure that we match exactly on the\n> whole word 'origin'. We can guarantee that the word boundaries are\n> spaces in our tests. The reason to use exact words is that sometimes a\n> refspec is supplied, such as \"+refs/heads/*:refs/remotes/origin/*\" which\n> would cause an incorrect replacement. The two commands are used because\n> there is not a clear POSIX way to match on word boundaries without\n> getting extremely pedantic about what possible characters we could have\n> at the boundaries.\n> \n> Reported-by: Randall Becker \n> Signed-off-by: Derrick Stolee \n> ---\n> t/t5510-fetch.sh | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index 4620f0ca7fa..8ca3aa5e931 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -853,7 +853,9 @@ test_configured_prune_type () {\n> \t\tthen\n> \t\t\tnew_cmdline=$cmdline_setup\n> \t\telse\n> -\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> +\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | \\\n> +\t\t\t\t\tsed \"s~origin ~'$remote_url' ~g\" | \\\n> +\t\t\t\t\tsed \"s~ origin~ '$remote_url'~g\")\n\n'sed' can run multiple commands at once, e.g. 'sed -e s/// -e s///'.\n\n> \t\tfi\n> \n> \t\tif test \"$fetch_prune_tags\" = 'true' ||\n> -- \n> 2.36.1.vfs.0.0\n> \n> \n> \n"},{"id":"457574","messageId":"220620.861qvjvzue.gmgdl@evledraar.gmail.com","threadId":"58041","inReplyTo":"495bd957-43dc-f252-657d-2969bb7ad5f3@github.com","subject":"Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-20T20:30:08Z","receivedAt":"2022-06-20T20:34:27Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 20 2022, Derrick Stolee wrote:\n\n> On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:\n>> On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n>\n>>> The issue is this line (some tabs removed):\n>>>\n>>> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>>> 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>>>\n>>> At this point, $remote_url contains the file path including the @ symbol. However,\n>>> this perl invocation is dropping everything starting at the @ to the next slash.\n>>>\n>>> I'm not sure of a better way to accomplish what is trying to be done here (replace\n>>> 'origin' with that specific url) without maybe causing other issues.\n>>>\n>>> This line was introduced by e1790f9245f (fetch tests: fetch as well as\n>>> fetch [], 2018-02-09).\n>> \n>> How about using sed instead of perl for this?\n>\n> I wasn't sure if using sed would create a different kind of replacement\n> problem, but using single-quotes seems to get around that kind of issue.\n>\n> Please see the patch below. I'm currently running CI in a GGG PR [1]\n>\n> [1] https://github.com/gitgitgadget/git/pull/1267\n>\n> Thanks,\n> -Stolee\n>\n>\n> --- >8 ---\n>\n> From 1df4fc66d4a62adc7087d7d22c8d78842b4e9b4d Mon Sep 17 00:00:00 2001\n> From: Derrick Stolee \n> Date: Mon, 20 Jun 2022 15:52:09 -0400\n> Subject: [PATCH] t5510: replace 'origin' with URL more carefully\n>\n> The many test_configured_prune tests in t5510-fetch.sh test many\n> combinations of --prune, --prune-tags, and using 'origin' or an explicit\n> URL. Some machinery was introduced in e1790f9245f (fetch tests: fetch\n> as well as fetch [], 2018-02-09) to replace\n> 'origin' with this explicit URL. This URL is a \"file:///\" URL for the\n> root of the $TRASH_DIRECTORY.\n>\n> However, if the current build tree has an '@' symbol, the replacement\n> using perl fails. It drops the '@' as well as anything else in that\n> directory name.\n>\n> You can verify this locally by cloning git.git into a \"victim@03\"\n> directory and running the test script.\n>\n> To resolve this issue, replace the perl invocation with two sed\n> commands. These two are used to ensure that we match exactly on the\n> whole word 'origin'. We can guarantee that the word boundaries are\n> spaces in our tests. The reason to use exact words is that sometimes a\n> refspec is supplied, such as \"+refs/heads/*:refs/remotes/origin/*\" which\n> would cause an incorrect replacement. The two commands are used because\n> there is not a clear POSIX way to match on word boundaries without\n> getting extremely pedantic about what possible characters we could have\n> at the boundaries.\n>\n> Reported-by: Randall Becker \n> Signed-off-by: Derrick Stolee \n> ---\n> t/t5510-fetch.sh | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index 4620f0ca7fa..8ca3aa5e931 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -853,7 +853,9 @@ test_configured_prune_type () {\n> \t\tthen\n> \t\t\tnew_cmdline=$cmdline_setup\n> \t\telse\n> -\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> +\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | \\\n> +\t\t\t\t\tsed \"s~origin ~'$remote_url' ~g\" | \\\n> +\t\t\t\t\tsed \"s~ origin~ '$remote_url'~g\")\n> \t\tfi\n> \n> \t\tif test \"$fetch_prune_tags\" = 'true' ||\n\nThanks for looking at this. Checking this out again this whole quoting\nmess is a bit of a ... mess, I wonder if there's some better way to\navoid this. Anyway:\n\nSo, is this functionally the same as:\n\t\n\tdiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n\tindex 4620f0ca7fa..9cd8b36f835 100755\n\t--- a/t/t5510-fetch.sh\n\t+++ b/t/t5510-fetch.sh\n\t@@ -853,7 +853,9 @@ test_configured_prune_type () {\n\t \t\tthen\n\t \t\t\tnew_cmdline=$cmdline_setup\n\t \t\telse\n\t-\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n\t+\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" |\n\t+\t\t\t\t perl -pe 's[origin ][\"'\"$remote_url\"'\" ]g' |\n\t+\t\t\t\t perl -pe 's[ origin][ \"'\"$remote_url\"'\"]g')\n\t \t\tfi\n\t \n\t \t\tif test \"$fetch_prune_tags\" = 'true' ||\n\n?\n\nI don't mind the migration to \"sed\", but doing so to fix a bug makes\nthis especially hard to analyze. I.e. you've gotten rid of the (?!/), I\nhaven't re-looked at this enough to see if/how that's important.\n\nI just came up with the above as a quick hack, but for any proper\nmigration to sed can't we do this in one command?\n\nIn any case you never need \"| \\\" in your scripts, just end the line with\n\"|\".\n"},{"id":"457575","messageId":"CAPc5daV29NuOkL1rZvzaqhe5fSrY2os42zsQRhtP6p6AJTp2NA@mail.gmail.com","threadId":"58041","inReplyTo":"220620.861qvjvzue.gmgdl@evledraar.gmail.com","subject":"Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-20T20:43:21Z","receivedAt":"2022-06-20T20:43:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I was expecting you to use \\Q...\\E (and passing $remote_url as an\nargument to perl script) actually. If you let $remote_url\ninterpolated by shell into a script of the host language, whether perl\nor sed, you'd be responsible for quoting it appropriately for the host\nlanguage yourself, and use of single-quote pair would not necessarily\nbe sufficient, no?\n\nOn Mon, Jun 20, 2022 at 1:34 PM Ævar Arnfjörð Bjarmason\n wrote:\n>\n>\n> On Mon, Jun 20 2022, Derrick Stolee wrote:\n>\n> > On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:\n> >> On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n> >\n> >>> The issue is this line (some tabs removed):\n> >>>\n> >>> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n> >>> 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> >>>\n> >>> At this point, $remote_url contains the file path including the @ symbol. However,\n> >>> this perl invocation is dropping everything starting at the @ to the next slash.\n> >>>\n> >>> I'm not sure of a better way to accomplish what is trying to be done here (replace\n> >>> 'origin' with that specific url) without maybe causing other issues.\n> >>>\n> >>> This line was introduced by e1790f9245f (fetch tests: fetch as well as\n> >>> fetch [], 2018-02-09).\n> >>\n> >> How about using sed instead of perl for this?\n> >\n> > I wasn't sure if using sed would create a different kind of replacement\n> > problem, but using single-quotes seems to get around that kind of issue.\n> >\n> > Please see the patch below. I'm currently running CI in a GGG PR [1]\n> >\n> > [1] https://github.com/gitgitgadget/git/pull/1267\n> >\n> > Thanks,\n> > -Stolee\n> >\n> >\n> > --- >8 ---\n> >\n> > From 1df4fc66d4a62adc7087d7d22c8d78842b4e9b4d Mon Sep 17 00:00:00 2001\n> > From: Derrick Stolee \n> > Date: Mon, 20 Jun 2022 15:52:09 -0400\n> > Subject: [PATCH] t5510: replace 'origin' with URL more carefully\n> >\n> > The many test_configured_prune tests in t5510-fetch.sh test many\n> > combinations of --prune, --prune-tags, and using 'origin' or an explicit\n> > URL. Some machinery was introduced in e1790f9245f (fetch tests: fetch\n> > as well as fetch [], 2018-02-09) to replace\n> > 'origin' with this explicit URL. This URL is a \"file:///\" URL for the\n> > root of the $TRASH_DIRECTORY.\n> >\n> > However, if the current build tree has an '@' symbol, the replacement\n> > using perl fails. It drops the '@' as well as anything else in that\n> > directory name.\n> >\n> > You can verify this locally by cloning git.git into a \"victim@03\"\n> > directory and running the test script.\n> >\n> > To resolve this issue, replace the perl invocation with two sed\n> > commands. These two are used to ensure that we match exactly on the\n> > whole word 'origin'. We can guarantee that the word boundaries are\n> > spaces in our tests. The reason to use exact words is that sometimes a\n> > refspec is supplied, such as \"+refs/heads/*:refs/remotes/origin/*\" which\n> > would cause an incorrect replacement. The two commands are used because\n> > there is not a clear POSIX way to match on word boundaries without\n> > getting extremely pedantic about what possible characters we could have\n> > at the boundaries.\n> >\n> > Reported-by: Randall Becker \n> > Signed-off-by: Derrick Stolee \n> > ---\n> > t/t5510-fetch.sh | 4 +++-\n> > 1 file changed, 3 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> > index 4620f0ca7fa..8ca3aa5e931 100755\n> > --- a/t/t5510-fetch.sh\n> > +++ b/t/t5510-fetch.sh\n> > @@ -853,7 +853,9 @@ test_configured_prune_type () {\n> > then\n> > new_cmdline=$cmdline_setup\n> > else\n> > - new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> > + new_cmdline=$(printf \"%s\" \"$cmdline\" | \\\n> > + sed \"s~origin ~'$remote_url' ~g\" | \\\n> > + sed \"s~ origin~ '$remote_url'~g\")\n> > fi\n> >\n> > if test \"$fetch_prune_tags\" = 'true' ||\n>\n> Thanks for looking at this. Checking this out again this whole quoting\n> mess is a bit of a ... mess, I wonder if there's some better way to\n> avoid this. Anyway:\n>\n> So, is this functionally the same as:\n>\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index 4620f0ca7fa..9cd8b36f835 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -853,7 +853,9 @@ test_configured_prune_type () {\n> then\n> new_cmdline=$cmdline_setup\n> else\n> - new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> + new_cmdline=$(printf \"%s\" \"$cmdline\" |\n> + perl -pe 's[origin ][\"'\"$remote_url\"'\" ]g' |\n> + perl -pe 's[ origin][ \"'\"$remote_url\"'\"]g')\n> fi\n>\n> if test \"$fetch_prune_tags\" = 'true' ||\n>\n> ?\n>\n> I don't mind the migration to \"sed\", but doing so to fix a bug makes\n> this especially hard to analyze. I.e. you've gotten rid of the (?!/), I\n> haven't re-looked at this enough to see if/how that's important.\n>\n> I just came up with the above as a quick hack, but for any proper\n> migration to sed can't we do this in one command?\n>\n> In any case you never need \"| \\\" in your scripts, just end the line with\n> \"|\".\n"},{"id":"457577","messageId":"00c101d884ec$133b6ec0$39b24c40$@nexbridge.com","threadId":"58041","inReplyTo":"CAPc5daV29NuOkL1rZvzaqhe5fSrY2os42zsQRhtP6p6AJTp2NA@mail.gmail.com","subject":"RE: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-06-20T21:24:02Z","receivedAt":"2022-06-20T21:24:14Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On June 20, 2022 4:43 PM, Junio C Hamano wrote:\n>I was expecting you to use \\Q...\\E (and passing $remote_url as an argument to\n>perl script) actually. If you let $remote_url interpolated by shell into a script of the\n>host language, whether perl or sed, you'd be responsible for quoting it\n>appropriately for the host language yourself, and use of single-quote pair would\n>not necessarily be sufficient, no?\n\nThat would make it easier to debug. In the mean time, I am trying to get the environment to test with the problematic path and am having issues after renaming the directory.\n\n>\n>On Mon, Jun 20, 2022 at 1:34 PM Ævar Arnfjörð Bjarmason \n>wrote:\n>>\n>>\n>> On Mon, Jun 20 2022, Derrick Stolee wrote:\n>>\n>> > On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:\n>> >> On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n>> >\n>> >>> The issue is this line (some tabs removed):\n>> >>>\n>> >>> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>> >>> 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>> >>>\n>> >>> At this point, $remote_url contains the file path including the @\n>> >>> symbol. However, this perl invocation is dropping everything starting at the\n>@ to the next slash.\n>> >>>\n>> >>> I'm not sure of a better way to accomplish what is trying to be\n>> >>> done here (replace 'origin' with that specific url) without maybe causing\n>other issues.\n>> >>>\n>> >>> This line was introduced by e1790f9245f (fetch tests: fetch \n>> >>> as well as fetch [], 2018-02-09).\n>> >>\n>> >> How about using sed instead of perl for this?\n>> >\n>> > I wasn't sure if using sed would create a different kind of\n>> > replacement problem, but using single-quotes seems to get around that kind\n>of issue.\n>> >\n>> > Please see the patch below. I'm currently running CI in a GGG PR [1]\n>> >\n>> > [1] https://github.com/gitgitgadget/git/pull/1267\n>> >\n>> > Thanks,\n>> > -Stolee\n>> >\n>> >\n>> > --- >8 ---\n>> >\n>> > From 1df4fc66d4a62adc7087d7d22c8d78842b4e9b4d Mon Sep 17 00:00:00\n>> > 2001\n>> > From: Derrick Stolee \n>> > Date: Mon, 20 Jun 2022 15:52:09 -0400\n>> > Subject: [PATCH] t5510: replace 'origin' with URL more carefully\n>> >\n>> > The many test_configured_prune tests in t5510-fetch.sh test many\n>> > combinations of --prune, --prune-tags, and using 'origin' or an\n>> > explicit URL. Some machinery was introduced in e1790f9245f (fetch\n>> > tests: fetch as well as fetch [], 2018-02-09)\n>> > to replace 'origin' with this explicit URL. This URL is a \"file:///\"\n>> > URL for the root of the $TRASH_DIRECTORY.\n>> >\n>> > However, if the current build tree has an '@' symbol, the\n>> > replacement using perl fails. It drops the '@' as well as anything\n>> > else in that directory name.\n>> >\n>> > You can verify this locally by cloning git.git into a \"victim@03\"\n>> > directory and running the test script.\n>> >\n>> > To resolve this issue, replace the perl invocation with two sed\n>> > commands. These two are used to ensure that we match exactly on the\n>> > whole word 'origin'. We can guarantee that the word boundaries are\n>> > spaces in our tests. The reason to use exact words is that sometimes\n>> > a refspec is supplied, such as \"+refs/heads/*:refs/remotes/origin/*\"\n>> > which would cause an incorrect replacement. The two commands are\n>> > used because there is not a clear POSIX way to match on word\n>> > boundaries without getting extremely pedantic about what possible\n>> > characters we could have at the boundaries.\n>> >\n>> > Reported-by: Randall Becker \n>> > Signed-off-by: Derrick Stolee \n>> > ---\n>> > t/t5510-fetch.sh | 4 +++-\n>> > 1 file changed, 3 insertions(+), 1 deletion(-)\n>> >\n>> > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index\n>> > 4620f0ca7fa..8ca3aa5e931 100755\n>> > --- a/t/t5510-fetch.sh\n>> > +++ b/t/t5510-fetch.sh\n>> > @@ -853,7 +853,9 @@ test_configured_prune_type () {\n>> > then\n>> > new_cmdline=$cmdline_setup\n>> > else\n>> > - new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>'s[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>> > + new_cmdline=$(printf \"%s\" \"$cmdline\" | \\\n>> > + sed \"s~origin ~'$remote_url' ~g\" | \\\n>> > + sed \"s~ origin~\n>> > + '$remote_url'~g\")\n>> > fi\n>> >\n>> > if test \"$fetch_prune_tags\" = 'true' ||\n>>\n>> Thanks for looking at this. Checking this out again this whole quoting\n>> mess is a bit of a ... mess, I wonder if there's some better way to\n>> avoid this. Anyway:\n>>\n>> So, is this functionally the same as:\n>>\n>> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n>> index 4620f0ca7fa..9cd8b36f835 100755\n>> --- a/t/t5510-fetch.sh\n>> +++ b/t/t5510-fetch.sh\n>> @@ -853,7 +853,9 @@ test_configured_prune_type () {\n>> then\n>> new_cmdline=$cmdline_setup\n>> else\n>> - new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>'s[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>> + new_cmdline=$(printf \"%s\" \"$cmdline\" |\n>> + perl -pe 's[origin ][\"'\"$remote_url\"'\" ]g' |\n>> + perl -pe 's[ origin][ \"'\"$remote_url\"'\"]g')\n>> fi\n>>\n>> if test \"$fetch_prune_tags\" = 'true' ||\n>>\n>> ?\n>>\n>> I don't mind the migration to \"sed\", but doing so to fix a bug makes\n>> this especially hard to analyze. I.e. you've gotten rid of the (?!/),\n>> I haven't re-looked at this enough to see if/how that's important.\n>>\n>> I just came up with the above as a quick hack, but for any proper\n>> migration to sed can't we do this in one command?\n>>\n>> In any case you never need \"| \\\" in your scripts, just end the line\n>> with \"|\".\n\n"},{"id":"457578","messageId":"220620.86wndbuiin.gmgdl@evledraar.gmail.com","threadId":"58041","inReplyTo":"CAPc5daV29NuOkL1rZvzaqhe5fSrY2os42zsQRhtP6p6AJTp2NA@mail.gmail.com","subject":"Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-20T21:33:16Z","receivedAt":"2022-06-20T21:33:57Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jun 20 2022, Junio C Hamano wrote:\n\n> I was expecting you to use \\Q...\\E (and passing $remote_url as an\n> argument to perl script) actually. If you let $remote_url\n> interpolated by shell into a script of the host language, whether perl\n> or sed, you'd be responsible for quoting it appropriately for the host\n> language yourself, and use of single-quote pair would not necessarily\n> be sufficient, no?\n\nYes, I wasn't aiming to try to come up with an ideal Perl version, just\nwondering why it needed the s/perl/sed/ while-we're-at-it...\n"},{"id":"457581","messageId":"00e001d884f3$79f8d6a0$6dea83e0$@nexbridge.com","threadId":"58041","inReplyTo":"495bd957-43dc-f252-657d-2969bb7ad5f3@github.com","subject":"RE: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-06-20T22:17:01Z","receivedAt":"2022-06-20T22:17:12Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"\n\n>-----Original Message-----\n>From: Derrick Stolee \n>Sent: June 20, 2022 4:00 PM\n>To: rsbecker@nexbridge.com; 'Junio C Hamano' \n>Cc: git@vger.kernel.org; avarab@gmail.com\n>Subject: Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1\n>\n>On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:\n>> On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n>\n>>> The issue is this line (some tabs removed):\n>>>\n>>> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>>> 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>>>\n>>> At this point, $remote_url contains the file path including the @\n>>> symbol. However, this perl invocation is dropping everything starting at the @\n>to the next slash.\n>>>\n>>> I'm not sure of a better way to accomplish what is trying to be done\n>>> here (replace 'origin' with that specific url) without maybe causing other issues.\n>>>\n>>> This line was introduced by e1790f9245f (fetch tests: fetch \n>>> as well as fetch [], 2018-02-09).\n>>\n>> How about using sed instead of perl for this?\n>\n>I wasn't sure if using sed would create a different kind of replacement problem,\n>but using single-quotes seems to get around that kind of issue.\n>\n>Please see the patch below. I'm currently running CI in a GGG PR [1]\n>\n>[1] https://github.com/gitgitgadget/git/pull/1267\n>\n>Thanks,\n>-Stolee\n>\n>\n>--- >8 ---\n>\n>>From 1df4fc66d4a62adc7087d7d22c8d78842b4e9b4d Mon Sep 17 00:00:00 2001\n>From: Derrick Stolee \n>Date: Mon, 20 Jun 2022 15:52:09 -0400\n>Subject: [PATCH] t5510: replace 'origin' with URL more carefully\n>\n>The many test_configured_prune tests in t5510-fetch.sh test many combinations\n>of --prune, --prune-tags, and using 'origin' or an explicit URL. Some machinery was\n>introduced in e1790f9245f (fetch tests: fetch as well as fetch\n>[], 2018-02-09) to replace 'origin' with this explicit URL. This URL is a\n>\"file:///\" URL for the root of the $TRASH_DIRECTORY.\n>\n>However, if the current build tree has an '@' symbol, the replacement using perl\n>fails. It drops the '@' as well as anything else in that directory name.\n>\n>You can verify this locally by cloning git.git into a \"victim@03\"\n>directory and running the test script.\n>\n>To resolve this issue, replace the perl invocation with two sed commands. These\n>two are used to ensure that we match exactly on the whole word 'origin'. We can\n>guarantee that the word boundaries are spaces in our tests. The reason to use\n>exact words is that sometimes a refspec is supplied, such as\n>\"+refs/heads/*:refs/remotes/origin/*\" which would cause an incorrect\n>replacement. The two commands are used because there is not a clear POSIX way\n>to match on word boundaries without getting extremely pedantic about what\n>possible characters we could have at the boundaries.\n>\n>Reported-by: Randall Becker \n>Signed-off-by: Derrick Stolee \n>---\n> t/t5510-fetch.sh | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n>\n>diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 4620f0ca7fa..8ca3aa5e931\n>100755\n>--- a/t/t5510-fetch.sh\n>+++ b/t/t5510-fetch.sh\n>@@ -853,7 +853,9 @@ test_configured_prune_type () {\n> \t\tthen\n> \t\t\tnew_cmdline=$cmdline_setup\n> \t\telse\n>-\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>'s[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>+\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | \\\n>+\t\t\t\t\tsed \"s~origin ~'$remote_url' ~g\" | \\\n>+\t\t\t\t\tsed \"s~ origin~ '$remote_url'~g\")\n> \t\tfi\n>\n> \t\tif test \"$fetch_prune_tags\" = 'true' ||\n\nOk, this seems to work. I have the test environment set up so can retest other options.\n\n"},{"id":"457583","messageId":"484a330e-0902-6e1b-8189-63c72dcea494@github.com","threadId":"58041","inReplyTo":"495bd957-43dc-f252-657d-2969bb7ad5f3@github.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)","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-20T22:20:07Z","receivedAt":"2022-06-20T22:20:14Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/20/2022 4:00 PM, Derrick Stolee wrote:\n> On 6/20/22 2:59 PM, rsbecker@nexbridge.com wrote:\n>> On June 20, 2022 2:46 PM, Derrick Stolee wrote:\n> \n>>> The issue is this line (some tabs removed):\n>>>\n>>> new_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe\n>>> 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n>>>\n>>> At this point, $remote_url contains the file path including the @ symbol. However,\n>>> this perl invocation is dropping everything starting at the @ to the next slash.\n>>>\n>>> I'm not sure of a better way to accomplish what is trying to be done here (replace\n>>> 'origin' with that specific url) without maybe causing other issues.\n>>>\n>>> This line was introduced by e1790f9245f (fetch tests: fetch as well as\n>>> fetch [], 2018-02-09).\n>>\n>> How about using sed instead of perl for this?\n> \n> I wasn't sure if using sed would create a different kind of replacement\n> problem, but using single-quotes seems to get around that kind of issue.\n> \n> Please see the patch below. I'm currently running CI in a GGG PR [1]\n\nHere is a v2 that updates the sed call based on the feedback so far.\n\nI'm basically switching to sed because I don't know perl and the\ndiscussion about how to fix this replacement command is completely over\nmy head. That makes me think that using a simpler find-and-replace tool\nlike sed would be good here.\n\nThanks,\n-Stolee\n\n--- >8 ---\n\nFrom 7fae22e0ad09af00de5a294f61dfd29cb349feeb Mon Sep 17 00:00:00 2001\nFrom: Derrick Stolee \nDate: Mon, 20 Jun 2022 15:52:09 -0400\nSubject: [PATCH v2] t5510: replace 'origin' with URL more carefully\n\nThe many test_configured_prune tests in t5510-fetch.sh test many\ncombinations of --prune, --prune-tags, and using 'origin' or an explicit\nURL. Some machinery was introduced in e1790f9245f (fetch tests: fetch\n as well as fetch [], 2018-02-09) to replace\n'origin' with this explicit URL. This URL is a \"file:///\" URL for the\nroot of the $TRASH_DIRECTORY.\n\nHowever, if the current build tree has an '@' symbol, the replacement\nusing perl fails. It drops the '@' as well as anything else in that\ndirectory name.\n\nYou can verify this locally by cloning git.git into a \"victim@03\"\ndirectory and running the test script.\n\nTo resolve this issue, replace the perl invocation with two sed\ncommands. These two are used to ensure that we match exactly on the\nwhole word 'origin'. We can guarantee that the word boundaries are\nspaces in our tests. The reason to use exact words is that sometimes a\nrefspec is supplied, such as \"+refs/heads/*:refs/remotes/origin/*\" which\nwould cause an incorrect replacement. The two commands are used because\nthere is not a clear POSIX way to match on word boundaries without\ngetting extremely pedantic about what possible characters we could have\nat the boundaries.\n\nReported-by: Randall Becker \nSigned-off-by: Derrick Stolee \n---\n t/t5510-fetch.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 4620f0ca7fa..c255a77e18a 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -853,7 +853,9 @@ test_configured_prune_type () {\n \t\tthen\n \t\t\tnew_cmdline=$cmdline_setup\n \t\telse\n-\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n+\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" |\n+\t\t\t\t\tsed -e \"s~origin ~'$remote_url' ~g\" \\\n+\t\t\t\t\t -e \"s~ origin~ '$remote_url'~g\")\n \t\tfi\n \n \t\tif test \"$fetch_prune_tags\" = 'true' ||\n-- \n2.36.1.220.g1fae7daf425\n\n\n"},{"id":"457588","messageId":"964a9ae5-35e7-8906-314a-f3c27624793d@kdbg.org","threadId":"58041","inReplyTo":"484a330e-0902-6e1b-8189-63c72dcea494@github.com","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2022-06-21T05:28:31Z","receivedAt":"2022-06-21T05:28:45Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 21.06.22 um 00:20 schrieb Derrick Stolee:\n> @@ -853,7 +853,9 @@ test_configured_prune_type () {\n> \t\tthen\n> \t\t\tnew_cmdline=$cmdline_setup\n> \t\telse\n> -\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> +\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" |\n> +\t\t\t\t\tsed -e \"s~origin ~'$remote_url' ~g\" \\\n> +\t\t\t\t\t -e \"s~ origin~ '$remote_url'~g\")\n\nI'm not sure if this is still an issue, but I recall that there are seds\nin the wild that do not grok an incomplete last line. Better make this\n`printf \"%s\\n\" \"$cmdline\"` if `echo \"$cmdline\"` is not an option.\n\n> \t\tfi\n> \n> \t\tif test \"$fetch_prune_tags\" = 'true' ||\n\n-- Hannes\n"},{"id":"457593","messageId":"YrFwcL2dRS/v7xAw@coredump.intra.peff.net","threadId":"58041","inReplyTo":"484a330e-0902-6e1b-8189-63c72dcea494@github.com","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-06-21T07:17:04Z","receivedAt":"2022-06-21T07:17:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 20, 2022 at 06:20:07PM -0400, Derrick Stolee wrote:\n\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index 4620f0ca7fa..c255a77e18a 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -853,7 +853,9 @@ test_configured_prune_type () {\n> \t\tthen\n> \t\t\tnew_cmdline=$cmdline_setup\n> \t\telse\n> -\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> +\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" |\n> +\t\t\t\t\tsed -e \"s~origin ~'$remote_url' ~g\" \\\n> +\t\t\t\t\t -e \"s~ origin~ '$remote_url'~g\")\n> \t\tfi\n\nDoesn't this introduce a new problem if $remote_url contains a tilde?\nUnlikely, but I thought the point of the exercise was defending against\nfunny paths.\n\nGetting this bullet-proof with sed is tricky, I think. I didn't follow\nall the other logic that that might need fixed, but handling this in\nperl is easy by passing the string on the command line, like:\n\n printf \"%s\" \"$cmdline\" |\n perl -pe '\n BEGIN { $url = shift }\n s[origin(?!/)]['\\''$url'\\'']g;\n ' \"$remote_url\"\n\nFor that matter, using printf and \"perl -p\" is a little silly, since we\nknow there is only a single string to modify. Perhaps:\n\n perl -e '\n my ($cmdline, $url) = @ARGV;\n $cmdline =~ s[origin(?!/)]['\\''$url'\\'']g;\n print $cmdline;\n ' -- \"$cmdline\" \"$remote_url\"\n\nAnd then further changes are easy:\n\n - you could replace the ad-hoc \"I hope single quotes are enough\"\n quoting of $url with a real regex\n\n - you can define $sq inside the perl script to avoid the gross '\\''\n quoting (or even avoid it entirely with quotemeta)\n\nSo perhaps something like:\n\n perl -e '\n my ($cmdline, $url) = @ARGV;\n $cmdline =~ s[origin(?!/)][quotemeta($url)]ge;\n print $cmdline;\n ' -- \"$cmdline\" \"$remote_url\"\n\nI don't mean to golf on this forever, but I wanted to show something\nconcrete since you said you don't know perl well. I just think moving to\nsed introduces more opportunities for errors here, not fewer. :)\n\n-Peff\n"},{"id":"457597","messageId":"20220621092915.GD1689@szeder.dev","threadId":"58041","inReplyTo":"YrFwcL2dRS/v7xAw@coredump.intra.peff.net","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2022-06-21T09:29:15Z","receivedAt":"2022-06-21T09:29:45Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jun 21, 2022 at 03:17:04AM -0400, Jeff King wrote:\n> On Mon, Jun 20, 2022 at 06:20:07PM -0400, Derrick Stolee wrote:\n> \n> > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> > index 4620f0ca7fa..c255a77e18a 100755\n> > --- a/t/t5510-fetch.sh\n> > +++ b/t/t5510-fetch.sh\n> > @@ -853,7 +853,9 @@ test_configured_prune_type () {\n> > \t\tthen\n> > \t\t\tnew_cmdline=$cmdline_setup\n> > \t\telse\n> > -\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n> > +\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" |\n> > +\t\t\t\t\tsed -e \"s~origin ~'$remote_url' ~g\" \\\n> > +\t\t\t\t\t -e \"s~ origin~ '$remote_url'~g\")\n> > \t\tfi\n> \n> Doesn't this introduce a new problem if $remote_url contains a tilde?\n> Unlikely, but I thought the point of the exercise was defending against\n> funny paths.\n\nAnd it doesn't replace a standalone \"origin\" word, either, which does\noccur a few times in the test script.\n\n> So perhaps something like:\n> \n> perl -e '\n> my ($cmdline, $url) = @ARGV;\n> $cmdline =~ s[origin(?!/)][quotemeta($url)]ge;\n\nI don't like this \"(?!/)\" magic, because I haven't got the slightest\nidea of what it might do by merely looking at it, and these characters\nare not exactly easy to search for.\n\nThe good old \"add a space prefix and suffix\" trick can help to easily\nmatch the \"origin\" word even when it stands alone, but, alas, the\nresult is still not as simple as I'd like with the \\s and the string\nconcatenation:\n\n perl -e '\n new_cmdline=$(perl -e '\n my ($cmdline, $url) = @ARGV;\n $cmdline =~ s[\\sorigin\\s][\" \" . quotemeta($url) . \" \"]ge;\n print $cmdline;\n ' -- \" $cmdline \" \"$remote_url\")\n\n(The 'sed' variant looks good to me:\n\n printf \" %s \" \"$cmdline\" |sed \"s~ origin ~ '$remote_url' ~g\"\n\nbut then again it just trades one set of troublesome characters for an\nother.)\n\n> print $cmdline;\n> ' -- \"$cmdline\" \"$remote_url\"\n> \n> I don't mean to golf on this forever, but I wanted to show something\n> concrete since you said you don't know perl well. I just think moving to\n> sed introduces more opportunities for errors here, not fewer. :)\n> \n> -Peff\n"},{"id":"457598","messageId":"YrGYZieKgm4z3mZh@coredump.intra.peff.net","threadId":"58041","inReplyTo":"20220621092915.GD1689@szeder.dev","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-06-21T10:07:34Z","receivedAt":"2022-06-21T10:07:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 21, 2022 at 11:29:15AM +0200, SZEDER Gábor wrote:\n\n> > So perhaps something like:\n> > \n> > perl -e '\n> > my ($cmdline, $url) = @ARGV;\n> > $cmdline =~ s[origin(?!/)][quotemeta($url)]ge;\n> \n> I don't like this \"(?!/)\" magic, because I haven't got the slightest\n> idea of what it might do by merely looking at it, and these characters\n> are not exactly easy to search for.\n\nYeah, I hadn't really dug into the rest of the thread and didn't\nunderstand what that part was trying to do. So I left it untouched in my\nexamples as an exercise for the reader. :)\n\n> The good old \"add a space prefix and suffix\" trick can help to easily\n> match the \"origin\" word even when it stands alone, but, alas, the\n> result is still not as simple as I'd like with the \\s and the string\n> concatenation:\n> \n> perl -e '\n> new_cmdline=$(perl -e '\n> my ($cmdline, $url) = @ARGV;\n> $cmdline =~ s[\\sorigin\\s][\" \" . quotemeta($url) . \" \"]ge;\n> print $cmdline;\n> ' -- \" $cmdline \" \"$remote_url\")\n\nIf you do:\n\n $url = quotemeta($url);\n\nthen you can drop the \"e\" from the regex, which gets rid of the gross\nconcatenation:\n\n $cmdline =~ s[\\sorigin\\s][ $url ];\n\nI think \"\\b\" for a word boundary would avoid the whitespace\nprefix/suffix hackery, but it also matches non-alphabetics (like \"/\").\nYou could use alternation, though, like:\n\n $ cmdline='origin notorigin origin originnot origin/foo origin'\n $ remote_url=real_url\n $ perl -e '\n my ($cmdline, $url) = @ARGV;\n $url = quotemeta($url);\n $cmdline =~ s/(\\s|^)origin(\\s|$)/$1$url$2/g;\n print $cmdline;\n ' \"$cmdline\" \"$remote_url\"\n real_url notorigin real_url originnot origin/foo real_url\n\nNegative lookbehind and lookahead get rid of the \"$1\" and \"$2\", but they\nare magical-looking, as you noted above. Possibly \"/x\" and some\nwhitespace would make the whole thing more readable.\n\n-Peff\n"},{"id":"457616","messageId":"012701d88576$0b6f7a80$224e6f80$@nexbridge.com","threadId":"58041","inReplyTo":"00a401d884d0$32885890$979909b0$@nexbridge.com","subject":"RE: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2022-06-21T13:51:39Z","receivedAt":"2022-06-21T13:51:58Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On June 20, 2022 2:04 PM, I wrote:\n>To: 'Junio C Hamano' \n>Cc: git@vger.kernel.org\n>Subject: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1\n>\n>Subtests t5510.69, 71, 97, 99, 125, 127, 147, 149, 151, 153, 155, 157, 159, 161, 163,\n>165, 167, 170 failed in rc1 on the NonStop ia64 platform. These were working in rc0,\n>but after analysis, I do not think that is relevant. Here is what we see for subtest\n>69:\n>\n>expecting success of 5510.69 'link prune fetch.prune=unset\n>remote.origin.prune=unset fetch.pruneTags=unset\n>remote.origin.pruneTags=unset --prune origin refs/tags/*:refs/tags/*;\n>branch:kept tag:pruned':\n>...\n> git fetch\n>\"file:///home/jenkins/.jenkins/workspace/Git_Pipeline@2/t/trash\n>directory.t5510-fetch/.\" \"+refs/heads/*:refs/remotes/origin/*\" && ...\n> git$git_fetch_c fetch --prune\n>\"file:///home/jenkins/.jenkins/workspace/Git_Pipeline/t/trash directory.t5510-\n>fetch/.\" refs/tags/*:refs/tags/* &&\n>\n>What seems to be happening is that the test script is computing the wrong parent\n>directory in the second case, cutting the @2. We use Jenkins for our builds and\n>occasional multiple builds run. The @2 is Jenkins way of running in another place.\n>This seems to be confusing the script parsing of the parent, causing the issue.\n>When I rebuilt and test in the primary directory, the failures disappeared because\n>no @2.\n>\n>t5562 also seems to have the same issue.\n\nA more complete list of tests failing in path with @2 is - and this is likely caused by different constructs:\n\nt5300, t5510, t5516, t5562, t5601, t7528.\n\n"},{"id":"457629","messageId":"xmqq1qviht4h.fsf@gitster.g","threadId":"58041","inReplyTo":"YrFwcL2dRS/v7xAw@coredump.intra.peff.net","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-21T16:35:26Z","receivedAt":"2022-06-21T16:35:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> And then further changes are easy:\n>\n> - you could replace the ad-hoc \"I hope single quotes are enough\"\n> quoting of $url with a real regex\n\nYes, this is what I found a bit queasy about in the whole thing.\n\n> - you can define $sq inside the perl script to avoid the gross '\\''\n> quoting (or even avoid it entirely with quotemeta)\n>\n> So perhaps something like:\n>\n> perl -e '\n> my ($cmdline, $url) = @ARGV;\n> $cmdline =~ s[origin(?!/)][quotemeta($url)]ge;\n> print $cmdline;\n> ' -- \"$cmdline\" \"$remote_url\"\n\nYup, a solution along that line was what I expected to see from\nthose who write Perl when I saw the discussion yesterday.\n\nFWIW, there are a few characters that won't be acceptable in the\npathname for our source tree, including a single quote (I think some\nof our build procedures are loose in quoting, unlike parts of the\nMakefile that use $(FOO_SQ) for that) and a colon (testing the\nversion of Git we just built needs the path of the build directory\nin GIT_EXEC_PATH, which is prepended to PATH at runtime during the\ntest, but the colon means that the path is split into two there), so\n\"I hope single quotes are enough\" is not making things worse in\npractice, but quotemeta() is so simple---allowing us not having to\nworry about quoting is a very good thing.\n\nThanks.\n\n\n"},{"id":"457639","messageId":"xmqqy1xpg3th.fsf@gitster.g","threadId":"58041","inReplyTo":"xmqq1qviht4h.fsf@gitster.g","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-21T20:27:22Z","receivedAt":"2022-06-21T20:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Yup, a solution along that line was what I expected to see from\n> those who write Perl when I saw the discussion yesterday.\n\nHere is what I queued tentatively. This is not exactly new;\ne1790f92 (fetch tests: fetch as well as fetch\n[], 2018-02-09) first appeared in v2.17.0 and we can\nlive with the same glitch for few more weeks ;-)\n\n----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----\nFrom: Derrick Stolee \nDate: Mon, 20 Jun 2022 15:52:09 -0400\nSubject: [PATCH] t5510: replace 'origin' with URL more carefully\n\nThe many test_configured_prune tests in t5510-fetch.sh test many\ncombinations of --prune, --prune-tags, and using 'origin' or an explicit\nURL. Some machinery was introduced in e1790f9245f (fetch tests: fetch\n as well as fetch [], 2018-02-09) to replace\n'origin' with this explicit URL. This URL is a \"file:///\" URL for the\nroot of the $TRASH_DIRECTORY.\n\nHowever, if the current build tree has an '@' symbol, the\nreplacement using perl fails. It drops the '@' as well as anything\nelse in that directory name. You can observe this locally by\ncloning git.git into a \"victim@03\" directory and running the test\nscript.\n\nAs we are writing in Perl anyway, pass in the shell variables involved\nto the script as arguments and perform necessary string transformations\ninside it, instead of assuming that it is sufficient to enclose the\n$remote_url variable inside a pair of single quotes.\n\nReported-by: Randall Becker \nOriginal-patch-by: Derrick Stolee \nHelped-by: Jeff King \nSigned-off-by: Junio C Hamano \n---\n t/t5510-fetch.sh | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 4620f0ca7f..b45879a760 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -853,7 +853,11 @@ test_configured_prune_type () {\n \t\tthen\n \t\t\tnew_cmdline=$cmdline_setup\n \t\telse\n-\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n+\t\t\tnew_cmdline=$(perl -e '\n+\t\t\t\tmy ($cmdline, $url) = @ARGV;\n+\t\t\t\t$cmdline =~ s[origin(?!/)][quotemeta($url)]ge;\n+\t\t\t\tprint $cmdline;\n+\t\t\t' -- \"$cmdline\" \"$remote_url\")\n \t\tfi\n \n \t\tif test \"$fetch_prune_tags\" = 'true' ||\n-- \n2.37.0-rc1-99-g4626346e14\n\n"},{"id":"457641","messageId":"dbfbbbd6-70bd-3830-8c87-9f50c346e8ea@github.com","threadId":"58041","inReplyTo":"xmqqy1xpg3th.fsf@gitster.g","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-21T21:13:35Z","receivedAt":"2022-06-21T21:23:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/21/2022 4:27 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> Yup, a solution along that line was what I expected to see from\n>> those who write Perl when I saw the discussion yesterday.\n> \n> Here is what I queued tentatively. This is not exactly new;\n> e1790f92 (fetch tests: fetch as well as fetch\n> [], 2018-02-09) first appeared in v2.17.0 and we can\n> live with the same glitch for few more weeks ;-)\n\nThere are also other tests that need similar updates before\nwe consider this topic done.\n \n> ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----\n> From: Derrick Stolee \n> Original-patch-by: Derrick Stolee \n\nI recommend updating the authorship to yourself or Peff, since I\nwas mostly useless in constructing the actual solution.\n\nThanks,\n-Stolee\n"},{"id":"457644","messageId":"xmqqa6a5g0m3.fsf@gitster.g","threadId":"58041","inReplyTo":"dbfbbbd6-70bd-3830-8c87-9f50c346e8ea@github.com","subject":"Re: [PATCH v2] t5510: replace 'origin' with URL more carefully (was Re: Test Failure t5510,t5562 - was RE: [ANNOUNCE] Git v2.37.0-rc1)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-21T21:36:36Z","receivedAt":"2022-06-21T21:36:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 6/21/2022 4:27 PM, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>> \n>>> Yup, a solution along that line was what I expected to see from\n>>> those who write Perl when I saw the discussion yesterday.\n>> \n>> Here is what I queued tentatively. This is not exactly new;\n>> e1790f92 (fetch tests: fetch as well as fetch\n>> [], 2018-02-09) first appeared in v2.17.0 and we can\n>> live with the same glitch for few more weeks ;-)\n>\n> There are also other tests that need similar updates before\n> we consider this topic done.\n\nIf we can check with single-quotes and dollar-signs in addition to\nat-mark, that would be interesting.\n\n> I recommend updating the authorship to yourself or Peff, since I\n> was mostly useless in constructing the actual solution.\n\nSure. That's what \"tentatively\" means. I didn't do anything (other\nthan cheering from the sidelines), so...\n\n\n"},{"id":"457651","messageId":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"xmqqa6a5g0m3.fsf@gitster.g","subject":"[PATCH 00/10] t5510: fix the quoting mess","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:12Z","receivedAt":"2022-06-21T22:34:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since the upthread quoting discussion pointed at a commit of mine I\nfelt obligated to look at this in a deeper way.\n\nWhen I added those tests I carried forward some technical debt where\nwe were passing arguments to a function as a string, instead of as\nmultiple arguments which we could get with \"$@\", in hindsight that was\na big mistake.\n\nAs far as I can tell the proposed\nhttps://lore.kernel.org/git/xmqqy1xpg3th.fsf@gitster.g/ fixes the\nissue at hand, but this is the deeper and I think better direction to\ngo in (but I could rebase in top of that fix if needed, although the\nconflict is trivial, keep this side).\n\nNow we don't use \"perl\", or \"sed\", or whatever to re-quote arguments\nin t/t5510-fetch.sh.\n\nThere's still some weirdness in that test, e.g. the whole behavior of\nset_config_tristate(), but I had to stop somewhere.\n\nÆvar Arnfjörð Bjarmason (10):\n fetch tests: remove redundant test_unconfig()\n fetch tests: use named, not positional parameters\n fetch tests: use \"local\", &&-chaining, style etc.\n fetch tests: add a helper to avoid boilerplate\n fetch tests: pass \"mode\" parameter first, pave way for \"$@\"\n fetch tests: pass a list, not a string of arguments\n fetch tests: remove lazy variable setup\n fetch tests: remove shelling out for previously \"lazy\" variables\n fetch tests: stop implicitly adding refspecs\n fetch tests: fix needless and buggy re-quoting\n\n t/t5510-fetch.sh | 289 +++++++++++++++++++++++++++++------------------\n 1 file changed, 178 insertions(+), 111 deletions(-)\n\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457652","messageId":"patch-01.10-6236f4475fc-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 01/10] fetch tests: remove redundant test_unconfig()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:13Z","receivedAt":"2022-06-21T22:34:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The test_unconfig() calls here were added as boilerplate in\n737c5a9cde7 (fetch: make --prune configurable, 2013-07-13), and then\nfaithfully reproduced in e249ce0ccdb (fetch tests: add scaffolding for\nthe new fetch.pruneTags, 2018-02-09). But they were never necessary,\nso let's remove them.\n\nThis actually improves our test coverage, as we'll now be asserting\nthat whatever configuration we leave here (in the \"one\" block below)\nwon't affect this particular \"git fetch\" command.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 4620f0ca7fa..d784a761ba0 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -874,10 +874,6 @@ test_configured_prune_type () {\n \t\tgit tag -f newtag &&\n \t\t(\n \t\t\tcd one &&\n-\t\t\ttest_unconfig fetch.prune &&\n-\t\t\ttest_unconfig fetch.pruneTags &&\n-\t\t\ttest_unconfig remote.origin.prune &&\n-\t\t\ttest_unconfig remote.origin.pruneTags &&\n \t\t\tgit fetch '\"$cmdline_setup\"' &&\n \t\t\tgit rev-parse --verify refs/remotes/origin/newbranch &&\n \t\t\tgit rev-parse --verify refs/tags/newtag\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457653","messageId":"patch-04.10-398818a5acb-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 04/10] fetch tests: add a helper to avoid boilerplate","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:16Z","receivedAt":"2022-06-21T22:34:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a new test_configured_prune_type_branch() helper to avoid the\nboilerplate introduced in 97716d217c1 (fetch: add a --prune-tags\noption and fetch.pruneTags config, 2018-02-09).\n\nBack then it was somewhat necessary, but since 6317972cff9 (fetch:\nmake the --prune-tags work with , 2018-02-09) these tests have\nbeen regular enough that we can always pass the \"kept\" argument for\n\"link\", and \"pruned\" for \"name\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 25 +++++++++++--------------\n 1 file changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 799e69dc1b1..5d118a6a806 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -1006,22 +1006,19 @@ test_configured_prune unset unset unset true pruned kept \\\n # remote. However, because there's no implicit\n # +refs/heads/*:refs/remotes/origin/* refspec and supplying it on the\n # command-line negates --prune-tags, the branches will not be pruned.\n+test_configured_prune_type_branch () {\n+\ttest_configured_prune_type \"$1\" \"$2\" \"$3\" \"$4\" pruned \"$6\" \"$7\" \"name\"\n+\ttest_configured_prune_type \"$1\" \"$2\" \"$3\" \"$4\" kept \"$6\" \"$7\" \"link\"\n+}\n test_configured_prune_type unset unset unset unset kept kept \"origin --prune-tags\" \"name\"\n test_configured_prune_type unset unset unset unset kept kept \"origin --prune-tags\" \"link\"\n-test_configured_prune_type unset unset unset unset pruned pruned \"origin --prune --prune-tags\" \"name\"\n-test_configured_prune_type unset unset unset unset kept pruned \"origin --prune --prune-tags\" \"link\"\n-test_configured_prune_type unset unset unset unset pruned pruned \"--prune --prune-tags origin\" \"name\"\n-test_configured_prune_type unset unset unset unset kept pruned \"--prune --prune-tags origin\" \"link\"\n-test_configured_prune_type unset unset true unset pruned pruned \"--prune origin\" \"name\"\n-test_configured_prune_type unset unset true unset kept pruned \"--prune origin\" \"link\"\n-test_configured_prune_type unset unset unset true pruned pruned \"--prune origin\" \"name\"\n-test_configured_prune_type unset unset unset true kept pruned \"--prune origin\" \"link\"\n-test_configured_prune_type true unset true unset pruned pruned \"origin\" \"name\"\n-test_configured_prune_type true unset true unset kept pruned \"origin\" \"link\"\n-test_configured_prune_type unset true true unset pruned pruned \"origin\" \"name\"\n-test_configured_prune_type unset true true unset kept pruned \"origin\" \"link\"\n-test_configured_prune_type unset true unset true pruned pruned \"origin\" \"name\"\n-test_configured_prune_type unset true unset true kept pruned \"origin\" \"link\"\n+test_configured_prune_type_branch unset unset unset unset - pruned \"origin --prune --prune-tags\"\n+test_configured_prune_type_branch unset unset unset unset - pruned \"--prune --prune-tags origin\"\n+test_configured_prune_type_branch unset unset true unset - pruned \"--prune origin\"\n+test_configured_prune_type_branch unset unset unset true - pruned \"--prune origin\"\n+test_configured_prune_type_branch true unset true unset - pruned \"origin\"\n+test_configured_prune_type_branch unset true true unset - pruned \"origin\"\n+test_configured_prune_type_branch unset true unset true - pruned \"origin\"\n \n # When all remote.origin.fetch settings are deleted a --prune\n # --prune-tags still implicitly supplies refs/tags/*:refs/tags/* so\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457654","messageId":"patch-03.10-02aad13748e-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 03/10] fetch tests: use \"local\", &&-chaining, style etc.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:15Z","receivedAt":"2022-06-21T22:34:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Make use of &&-chaining, the \"local\" keyword, the more idiomatic \"test\n-z\" over comparison against \"\" etc. in the\n\"test_configured_prune_type()\" function.\n\nLet's also move the wrapper function to below the function itself. In\ne1790f9245f (fetch tests: fetch as well as fetch\n[], 2018-02-09) placed in there to minimize the diff\nsize. While it's not incorrect to define these in this order (this\nisn't C), it's less confusing this way.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 40 +++++++++++++++++++++-------------------\n 1 file changed, 21 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 37e12de2475..799e69dc1b1 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -821,20 +821,15 @@ set_config_tristate () {\n \tesac\n }\n \n-test_configured_prune () {\n-\ttest_configured_prune_type \"$@\" \"name\"\n-\ttest_configured_prune_type \"$@\" \"link\"\n-}\n-\n test_configured_prune_type () {\n-\tfetch_prune=$1\n-\tremote_origin_prune=$2\n-\tfetch_prune_tags=$3\n-\tremote_origin_prune_tags=$4\n-\texpected_branch=$5\n-\texpected_tag=$6\n-\tcmdline=$7\n-\tmode=$8\n+\tlocal fetch_prune=\"$1\" &&\n+\tlocal remote_origin_prune=\"$2\" &&\n+\tlocal fetch_prune_tags=\"$3\" &&\n+\tlocal remote_origin_prune_tags=\"$4\" &&\n+\tlocal expected_branch=\"$5\" &&\n+\tlocal expected_tag=\"$6\" &&\n+\tlocal cmdline=\"$7\" &&\n+\tlocal mode=\"$8\" &&\n \n \tif test -z \"$cmdline_setup\"\n \tthen\n@@ -843,18 +838,18 @@ test_configured_prune_type () {\n \t\t\tremote_fetch=\"$(git -C one config remote.origin.fetch)\" &&\n \t\t\tcmdline_setup=\"\\\"$remote_url\\\" \\\"$remote_fetch\\\"\"\n \t\t'\n-\tfi\n+\tfi &&\n \n \tif test \"$mode\" = 'link'\n \tthen\n-\t\tnew_cmdline=\"\"\n+\t\tnew_cmdline=\"\" &&\n \n-\t\tif test \"$cmdline\" = \"\"\n+\t\tif test -z \"$cmdline\"\n \t\tthen\n \t\t\tnew_cmdline=$cmdline_setup\n \t\telse\n \t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n-\t\tfi\n+\t\tfi &&\n \n \t\tif test \"$fetch_prune_tags\" = 'true' ||\n \t\t test \"$remote_origin_prune_tags\" = 'true'\n@@ -863,10 +858,10 @@ test_configured_prune_type () {\n \t\t\tthen\n \t\t\t\tnew_cmdline=\"$new_cmdline refs/tags/*:refs/tags/*\"\n \t\t\tfi\n-\t\tfi\n+\t\tfi &&\n \n \t\tcmdline=\"$new_cmdline\"\n-\tfi\n+\tfi &&\n \n \ttest_expect_success \"$mode prune fetch.prune=$fetch_prune remote.origin.prune=$remote_origin_prune fetch.pruneTags=$fetch_prune_tags remote.origin.pruneTags=$remote_origin_prune_tags${cmdline:+ $cmdline}; branch:$expected_branch tag:$expected_tag\" '\n \t\t# make sure a newbranch is there in . and also in one\n@@ -915,6 +910,13 @@ test_configured_prune_type () {\n \t\t\tesac\n \t\t)\n \t'\n+\n+\treturn 0\n+}\n+\n+test_configured_prune () {\n+\ttest_configured_prune_type \"$@\" \"name\" &&\n+\ttest_configured_prune_type \"$@\" \"link\"\n }\n \n # $1 config: fetch.prune\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457655","messageId":"patch-02.10-88bde1a82a6-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 02/10] fetch tests: use named, not positional parameters","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:14Z","receivedAt":"2022-06-21T22:34:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Use the named parameters we've already unpacked from \"$@\" rather than\nthe positional paremeters. There's no functional changes here.\n\nThis changes code that dates back to the initial introduction of the\nfunction in 737c5a9cde7 (fetch: make --prune configurable,\n2013-07-13), although e.g. e249ce0ccdb (fetch tests: add scaffolding\nfor the new fetch.pruneTags, 2018-02-09) added many more parameters.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex d784a761ba0..37e12de2475 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -868,7 +868,7 @@ test_configured_prune_type () {\n \t\tcmdline=\"$new_cmdline\"\n \tfi\n \n-\ttest_expect_success \"$mode prune fetch.prune=$1 remote.origin.prune=$2 fetch.pruneTags=$3 remote.origin.pruneTags=$4${7:+ $7}; branch:$5 tag:$6\" '\n+\ttest_expect_success \"$mode prune fetch.prune=$fetch_prune remote.origin.prune=$remote_origin_prune fetch.pruneTags=$fetch_prune_tags remote.origin.pruneTags=$remote_origin_prune_tags${cmdline:+ $cmdline}; branch:$expected_branch tag:$expected_tag\" '\n \t\t# make sure a newbranch is there in . and also in one\n \t\tgit branch -f newbranch &&\n \t\tgit tag -f newtag &&\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457657","messageId":"patch-05.10-a3fb3174bde-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 05/10] fetch tests: pass \"mode\" parameter first, pave way for \"$@\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:17Z","receivedAt":"2022-06-21T22:34:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"$mode\" parameter to be passed first, and setup a\ncommand-line parser we'll be able to use for getting rid of many of\nthe boilerplate parameters.\n\nThis will allow us to unquote the command-line argument, and process\nfetch arguments as a list of \"$@\". For now we need to do more work to\nunpack these, but in a subsequent commit we'll be able to make the\nshell quote handling here much simpler.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 43 +++++++++++++++++++++++++++++++++----------\n 1 file changed, 33 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 5d118a6a806..477b6dd4953 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -822,14 +822,29 @@ set_config_tristate () {\n }\n \n test_configured_prune_type () {\n+\tmode= &&\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t--mode)\n+\t\t\tmode=\"$2\" &&\n+\t\t\tshift\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac &&\n+\t\tshift\n+\tdone &&\n+\n \tlocal fetch_prune=\"$1\" &&\n \tlocal remote_origin_prune=\"$2\" &&\n \tlocal fetch_prune_tags=\"$3\" &&\n \tlocal remote_origin_prune_tags=\"$4\" &&\n \tlocal expected_branch=\"$5\" &&\n \tlocal expected_tag=\"$6\" &&\n-\tlocal cmdline=\"$7\" &&\n-\tlocal mode=\"$8\" &&\n+\tshift 6 &&\n+\tlocal cmdline=\"$@\" &&\n \n \tif test -z \"$cmdline_setup\"\n \tthen\n@@ -915,8 +930,8 @@ test_configured_prune_type () {\n }\n \n test_configured_prune () {\n-\ttest_configured_prune_type \"$@\" \"name\" &&\n-\ttest_configured_prune_type \"$@\" \"link\"\n+\ttest_configured_prune_type --mode name \"$@\" &&\n+\ttest_configured_prune_type --mode link \"$@\"\n }\n \n # $1 config: fetch.prune\n@@ -1007,11 +1022,19 @@ test_configured_prune unset unset unset true pruned kept \\\n # +refs/heads/*:refs/remotes/origin/* refspec and supplying it on the\n # command-line negates --prune-tags, the branches will not be pruned.\n test_configured_prune_type_branch () {\n-\ttest_configured_prune_type \"$1\" \"$2\" \"$3\" \"$4\" pruned \"$6\" \"$7\" \"name\"\n-\ttest_configured_prune_type \"$1\" \"$2\" \"$3\" \"$4\" kept \"$6\" \"$7\" \"link\"\n+\tlocal cfg_fp=\"$1\" &&\n+\tlocal cfg_rnp=\"$2\" &&\n+\tlocal cfg_fpt=\"$3\" &&\n+\tlocal cfg_rnpt=\"$4\" &&\n+\tlocal arg_branch=\"$5\" &&\n+\tlocal arg_tag=\"$6\" &&\n+\tshift 6 &&\n+\n+\ttest_configured_prune_type --mode name \"$cfg_fp\" \"$cfg_rnp\" \"$cfg_fpt\" \"$cfg_rnpt\" pruned \"$arg_tag\" \"$@\"\n+\ttest_configured_prune_type --mode link \"$cfg_fp\" \"$cfg_rnp\" \"$cfg_fpt\" \"$cfg_rnpt\" kept \"$arg_tag\" \"$@\"\n }\n-test_configured_prune_type unset unset unset unset kept kept \"origin --prune-tags\" \"name\"\n-test_configured_prune_type unset unset unset unset kept kept \"origin --prune-tags\" \"link\"\n+test_configured_prune_type --mode name unset unset unset unset kept kept \"origin --prune-tags\"\n+test_configured_prune_type --mode link unset unset unset unset kept kept \"origin --prune-tags\"\n test_configured_prune_type_branch unset unset unset unset - pruned \"origin --prune --prune-tags\"\n test_configured_prune_type_branch unset unset unset unset - pruned \"--prune --prune-tags origin\"\n test_configured_prune_type_branch unset unset true unset - pruned \"--prune origin\"\n@@ -1029,8 +1052,8 @@ test_expect_success 'remove remote.origin.fetch \"one\"' '\n \t\tgit config --unset-all remote.origin.fetch\n \t)\n '\n-test_configured_prune_type unset unset unset unset kept pruned \"origin --prune --prune-tags\" \"name\"\n-test_configured_prune_type unset unset unset unset kept pruned \"origin --prune --prune-tags\" \"link\"\n+test_configured_prune_type --mode name unset unset unset unset kept pruned \"origin --prune --prune-tags\"\n+test_configured_prune_type --mode link unset unset unset unset kept pruned \"origin --prune --prune-tags\"\n \n test_expect_success 'all boundary commits are excluded' '\n \ttest_commit base &&\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457658","messageId":"patch-06.10-65c35361f88-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 06/10] fetch tests: pass a list, not a string of arguments","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:18Z","receivedAt":"2022-06-21T22:34:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Follow-up on the preceding commit where we've started to intercept\nthese arguments as \"$@\" and pass them as a list, not as a string,\nbefore we'd only have a \"$@\" equivalent to \"$1\".\n\nWe're still not doing anything really useful with these, but this is\ngetting us towards fixing the quote handling in\n\"test_configured_prune_type()\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 116 +++++++++++++++++++++++------------------------\n 1 file changed, 58 insertions(+), 58 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 477b6dd4953..c56a00f1a17 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -940,82 +940,82 @@ test_configured_prune () {\n # $4 config: remote..pruneTags\n # $5 expect: branch to be pruned?\n # $6 expect: tag to be pruned?\n-# $7 git-fetch $cmdline:\n+# $7... git-fetch $cmdline:\n #\n-# $1 $2 $3 $4 $5 $6 $7\n-test_configured_prune unset unset unset unset kept kept \"\"\n-test_configured_prune unset unset unset unset kept kept \"--no-prune\"\n-test_configured_prune unset unset unset unset pruned kept \"--prune\"\n+# $1 $2 $3 $4 $5 $6 $7...\n+test_configured_prune unset unset unset unset kept kept\n+test_configured_prune unset unset unset unset kept kept --no-prune\n+test_configured_prune unset unset unset unset pruned kept --prune\n test_configured_prune unset unset unset unset kept pruned \\\n-\t\"--prune origin refs/tags/*:refs/tags/*\"\n+\t--prune origin \"refs/tags/*:refs/tags/*\"\n test_configured_prune unset unset unset unset pruned pruned \\\n-\t\"--prune origin refs/tags/*:refs/tags/* +refs/heads/*:refs/remotes/origin/*\"\n+\t--prune origin \"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n \n-test_configured_prune false unset unset unset kept kept \"\"\n-test_configured_prune false unset unset unset kept kept \"--no-prune\"\n-test_configured_prune false unset unset unset pruned kept \"--prune\"\n+test_configured_prune false unset unset unset kept kept\n+test_configured_prune false unset unset unset kept kept --no-prune\n+test_configured_prune false unset unset unset pruned kept --prune\n \n-test_configured_prune true unset unset unset pruned kept \"\"\n-test_configured_prune true unset unset unset pruned kept \"--prune\"\n-test_configured_prune true unset unset unset kept kept \"--no-prune\"\n+test_configured_prune true unset unset unset pruned kept\n+test_configured_prune true unset unset unset pruned kept --prune\n+test_configured_prune true unset unset unset kept kept --no-prune\n \n-test_configured_prune unset false unset unset kept kept \"\"\n-test_configured_prune unset false unset unset kept kept \"--no-prune\"\n-test_configured_prune unset false unset unset pruned kept \"--prune\"\n+test_configured_prune unset false unset unset kept kept\n+test_configured_prune unset false unset unset kept kept --no-prune\n+test_configured_prune unset false unset unset pruned kept --prune\n \n-test_configured_prune false false unset unset kept kept \"\"\n-test_configured_prune false false unset unset kept kept \"--no-prune\"\n-test_configured_prune false false unset unset pruned kept \"--prune\"\n+test_configured_prune false false unset unset kept kept\n+test_configured_prune false false unset unset kept kept --no-prune\n+test_configured_prune false false unset unset pruned kept --prune\n test_configured_prune false false unset unset kept pruned \\\n-\t\"--prune origin refs/tags/*:refs/tags/*\"\n+\t--prune origin \"refs/tags/*:refs/tags/*\"\n test_configured_prune false false unset unset pruned pruned \\\n-\t\"--prune origin refs/tags/*:refs/tags/* +refs/heads/*:refs/remotes/origin/*\"\n+\t--prune origin \"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n \n-test_configured_prune true false unset unset kept kept \"\"\n-test_configured_prune true false unset unset pruned kept \"--prune\"\n-test_configured_prune true false unset unset kept kept \"--no-prune\"\n+test_configured_prune true false unset unset kept kept\n+test_configured_prune true false unset unset pruned kept --prune\n+test_configured_prune true false unset unset kept kept --no-prune\n \n-test_configured_prune unset true unset unset pruned kept \"\"\n-test_configured_prune unset true unset unset kept kept \"--no-prune\"\n-test_configured_prune unset true unset unset pruned kept \"--prune\"\n+test_configured_prune unset true unset unset pruned kept\n+test_configured_prune unset true unset unset kept kept --no-prune\n+test_configured_prune unset true unset unset pruned kept --prune\n \n-test_configured_prune false true unset unset pruned kept \"\"\n-test_configured_prune false true unset unset kept kept \"--no-prune\"\n-test_configured_prune false true unset unset pruned kept \"--prune\"\n+test_configured_prune false true unset unset pruned kept\n+test_configured_prune false true unset unset kept kept --no-prune\n+test_configured_prune false true unset unset pruned kept --prune\n \n-test_configured_prune true true unset unset pruned kept \"\"\n-test_configured_prune true true unset unset pruned kept \"--prune\"\n-test_configured_prune true true unset unset kept kept \"--no-prune\"\n+test_configured_prune true true unset unset pruned kept\n+test_configured_prune true true unset unset pruned kept --prune\n+test_configured_prune true true unset unset kept kept --no-prune\n test_configured_prune true true unset unset kept pruned \\\n-\t\"--prune origin refs/tags/*:refs/tags/*\"\n+\t--prune origin \"refs/tags/*:refs/tags/*\"\n test_configured_prune true true unset unset pruned pruned \\\n-\t\"--prune origin refs/tags/*:refs/tags/* +refs/heads/*:refs/remotes/origin/*\"\n+\t--prune origin \"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n \n # --prune-tags on its own does nothing, needs --prune as well, same\n # for fetch.pruneTags without fetch.prune\n-test_configured_prune unset unset unset unset kept kept \"--prune-tags\"\n-test_configured_prune unset unset true unset kept kept \"\"\n-test_configured_prune unset unset unset true kept kept \"\"\n+test_configured_prune unset unset unset unset kept kept --prune-tags\n+test_configured_prune unset unset true unset kept kept\n+test_configured_prune unset unset unset true kept kept\n \n # These will prune the tags\n-test_configured_prune unset unset unset unset pruned pruned \"--prune --prune-tags\"\n-test_configured_prune true unset true unset pruned pruned \"\"\n-test_configured_prune unset true unset true pruned pruned \"\"\n+test_configured_prune unset unset unset unset pruned pruned --prune --prune-tags\n+test_configured_prune true unset true unset pruned pruned\n+test_configured_prune unset true unset true pruned pruned\n \n # remote..pruneTags overrides fetch.pruneTags, just like\n # remote..prune overrides fetch.prune if set.\n-test_configured_prune true unset true unset pruned pruned \"\"\n-test_configured_prune false true false true pruned pruned \"\"\n-test_configured_prune true false true false kept kept \"\"\n+test_configured_prune true unset true unset pruned pruned\n+test_configured_prune false true false true pruned pruned\n+test_configured_prune true false true false kept kept\n \n # When --prune-tags is supplied it's ignored if an explicit refspec is\n # given, same for the configuration options.\n test_configured_prune unset unset unset unset pruned kept \\\n-\t\"--prune --prune-tags origin +refs/heads/*:refs/remotes/origin/*\"\n+\t--prune --prune-tags origin \"+refs/heads/*:refs/remotes/origin/*\"\n test_configured_prune unset unset true unset pruned kept \\\n-\t\"--prune origin +refs/heads/*:refs/remotes/origin/*\"\n+\t--prune origin \"+refs/heads/*:refs/remotes/origin/*\"\n test_configured_prune unset unset unset true pruned kept \\\n-\t\"--prune origin +refs/heads/*:refs/remotes/origin/*\"\n+\t--prune origin \"+refs/heads/*:refs/remotes/origin/*\"\n \n # Pruning that also takes place if a file:// url replaces a named\n # remote. However, because there's no implicit\n@@ -1033,15 +1033,15 @@ test_configured_prune_type_branch () {\n \ttest_configured_prune_type --mode name \"$cfg_fp\" \"$cfg_rnp\" \"$cfg_fpt\" \"$cfg_rnpt\" pruned \"$arg_tag\" \"$@\"\n \ttest_configured_prune_type --mode link \"$cfg_fp\" \"$cfg_rnp\" \"$cfg_fpt\" \"$cfg_rnpt\" kept \"$arg_tag\" \"$@\"\n }\n-test_configured_prune_type --mode name unset unset unset unset kept kept \"origin --prune-tags\"\n-test_configured_prune_type --mode link unset unset unset unset kept kept \"origin --prune-tags\"\n-test_configured_prune_type_branch unset unset unset unset - pruned \"origin --prune --prune-tags\"\n-test_configured_prune_type_branch unset unset unset unset - pruned \"--prune --prune-tags origin\"\n-test_configured_prune_type_branch unset unset true unset - pruned \"--prune origin\"\n-test_configured_prune_type_branch unset unset unset true - pruned \"--prune origin\"\n-test_configured_prune_type_branch true unset true unset - pruned \"origin\"\n-test_configured_prune_type_branch unset true true unset - pruned \"origin\"\n-test_configured_prune_type_branch unset true unset true - pruned \"origin\"\n+test_configured_prune_type --mode name unset unset unset unset kept kept origin --prune-tags\n+test_configured_prune_type --mode link unset unset unset unset kept kept origin --prune-tags\n+test_configured_prune_type_branch unset unset unset unset - pruned origin --prune --prune-tags\n+test_configured_prune_type_branch unset unset unset unset - pruned --prune --prune-tags origin\n+test_configured_prune_type_branch unset unset true unset - pruned --prune origin\n+test_configured_prune_type_branch unset unset unset true - pruned --prune origin\n+test_configured_prune_type_branch true unset true unset - pruned origin\n+test_configured_prune_type_branch unset true true unset - pruned origin\n+test_configured_prune_type_branch unset true unset true - pruned origin\n \n # When all remote.origin.fetch settings are deleted a --prune\n # --prune-tags still implicitly supplies refs/tags/*:refs/tags/* so\n@@ -1052,8 +1052,8 @@ test_expect_success 'remove remote.origin.fetch \"one\"' '\n \t\tgit config --unset-all remote.origin.fetch\n \t)\n '\n-test_configured_prune_type --mode name unset unset unset unset kept pruned \"origin --prune --prune-tags\"\n-test_configured_prune_type --mode link unset unset unset unset kept pruned \"origin --prune --prune-tags\"\n+test_configured_prune_type --mode name unset unset unset unset kept pruned origin --prune --prune-tags\n+test_configured_prune_type --mode link unset unset unset unset kept pruned origin --prune --prune-tags\n \n test_expect_success 'all boundary commits are excluded' '\n \ttest_commit base &&\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457659","messageId":"patch-07.10-267a6a6ab8f-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 07/10] fetch tests: remove lazy variable setup","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:19Z","receivedAt":"2022-06-21T22:34:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the lazy test setup added in e1790f9245f (fetch tests: fetch\n as well as fetch [], 2018-02-09) to make it clear\nthat these variables aren't changing across runs.\n\nWe can also do away with the shell invocations here, but let's do that\nin a subsequent commit, for now this shows that it's safe to do this\nmore than once.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex c56a00f1a17..7cfef0082c0 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -846,14 +846,9 @@ test_configured_prune_type () {\n \tshift 6 &&\n \tlocal cmdline=\"$@\" &&\n \n-\tif test -z \"$cmdline_setup\"\n-\tthen\n-\t\ttest_expect_success 'setup cmdline_setup variable for subsequent test' '\n-\t\t\tremote_url=\"file://$(git -C one config remote.origin.url)\" &&\n-\t\t\tremote_fetch=\"$(git -C one config remote.origin.fetch)\" &&\n-\t\t\tcmdline_setup=\"\\\"$remote_url\\\" \\\"$remote_fetch\\\"\"\n-\t\t'\n-\tfi &&\n+\tremote_url=\"file://$(git -C one config remote.origin.url)\" &&\n+\tremote_fetch=\"$(git -C one config remote.origin.fetch)\" &&\n+\tcmdline_setup=\"\\\"$remote_url\\\" \\\"$remote_fetch\\\"\" &&\n \n \tif test \"$mode\" = 'link'\n \tthen\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457660","messageId":"patch-08.10-a0d645b7146-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 08/10] fetch tests: remove shelling out for previously \"lazy\" variables","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:20Z","receivedAt":"2022-06-21T22:34:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the variable assignments added in e1790f9245f (fetch tests:\nfetch as well as fetch [], 2018-02-09). As of the\nlast commit this test was slower as we'd do this on every run, but it\nwas easier to see what was going on.\n\nBut let's instead set this on the basis of $TRASH_DIRECTORY, which is\na lot more obvious than the roundabout way of getting the\nconfiguration from the repo that we created earlier with:\n\n git clone . one\n\nSubsequent commits will make use of the new \"$refspec_heads\" for other\npurposes, so let's declare it as a variable, and use it instead of\n\"$remote_fetch\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 7cfef0082c0..54c7c86e5ca 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -837,6 +837,8 @@ test_configured_prune_type () {\n \t\tshift\n \tdone &&\n \n+\tlocal refspec_heads='+refs/heads/*:refs/remotes/origin/*' &&\n+\n \tlocal fetch_prune=\"$1\" &&\n \tlocal remote_origin_prune=\"$2\" &&\n \tlocal fetch_prune_tags=\"$3\" &&\n@@ -846,9 +848,8 @@ test_configured_prune_type () {\n \tshift 6 &&\n \tlocal cmdline=\"$@\" &&\n \n-\tremote_url=\"file://$(git -C one config remote.origin.url)\" &&\n-\tremote_fetch=\"$(git -C one config remote.origin.fetch)\" &&\n-\tcmdline_setup=\"\\\"$remote_url\\\" \\\"$remote_fetch\\\"\" &&\n+\tlocal remote_url=\"file://$TRASH_DIRECTORY/.\" &&\n+\tlocal cmdline_setup=\"\\\"$remote_url\\\" \\\"$refspec_heads\\\"\"\n \n \tif test \"$mode\" = 'link'\n \tthen\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457661","messageId":"patch-09.10-99832815abd-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 09/10] fetch tests: stop implicitly adding refspecs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:21Z","receivedAt":"2022-06-21T22:34:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the implicit behavior added in 97716d217c1 (fetch: add a\n--prune-tags option and fetch.pruneTags config, 2018-02-09) of\nproviding the heads/tags refspecs and the \"$remote_url\" for tests that\nneeded the \"--mode link\".\n\nIn that case we need the URI on the command-line, and would then add\nthese refspecs.\n\nNote that this was added in other cases, but these are the only ones\nwhere it mattered, i.e. we redundantly modified the command-line\nbefore.\n\nYes, the \"\\\"$remote_url\\\"\" quoting here is buggy, but so is the\npre-image in the same way, but removing this edge case will make it\neasier to eventually deal with that. So let's make that variable\nnon-local for now and pass it in.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 42 +++++++++++++++++++++++++-----------------\n 1 file changed, 25 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 54c7c86e5ca..73964bebced 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -848,7 +848,7 @@ test_configured_prune_type () {\n \tshift 6 &&\n \tlocal cmdline=\"$@\" &&\n \n-\tlocal remote_url=\"file://$TRASH_DIRECTORY/.\" &&\n+\tremote_url=\"file://$TRASH_DIRECTORY/.\" && # NOT local yet!\n \tlocal cmdline_setup=\"\\\"$remote_url\\\" \\\"$refspec_heads\\\"\"\n \n \tif test \"$mode\" = 'link'\n@@ -862,15 +862,6 @@ test_configured_prune_type () {\n \t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n \t\tfi &&\n \n-\t\tif test \"$fetch_prune_tags\" = 'true' ||\n-\t\t test \"$remote_origin_prune_tags\" = 'true'\n-\t\tthen\n-\t\t\tif ! printf '%s' \"$cmdline\\n\" | grep -q refs/remotes/origin/\n-\t\t\tthen\n-\t\t\t\tnew_cmdline=\"$new_cmdline refs/tags/*:refs/tags/*\"\n-\t\t\tfi\n-\t\tfi &&\n-\n \t\tcmdline=\"$new_cmdline\"\n \tfi &&\n \n@@ -990,19 +981,36 @@ test_configured_prune true true unset unset pruned pruned \\\n # --prune-tags on its own does nothing, needs --prune as well, same\n # for fetch.pruneTags without fetch.prune\n test_configured_prune unset unset unset unset kept kept --prune-tags\n-test_configured_prune unset unset true unset kept kept\n-test_configured_prune unset unset unset true kept kept\n+test_configured_prune_type --mode name unset unset true unset kept kept\n+test_configured_prune_type --mode link unset unset true unset kept kept \\\n+\torigin \"refs/tags/*:refs/tags/*\"\n+test_configured_prune_type --mode name unset unset unset true kept kept\n+test_configured_prune_type --mode link unset unset unset true kept kept \\\n+\torigin \"refs/tags/*:refs/tags/*\"\n \n # These will prune the tags\n test_configured_prune unset unset unset unset pruned pruned --prune --prune-tags\n-test_configured_prune true unset true unset pruned pruned\n-test_configured_prune unset true unset true pruned pruned\n+\n+test_configured_prune_type --mode name true unset true unset pruned pruned\n+test_configured_prune_type --mode link true unset true unset pruned pruned \\\n+\t\"\\\"$remote_url\\\"\" \\\n+\t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n+test_configured_prune_type --mode name unset true unset true pruned pruned\n+test_configured_prune_type --mode link unset true unset true pruned pruned \\\n+\t\"\\\"$remote_url\\\"\" \\\n+\t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n \n # remote..pruneTags overrides fetch.pruneTags, just like\n # remote..prune overrides fetch.prune if set.\n-test_configured_prune true unset true unset pruned pruned\n-test_configured_prune false true false true pruned pruned\n-test_configured_prune true false true false kept kept\n+test_configured_prune_type --mode name true unset true unset pruned pruned\n+test_configured_prune_type --mode link true unset true unset pruned pruned \\\n+\t\"\\\"$remote_url\\\"\" \\\n+\t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n+test_configured_prune_type --mode name false true false true pruned pruned\n+test_configured_prune_type --mode link false true false true pruned pruned \\\n+\t\"\\\"$remote_url\\\"\" \\\n+\t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n+test_configured_prune true false true false kept kept\n \n # When --prune-tags is supplied it's ignored if an explicit refspec is\n # given, same for the configuration options.\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457662","messageId":"patch-10.10-54129b94a77-20220621T222854Z-avarab@gmail.com","threadId":"58041","inReplyTo":"cover-00.10-00000000000-20220621T222854Z-avarab@gmail.com","subject":"[PATCH 10/10] fetch tests: fix needless and buggy re-quoting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-21T22:34:22Z","receivedAt":"2022-06-21T22:34:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the test_configured_prune_type() function to take full\nadvantage of its arguments being passed in as a list, rather than\nneeding hacks to work around its quoting issues.\n\nWhen the test_configured_prune() function was implemented in\n737c5a9cde7 (fetch: make --prune configurable, 2013-07-13) it passed\nin its arguments to \"git fetch\" as one argument (although at the time\nonly one argument was passed)..\n\nThen in preparation for passing more arguments the first quoting hack\nwas added in 82f34e03e91 (fetch tests: double quote a variable for\ninterpolation, 2018-02-09), with e1790f9245f (fetch tests: fetch \n as well as fetch [], 2018-02-09) following after\nthat. This was all to implement the \"git fetch --prune-tags\" feature\nin 97716d217c1 (fetch: add a --prune-tags option and fetch.pruneTags\nconfig, 2018-02-09).\n\nAt the time the edge cases introduced by this quoting were a known\nissue, but the alternative was a larger refactoring of this test\nfile. In preceding commits we've done that refactoring, so let's\nfinally take advantage of it. As reported in [1] the existing\nworkaround(s) weren't enough.\n\nWe can now drop the \"perl\" (or \"sed\" or whatever, see [2]), this will\nalso handle other cases, such as those mentioned in [3].\n\n1. https://lore.kernel.org/git/00a401d884d0$32885890$979909b0$@nexbridge.com/\n2. https://lore.kernel.org/git/495bd957-43dc-f252-657d-2969bb7ad5f3@github.com/\n3. https://lore.kernel.org/git/YrFwcL2dRS%2Fv7xAw@coredump.intra.peff.net/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t5510-fetch.sh | 79 +++++++++++++++++++++++++++++++++++++-----------\n 1 file changed, 62 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 73964bebced..730968d4cd7 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -837,6 +837,7 @@ test_configured_prune_type () {\n \t\tshift\n \tdone &&\n \n+\tlocal refspec_tags='refs/tags/*:refs/tags/*' &&\n \tlocal refspec_heads='+refs/heads/*:refs/remotes/origin/*' &&\n \n \tlocal fetch_prune=\"$1\" &&\n@@ -846,23 +847,59 @@ test_configured_prune_type () {\n \tlocal expected_branch=\"$5\" &&\n \tlocal expected_tag=\"$6\" &&\n \tshift 6 &&\n-\tlocal cmdline=\"$@\" &&\n+\tlocal cmdline=\"\" &&\n+\tlocal arg_fetch_prune=\"\" &&\n+\tlocal arg_fetch_no_prune=\"\" &&\n+\tlocal arg_fetch_prune_tags=\"\" &&\n+\tlocal arg_fetch_origin=\"\" &&\n+\tlocal arg_fetch_url=\"\" &&\n+\tlocal arg_fetch_refspec_tags=\"\" &&\n+\tlocal arg_fetch_refspec_heads=\"\" &&\n+\twhile test $# != 0\n+\tdo\n+\t\tcmdline=\"${cmdline:+$cmdline }$1\" &&\n+\t\tcase \"$1\" in\n+\t\t--prune)\n+\t\t\targ_fetch_prune=t\n+\t\t\t;;\n+\t\t--no-prune)\n+\t\t\targ_fetch_no_prune=t\n+\t\t\t;;\n+\t\t--prune-tags)\n+\t\t\targ_fetch_prune_tags=t\n+\t\t\t;;\n+\t\torigin)\n+\t\t\targ_fetch_origin=t\n+\t\t\t;;\n+\t\tREMOTE_URL)\n+\t\t\targ_fetch_url=t\n+\t\t\t;;\n+\t\t$refspec_tags)\n+\t\t\targ_fetch_refspec_tags=t\n+\t\t\t;;\n+\t\t$refspec_heads)\n+\t\t\targ_fetch_refspec_heads=t\n+\t\t\t;;\n+\t\t*)\n+\t\t\tBUG \"unknown argument: '$1'\"\n+\t\t\t;;\n+\t\tesac &&\n+\t\tshift\n+\tdone &&\n \n-\tremote_url=\"file://$TRASH_DIRECTORY/.\" && # NOT local yet!\n-\tlocal cmdline_setup=\"\\\"$remote_url\\\" \\\"$refspec_heads\\\"\"\n+\tlocal remote_url=\"file://$TRASH_DIRECTORY/.\" &&\n \n \tif test \"$mode\" = 'link'\n \tthen\n-\t\tnew_cmdline=\"\" &&\n-\n \t\tif test -z \"$cmdline\"\n \t\tthen\n-\t\t\tnew_cmdline=$cmdline_setup\n-\t\telse\n-\t\t\tnew_cmdline=$(printf \"%s\" \"$cmdline\" | perl -pe 's[origin(?!/)][\"'\"$remote_url\"'\"]g')\n-\t\tfi &&\n-\n-\t\tcmdline=\"$new_cmdline\"\n+\t\t\targ_fetch_refspec_heads=t\n+\t\t\targ_fetch_url=t\n+\t\telif test -n \"$arg_fetch_origin\"\n+\t\tthen\n+\t\t\targ_fetch_origin=\n+\t\t\targ_fetch_url=t\n+\t\tfi\n \tfi &&\n \n \ttest_expect_success \"$mode prune fetch.prune=$fetch_prune remote.origin.prune=$remote_origin_prune fetch.pruneTags=$fetch_prune_tags remote.origin.pruneTags=$remote_origin_prune_tags${cmdline:+ $cmdline}; branch:$expected_branch tag:$expected_tag\" '\n@@ -871,7 +908,7 @@ test_configured_prune_type () {\n \t\tgit tag -f newtag &&\n \t\t(\n \t\t\tcd one &&\n-\t\t\tgit fetch '\"$cmdline_setup\"' &&\n+\t\t\tgit fetch \"$remote_url\" \"$refspec_heads\" &&\n \t\t\tgit rev-parse --verify refs/remotes/origin/newbranch &&\n \t\t\tgit rev-parse --verify refs/tags/newtag\n \t\t) &&\n@@ -893,7 +930,15 @@ test_configured_prune_type () {\n \t\t\tthen\n \t\t\t\tgit_fetch_c=\"\"\n \t\t\tfi &&\n-\t\t\tgit$git_fetch_c fetch '\"$cmdline\"' &&\n+\t\t\tgit$git_fetch_c fetch \\\n+\t\t\t\t${arg_fetch_prune:+--prune} \\\n+\t\t\t\t${arg_fetch_no_prune:+--no-prune} \\\n+\t\t\t\t${arg_fetch_prune_tags:+--prune-tags} \\\n+\t\t\t\t${arg_fetch_origin:+origin} \\\n+\t\t\t\t${arg_fetch_url:+\"$remote_url\"} \\\n+\t\t\t\t${arg_fetch_refspec_tags:+\"refs/tags/*:refs/tags/*\"} \\\n+\t\t\t\t${arg_fetch_refspec_heads:+\"+refs/heads/*:refs/remotes/origin/*\"} &&\n+\n \t\t\tcase \"$expected_branch\" in\n \t\t\tpruned)\n \t\t\t\ttest_must_fail git rev-parse --verify refs/remotes/origin/newbranch\n@@ -993,22 +1038,22 @@ test_configured_prune unset unset unset unset pruned pruned --prune --prune-tags\n \n test_configured_prune_type --mode name true unset true unset pruned pruned\n test_configured_prune_type --mode link true unset true unset pruned pruned \\\n-\t\"\\\"$remote_url\\\"\" \\\n+\tREMOTE_URL \\\n \t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n test_configured_prune_type --mode name unset true unset true pruned pruned\n test_configured_prune_type --mode link unset true unset true pruned pruned \\\n-\t\"\\\"$remote_url\\\"\" \\\n+\tREMOTE_URL \\\n \t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n \n # remote..pruneTags overrides fetch.pruneTags, just like\n # remote..prune overrides fetch.prune if set.\n test_configured_prune_type --mode name true unset true unset pruned pruned\n test_configured_prune_type --mode link true unset true unset pruned pruned \\\n-\t\"\\\"$remote_url\\\"\" \\\n+\tREMOTE_URL \\\n \t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n test_configured_prune_type --mode name false true false true pruned pruned\n test_configured_prune_type --mode link false true false true pruned pruned \\\n-\t\"\\\"$remote_url\\\"\" \\\n+\tREMOTE_URL \\\n \t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n test_configured_prune true false true false kept kept\n \n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457681","messageId":"xmqq4k0d8cts.fsf@gitster.g","threadId":"58041","inReplyTo":"patch-01.10-6236f4475fc-20220621T222854Z-avarab@gmail.com","subject":"Re: [PATCH 01/10] fetch tests: remove redundant test_unconfig()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-22T05:52:15Z","receivedAt":"2022-06-22T05:52:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The test_unconfig() calls here were added as boilerplate in\n> 737c5a9cde7 (fetch: make --prune configurable, 2013-07-13), and then\n> faithfully reproduced in e249ce0ccdb (fetch tests: add scaffolding for\n> the new fetch.pruneTags, 2018-02-09). But they were never necessary,\n\nunnecessary because ...?\n\nis it because nothing has happened in this directory before?\n\nis it because these will all be overriden by the command line\noptions?\n\nis it because of something else?\n\n> so let's remove them.\n\n\n> This actually improves our test coverage, as we'll now be asserting\n> that whatever configuration we leave here (in the \"one\" block below)\n> won't affect this particular \"git fetch\" command.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> t/t5510-fetch.sh | 4 ----\n> 1 file changed, 4 deletions(-)\n>\n> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\n> index 4620f0ca7fa..d784a761ba0 100755\n> --- a/t/t5510-fetch.sh\n> +++ b/t/t5510-fetch.sh\n> @@ -874,10 +874,6 @@ test_configured_prune_type () {\n> \t\tgit tag -f newtag &&\n> \t\t(\n> \t\t\tcd one &&\n> -\t\t\ttest_unconfig fetch.prune &&\n> -\t\t\ttest_unconfig fetch.pruneTags &&\n> -\t\t\ttest_unconfig remote.origin.prune &&\n> -\t\t\ttest_unconfig remote.origin.pruneTags &&\n> \t\t\tgit fetch '\"$cmdline_setup\"' &&\n> \t\t\tgit rev-parse --verify refs/remotes/origin/newbranch &&\n> \t\t\tgit rev-parse --verify refs/tags/newtag\n"},{"id":"457682","messageId":"xmqqy1xp6xup.fsf@gitster.g","threadId":"58041","inReplyTo":"patch-05.10-a3fb3174bde-20220621T222854Z-avarab@gmail.com","subject":"Re: [PATCH 05/10] fetch tests: pass \"mode\" parameter first, pave way for \"$@\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-22T06:01:02Z","receivedAt":"2022-06-22T06:01:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> +\tlocal cmdline=\"$@\" &&\n\nUnless you are using the magic trait of \"$@\" that lets you pass each\npositional parameter without getting split at internal $IFS, it is\nless confusing to write \"$*\", which will make it obvious that a\nlater use of $cmdline will lose the distinction between\ninter-parameter spaces and a whitespace that was embedded inside\nan individual parameter.\n\n> @@ -915,8 +930,8 @@ test_configured_prune_type () {\n> }\n> \n> test_configured_prune () {\n> -\ttest_configured_prune_type \"$@\" \"name\" &&\n> -\ttest_configured_prune_type \"$@\" \"link\"\n> +\ttest_configured_prune_type --mode name \"$@\" &&\n> +\ttest_configured_prune_type --mode link \"$@\"\n> }\n> \n> # $1 config: fetch.prune\n> @@ -1007,11 +1022,19 @@ test_configured_prune unset unset unset true pruned kept \\\n> # +refs/heads/*:refs/remotes/origin/* refspec and supplying it on the\n> # command-line negates --prune-tags, the branches will not be pruned.\n> test_configured_prune_type_branch () {\n> -\ttest_configured_prune_type \"$1\" \"$2\" \"$3\" \"$4\" pruned \"$6\" \"$7\" \"name\"\n> -\ttest_configured_prune_type \"$1\" \"$2\" \"$3\" \"$4\" kept \"$6\" \"$7\" \"link\"\n> +\tlocal cfg_fp=\"$1\" &&\n> +\tlocal cfg_rnp=\"$2\" &&\n> +\tlocal cfg_fpt=\"$3\" &&\n> +\tlocal cfg_rnpt=\"$4\" &&\n> +\tlocal arg_branch=\"$5\" &&\n> +\tlocal arg_tag=\"$6\" &&\n> +\tshift 6 &&\n\nUnless we plan to never allow new parameters to be added (or\nexisting one retired) from this helper, it would probably be easier\nto maintain if you wrote\n\n\tlocal cfg_fp=\"$1\" && shift &&\n\tlocal cfg_rnp=\"$1\" && shift &&\n\t...\n\n"},{"id":"457683","messageId":"xmqqsfnx6xbh.fsf@gitster.g","threadId":"58041","inReplyTo":"patch-10.10-54129b94a77-20220621T222854Z-avarab@gmail.com","subject":"Re: [PATCH 10/10] fetch tests: fix needless and buggy re-quoting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-22T06:12:34Z","receivedAt":"2022-06-22T06:12:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> -\tremote_url=\"file://$TRASH_DIRECTORY/.\" && # NOT local yet!\n> -\tlocal cmdline_setup=\"\\\"$remote_url\\\" \\\"$refspec_heads\\\"\"\n\nGood riddance ;-)\n\n> +\tlocal remote_url=\"file://$TRASH_DIRECTORY/.\" &&\n> \n> ...\n> -\t\t\tgit fetch '\"$cmdline_setup\"' &&\n> +\t\t\tgit fetch \"$remote_url\" \"$refspec_heads\" &&\n> ...\n> +\t\t\tgit$git_fetch_c fetch \\\n> +\t\t\t\t${arg_fetch_prune:+--prune} \\\n> +\t\t\t\t${arg_fetch_no_prune:+--no-prune} \\\n> +\t\t\t\t${arg_fetch_prune_tags:+--prune-tags} \\\n> +\t\t\t\t${arg_fetch_origin:+origin} \\\n> +\t\t\t\t${arg_fetch_url:+\"$remote_url\"} \\\n> +\t\t\t\t${arg_fetch_refspec_tags:+\"refs/tags/*:refs/tags/*\"} \\\n> +\t\t\t\t${arg_fetch_refspec_heads:+\"+refs/heads/*:refs/remotes/origin/*\"} &&\n> +\n\nThis makes it a lot clearer, with no perl, no sed, no eval. It does\nbecome louder, but should be easier to follow in general ...\n\n> test_configured_prune_type --mode link true unset true unset pruned pruned \\\n> -\t\"\\\"$remote_url\\\"\" \\\n> +\tREMOTE_URL \\\n> \t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n\n... except for a magic like this one.\n\nWe may remember the REMOTE_URL -> $remote_url trick used here this\nweek, but I am not sure if we find it sensible in 3 months.\n\nBut overall I think this makes it simpler. I am not 100% sold on\nthe necessity of lengthy earlier steps, though.\n\nThanks.\n\n\n\n"},{"id":"457690","messageId":"69256646-13b0-5619-3161-8d8e319fad50@github.com","threadId":"58041","inReplyTo":"xmqqsfnx6xbh.fsf@gitster.g","subject":"Re: [PATCH 10/10] fetch tests: fix needless and buggy re-quoting","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-22T11:25:05Z","receivedAt":"2022-06-22T11:25:10Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/22/22 2:12 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n\n> This makes it a lot clearer, with no perl, no sed, no eval. It does\n> become louder, but should be easier to follow in general ...\n> \n>> test_configured_prune_type --mode link true unset true unset pruned pruned \\\n>> -\t\"\\\"$remote_url\\\"\" \\\n>> +\tREMOTE_URL \\\n>> \t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n> \n> ... except for a magic like this one.\n> \n> We may remember the REMOTE_URL -> $remote_url trick used here this\n> week, but I am not sure if we find it sensible in 3 months.\n> \n> But overall I think this makes it simpler. I am not 100% sold on\n> the necessity of lengthy earlier steps, though.\n\nWhen I saw that this was a series with 10 patches (without reading\nanything else) I expected that you had created a test-lib.sh helper\nthat allowed replacing a word in a string with another string, and\nthen the remaining patches were fixing the other tests that have\nsimilar breaks when using \"@\" in the path.\n\n(Heck, I'd even take a \"test-tool replace-word \n\" implementation to avoid all of these issues we have\ndue to using scripting languages that rely on special characters\nto define the match and replacement operation.)\n\nIt seems like this isn't the last time we are going to have a\nproblem with string replacement like this, and having a well-defined\nhelper would go far.\n\nThe rest of the changes to the test script seem more complicated\nthan necessary for what _should_ be a simple problem.\n\nThanks,\n-Stolee\n"},{"id":"457713","messageId":"220622.86iloslnlt.gmgdl@evledraar.gmail.com","threadId":"58041","inReplyTo":"69256646-13b0-5619-3161-8d8e319fad50@github.com","subject":"Re: [PATCH 10/10] fetch tests: fix needless and buggy re-quoting","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-22T15:21:24Z","receivedAt":"2022-06-22T15:33:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jun 22 2022, Derrick Stolee wrote:\n\n> On 6/22/22 2:12 AM, Junio C Hamano wrote:\n>> Ævar Arnfjörð Bjarmason writes:\n>\n>> This makes it a lot clearer, with no perl, no sed, no eval. It does\n>> become louder, but should be easier to follow in general ...\n>> \n>>> test_configured_prune_type --mode link true unset true unset pruned pruned \\\n>>> -\t\"\\\"$remote_url\\\"\" \\\n>>> +\tREMOTE_URL \\\n>>> \t\"refs/tags/*:refs/tags/*\" \"+refs/heads/*:refs/remotes/origin/*\"\n>> \n>> ... except for a magic like this one.\n>> \n>> We may remember the REMOTE_URL -> $remote_url trick used here this\n>> week, but I am not sure if we find it sensible in 3 months.\n>> \n>> But overall I think this makes it simpler. I am not 100% sold on\n>> the necessity of lengthy earlier steps, though.\n>\n> When I saw that this was a series with 10 patches (without reading\n> anything else) I expected that you had created a test-lib.sh helper\n> that allowed replacing a word in a string with another string, and\n> then the remaining patches were fixing the other tests that have\n> similar breaks when using \"@\" in the path.\n\nI guess we could have such a helper, but I can't imagine a use-case for\nit where the answer wouldn't be the same as what this series suggests:\nJust stop passing a quoted argument list as one giant string.\n\n> (Heck, I'd even take a \"test-tool replace-word \n> \" implementation [...]\n\nIf we need to replace a string we can use sed or perl, but much better\nis to not need to do so in the first place. E.g. the \"origin\" match we\nhad before is now just \"origin\" in a case statement. I'm assuming that\nyou mean a test-tool to do something similar to strvec_split() (or\nwhatever the quoted variant was?).\n\n> to avoid all of these issues we have\n> due to using scripting languages that rely on special characters\n> to define the match and replacement operation.)\n\nWe've got plenty of issues inherent in shellscript as a language with no\nlittle in the way of complex types (e.g. no hashes).\n\nBut in this case the language gives us the right type (list), we just\nweren't using it. No?\n\n> It seems like this isn't the last time we are going to have a\n> problem with string replacement like this, and having a well-defined\n> helper would go far.\n\n...I guess I'm not seeing the use-case for it, in this case it was \"just\npass a list then\", wouldn't that be the case in other similar scenarios.\n\n> The rest of the changes to the test script seem more complicated\n> than necessary for what _should_ be a simple problem.\n\nI tried to optimize it for ease of review as opposed to diff size or\npatch numbers, so e.g. doing mechanical replacements in their own\ncommits.\n\nDo you have any specific things in mind that you think are too\ncomplicated?\n\nYes, it should ideally not be such a pain, but as we made wide use of a\nstring instead of a list digging ourselves out of that hole took some\ndoing.\n\nBut I really think fixing the underlying issue is worth it, as opposed\nto just plastering over it.\n"},{"id":"457715","messageId":"xmqq1qvg7le7.fsf@gitster.g","threadId":"58041","inReplyTo":"69256646-13b0-5619-3161-8d8e319fad50@github.com","subject":"Re: [PATCH 10/10] fetch tests: fix needless and buggy re-quoting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-22T15:44:48Z","receivedAt":"2022-06-22T15:44:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> It seems like this isn't the last time we are going to have a\n> problem with string replacement like this, and having a well-defined\n> helper would go far.\n\nI think the idea of [10/10] is to use shell itself as a well-defined\nhelper, with \"string replacement\" being \"$variable_interpolation\".\n\nWhich isn't a bad approach, I would say.\n\n> The rest of the changes to the test script seem more complicated\n> than necessary for what _should_ be a simple problem.\n\nTrue. I am not sure which parts are unnecessary \"churn while at it,\nburying the most interesting and beneficial one at the end as a\nhostage\", and which ones are absolutely needed to reach [10/10].\nPerhaps all of them are the latter? I dunno.\n\n\n"}]}