{"thread":{"id":"63491","subject":"[PATCH] cvsserver: avoid precedence problem between ! and %s","startedAt":"2025-05-21T07:45:05Z","lastAt":"2025-05-27T15:52:54Z","messageCount":21,"participants":["Ondřej Pohořelský via GitGitGadget","Kristoffer Haugsbakk","Junio C Hamano","brian m. carlson","Ondrej Pohorelsky","Jeff King","Todd Zullinger","Matthew Ogilvie"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"518544","messageId":"pull.1925.git.1747813502225.gitgitgadget@gmail.com","threadId":"63491","inReplyTo":null,"subject":"[PATCH] cvsserver: avoid precedence problem between ! and %s","fromName":"Ondřej Pohořelský via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-21T07:45:02Z","receivedAt":"2025-05-21T07:45:05Z","isPatch":true,"sender":{"key":"name:Ondřej Pohořelský","avatar":null},"body":"From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n\nWith perl-5.41.4 and newer, git-cvsserver fails to build because of\npossible precedence problem[0]\n\nAdded parentheses avoid this issue.\n\nFull credit for finding the issue and coming up with the fix goes to\nJitka Plesnikova (jplesnik@redhat.com)\n\n[0] https://metacpan.org/release/ETHER/perl-5.41.12/view/pod/perl5414delta.pod#New-Warnings\n\nSigned-off-by: Ondřej Pohořelský \n---\n cvsserver: avoid precedence problem between ! and %s\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1925%2Fopohorel%2Fcvsserver_parentheses-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1925/opohorel/cvsserver_parentheses-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1925\n\n git-cvsserver.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex a4e1bad33ca..076c10cb2c2 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -5009,7 +5009,7 @@ sub escapeRefName\n # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n # desired ASCII character byte. (for anything else)\n \n- if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n+ if(! ($refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/))\n {\n $refName=~s/_-/_-u--/g;\n $refName=~s/\\./_-p-/g;\n\nbase-commit: cb96e1697ad6e54d11fc920c95f82977f8e438f8\n-- \ngitgitgadget\n"},{"id":"518545","messageId":"b2abe6c5-042d-4842-9928-39b7fb7b2c0a@app.fastmail.com","threadId":"63491","inReplyTo":"pull.1925.git.1747813502225.gitgitgadget@gmail.com","subject":"Re: [PATCH] cvsserver: avoid precedence problem between ! and %s","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-05-21T07:53:07Z","receivedAt":"2025-05-21T07:53:28Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"Hi\n\nOn Wed, May 21, 2025, at 09:45, Ondřej Pohořelský via GitGitGadget wrote:\n> From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n>\n> With perl-5.41.4 and newer, git-cvsserver fails to build because of\n> possible precedence problem[0]\n>\n> Added parentheses avoid this issue.\n>\n> Full credit for finding the issue and coming up with the fix goes to\n> Jitka Plesnikova (jplesnik@redhat.com)\n\nYou can mention the person in the trailer section above your signoff. For example:\n\n Helped-by: Jitka Plesnikova \n Signed-off-by: Ondřej Pohořelský \n\nOr choose one of the other common ones (from `Documentation/SubmittingPatches`):\n\n If you like, you can put extra trailers at the end:\n\n . `Reported-by:` is used to credit someone who found the bug that\n the patch attempts to fix.\n . `Acked-by:` says that the person who is more familiar with the area\n the patch attempts to modify liked the patch.\n . `Reviewed-by:`, unlike the other trailers, can only be offered by the\n reviewers themselves when they are completely satisfied with the\n patch after a detailed analysis.\n . `Tested-by:` is used to indicate that the person applied the patch\n and found it to have the desired effect.\n . `Co-authored-by:` is used to indicate that people exchanged drafts\n of a patch before submitting it.\n . `Helped-by:` is used to credit someone who suggested ideas for\n changes without providing the precise changes in patch form.\n . `Mentored-by:` is used to credit someone with helping develop a\n patch as part of a mentorship program (e.g., GSoC or Outreachy).\n . `Suggested-by:` is used to credit someone with suggesting the idea\n for a patch.\n\n-- \nKristoffer Haugsbakk\n\n\n"},{"id":"518556","messageId":"pull.1925.v2.git.1747822992457.gitgitgadget@gmail.com","threadId":"63491","inReplyTo":"pull.1925.git.1747813502225.gitgitgadget@gmail.com","subject":"[PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Ondřej Pohořelský via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-21T10:23:12Z","receivedAt":"2025-05-21T10:23:15Z","isPatch":true,"sender":{"key":"name:Ondřej Pohořelský","avatar":null},"body":"From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n\nWith perl-5.41.4 and newer, git-cvsserver fails to build because of\npossible precedence problem[0]\n\nAdded parentheses avoid this issue.\n\n[0] https://metacpan.org/release/ETHER/perl-5.41.12/view/pod/perl5414delta.pod#New-Warnings\n\nReported-by: Jitka Plesnikova \nSuggested-by: Jitka Plesnikova \nSigned-off-by: Ondřej Pohořelský \n---\n cvsserver: avoid precedence problem between ! and %s\n \n cc: \"Kristoffer Haugsbakk\" kristofferhaugsbakk@fastmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1925%2Fopohorel%2Fcvsserver_parentheses-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1925/opohorel/cvsserver_parentheses-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1925\n\nRange-diff vs v1:\n\n 1: 42c03c4b044 ! 1: a15f924657c cvsserver: avoid precedence problem between ! and %s\n @@ Commit message\n \n Added parentheses avoid this issue.\n \n - Full credit for finding the issue and coming up with the fix goes to\n - Jitka Plesnikova (jplesnik@redhat.com)\n -\n [0] https://metacpan.org/release/ETHER/perl-5.41.12/view/pod/perl5414delta.pod#New-Warnings\n \n + Reported-by: Jitka Plesnikova \n + Suggested-by: Jitka Plesnikova \n Signed-off-by: Ondřej Pohořelský \n \n ## git-cvsserver.perl ##\n\n\n git-cvsserver.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex a4e1bad33ca..076c10cb2c2 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -5009,7 +5009,7 @@ sub escapeRefName\n # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n # desired ASCII character byte. (for anything else)\n \n- if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n+ if(! ($refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/))\n {\n $refName=~s/_-/_-u--/g;\n $refName=~s/\\./_-p-/g;\n\nbase-commit: cb96e1697ad6e54d11fc920c95f82977f8e438f8\n-- \ngitgitgadget\n"},{"id":"518583","messageId":"xmqqplg2c8ow.fsf@gitster.g","threadId":"63491","inReplyTo":"pull.1925.git.1747813502225.gitgitgadget@gmail.com","subject":"Re: [PATCH] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T14:58:07Z","receivedAt":"2025-05-21T14:58:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ondřej Pohořelský via GitGitGadget\" \nwrites:\n\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index a4e1bad33ca..076c10cb2c2 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -5009,7 +5009,7 @@ sub escapeRefName\n> # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n> # desired ASCII character byte. (for anything else)\n> \n> - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n> + if(! ($refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/))\n\nInteresting. Shouldn't it be using !~ instead if it wants to assert\nthat the refname does not match the pattern?\n\n\n\n> {\n> $refName=~s/_-/_-u--/g;\n> $refName=~s/\\./_-p-/g;\n>\n> base-commit: cb96e1697ad6e54d11fc920c95f82977f8e438f8\n"},{"id":"518597","messageId":"xmqqldqqarim.fsf@gitster.g","threadId":"63491","inReplyTo":"pull.1925.v2.git.1747822992457.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T15:54:25Z","receivedAt":"2025-05-21T15:54:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ondřej Pohořelský via GitGitGadget\" \nwrites:\n\n> From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n>\n> With perl-5.41.4 and newer, git-cvsserver fails to build because of\n> possible precedence problem[0]\n>\n> Added parentheses avoid this issue.\n>\n> [0] https://metacpan.org/release/ETHER/perl-5.41.12/view/pod/perl5414delta.pod#New-Warnings\n>\n> Reported-by: Jitka Plesnikova \n> Suggested-by: Jitka Plesnikova \n> Signed-off-by: Ondřej Pohořelský \n> ---\n> git-cvsserver.perl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index a4e1bad33ca..076c10cb2c2 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -5009,7 +5009,7 @@ sub escapeRefName\n> # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n> # desired ASCII character byte. (for anything else)\n> \n> - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n> + if(! ($refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/))\n> {\n> $refName=~s/_-/_-u--/g;\n> $refName=~s/\\./_-p-/g;\n\nThanks, will queue.\n\n"},{"id":"518599","messageId":"xmqqh61ear4s.fsf@gitster.g","threadId":"63491","inReplyTo":"pull.1925.v2.git.1747822992457.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T16:02:43Z","receivedAt":"2025-05-21T16:02:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ondřej Pohořelský via GitGitGadget\" \nwrites:\n\n> From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n>\n> With perl-5.41.4 and newer, git-cvsserver fails to build because of\n> possible precedence problem[0]\n\nWhat is the exact symptom? As Perl is not a language to compile and\nrun separately, \"fails to build\" does not look like what exactly is\ngoing on. \"gives a warning and then refuses to run\"? \"gives a warning\nbefore running\"? Something else?\n\n> Added parentheses avoid this issue.\n\nWe phrase such \"this is how the patch addresses the issue\" statement\nin imperative, as if we are telling the codebase to become-like-so,\ne.g., \"Enclose the pattern matching =~ in parentheses to force the\nright order of binding\", or something like that.\n\n"},{"id":"518602","messageId":"xmqq1pshc2vs.fsf@gitster.g","threadId":"63491","inReplyTo":"xmqqh61ear4s.fsf@gitster.g","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T17:03:35Z","receivedAt":"2025-05-21T17:03:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Ondřej Pohořelský via GitGitGadget\" \n> writes:\n>\n>> From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n>>\n>> With perl-5.41.4 and newer, git-cvsserver fails to build because of\n>> possible precedence problem[0]\n>\n> What is the exact symptom? As Perl is not a language to compile and\n> run separately, \"fails to build\" does not look like what exactly is\n> going on. \"gives a warning and then refuses to run\"? \"gives a warning\n> before running\"? Something else?\n\nStepping back a bit, the original that the new warning complains\nabout is this:\n\n- if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n\nAnd the complaint is that due to operator binding precedence, this\ndoes\n\n (!$refname) =~ /pattern/\n\nUnless the behaviour has changed as well as warning, which is highly\nunlikely, doesn't it mean that the code was wrong, with or without\nthe warning, all along? The intent of the code was to see if the\nrefname conformed to dotted decimal, and if it does not, the refname\ngets munged in the block guarded by that if (condition). But the\ncondition was a total nonsense. !$refname would most likely to be\nan empty string (unless $refname contains '0' or an empty string),\nwhich would not match the /^[1-9][0-9]*(\\.[1-9][0-9]*)*$/ pattern,\nso we probably have always munged the $refName in escapeRefName sub.\n\nWhich I do not see used anywhere in the program, though. There is a\ncall to unescapeRefname method, but it seems to me that\nescapeRefName is never called.\n\nWhat made you send a patch for this program? Do you or anybody you\nknow use git-cvsserver? Unless I am reading the program\nincorrectly, despite the claim in front of that escapeRefName sub\nthat we avoid sending a tag whose name is not something CVS would be\nhappy with, we did not sanitize the refs and relied solely on the\nusers' repository to use only safe characters in the refs to keep\nCVS clients happy, and the fact that this expression used as if()\ncondition is totally broken does not really make any difference,\nsince it is in an unused sub. I have to wonder if (1) it is a\nbetter fix to just remove the unused sub, and/or (2) perhaps nobody\nuses cvsserver to allow cvs clients to talk to a Git repository?\n\n>> Added parentheses avoid this issue.\n>\n> We phrase such \"this is how the patch addresses the issue\" statement\n> in imperative, as if we are telling the codebase to become-like-so,\n> e.g., \"Enclose the pattern matching =~ in parentheses to force the\n> right order of binding\", or something like that.\n"},{"id":"518623","messageId":"aC5KRBop9m3K5JtE@tapette.crustytoothpaste.net","threadId":"63491","inReplyTo":"xmqqplg2c8ow.fsf@gitster.g","subject":"Re: [PATCH] cvsserver: avoid precedence problem between ! and %s","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-05-21T21:48:52Z","receivedAt":"2025-05-21T21:49:00Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-05-21 at 14:58:07, Junio C Hamano wrote:\n> \"Ondřej Pohořelský via GitGitGadget\" \n> writes:\n> \n> > diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> > index a4e1bad33ca..076c10cb2c2 100755\n> > --- a/git-cvsserver.perl\n> > +++ b/git-cvsserver.perl\n> > @@ -5009,7 +5009,7 @@ sub escapeRefName\n> > # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n> > # desired ASCII character byte. (for anything else)\n> > \n> > - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n> > + if(! ($refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/))\n> \n> Interesting. Shouldn't it be using !~ instead if it wants to assert\n> that the refname does not match the pattern?\n\nYes, it should. It's likely the reason this is getting a warning is\nthat `!` is higher precedence than `=~` and `!~` (see `man perlop`) and\nswitching to `!~` is the customary way of writing this.\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"518648","messageId":"CA+B51BGLK-3R9ev4a8EwkGHQEBi2QhgxvAd0CHMbphrxPM74eg@mail.gmail.com","threadId":"63491","inReplyTo":"xmqq1pshc2vs.fsf@gitster.g","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Ondrej Pohorelsky","fromEmail":"opohorel@redhat.com","sentAt":"2025-05-22T07:19:18Z","receivedAt":"2025-05-22T07:19:33Z","isPatch":true,"sender":{"key":"opohorel@redhat.com","avatar":"https://avatars.githubusercontent.com/u/35430604?v=4"},"body":"On Wed, May 21, 2025 at 7:03 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > \"Ondřej Pohořelský via GitGitGadget\" \n> > writes:\n> >\n> >> From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n> >>\n> >> With perl-5.41.4 and newer, git-cvsserver fails to build because of\n> >> possible precedence problem[0]\n> >\n> > What is the exact symptom? As Perl is not a language to compile and\n> > run separately, \"fails to build\" does not look like what exactly is\n> > going on. \"gives a warning and then refuses to run\"? \"gives a warning\n> > before running\"? Something else?\n>\n> Stepping back a bit, the original that the new warning complains\n> about is this:\n>\n> - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n>\n> And the complaint is that due to operator binding precedence, this\n> does\n>\n> (!$refname) =~ /pattern/\n>\n> Unless the behaviour has changed as well as warning, which is highly\n> unlikely, doesn't it mean that the code was wrong, with or without\n> the warning, all along? The intent of the code was to see if the\n> refname conformed to dotted decimal, and if it does not, the refname\n> gets munged in the block guarded by that if (condition). But the\n> condition was a total nonsense. !$refname would most likely to be\n> an empty string (unless $refname contains '0' or an empty string),\n> which would not match the /^[1-9][0-9]*(\\.[1-9][0-9]*)*$/ pattern,\n> so we probably have always munged the $refName in escapeRefName sub.\n>\n> Which I do not see used anywhere in the program, though. There is a\n> call to unescapeRefname method, but it seems to me that\n> escapeRefName is never called.\n>\n> What made you send a patch for this program? Do you or anybody you\n> know use git-cvsserver? Unless I am reading the program\n> incorrectly, despite the claim in front of that escapeRefName sub\n> that we avoid sending a tag whose name is not something CVS would be\n> happy with, we did not sanitize the refs and relied solely on the\n> users' repository to use only safe characters in the refs to keep\n> CVS clients happy, and the fact that this expression used as if()\n> condition is totally broken does not really make any difference,\n> since it is in an unused sub. I have to wonder if (1) it is a\n> better fix to just remove the unused sub, and/or (2) perhaps nobody\n> uses cvsserver to allow cvs clients to talk to a Git repository?\n>\n\n\nWhat I meant by 'does not build' is that the warnings that were added\nin the newest Perl release populate the cvs.log when running the test\nsuite.\nThis causes some tests from t9402-git-cvsserver-refs.sh to fail, which\nthen fails the whole build in Fedora.\nTests that are affected are t9402.30, t9402.31, t9402.32, t9402.34.\n\n\n> >> Added parentheses avoid this issue.\n> >\n> > We phrase such \"this is how the patch addresses the issue\" statement\n> > in imperative, as if we are telling the codebase to become-like-so,\n> > e.g., \"Enclose the pattern matching =~ in parentheses to force the\n> > right order of binding\", or something like that.\n>\n\n\nI'll rephrase the commit message to meet this requirement.\n\n\nOn Wed, May 21, 2025 at 7:03 PM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > \"Ondřej Pohořelský via GitGitGadget\" \n> > writes:\n> >\n> >> From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n> >>\n> >> With perl-5.41.4 and newer, git-cvsserver fails to build because of\n> >> possible precedence problem[0]\n> >\n> > What is the exact symptom? As Perl is not a language to compile and\n> > run separately, \"fails to build\" does not look like what exactly is\n> > going on. \"gives a warning and then refuses to run\"? \"gives a warning\n> > before running\"? Something else?\n>\n> Stepping back a bit, the original that the new warning complains\n> about is this:\n>\n> - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n>\n> And the complaint is that due to operator binding precedence, this\n> does\n>\n> (!$refname) =~ /pattern/\n>\n> Unless the behaviour has changed as well as warning, which is highly\n> unlikely, doesn't it mean that the code was wrong, with or without\n> the warning, all along? The intent of the code was to see if the\n> refname conformed to dotted decimal, and if it does not, the refname\n> gets munged in the block guarded by that if (condition). But the\n> condition was a total nonsense. !$refname would most likely to be\n> an empty string (unless $refname contains '0' or an empty string),\n> which would not match the /^[1-9][0-9]*(\\.[1-9][0-9]*)*$/ pattern,\n> so we probably have always munged the $refName in escapeRefName sub.\n>\n> Which I do not see used anywhere in the program, though. There is a\n> call to unescapeRefname method, but it seems to me that\n> escapeRefName is never called.\n>\n> What made you send a patch for this program? Do you or anybody you\n> know use git-cvsserver? Unless I am reading the program\n> incorrectly, despite the claim in front of that escapeRefName sub\n> that we avoid sending a tag whose name is not something CVS would be\n> happy with, we did not sanitize the refs and relied solely on the\n> users' repository to use only safe characters in the refs to keep\n> CVS clients happy, and the fact that this expression used as if()\n> condition is totally broken does not really make any difference,\n> since it is in an unused sub. I have to wonder if (1) it is a\n> better fix to just remove the unused sub, and/or (2) perhaps nobody\n> uses cvsserver to allow cvs clients to talk to a Git repository?\n>\n> >> Added parentheses avoid this issue.\n> >\n> > We phrase such \"this is how the patch addresses the issue\" statement\n> > in imperative, as if we are telling the codebase to become-like-so,\n> > e.g., \"Enclose the pattern matching =~ in parentheses to force the\n> > right order of binding\", or something like that.\n>\n\n\n-- \n\nOndřej Pohořelský\n\nSoftware Engineer\n\nRed Hat\n\nopohorel@redhat.com\n\n"},{"id":"518649","messageId":"CA+B51BEqMPPmbaiXCFKXoPvufS7NnT-wxJCW5VgmtP05xUbrcw@mail.gmail.com","threadId":"63491","inReplyTo":"aC5KRBop9m3K5JtE@tapette.crustytoothpaste.net","subject":"Re: [PATCH] cvsserver: avoid precedence problem between ! and %s","fromName":"Ondrej Pohorelsky","fromEmail":"opohorel@redhat.com","sentAt":"2025-05-22T07:31:20Z","receivedAt":"2025-05-22T07:31:36Z","isPatch":true,"sender":{"key":"opohorel@redhat.com","avatar":"https://avatars.githubusercontent.com/u/35430604?v=4"},"body":"Looking at the code, we were not exactly sure how the code should\nwork, so we picked the solution with the least impact that suppresses\nthe warning and doesn't break anything.\nI'll change the commit to use `!~` instead.\n\n\nOn Wed, May 21, 2025 at 11:49 PM brian m. carlson\n wrote:\n>\n> On 2025-05-21 at 14:58:07, Junio C Hamano wrote:\n> > \"Ondřej Pohořelský via GitGitGadget\" \n> > writes:\n> >\n> > > diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> > > index a4e1bad33ca..076c10cb2c2 100755\n> > > --- a/git-cvsserver.perl\n> > > +++ b/git-cvsserver.perl\n> > > @@ -5009,7 +5009,7 @@ sub escapeRefName\n> > > # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n> > > # desired ASCII character byte. (for anything else)\n> > >\n> > > - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n> > > + if(! ($refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/))\n> >\n> > Interesting. Shouldn't it be using !~ instead if it wants to assert\n> > that the refname does not match the pattern?\n>\n> Yes, it should. It's likely the reason this is getting a warning is\n> that `!` is higher precedence than `=~` and `!~` (see `man perlop`) and\n> switching to `!~` is the customary way of writing this.\n> --\n> brian m. carlson (they/them)\n> Toronto, Ontario, CA\n\n\n\n-- \n\nOndřej Pohořelský\n\nSoftware Engineer\n\nRed Hat\n\nopohorel@redhat.com\n\n"},{"id":"518656","messageId":"pull.1925.v3.git.1747913206622.gitgitgadget@gmail.com","threadId":"63491","inReplyTo":"pull.1925.v2.git.1747822992457.gitgitgadget@gmail.com","subject":"[PATCH v3] cvsserver: avoid precedence problem between ! and %s","fromName":"Ondřej Pohořelský via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-22T11:26:46Z","receivedAt":"2025-05-22T11:26:49Z","isPatch":true,"sender":{"key":"name:Ondřej Pohořelský","avatar":null},"body":"From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n\nWith perl-5.41.4 and newer, test t9402-git-cvsserver-refs.sh\n(specifically t9402.30, t9402.31, t9402.32, t9402.34) fails, because\nof the new warnings[0] populating cvs.log.\n\nUse the 'does not match' operator '!~' directly to express the\nnegated pattern match, resolving the precedence issue.\n\n[0] https://metacpan.org/release/ETHER/perl-5.41.12/view/pod/perl5414delta.pod#New-Warnings\n\nReported-by: Jitka Plesnikova \nSuggested-by: Jitka Plesnikova \nSigned-off-by: Ondřej Pohořelský \n---\n cvsserver: avoid precedence problem between ! and %s\n \n cc: \"Kristoffer Haugsbakk\" kristofferhaugsbakk@fastmail.com cc: \"brian\n m. carlson\" sandals@crustytoothpaste.net\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1925%2Fopohorel%2Fcvsserver_parentheses-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1925/opohorel/cvsserver_parentheses-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1925\n\nRange-diff vs v2:\n\n 1: a15f924657c ! 1: b7563182492 cvsserver: avoid precedence problem between ! and %s\n @@ Metadata\n ## Commit message ##\n cvsserver: avoid precedence problem between ! and %s\n \n - With perl-5.41.4 and newer, git-cvsserver fails to build because of\n - possible precedence problem[0]\n + With perl-5.41.4 and newer, test t9402-git-cvsserver-refs.sh\n + (specifically t9402.30, t9402.31, t9402.32, t9402.34) fails, because\n + of the new warnings[0] populating cvs.log.\n \n - Added parentheses avoid this issue.\n + Use the 'does not match' operator '!~' directly to express the\n + negated pattern match, resolving the precedence issue.\n \n [0] https://metacpan.org/release/ETHER/perl-5.41.12/view/pod/perl5414delta.pod#New-Warnings\n \n @@ git-cvsserver.perl: sub escapeRefName\n # desired ASCII character byte. (for anything else)\n \n - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n -+ if(! ($refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/))\n ++ if ($refName !~ /^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n {\n $refName=~s/_-/_-u--/g;\n $refName=~s/\\./_-p-/g;\n\n\n git-cvsserver.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex a4e1bad33ca..7ccd720019b 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -5009,7 +5009,7 @@ sub escapeRefName\n # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n # desired ASCII character byte. (for anything else)\n \n- if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n+ if ($refName !~ /^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n {\n $refName=~s/_-/_-u--/g;\n $refName=~s/\\./_-p-/g;\n\nbase-commit: cb96e1697ad6e54d11fc920c95f82977f8e438f8\n-- \ngitgitgadget\n"},{"id":"518668","messageId":"xmqq7c287i7n.fsf@gitster.g","threadId":"63491","inReplyTo":"CA+B51BGLK-3R9ev4a8EwkGHQEBi2QhgxvAd0CHMbphrxPM74eg@mail.gmail.com","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-22T15:55:56Z","receivedAt":"2025-05-22T15:55:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ondrej Pohorelsky writes:\n\n>> What made you send a patch for this program? Do you or anybody you\n>> know use git-cvsserver? Unless I am reading the program\n>> incorrectly, despite the claim in front of that escapeRefName sub\n>> that we avoid sending a tag whose name is not something CVS would be\n>> happy with, we did not sanitize the refs and relied solely on the\n>> users' repository to use only safe characters in the refs to keep\n>> CVS clients happy, and the fact that this expression used as if()\n>> condition is totally broken does not really make any difference,\n>> since it is in an unused sub. I have to wonder if (1) it is a\n>> better fix to just remove the unused sub, and/or (2) perhaps nobody\n>> uses cvsserver to allow cvs clients to talk to a Git repository?\n\nBelow you mention you found it from test failures. Nice to know\nthat you weren't actually using it ;-)\n\nStill, I would welcome second and third set of eyeballs to see if\nthis is a dead code that the \"compiler\" is complaining about. If\nso, we can remove that unused code instead of fixing it.\n\n> What I meant by 'does not build' is that the warnings that were added\n> in the newest Perl release populate the cvs.log when running the test\n> suite.\n> This causes some tests from t9402-git-cvsserver-refs.sh to fail, which\n> then fails the whole build in Fedora.\n> Tests that are affected are t9402.30, t9402.31, t9402.32, t9402.34.\n>\n>\n>> >> Added parentheses avoid this issue.\n>> >\n>> > We phrase such \"this is how the patch addresses the issue\" statement\n>> > in imperative, as if we are telling the codebase to become-like-so,\n>> > e.g., \"Enclose the pattern matching =~ in parentheses to force the\n>> > right order of binding\", or something like that.\n>\n> I'll rephrase the commit message to meet this requirement.\n\nAlso please update the earlier part of the log message to clarify\nwhat the end-user observable symptoms are (e.g. \"gives warnings\nbefore doing its thing? or errors out and does not run? or something\nelse?\").\n\nThanks.\n"},{"id":"518673","messageId":"20250522170536.GB1613@coredump.intra.peff.net","threadId":"63491","inReplyTo":"xmqq7c287i7n.fsf@gitster.g","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-22T17:05:36Z","receivedAt":"2025-05-22T17:05:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 22, 2025 at 08:55:56AM -0700, Junio C Hamano wrote:\n\n> Ondrej Pohorelsky writes:\n> \n> >> What made you send a patch for this program? Do you or anybody you\n> >> know use git-cvsserver? Unless I am reading the program\n> >> incorrectly, despite the claim in front of that escapeRefName sub\n> >> that we avoid sending a tag whose name is not something CVS would be\n> >> happy with, we did not sanitize the refs and relied solely on the\n> >> users' repository to use only safe characters in the refs to keep\n> >> CVS clients happy, and the fact that this expression used as if()\n> >> condition is totally broken does not really make any difference,\n> >> since it is in an unused sub. I have to wonder if (1) it is a\n> >> better fix to just remove the unused sub, and/or (2) perhaps nobody\n> >> uses cvsserver to allow cvs clients to talk to a Git repository?\n> \n> Below you mention you found it from test failures. Nice to know\n> that you weren't actually using it ;-)\n> \n> Still, I would welcome second and third set of eyeballs to see if\n> this is a dead code that the \"compiler\" is complaining about. If\n> so, we can remove that unused code instead of fixing it.\n\nI agree that the code does not appear to be called, and doing this:\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex a4e1bad33c..cc891eba67 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -5009,6 +5009,7 @@ sub escapeRefName\n # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n # desired ASCII character byte. (for anything else)\n \n+ die \"foo\";\n if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n {\n $refName=~s/_-/_-u--/g;\n\nstill lets t9402 pass. I suspect the issue is that perl complains to\nstderr while parsing the file (polluting the log), not when actually\nrunning the code.\n\n-Peff\n"},{"id":"518679","messageId":"aC9lM12GyntAp2tR@teonanacatl.net","threadId":"63491","inReplyTo":"20250522170536.GB1613@coredump.intra.peff.net","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2025-05-22T17:56:03Z","receivedAt":"2025-05-22T17:56:06Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> On Thu, May 22, 2025 at 08:55:56AM -0700, Junio C Hamano wrote:\n> \n>> Ondrej Pohorelsky writes:\n>> \n>>>> What made you send a patch for this program? Do you or anybody you\n>>>> know use git-cvsserver? Unless I am reading the program\n>>>> incorrectly, despite the claim in front of that escapeRefName sub\n>>>> that we avoid sending a tag whose name is not something CVS would be\n>>>> happy with, we did not sanitize the refs and relied solely on the\n>>>> users' repository to use only safe characters in the refs to keep\n>>>> CVS clients happy, and the fact that this expression used as if()\n>>>> condition is totally broken does not really make any difference,\n>>>> since it is in an unused sub. I have to wonder if (1) it is a\n>>>> better fix to just remove the unused sub, and/or (2) perhaps nobody\n>>>> uses cvsserver to allow cvs clients to talk to a Git repository?\n>> \n>> Below you mention you found it from test failures. Nice to know\n>> that you weren't actually using it ;-)\n>> \n>> Still, I would welcome second and third set of eyeballs to see if\n>> this is a dead code that the \"compiler\" is complaining about. If\n>> so, we can remove that unused code instead of fixing it.\n> \n> I agree that the code does not appear to be called, and doing this:\n> \n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index a4e1bad33c..cc891eba67 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -5009,6 +5009,7 @@ sub escapeRefName\n> # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n> # desired ASCII character byte. (for anything else)\n> \n> + die \"foo\";\n> if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n> {\n> $refName=~s/_-/_-u--/g;\n> \n> still lets t9402 pass. I suspect the issue is that perl complains to\n> stderr while parsing the file (polluting the log), not when actually\n> running the code.\n\nJust for curiosity, the only commit found with escapeRefName\nis when it was added:\n\n $ git log -G '\\bescapeRefName\\b' -- git-cvsserver.perl\n commit 51a7e6dbc9\n Author: Matthew Ogilvie \n Date: Sat Oct 13 23:42:26 2012 -0600\n\n\tcvsserver: define a tag name character escape mechanism\n\t\n\tCVS tags are officially only allowed to use [-_0-9A-Za-f]. Git\n\trefs commonly uses other characters, especially [./]. Such characters\n\tneed to be escaped from CVS in order to be referenced.\n\t\n\tThis just defines functions to escape/unescape names. The functions\n\tare not used yet.\n\t\n\tSigned-off-by: Matthew Ogilvie \n\tSigned-off-by: Junio C Hamano \n\nA subsequent commit, 658b57ad52 (cvsserver: add misc commit\nlookup, file meta data, and file listing functions,\n2012-10-13), made use of unescapeRefName; escapeRefName\nseems to have _never_ been used.\n\n-- \nTodd\n"},{"id":"518691","messageId":"xmqqtt5c5viq.fsf@gitster.g","threadId":"63491","inReplyTo":"aC9lM12GyntAp2tR@teonanacatl.net","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-22T18:51:25Z","receivedAt":"2025-05-22T18:51:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger writes:\n\n> Just for curiosity, the only commit found with escapeRefName\n> is when it was added:\n>\n> $ git log -G '\\bescapeRefName\\b' -- git-cvsserver.perl\n> commit 51a7e6dbc9\n> Author: Matthew Ogilvie \n> Date: Sat Oct 13 23:42:26 2012 -0600\n>\n> \tcvsserver: define a tag name character escape mechanism\n> \t\n> \tCVS tags are officially only allowed to use [-_0-9A-Za-f]. Git\n> \trefs commonly uses other characters, especially [./]. Such characters\n> \tneed to be escaped from CVS in order to be referenced.\n> \t\n> \tThis just defines functions to escape/unescape names. The functions\n> \tare not used yet.\n> \t\n> \tSigned-off-by: Matthew Ogilvie \n> \tSigned-off-by: Junio C Hamano \n>\n> A subsequent commit, 658b57ad52 (cvsserver: add misc commit\n> lookup, file meta data, and file listing functions,\n> 2012-10-13), made use of unescapeRefName; escapeRefName\n> seems to have _never_ been used.\n\nOK, so we can safely remove it, it seems ;-) I wonder what, if any,\nthe unescaping side is unescaping, if we are not doing the escaping.\n\nThanks for digging.\n"},{"id":"518749","messageId":"aC_90R3ohRRBVIV7@comcast.net","threadId":"63491","inReplyTo":"xmqqtt5c5viq.fsf@gitster.g","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Matthew Ogilvie","fromEmail":"mmogilvi+git@zoho.com","sentAt":"2025-05-23T04:47:13Z","receivedAt":"2025-05-23T04:47:27Z","isPatch":true,"sender":{"key":"mmogilvi+git@zoho.com","avatar":null},"body":"On Thu, May 22, 2025 at 11:51:25AM -0700, Junio C Hamano wrote:\n> Todd Zullinger writes:\n> \n> > Just for curiosity, the only commit found with escapeRefName\n> > is when it was added:\n> >\n> > $ git log -G '\\bescapeRefName\\b' -- git-cvsserver.perl\n> > commit 51a7e6dbc9\n> > Author: Matthew Ogilvie \n> > Date: Sat Oct 13 23:42:26 2012 -0600\n> >\n> > \tcvsserver: define a tag name character escape mechanism\n> > \t\n> > \tCVS tags are officially only allowed to use [-_0-9A-Za-f]. Git\n> > \trefs commonly uses other characters, especially [./]. Such characters\n> > \tneed to be escaped from CVS in order to be referenced.\n> > \t\n> > \tThis just defines functions to escape/unescape names. The functions\n> > \tare not used yet.\n> > \t\n> > \tSigned-off-by: Matthew Ogilvie \n> > \tSigned-off-by: Junio C Hamano \n> >\n> > A subsequent commit, 658b57ad52 (cvsserver: add misc commit\n> > lookup, file meta data, and file listing functions,\n> > 2012-10-13), made use of unescapeRefName; escapeRefName\n> > seems to have _never_ been used.\n> \n> OK, so we can safely remove it, it seems ;-) I wonder what, if any,\n> the unescaping side is unescaping, if we are not doing the escaping.\n> \n> Thanks for digging.\n\nFYI:\n\nOne intent is that the user might do the escaping manually, if\nthey need to refer to a git refspec that is not legal in CVS.\nFor example, \"cvs update -r pu_-s-mo_-s-experiment1\" instead of\n\"cvs update -r pu/mo/experiment1\". To some extent the function\ncould be considered a form of documentation of how you would do\nthis manually.\n\nAlso, the fact escapeRefName() isn't called suggests that there\nmight be other bugs. There is a test case in t9402 that\ntests arguments \"-r heads/b1\" with a comment that \"This is not\nreally legal CVS, but it seems to work anyway\". I haven't fully\ntracked it down, but I suspect that might end up putting a\nliteral \"heads/b1\" in the CVS sandbox's \"CVS/Entries\" file. If so,\nthat is invalid, because Entries uses slash for its own field\nseparator. If we added more tests immediately after it\n*without* a different \"-r\" (which is very high priority\nwhen resolving which version to update to), they would likely fail.\nIt might make sense to put an escapeRefName(unescapeRefName()) nested\ncall somewhere to protect against things like this test case...\n\nHowever, despite writing and (incompletely) testing this code, I\nhave never *really* used it, and probably never will. So I'm not\nin a hurry to try to test or fix it further...\n\n(For that matter, has anyone ever heard of anyone actually using\ngit-cvsserver at all? I think I would be surprised if there was anyone\nusing it, especially so many years after CVS stopped being maintained\nat all.)\n\n - Matthew Ogilvie\n"},{"id":"518774","messageId":"xmqqwma7z5th.fsf@gitster.g","threadId":"63491","inReplyTo":"aC_90R3ohRRBVIV7@comcast.net","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-23T15:48:26Z","receivedAt":"2025-05-23T15:48:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthew Ogilvie writes:\n\n> However, despite writing and (incompletely) testing this code, I\n> have never *really* used it, and probably never will. So I'm not\n> in a hurry to try to test or fix it further...\n>\n> (For that matter, has anyone ever heard of anyone actually using\n> git-cvsserver at all? I think I would be surprised if there was anyone\n> using it, especially so many years after CVS stopped being maintained\n> at all.)\n\n;-)\n"},{"id":"518929","messageId":"pull.1925.v4.git.1748267305871.gitgitgadget@gmail.com","threadId":"63491","inReplyTo":"pull.1925.v3.git.1747913206622.gitgitgadget@gmail.com","subject":"[PATCH v4] cvsserver: remove unused escapeRefName function","fromName":"Ondřej Pohořelský via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-05-26T13:48:25Z","receivedAt":"2025-05-26T13:48:29Z","isPatch":true,"sender":{"key":"name:Ondřej Pohořelský","avatar":null},"body":"From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= \n\nFunction 'escapeRefName' introduced in 51a7e6dbc9 has never been used.\n\nDespite being dead code, changes in Perl 5.41.4 exposed precedence\nwarning withing its logic, which then caused test failures in t9402 by\nlogging the warnings to stderr while parsing the code. The affected\ntests are t9402.30, t9402.31, t9402.32 and t9402.34.\n\nRemove this unused function to simplify the codebase and stop the\nwarnings and test failures. Its corresponding unescapeRefName function,\nwhich remains in use, has had its comments updated.\n\nReported-by: Jitka Plesnikova \nSigned-off-by: Ondřej Pohořelský \n---\n cvsserver: avoid precedence problem between ! and %s\n \n cc: \"Kristoffer Haugsbakk\" kristofferhaugsbakk@fastmail.com cc: \"brian\n m. carlson\" sandals@crustytoothpaste.net cc: Jeff King peff@peff.net cc:\n Todd Zullinger tmz@pobox.com cc: Matthew Ogilvie mmogilvi+git@zoho.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1925%2Fopohorel%2Fcvsserver_parentheses-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1925/opohorel/cvsserver_parentheses-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1925\n\nRange-diff vs v3:\n\n 1: b7563182492 ! 1: ce853594ceb cvsserver: avoid precedence problem between ! and %s\n @@ Metadata\n Author: Ondřej Pohořelský \n \n ## Commit message ##\n - cvsserver: avoid precedence problem between ! and %s\n + cvsserver: remove unused escapeRefName function\n \n - With perl-5.41.4 and newer, test t9402-git-cvsserver-refs.sh\n - (specifically t9402.30, t9402.31, t9402.32, t9402.34) fails, because\n - of the new warnings[0] populating cvs.log.\n + Function 'escapeRefName' introduced in 51a7e6dbc9 has never been used.\n \n - Use the 'does not match' operator '!~' directly to express the\n - negated pattern match, resolving the precedence issue.\n + Despite being dead code, changes in Perl 5.41.4 exposed precedence\n + warning withing its logic, which then caused test failures in t9402 by\n + logging the warnings to stderr while parsing the code. The affected\n + tests are t9402.30, t9402.31, t9402.32 and t9402.34.\n \n - [0] https://metacpan.org/release/ETHER/perl-5.41.12/view/pod/perl5414delta.pod#New-Warnings\n + Remove this unused function to simplify the codebase and stop the\n + warnings and test failures. Its corresponding unescapeRefName function,\n + which remains in use, has had its comments updated.\n \n Reported-by: Jitka Plesnikova \n - Suggested-by: Jitka Plesnikova \n Signed-off-by: Ondřej Pohořelský \n \n ## git-cvsserver.perl ##\n +@@ git-cvsserver.perl: sub gethistorydense\n + return $result;\n + }\n + \n +-=head2 escapeRefName\n ++=head2 unescapeRefName\n + \n +-Apply an escape mechanism to compensate for characters that\n ++Undo an escape mechanism to compensate for characters that\n + git ref names can have that CVS tags can not.\n + \n + =cut\n +-sub escapeRefName\n ++sub unescapeRefName\n + {\n + my($self,$refName)=@_;\n + \n @@ git-cvsserver.perl: sub escapeRefName\n # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n # desired ASCII character byte. (for anything else)\n \n - if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n -+ if ($refName !~ /^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n - {\n - $refName=~s/_-/_-u--/g;\n - $refName=~s/\\./_-p-/g;\n +- {\n +- $refName=~s/_-/_-u--/g;\n +- $refName=~s/\\./_-p-/g;\n +- $refName=~s%/%_-s-%g;\n +- $refName=~s/[^-_a-zA-Z0-9]/sprintf(\"_-%02x-\",$1)/eg;\n +- }\n +-}\n +-\n +-=head2 unescapeRefName\n +-\n +-Undo an escape mechanism to compensate for characters that\n +-git ref names can have that CVS tags can not.\n +-\n +-=cut\n +-sub unescapeRefName\n +-{\n +- my($self,$refName)=@_;\n +-\n +- # see escapeRefName() for description of escape mechanism.\n +-\n + $refName=~s/_-([spu]|[0-9a-f][0-9a-f])-/unescapeRefNameChar($1)/eg;\n + \n + # allowed tag names\n\n\n git-cvsserver.perl | 27 +++------------------------\n 1 file changed, 3 insertions(+), 24 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex a4e1bad33ca..d8d5422cbca 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -4986,13 +4986,13 @@ sub gethistorydense\n return $result;\n }\n \n-=head2 escapeRefName\n+=head2 unescapeRefName\n \n-Apply an escape mechanism to compensate for characters that\n+Undo an escape mechanism to compensate for characters that\n git ref names can have that CVS tags can not.\n \n =cut\n-sub escapeRefName\n+sub unescapeRefName\n {\n my($self,$refName)=@_;\n \n@@ -5009,27 +5009,6 @@ sub escapeRefName\n # = \"_-xx-\" Where \"xx\" is the hexadecimal representation of the\n # desired ASCII character byte. (for anything else)\n \n- if(! $refName=~/^[1-9][0-9]*(\\.[1-9][0-9]*)*$/)\n- {\n- $refName=~s/_-/_-u--/g;\n- $refName=~s/\\./_-p-/g;\n- $refName=~s%/%_-s-%g;\n- $refName=~s/[^-_a-zA-Z0-9]/sprintf(\"_-%02x-\",$1)/eg;\n- }\n-}\n-\n-=head2 unescapeRefName\n-\n-Undo an escape mechanism to compensate for characters that\n-git ref names can have that CVS tags can not.\n-\n-=cut\n-sub unescapeRefName\n-{\n- my($self,$refName)=@_;\n-\n- # see escapeRefName() for description of escape mechanism.\n-\n $refName=~s/_-([spu]|[0-9a-f][0-9a-f])-/unescapeRefNameChar($1)/eg;\n \n # allowed tag names\n\nbase-commit: cb96e1697ad6e54d11fc920c95f82977f8e438f8\n-- \ngitgitgadget\n"},{"id":"518933","messageId":"CA+B51BFJ9abjP5pDYwV1-mHpwg_n-jjz4_YX+nm9wOYF4nKuGQ@mail.gmail.com","threadId":"63491","inReplyTo":"xmqqwma7z5th.fsf@gitster.g","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Ondrej Pohorelsky","fromEmail":"opohorel@redhat.com","sentAt":"2025-05-26T13:56:13Z","receivedAt":"2025-05-26T13:56:28Z","isPatch":true,"sender":{"key":"opohorel@redhat.com","avatar":"https://avatars.githubusercontent.com/u/35430604?v=4"},"body":"I've just submitted v4, which removes the 'escapeRefName' function, so\nwe avoid the warnings and test failures when we build with new Perl\nreleases.\nI think the next step would be to remove whole git-cvsserver as was\nsaid earlier. I'll take a look what it is going to take and submit a\npatch with the removal later, if that's ok\n\n\nOn Fri, May 23, 2025 at 5:48 PM Junio C Hamano wrote:\n>\n> Matthew Ogilvie writes:\n>\n> > However, despite writing and (incompletely) testing this code, I\n> > have never *really* used it, and probably never will. So I'm not\n> > in a hurry to try to test or fix it further...\n> >\n> > (For that matter, has anyone ever heard of anyone actually using\n> > git-cvsserver at all? I think I would be surprised if there was anyone\n> > using it, especially so many years after CVS stopped being maintained\n> > at all.)\n>\n> ;-)\n>\n\n\n-- \n\nOndřej Pohořelský\n\nSoftware Engineer\n\nRed Hat\n\nopohorel@redhat.com\n\n"},{"id":"518995","messageId":"xmqqv7pmqdo4.fsf@gitster.g","threadId":"63491","inReplyTo":"pull.1925.v4.git.1748267305871.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] cvsserver: remove unused escapeRefName function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-27T15:24:59Z","receivedAt":"2025-05-27T15:25:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ondřej Pohořelský via GitGitGadget\" \nwrites:\n\n> Function 'escapeRefName' introduced in 51a7e6dbc9 has never been used.\n>\n> Despite being dead code, changes in Perl 5.41.4 exposed precedence\n> warning withing its logic, which then caused test failures in t9402 by\n> logging the warnings to stderr while parsing the code. The affected\n> tests are t9402.30, t9402.31, t9402.32 and t9402.34.\n>\n> Remove this unused function to simplify the codebase and stop the\n> warnings and test failures. Its corresponding unescapeRefName function,\n> which remains in use, has had its comments updated.\n\n\nVery clearly explained and looking good. Thanks.\n\nWill queue with \"withing\" -> \"within\".\n\n"},{"id":"518998","messageId":"xmqq7c22qcdp.fsf@gitster.g","threadId":"63491","inReplyTo":"CA+B51BFJ9abjP5pDYwV1-mHpwg_n-jjz4_YX+nm9wOYF4nKuGQ@mail.gmail.com","subject":"Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-27T15:52:50Z","receivedAt":"2025-05-27T15:52:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ondrej Pohorelsky writes:\n\n> I've just submitted v4, which removes the 'escapeRefName' function, so\n> we avoid the warnings and test failures when we build with new Perl\n> releases.\n\nGreat, thanks.\n\n> I think the next step would be to remove whole git-cvsserver as was\n> said earlier. I'll take a look what it is going to take and submit a\n> patch with the removal later, if that's ok\n\nIt probably needs to follow the pattern established for all the\nother recent topics that touched Documentation/BreakingChanges file.\n\n"}]}