{"thread":{"id":"59988","subject":"[PATCH] t2400: Fix test failures when using grep 2.5","startedAt":"2023-07-15T02:55:45Z","lastAt":"2023-07-28T13:09:28Z","messageCount":25,"participants":["Jacob Abel","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479539","messageId":"20230715025512.7574-1-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":null,"subject":"[PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-15T02:55:21Z","receivedAt":"2023-07-15T02:55:45Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Replace all cases of `\\s` with `[[:space:]]` as older versions of GNU\ngrep (and from what it seems most versions of BSD grep) do not handle\n`\\s`.\n\nFor the same reason all cases of `\\S` are replaced with `[^[:space:]]`.\nReplacing `\\S` also needs to occur as `\\S` is technically PCRE and not\npart of ERE even though most modern versions of grep accept it as ERE.\n\nSigned-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\nThis patch is in response to build failures on GGG's Cirrus CI \nfreebsd_12 build jobs[1] and was prompted by a discussion thread [2].\n\nThese failures seem to be caused by the behavior outlined in [3]. \nWeirdly however they only seem to occur on the FreeBSD CI but not the \nMac OS CI for some reason despite Mac OS using FreeBSD grep.\n\n1. https://github.com/gitgitgadget/git/pull/1550/checks?check_run_id=14949695859\n2. https://lore.kernel.org/git/CALnO6CDryTsguLshcQxx97ZxyY42Twu2hC2y1bLOsS-9zbqXMA@mail.gmail.com/\n3. https://stackoverflow.com/questions/4233159/grep-regex-whitespace-behavior\n\n t/t2400-worktree-add.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 0ac468e69e..7f19bdabff 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -417,9 +417,9 @@ test_wt_add_orphan_hint () {\n \t\tgrep \"hint: If you meant to create a worktree containing a new orphan branch\" actual &&\n \t\tif [ $use_branch -eq 1 ]\n \t\tthen\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan -b [^[:space:]]+ [^[:space:]]+[[:space:]]*$\" actual\n \t\telse\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan [^[:space:]]+[[:space:]]*$\" actual\n \t\tfi\n \n \t'\n@@ -709,7 +709,7 @@ test_dwim_orphan () {\n \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n-\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n+\tlocal invalid_ref_regex=\"^fatal: invalid reference:[[:space:]]\\+.*\" &&\n \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n \n \tlocal git_ns=\"repo\" &&\n@@ -998,8 +998,8 @@ test_dwim_orphan () {\n \t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n-\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n-\t\t\t\t\tgrep \"HEAD contents:\\s*.$headcontents.\" actual &&\n+\t\t\t\t\tgrep \"HEAD path:[[:space:]]*.$headpath.\" actual &&\n+\t\t\t\t\tgrep \"HEAD contents:[[:space:]]*.$headcontents.\" actual &&\n \t\t\t\t\tgrep \"$orphan_hint\" actual &&\n \t\t\t\t\t! grep \"$info_text\" actual\n \t\t\t\tfi &&\n\nbase-commit: 830b4a04c45bf0a6db26defe02ed1f490acd18ee\n-- \n2.39.3\n\n\n"},{"id":"479543","messageId":"2e22a23f-576f-7a42-ace8-624a5362d9f4@gmail.com","threadId":"59988","inReplyTo":"20230715025512.7574-1-jacobabel@nullpo.dev","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-07-15T08:59:23Z","receivedAt":"2023-07-15T08:59:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Jocab\n\nOn 15/07/2023 03:55, Jacob Abel wrote:\n> Replace all cases of `\\s` with `[[:space:]]` as older versions of GNU\n> grep (and from what it seems most versions of BSD grep) do not handle\n> `\\s`.\n>\n> For the same reason all cases of `\\S` are replaced with `[^[:space:]]`.\n> Replacing `\\S` also needs to occur as `\\S` is technically PCRE and not\n> part of ERE even though most modern versions of grep accept it as ERE.\n\nThanks for working on this fix. Having looked at the changes I think it \nwould be better just be using a space character in a lot of these \nexpressions - see below.\n\n> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n> ---\n> This patch is in response to build failures on GGG's Cirrus CI\n> freebsd_12 build jobs[1] and was prompted by a discussion thread [2].\n> \n> These failures seem to be caused by the behavior outlined in [3].\n> Weirdly however they only seem to occur on the FreeBSD CI but not the\n> Mac OS CI for some reason despite Mac OS using FreeBSD grep.\n> \n> 1. https://github.com/gitgitgadget/git/pull/1550/checks?check_run_id=14949695859\n> 2. https://lore.kernel.org/git/CALnO6CDryTsguLshcQxx97ZxyY42Twu2hC2y1bLOsS-9zbqXMA@mail.gmail.com/\n> 3. https://stackoverflow.com/questions/4233159/grep-regex-whitespace-behavior\n> \n>   t/t2400-worktree-add.sh | 10 +++++-----\n>   1 file changed, 5 insertions(+), 5 deletions(-)\n> \n> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\n> index 0ac468e69e..7f19bdabff 100755\n> --- a/t/t2400-worktree-add.sh\n> +++ b/t/t2400-worktree-add.sh\n> @@ -417,9 +417,9 @@ test_wt_add_orphan_hint () {\n>   \t\tgrep \"hint: If you meant to create a worktree containing a new orphan branch\" actual &&\n>   \t\tif [ $use_branch -eq 1 ]\n>   \t\tthen\n> -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n> +\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan -b [^[:space:]]+ [^[:space:]]+[[:space:]]*$\" actual\n\nWe know that \"hint:\" is followed by a single space and all we're really \ninterested in is that we print something after the \"-b \" so we can \nsimplify this to\n\n\tgrep \"^hint: git worktree add --orphan -b [^ ]\"\n\nI think the same applies to most of the other expressions changed in \nthis patch.\n\n>   \t\telse\n> -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n> +\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan [^[:space:]]+[[:space:]]*$\" actual\n>   \t\tfi\n>   \n>   \t'\n> @@ -709,7 +709,7 @@ test_dwim_orphan () {\n>   \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n>   \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n>   \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n> -\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n> +\tlocal invalid_ref_regex=\"^fatal: invalid reference:[[:space:]]\\+.*\" &&\n>   \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n>   \n>   \tlocal git_ns=\"repo\" &&\n> @@ -998,8 +998,8 @@ test_dwim_orphan () {\n>   \t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n\nI'm a bit confused by the --sq here - why does it need to be shell \nquoted when it is always used inside double quotes? Also when the \nreftable backend is used I'm not sure that HEAD is actually a file in \n$GIT_DIR anymore (that's less of an issue at the moment as that backend \nis not is use yet).\n\n>   \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n>   \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n> -\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n> -\t\t\t\t\tgrep \"HEAD contents:\\s*.$headcontents.\" actual &&\n> +\t\t\t\t\tgrep \"HEAD path:[[:space:]]*.$headpath.\" actual &&\n> +\t\t\t\t\tgrep \"HEAD contents:[[:space:]]*.$headcontents.\" actual &&\n\nUsing grep like this makes it harder to debug test failures as one has \nto run the test with \"-x\" in order to try and figure out which grep \nactually failed. I think here we can replace the sequence of \"grep\"s \nwith \"test_cmp\"\n\n\tcat >expect <<-EOF &&\n\tHEAD points to an invalid (or orphaned) reference\n\tHEAD path: $headpath\n\tHEAD contents: $headcontents\n\tEOF\n\n\ttest_cmp expect actual\n\nBest Wishes\n\nPhillip\n\n>   \t\t\t\t\tgrep \"$orphan_hint\" actual &&\n>   \t\t\t\t\t! grep \"$info_text\" actual\n>   \t\t\t\tfi &&\n> \n> base-commit: 830b4a04c45bf0a6db26defe02ed1f490acd18ee\n"},{"id":"479548","messageId":"vn5sylull5lqpitsanlyan5fafxj5dhrxgo6k65c462dhqjbno@uwghfyfdixtk","threadId":"59988","inReplyTo":"2e22a23f-576f-7a42-ace8-624a5362d9f4@gmail.com","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-15T23:15:28Z","receivedAt":"2023-07-15T23:15:40Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"On 23/07/15 09:59AM, Phillip Wood wrote:\n> Hi Jocab\n> \n> On 15/07/2023 03:55, Jacob Abel wrote:\n> > [...]\n> \n> Thanks for working on this fix. Having looked at the changes I think it\n> would be better just be using a space character in a lot of these\n> expressions - see below.\n> \n> > [...]\n> >\n> > -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n> > +\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan -b [^[:space:]]+ [^[:space:]]+[[:space:]]*$\" actual\n> \n> We know that \"hint:\" is followed by a single space and all we're really\n> interested in is that we print something after the \"-b \" so we can\n> simplify this to\n> \n> \tgrep \"^hint: git worktree add --orphan -b [^ ]\"\n> \n> I think the same applies to most of the other expressions changed in\n> this patch.\n\nThis wouldn't work as it's `hint: ` followed by a `\\t` as the command\nis indented in the text block. So I just went with `[[:space:]]+` as I\ndidn't want to have to worry about whether some platforms expand the\ntab to spaces or how many spaces. I'll make the rest of the suggested\nchanges though.\n\n> > [...]\n> > @@ -998,8 +998,8 @@ test_dwim_orphan () {\n> >   \t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n> \n> I'm a bit confused by the --sq here - why does it need to be shell\n> quoted when it is always used inside double quotes? \n\nTo be honest I can't remember if this specifically needs to be in\nquotes or not however I had a lot of trouble during the development of\nthat patchset with things escaping quotes and causing breakages in the\ntests so if it isn't currently harmful I'd personally prefer to leave\nit as is.\n\n> Also when the reftable backend is used I'm not sure that HEAD is\n> actually a file in $GIT_DIR anymore (that's less of an issue at the\n> moment as that backend is not is use yet).\n\nIf there is documentation (or discussions) on how to use this backend\nproperly I'd appreciate a link and I can try workshopping a better\nsolution then. The warning included in the original patchset reads\nfrom that HEAD file as well so it would also need to be adapted. \n\nThe reason I did it this way is because I didn't see any easy way to\nget the raw contents of the HEAD when it was invalid. If there is a\ncleaner/safer/more portable way to view those contents when HEAD\npoints to an invalid or unborn reference, I'd be willing to work on a\nfollowup patch down the line.\n\n> > [...]\n> \n> Using grep like this makes it harder to debug test failures as one has\n> to run the test with \"-x\" in order to try and figure out which grep\n> actually failed. I think here we can replace the sequence of \"grep\"s\n> with \"test_cmp\"\n> \n> \tcat >expect <<-EOF &&\n> \tHEAD points to an invalid (or orphaned) reference\n> \tHEAD path: $headpath\n> \tHEAD contents: $headcontents\n> \tEOF\n> \n> \ttest_cmp expect actual\n\nI'll make these changes.\n\n> [...]\n\n"},{"id":"479550","messageId":"bi4cirtujekrbbzdyjdimgv7shqumvniwqmsyoryfjd4ar2nan@66kcdfsdah3m","threadId":"59988","inReplyTo":"2e22a23f-576f-7a42-ace8-624a5362d9f4@gmail.com","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-15T23:36:09Z","receivedAt":"2023-07-15T23:36:25Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"On 23/07/15 07:15PM, Jacob Abel wrote:\n> On 23/07/15 09:59AM, Phillip Wood wrote:\n> >\n> > [...]\n>\n> [...]\n>\n> > \n> > Using grep like this makes it harder to debug test failures as one has\n> > to run the test with \"-x\" in order to try and figure out which grep\n> > actually failed. I think here we can replace the sequence of \"grep\"s\n> > with \"test_cmp\"\n> > \n> > \tcat >expect <<-EOF &&\n> > \tHEAD points to an invalid (or orphaned) reference\n> > \tHEAD path: $headpath\n> > \tHEAD contents: $headcontents\n> > \tEOF\n> > \n> > \ttest_cmp expect actual\n> \n> I'll make these changes.\n\nActually now that I'm sitting down to make these changes, I'm a bit\nhesitant as to whether this would be a cleaner solution than what is\ncurrently there. I say this as the contents of `actual` are about\n10-12 lines in length and some of those lines would vary between the\ndifferent tests that use this test function. Those specific details\ndon't need to be tested for as they are validated in prior tests and\nbuilding a perfectly matching `expected` would further complicate that\nfairly large test function.\n\n> \n> > [...]\n\n"},{"id":"479552","messageId":"xmqqilakll2m.fsf@gitster.g","threadId":"59988","inReplyTo":"vn5sylull5lqpitsanlyan5fafxj5dhrxgo6k65c462dhqjbno@uwghfyfdixtk","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-16T01:08:33Z","receivedAt":"2023-07-16T01:08:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Abel <jacobabel@nullpo.dev> writes:\n\n>> > @@ -998,8 +998,8 @@ test_dwim_orphan () {\n>> >   \t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n>> \n>> I'm a bit confused by the --sq here - why does it need to be shell\n>> quoted when it is always used inside double quotes? \n>\n> To be honest I can't remember if this specifically needs to be in\n> quotes or not however I had a lot of trouble during the development of\n> that patchset with things escaping quotes and causing breakages in the\n> tests so if it isn't currently harmful I'd personally prefer to leave\n> it as is.\n\nQuoting is sometimes tricky enough that \"this happens to work for me\nbut I do not know why it works\" is asking for trouble in somebody\nelse's environment.  If the form in the patch is correct, but tricky\nfor others to understand, you'd need to pick it apart and document\nhow it works (and if you cannot do so, ask for help by somebody who\ncan, or simplify it enough so that you can explain it yourself).\n\n    headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n\nIn this case, \"--sq\" is a noop that only confuses readers, I think,\nand I would drop it if I were you.  \"--git-path HEAD\" is given by\nthis call chain:\n\n   builtin/rev-parse.c:cmd_rev_parse() \n   -> builtin/rev-parse.c:print_path()\n      -> transform path depending on the path format\n         -> puts()\n\nand nowhere in this chain \"output_sq\" (which is set by \"--sq\") is\neven checked.  The transformations are all about relative, prefix,\netc., and never about quoting.\n\nThe original test script t2400 (before your patch) does look crappy\nwith full of long lines and coding style violations (none of which\nis your fault), and it may need to be cleaned up once this patch\nsettles.\n\nThanks.\n"},{"id":"479555","messageId":"bj27nq5aputhd66rkqer37vuc7qogpmn6nqyusladdy4k5it7k@u3yvvivrixsy","threadId":"59988","inReplyTo":"xmqqilakll2m.fsf@gitster.g","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-16T02:55:56Z","receivedAt":"2023-07-16T02:56:10Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"On 23/07/15 06:08PM, Junio C Hamano wrote:\n> Jacob Abel <jacobabel@nullpo.dev> writes:\n> \n> >> > @@ -998,8 +998,8 @@ test_dwim_orphan () {\n> >> >   \t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n> >>\n> >> I'm a bit confused by the --sq here - why does it need to be shell\n> >> quoted when it is always used inside double quotes?\n> >\n> > To be honest I can't remember if this specifically needs to be in\n> > quotes or not however I had a lot of trouble during the development of\n> > that patchset with things escaping quotes and causing breakages in the\n> > tests so if it isn't currently harmful I'd personally prefer to leave\n> > it as is.\n> \n> Quoting is sometimes tricky enough that \"this happens to work for me\n> but I do not know why it works\" is asking for trouble in somebody\n> else's environment.  If the form in the patch is correct, but tricky\n> for others to understand, you'd need to pick it apart and document\n> how it works (and if you cannot do so, ask for help by somebody who\n> can, or simplify it enough so that you can explain it yourself).\n\nYes Apologies. That was kind of a cop-out on my part as I was hesitant\nto add additional changes that could potentially introduce new issues\nto this patch as it is already addressing a fairly obscure issue.\n\n> \n>     headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n> \n> In this case, \"--sq\" is a noop that only confuses readers, I think,\n> and I would drop it if I were you.  \"--git-path HEAD\" is given by\n> this call chain:\n> \n>    builtin/rev-parse.c:cmd_rev_parse()\n>    -> builtin/rev-parse.c:print_path()\n>       -> transform path depending on the path format\n>          -> puts()\n> \n> and nowhere in this chain \"output_sq\" (which is set by \"--sq\") is\n> even checked.  The transformations are all about relative, prefix,\n> etc., and never about quoting.\n\nUnderstood. I tried running it with `--sq` removed and it seems to\nwork as you and Phillip expected so I'm adding that to v2.\n\n> \n> The original test script t2400 (before your patch) does look crappy\n> with full of long lines and coding style violations (none of which\n> is your fault), and it may need to be cleaned up once this patch\n> settles.\n> \n> Thanks.\n\nI may give that cleanup a shot some time down the line if nobody else\ntakes a crack at it first.\n\n"},{"id":"479556","messageId":"20230716033743.18200-1-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230715025512.7574-1-jacobabel@nullpo.dev","subject":"[PATCH v2] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-16T03:38:42Z","receivedAt":"2023-07-16T03:52:05Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Replace all cases of `\\s` with `[[:blank:]]` or ` ` as older versions\nof GNU grep (and from what it seems most versions of BSD grep) do not\nhandle `\\s`.\n\nFor the same reason all cases of `\\S` are replaced with `[^ ]`. It's not\nan exact replacement (as it does not match tabs) but it is close enough\nfor this use case.\n\nReplacing `\\S` also needs to occur as `\\S` is technically PCRE and not\npart of ERE even though most modern versions of grep accept it as ERE.\n\nThis commit also drops `--sq` from a rev-parse call as it appears to be\na no-op.\n\nSigned-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\nThis patch is in response to build failures on GGG's Cirrus CI \nfreebsd_12 build jobs[1] and was prompted by a discussion thread [2].\nThese failures seem to be caused by the behavior outlined in [3]. \n\nChanges from v1:\n  * Change `[[:space:]]` to ` ` where possible and `[[:blank:]]` \n    (tabs and spaces) otherwise as it is more accurate than `[[:space:]]` [4].\n  * Change `[^[:space:]]` to `[^ ]` [4]. Technically `[^[:blank:]]` would be\n    more accurate but tabs shouldn't be present where this `[^ ]` is used.\n  * Drop `--sq` from rev-parse [4] (after further discussion [5]).\n  * Update commit message to match changes.\n\n1. https://github.com/gitgitgadget/git/pull/1550/checks?check_run_id=14949695859\n2. https://lore.kernel.org/git/CALnO6CDryTsguLshcQxx97ZxyY42Twu2hC2y1bLOsS-9zbqXMA@mail.gmail.com/\n3. https://stackoverflow.com/questions/4233159/grep-regex-whitespace-behavior\n4. https://lore.kernel.org/git/vn5sylull5lqpitsanlyan5fafxj5dhrxgo6k65c462dhqjbno@uwghfyfdixtk/\n5. https://lore.kernel.org/git/bj27nq5aputhd66rkqer37vuc7qogpmn6nqyusladdy4k5it7k@u3yvvivrixsy/\n\nRange-diff against v1:\n1:  39f57add45 ! 1:  ef4ebd7350 t2400: Fix test failures when using grep 2.5\n    @@ Metadata\n      ## Commit message ##\n         t2400: Fix test failures when using grep 2.5\n     \n    -    Replace all cases of `\\s` with `[[:space:]]` as older versions of GNU\n    -    grep (and from what it seems most versions of BSD grep) do not handle\n    -    `\\s`.\n    +    Replace all cases of `\\s` with `[[:blank:]]` or ` ` as older versions\n    +    of GNU grep (and from what it seems most versions of BSD grep) do not\n    +    handle `\\s`.\n    +\n    +    For the same reason all cases of `\\S` are replaced with `[^ ]`. It's not\n    +    an exact replacement (as it does not match tabs) but it is close enough\n    +    for this use case.\n     \n    -    For the same reason all cases of `\\S` are replaced with `[^[:space:]]`.\n         Replacing `\\S` also needs to occur as `\\S` is technically PCRE and not\n         part of ERE even though most modern versions of grep accept it as ERE.\n     \n    +    This commit also drops `--sq` from a rev-parse call as it appears to be\n    +    a no-op.\n    +\n         Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n     \n      ## t/t2400-worktree-add.sh ##\n    @@ t/t2400-worktree-add.sh: test_wt_add_orphan_hint () {\n      \t\tif [ $use_branch -eq 1 ]\n      \t\tthen\n     -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n    -+\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan -b [^[:space:]]+ [^[:space:]]+[[:space:]]*$\" actual\n    ++\t\t\tgrep -E \"^hint:[[:blank:]]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n      \t\telse\n     -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n    -+\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan [^[:space:]]+[[:space:]]*$\" actual\n    ++\t\t\tgrep -E \"^hint:[[:blank:]]+git worktree add --orphan [^ ]+$\" actual\n      \t\tfi\n      \n      \t'\n    @@ t/t2400-worktree-add.sh: test_dwim_orphan () {\n      \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n      \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n     -\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n    -+\tlocal invalid_ref_regex=\"^fatal: invalid reference:[[:space:]]\\+.*\" &&\n    ++\tlocal invalid_ref_regex=\"^fatal: invalid reference: .*\" &&\n      \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n      \n      \tlocal git_ns=\"repo\" &&\n     @@ t/t2400-worktree-add.sh: test_dwim_orphan () {\n    - \t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n    + \t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n    + \t\t\t\t\t! grep \"$orphan_hint\" actual\n    + \t\t\t\telse\n    +-\t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n    ++\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n      \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n      \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n     -\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n     -\t\t\t\t\tgrep \"HEAD contents:\\s*.$headcontents.\" actual &&\n    -+\t\t\t\t\tgrep \"HEAD path:[[:space:]]*.$headpath.\" actual &&\n    -+\t\t\t\t\tgrep \"HEAD contents:[[:space:]]*.$headcontents.\" actual &&\n    ++\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n    ++\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n      \t\t\t\t\tgrep \"$orphan_hint\" actual &&\n      \t\t\t\t\t! grep \"$info_text\" actual\n      \t\t\t\tfi &&\n\n t/t2400-worktree-add.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 0ac468e69e..1b693dfca9 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -417,9 +417,9 @@ test_wt_add_orphan_hint () {\n \t\tgrep \"hint: If you meant to create a worktree containing a new orphan branch\" actual &&\n \t\tif [ $use_branch -eq 1 ]\n \t\tthen\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint:[[:blank:]]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n \t\telse\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint:[[:blank:]]+git worktree add --orphan [^ ]+$\" actual\n \t\tfi\n \n \t'\n@@ -709,7 +709,7 @@ test_dwim_orphan () {\n \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n-\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n+\tlocal invalid_ref_regex=\"^fatal: invalid reference: .*\" &&\n \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n \n \tlocal git_ns=\"repo\" &&\n@@ -995,11 +995,11 @@ test_dwim_orphan () {\n \t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n \t\t\t\t\t! grep \"$orphan_hint\" actual\n \t\t\t\telse\n-\t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n+\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n-\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n-\t\t\t\t\tgrep \"HEAD contents:\\s*.$headcontents.\" actual &&\n+\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n+\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n \t\t\t\t\tgrep \"$orphan_hint\" actual &&\n \t\t\t\t\t! grep \"$info_text\" actual\n \t\t\t\tfi &&\n-- \n2.39.3\n\n\n"},{"id":"479560","messageId":"3f3a3f5b-70fd-ec3f-acbb-d585b5eb6cbc@gmail.com","threadId":"59988","inReplyTo":"vn5sylull5lqpitsanlyan5fafxj5dhrxgo6k65c462dhqjbno@uwghfyfdixtk","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-07-16T15:34:52Z","receivedAt":"2023-07-16T15:35:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Jacob\n\nOn 16/07/2023 00:15, Jacob Abel wrote:\n> On 23/07/15 09:59AM, Phillip Wood wrote:\n>> Hi Jocab\n>>\n>> On 15/07/2023 03:55, Jacob Abel wrote:\n>>> [...]\n>>\n>> Thanks for working on this fix. Having looked at the changes I think it\n>> would be better just be using a space character in a lot of these\n>> expressions - see below.\n\nOne thing I forgot to mention was that I think it would be better to \nexplain in the commit message that \"\\s\" etc. are not part of POSIX EREs \nand that is why they do not work.\n\n>>> [...]\n>>>\n>>> -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n>>> +\t\t\tgrep -E \"^hint:[[:space:]]+git worktree add --orphan -b [^[:space:]]+ [^[:space:]]+[[:space:]]*$\" actual\n>>\n>> We know that \"hint:\" is followed by a single space and all we're really\n>> interested in is that we print something after the \"-b \" so we can\n>> simplify this to\n>>\n>> \tgrep \"^hint: git worktree add --orphan -b [^ ]\"\n>>\n>> I think the same applies to most of the other expressions changed in\n>> this patch.\n> \n> This wouldn't work as it's `hint: ` followed by a `\\t` as the command\n> is indented in the text block.\n\nOh so we need to search for a space followed by a tab after \"hint:\" \nthen. As an aside we often just use four spaces to indent commands in \nadvice messages (see the output of git -C .. grep '\"    git' \\*.c)\n\n> So I just went with `[[:space:]]+` as I\n> didn't want to have to worry about whether some platforms expand the\n> tab to spaces or how many spaces.\n\nIs that a thing?\n\n> I'll make the rest of the suggested\n> changes though.\n> \n>>> [...]\n>>> @@ -998,8 +998,8 @@ test_dwim_orphan () {\n>>>    \t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n>>\n>> I'm a bit confused by the --sq here - why does it need to be shell\n>> quoted when it is always used inside double quotes?\n> \n> To be honest I can't remember if this specifically needs to be in\n> quotes or not however I had a lot of trouble during the development of\n> that patchset with things escaping quotes and causing breakages in the\n> tests so if it isn't currently harmful I'd personally prefer to leave\n> it as is.\n> \n>> Also when the reftable backend is used I'm not sure that HEAD is\n>> actually a file in $GIT_DIR anymore (that's less of an issue at the\n>> moment as that backend is not is use yet).\n> \n> If there is documentation (or discussions) on how to use this backend\n> properly I'd appreciate a link and I can try workshopping a better\n> solution then. The warning included in the original patchset reads\n> from that HEAD file as well so it would also need to be adapted.\n\nI'm afraid I don't have anything specific, there were some patches a \nwhile ago such as dd8468ef00 (t5601: read HEAD using rev-parse, \n2021-05-31) that stopped reading HEAD from the filesystem.\n\n> The reason I did it this way is because I didn't see any easy way to\n> get the raw contents of the HEAD when it was invalid. If there is a\n> cleaner/safer/more portable way to view those contents when HEAD\n> points to an invalid or unborn reference, I'd be willing to work on a\n> followup patch down the line.\n\nI think it might be better to just diagnose if HEAD is a dangling \nsymbolic-ref or contains an invalid oid and leave it at that. See the \ndocumentation in refs.h for refs_resolve_ref_unsafe() for how to check \nif HEAD is a dangling symbolic ref - if rego_get_oid(repo, \"HEAD\") fails \nand it is not a dangling symbolic ref then it contains an invalid oid.\n\nBest Wishes\n\nPhillip\n\n>>> [...]\n>>\n>> Using grep like this makes it harder to debug test failures as one has\n>> to run the test with \"-x\" in order to try and figure out which grep\n>> actually failed. I think here we can replace the sequence of \"grep\"s\n>> with \"test_cmp\"\n>>\n>> \tcat >expect <<-EOF &&\n>> \tHEAD points to an invalid (or orphaned) reference\n>> \tHEAD path: $headpath\n>> \tHEAD contents: $headcontents\n>> \tEOF\n>>\n>> \ttest_cmp expect actual\n> \n> I'll make these changes.\n> \n>> [...]\n> \n"},{"id":"479566","messageId":"xmqqo7kbjm80.fsf@gitster.g","threadId":"59988","inReplyTo":"3f3a3f5b-70fd-ec3f-acbb-d585b5eb6cbc@gmail.com","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-17T02:38:55Z","receivedAt":"2023-07-17T02:39:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> One thing I forgot to mention was that I think it would be better to\n> explain in the commit message that \"\\s\" etc. are not part of POSIX\n> EREs and that is why they do not work.\n\nYes, that is a very good point.  We have been burned by regular\nexpression implementations that use or do not use \"enhanced\" bit in\nthe recent past, IIRC.\n\n> I think it might be better to just diagnose if HEAD is a dangling\n> symbolic-ref or contains an invalid oid and leave it at that. See the\n> documentation in refs.h for refs_resolve_ref_unsafe() for how to check\n> if HEAD is a dangling symbolic ref - if rego_get_oid(repo, \"HEAD\")\n> fails and it is not a dangling symbolic ref then it contains an\n> invalid oid.\n\nSounds doable and sensible.  If we can easily test it without\npeeking into the filesystem, that would be very good.\n\nThanks.\n\n\n"},{"id":"479593","messageId":"dyzkftugvd5b4f4wxsg6773fkrdrnbync6idvvi6h7cuuto36w@dbzjnkj3mh2l","threadId":"59988","inReplyTo":"3f3a3f5b-70fd-ec3f-acbb-d585b5eb6cbc@gmail.com","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-18T00:44:47Z","receivedAt":"2023-07-18T00:45:04Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"On 23/07/16 04:34PM, Phillip Wood wrote:\n> Hi Jacob\n> \n> [...]\n> \n> One thing I forgot to mention was that I think it would be better to\n> explain in the commit message that \"\\s\" etc. are not part of POSIX EREs\n> and that is why they do not work.\n\nNoted. Will do.\n\n> [...]\n> \n> Oh so we need to search for a space followed by a tab after \"hint:\"\n> then. \n\nOkay. I think `\\t` is PCRE so I'll just update the string in \n`builtin/worktree.c` so we can just do `[ ]+` instead. \n\n> As an aside we often just use four spaces to indent commands in\n> advice messages (see the output of git -C .. grep '\"    git' \\*.c)\n\nApologies. When writing up that original patchset I based the\nformatting of the advice based on the ones in `builtin/add.c` which\nseems to also use `\\t`.\n\n> \n> > So I just went with `[[:space:]]+` as I\n> > didn't want to have to worry about whether some platforms expand the\n> > tab to spaces or how many spaces.\n> \n> Is that a thing?\n\nIt might be? I know copying text through tmux tends to expand tabs to\nspaces for me so I figured some other tools or those same tools on\ndifferent platforms might do things like that as well. To be honest I\nhave no idea and figured that I'd just CYA by making it work in the\ncase that it did than trying to guarantee that it wouldn't happen.\n\n> > [...]\n> >\n> > If there is documentation (or discussions) on how to use this backend\n> > properly I'd appreciate a link and I can try workshopping a better\n> > solution then. The warning included in the original patchset reads\n> > from that HEAD file as well so it would also need to be adapted.\n> \n> I'm afraid I don't have anything specific, there were some patches a\n> while ago such as dd8468ef00 (t5601: read HEAD using rev-parse,\n> 2021-05-31) that stopped reading HEAD from the filesystem.\n\nNoted.\n\n> > [...]\n> \n> I think it might be better to just diagnose if HEAD is a dangling\n> symbolic-ref or contains an invalid oid and leave it at that. See the\n> documentation in refs.h for refs_resolve_ref_unsafe() for how to check\n> if HEAD is a dangling symbolic ref - if rego_get_oid(repo, \"HEAD\") fails\n> and it is not a dangling symbolic ref then it contains an invalid oid.\n\nUnderstood. I'll start working on a separate patch to update that\nwarning once this patch settles then.\n\n> \n> [...]\n\n"},{"id":"479597","messageId":"44671697-e9e6-d75a-30b9-7dccffcc792a@gmail.com","threadId":"59988","inReplyTo":"dyzkftugvd5b4f4wxsg6773fkrdrnbync6idvvi6h7cuuto36w@dbzjnkj3mh2l","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-07-18T13:36:10Z","receivedAt":"2023-07-18T13:36:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 18/07/2023 01:44, Jacob Abel wrote:\n> On 23/07/16 04:34PM, Phillip Wood wrote:\n>> Oh so we need to search for a space followed by a tab after \"hint:\"\n>> then.\n> \n> Okay. I think `\\t` is PCRE so I'll just update the string in\n> `builtin/worktree.c` so we can just do `[ ]+` instead.\n> \n>> As an aside we often just use four spaces to indent commands in\n>> advice messages (see the output of git -C .. grep '\"    git' \\*.c)\n> \n> Apologies. When writing up that original patchset I based the\n> formatting of the advice based on the ones in `builtin/add.c` which\n> seems to also use `\\t`.\n\nThe existing code is not consistent on this point but I think there are \nmore instances of \"    \" than \"\\t\". Using \"    \" makes the indentation \nconsistent as the \"hint: \" prefix is translated so we don't know how far \nthe next tab stop will be from the end of the prefix.\n\n>>\n>>> So I just went with `[[:space:]]+` as I\n>>> didn't want to have to worry about whether some platforms expand the\n>>> tab to spaces or how many spaces.\n>>\n>> Is that a thing?\n> \n> It might be? I know copying text through tmux tends to expand tabs to\n> spaces for me so I figured some other tools or those same tools on\n> different platforms might do things like that as well. To be honest I\n> have no idea and figured that I'd just CYA by making it work in the\n> case that it did than trying to guarantee that it wouldn't happen.\n\nIn the test we are redirecting the output to a file so things like tmux \ndo not come into play. I think it would be a bit odd for the system libc \nto convert tabs to spaces.\n\n>>> [...]\n>>\n>> I think it might be better to just diagnose if HEAD is a dangling\n>> symbolic-ref or contains an invalid oid and leave it at that. See the\n>> documentation in refs.h for refs_resolve_ref_unsafe() for how to check\n>> if HEAD is a dangling symbolic ref - if rego_get_oid(repo, \"HEAD\") fails\n>> and it is not a dangling symbolic ref then it contains an invalid oid.\n> \n> Understood. I'll start working on a separate patch to update that\n> warning once this patch settles then.\n\nThat's great. I think just telling the user something like\n\n    branch 'main' does not exist\n\nwhen HEAD contains the dangling symbolic ref \"refs/heads/main\" and\n\n     HEAD is corrupt\n\nwhen it is not a symbolic ref and repo_get_oid() fails would be fine.\n\nBest Wishes\n\nPhillip\n"},{"id":"479707","messageId":"feeobdkifsgoo7rrx4r45qh3v5d2qgktmaggwhmdnh2h5hl7zw@k37fiqyh3irq","threadId":"59988","inReplyTo":"44671697-e9e6-d75a-30b9-7dccffcc792a@gmail.com","subject":"Re: [PATCH] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-21T04:35:19Z","receivedAt":"2023-07-21T04:35:39Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"On 23/07/18 02:36PM, Phillip Wood wrote:\n> [...]\n> \n> The existing code is not consistent on this point but I think there are\n> more instances of \"    \" than \"\\t\". Using \"    \" makes the indentation\n> consistent as the \"hint: \" prefix is translated so we don't know how far\n> the next tab stop will be from the end of the prefix.\n\nAgreed.\n\n> > [...]\n> \n> In the test we are redirecting the output to a file so things like tmux\n> do not come into play. I think it would be a bit odd for the system libc\n> to convert tabs to spaces.\n\nUnderstood. It was just a bit of paranoia on my part then.\n\n> [...]\n>\n> > Understood. I'll start working on a separate patch to update that\n> > warning once this patch settles then.\n> \n> That's great. I think just telling the user something like\n> \n>     branch 'main' does not exist\n> \n> when HEAD contains the dangling symbolic ref \"refs/heads/main\" and\n> \n>      HEAD is corrupt\n> \n> when it is not a symbolic ref and repo_get_oid() fails would be fine.\n\nNoted. I'll workshop it a bit before I put v1 of that patch out.\n\n"},{"id":"479708","messageId":"20230721044012.24360-1-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230716033743.18200-1-jacobabel@nullpo.dev","subject":"[PATCH v3 0/3] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-21T04:40:28Z","receivedAt":"2023-07-21T04:41:21Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"This patchset is in response to build failures on GGG's Cirrus CI \nfreebsd_12 build jobs[1] and was prompted by a discussion thread [2].\nThese failures seem to be caused by the behavior outlined in [3]. \n\nNote: I jumped the gun on v2 a bit as discussions for v1 were still in\nprogress so these are all changes suggested off v1.\n\nChanges from v2:\n  * Split `--sq` change out into separate patch (from 3/3 to 1/3).\n  * Convert tab in advice to space to match coding convention and to\n    allow regex to be further simplified [4].\n  * Simplified regex [4].\n  * Reworded commit message for patch 3/3 to better document reason for\n    change [4].\n\n1. https://github.com/gitgitgadget/git/pull/1550/checks?check_run_id=14949695859\n2. https://lore.kernel.org/git/CALnO6CDryTsguLshcQxx97ZxyY42Twu2hC2y1bLOsS-9zbqXMA@mail.gmail.com/\n3. https://stackoverflow.com/questions/4233159/grep-regex-whitespace-behavior\n4. https://lore.kernel.org/git/3f3a3f5b-70fd-ec3f-acbb-d585b5eb6cbc@gmail.com/\n\nJacob Abel (3):\n  t2400: drop no-op `--sq` from rev-parse call\n  builtin/worktree.c: convert tab in advice to space\n  t2400: rewrite regex to avoid unintentional PCRE\n\n builtin/worktree.c      |  4 ++--\n t/t2400-worktree-add.sh | 12 ++++++------\n 2 files changed, 8 insertions(+), 8 deletions(-)\n\nRange-diff against v2:\n-:  ---------- > 1:  96c21c5bee t2400: drop no-op `--sq` from rev-parse call\n-:  ---------- > 2:  ebfba2d602 builtin/worktree.c: convert tab in advice to space\n1:  ef4ebd7350 ! 3:  dee0c8f350 t2400: Fix test failures when using grep 2.5\n    @@ Metadata\n     Author: Jacob Abel <jacobabel@nullpo.dev>\n     \n      ## Commit message ##\n    -    t2400: Fix test failures when using grep 2.5\n    +    t2400: rewrite regex to avoid unintentional PCRE\n     \n    -    Replace all cases of `\\s` with `[[:blank:]]` or ` ` as older versions\n    -    of GNU grep (and from what it seems most versions of BSD grep) do not\n    -    handle `\\s`.\n    +    Replace all cases of `\\s` with ` ` as it is not part of POSIX BRE or ERE\n    +    and therefore not all versions of grep handle it without PCRE support.\n     \n         For the same reason all cases of `\\S` are replaced with `[^ ]`. It's not\n    -    an exact replacement (as it does not match tabs) but it is close enough\n    -    for this use case.\n    -\n    -    Replacing `\\S` also needs to occur as `\\S` is technically PCRE and not\n    -    part of ERE even though most modern versions of grep accept it as ERE.\n    -\n    -    This commit also drops `--sq` from a rev-parse call as it appears to be\n    -    a no-op.\n    +    an exact replacement but it is close enough for this use case.\n     \n         Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n     \n    @@ t/t2400-worktree-add.sh: test_wt_add_orphan_hint () {\n      \t\tif [ $use_branch -eq 1 ]\n      \t\tthen\n     -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n    -+\t\t\tgrep -E \"^hint:[[:blank:]]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n    ++\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n      \t\telse\n     -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n    -+\t\t\tgrep -E \"^hint:[[:blank:]]+git worktree add --orphan [^ ]+$\" actual\n    ++\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan [^ ]+$\" actual\n      \t\tfi\n      \n      \t'\n    @@ t/t2400-worktree-add.sh: test_dwim_orphan () {\n      \n      \tlocal git_ns=\"repo\" &&\n     @@ t/t2400-worktree-add.sh: test_dwim_orphan () {\n    - \t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n    - \t\t\t\t\t! grep \"$orphan_hint\" actual\n    - \t\t\t\telse\n    --\t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n    -+\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n    + \t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n      \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n      \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n     -\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n-- \n2.39.3\n\n\n"},{"id":"479710","messageId":"20230721044012.24360-2-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230721044012.24360-1-jacobabel@nullpo.dev","subject":"[PATCH v3 1/3] t2400: drop no-op `--sq` from rev-parse call","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-21T04:40:37Z","receivedAt":"2023-07-21T04:41:25Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\n t/t2400-worktree-add.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 0ac468e69e..e106540c6d 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -995,7 +995,7 @@ test_dwim_orphan () {\n \t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n \t\t\t\t\t! grep \"$orphan_hint\" actual\n \t\t\t\telse\n-\t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n+\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n \t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n-- \n2.39.3\n\n\n"},{"id":"479709","messageId":"20230721044012.24360-3-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230721044012.24360-1-jacobabel@nullpo.dev","subject":"[PATCH v3 2/3] builtin/worktree.c: convert tab in advice to space","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-21T04:40:44Z","receivedAt":"2023-07-21T04:41:27Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\n builtin/worktree.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 7c114d56a3..3cdcb86cd4 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -54,14 +54,14 @@\n \t\"(branch with no commits) for this repository, you can do so\\n\" \\\n \t\"using the --orphan flag:\\n\" \\\n \t\"\\n\" \\\n-\t\"\tgit worktree add --orphan -b %s %s\\n\")\n+\t\"    git worktree add --orphan -b %s %s\\n\")\n \n #define WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT \\\n \t_(\"If you meant to create a worktree containing a new orphan branch\\n\" \\\n \t\"(branch with no commits) for this repository, you can do so\\n\" \\\n \t\"using the --orphan flag:\\n\" \\\n \t\"\\n\" \\\n-\t\"\tgit worktree add --orphan %s\\n\")\n+\t\"    git worktree add --orphan %s\\n\")\n \n static const char * const git_worktree_usage[] = {\n \tBUILTIN_WORKTREE_ADD_USAGE,\n-- \n2.39.3\n\n\n"},{"id":"479711","messageId":"20230721044012.24360-4-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230721044012.24360-1-jacobabel@nullpo.dev","subject":"[PATCH v3 3/3] t2400: rewrite regex to avoid unintentional PCRE","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-21T04:40:56Z","receivedAt":"2023-07-21T04:41:37Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Replace all cases of `\\s` with ` ` as it is not part of POSIX BRE or ERE\nand therefore not all versions of grep handle it without PCRE support.\n\nFor the same reason all cases of `\\S` are replaced with `[^ ]`. It's not\nan exact replacement but it is close enough for this use case.\n\nSigned-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\n t/t2400-worktree-add.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex e106540c6d..eafecdf7ce 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -417,9 +417,9 @@ test_wt_add_orphan_hint () {\n \t\tgrep \"hint: If you meant to create a worktree containing a new orphan branch\" actual &&\n \t\tif [ $use_branch -eq 1 ]\n \t\tthen\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n \t\telse\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan [^ ]+$\" actual\n \t\tfi\n \n \t'\n@@ -709,7 +709,7 @@ test_dwim_orphan () {\n \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n-\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n+\tlocal invalid_ref_regex=\"^fatal: invalid reference: .*\" &&\n \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n \n \tlocal git_ns=\"repo\" &&\n@@ -998,8 +998,8 @@ test_dwim_orphan () {\n \t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n-\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n-\t\t\t\t\tgrep \"HEAD contents:\\s*.$headcontents.\" actual &&\n+\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n+\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n \t\t\t\t\tgrep \"$orphan_hint\" actual &&\n \t\t\t\t\t! grep \"$info_text\" actual\n \t\t\t\tfi &&\n-- \n2.39.3\n\n\n"},{"id":"479738","messageId":"xmqqv8edwb0k.fsf@gitster.g","threadId":"59988","inReplyTo":"20230721044012.24360-4-jacobabel@nullpo.dev","subject":"Re: [PATCH v3 3/3] t2400: rewrite regex to avoid unintentional PCRE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-21T15:16:11Z","receivedAt":"2023-07-21T15:16:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Abel <jacobabel@nullpo.dev> writes:\n\n> Replace all cases of `\\s` with ` ` as it is not part of POSIX BRE or ERE\n> and therefore not all versions of grep handle it without PCRE support.\n\nGood point.  But the patch replaces them with \"[ ]\" instead, which\nprobably is not a good idea for readability.\n\nTechnically speaking, there is no regular expression library that\nsupports PCRE per-se; treating \\S, \\s, \\d and the like the same way\nas PCRE is a GNU extension in the glibc land, and a simlar \"enhanced\nmode\" can be requested by passing REG_ENHANCED bit to regcomp(3) at\nruntime in the BSD land including macOS.  I would suggest just\ndropping \"without PCRE support\" for brevity, as \"not all versions of\ngrep handle it\" is sufficient here.\n\n> For the same reason all cases of `\\S` are replaced with `[^ ]`.\n> It's not an exact replacement but it is close enough for this\n> use case.\n\nGood.\n\n> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n> ---\n>  t/t2400-worktree-add.sh | 10 +++++-----\n>  1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\n> index e106540c6d..eafecdf7ce 100755\n> --- a/t/t2400-worktree-add.sh\n> +++ b/t/t2400-worktree-add.sh\n> @@ -417,9 +417,9 @@ test_wt_add_orphan_hint () {\n>  \t\tgrep \"hint: If you meant to create a worktree containing a new orphan branch\" actual &&\n>  \t\tif [ $use_branch -eq 1 ]\n>  \t\tthen\n> -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n> +\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n>  \t\telse\n> -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n> +\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan [^ ]+$\" actual\n>  \t\tfi\n\nJust a single space would be fine without [bracket].  I think older\ntests use (literally) HT and SP inside [], many of them may still\nsurvive.\n\n> @@ -709,7 +709,7 @@ test_dwim_orphan () {\n>  \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n>  \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n>  \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n> -\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n> +\tlocal invalid_ref_regex=\"^fatal: invalid reference: .*\" &&\n\nFeeding \"<something>\\+\" to BRE (this pattern is later used with\n'grep' but not with 'egrep' or 'grep -E') and expecting it to mean 1\nor more is a GNU extension, and in this case \"there must be a SP\nafter colon\" is much easier to see, which is what the updated one\nuses.  Good.\n\nBy the way, you can drop the \".*\" at the end of the pattern, because\nthe match is not anchored at the tail end.\n\n>  \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n\nThis should also be corrected, I think.\n\n\t\"fatal: '[a-z-]\\{1,\\}' and '[a-z-]\\{1,\\}' cannot be used together\"\n\nor even simpler,\n\n\t\"fatal: '[a-z-]*' and '[a-z-]*' cannot be used together\"\n\nto avoid \\+ in BRE (see above).  \"[-a-z]\" (to show '-' at the\nbeginning) may make it easier to read by letting the hyphen-minus\nstand out more, as we know we are giving two command line option\nnames and in a command line option name, the first letter is always\nhyphen-minus.  But that is more of personal taste, not correctness.\n\n> @@ -998,8 +998,8 @@ test_dwim_orphan () {\n>  \t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n>  \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n>  \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n> -\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n> -\t\t\t\t\tgrep \"HEAD contents:\\s*.$headcontents.\" actual &&\n> +\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n> +\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n>  \t\t\t\t\tgrep \"$orphan_hint\" actual &&\n>  \t\t\t\t\t! grep \"$info_text\" actual\n>  \t\t\t\tfi &&\n\nThanks.\n"},{"id":"479740","messageId":"xmqqiladw9h7.fsf@gitster.g","threadId":"59988","inReplyTo":"xmqqv8edwb0k.fsf@gitster.g","subject":"Re: [PATCH v3 3/3] t2400: rewrite regex to avoid unintentional PCRE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-21T15:49:24Z","receivedAt":"2023-07-21T15:49:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Just for reference, here is a summary, as a squashable commit, of\nthe message I am responding to.\n\n---- >8 ----\nSubject: [PATCH] SQUASH???\n\n(remove this part of the message while squashing)\nUse ` `, not `[ ]`, as the proposed log message described.\n\n(append to the proposed log message of the previous)\nAlso, do not write `\\+` in BRE and expect it to mean 1 or more;\nit is a GNU extension that may not work everywhere.\n\nRemove '.*' at the end of a pattern that is not right-anchored.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t2400-worktree-add.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex eafecdf7ce..aee5eba980 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -417,9 +417,9 @@ test_wt_add_orphan_hint () {\n \t\tgrep \"hint: If you meant to create a worktree containing a new orphan branch\" actual &&\n \t\tif [ $use_branch -eq 1 ]\n \t\tthen\n-\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n+\t\t\tgrep -E \"^hint: +git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n \t\telse\n-\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan [^ ]+$\" actual\n+\t\t\tgrep -E \"^hint: +git worktree add --orphan [^ ]+$\" actual\n \t\tfi\n \n \t'\n@@ -709,8 +709,8 @@ test_dwim_orphan () {\n \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n-\tlocal invalid_ref_regex=\"^fatal: invalid reference: .*\" &&\n-\tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n+\tlocal invalid_ref_regex=\"^fatal: invalid reference: \" &&\n+\tlocal bad_combo_regex=\"^fatal: '[-a-z]*' and '[-a-z-]*' cannot be used together\" &&\n \n \tlocal git_ns=\"repo\" &&\n \tlocal dashc_args=\"-C $git_ns\" &&\n-- \n2.41.0-376-gcba07a324d\n\n"},{"id":"479774","messageId":"axnxvnmo6ekhhccppinji73ivlandwuqs44epmq4pdefm7ukiv@ejz7bee5xjli","threadId":"59988","inReplyTo":"xmqqv8edwb0k.fsf@gitster.g","subject":"Re: [PATCH v3 3/3] t2400: rewrite regex to avoid unintentional PCRE","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-22T02:36:27Z","receivedAt":"2023-07-22T02:36:39Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"On 23/07/21 08:16AM, Junio C Hamano wrote:\n> Jacob Abel <jacobabel@nullpo.dev> writes:\n> \n> > Replace all cases of `\\s` with ` ` as it is not part of POSIX BRE or ERE\n> > and therefore not all versions of grep handle it without PCRE support.\n> \n> Good point.  But the patch replaces them with \"[ ]\" instead, which\n> probably is not a good idea for readability.\n\nUsing `[ ]` over ` ` is just a personal thing I picked up to keep\nmyself from forgetting the space was intentional. I can see how that\ncan come across as confusing though so I'll make sure to update that.\n\n> Technically speaking, there is no regular expression library that\n> supports PCRE per-se; treating \\S, \\s, \\d and the like the same way\n> as PCRE is a GNU extension in the glibc land, and a simlar \"enhanced\n> mode\" can be requested by passing REG_ENHANCED bit to regcomp(3) at\n> runtime in the BSD land including macOS.  I would suggest just\n> dropping \"without PCRE support\" for brevity, as \"not all versions of\n> grep handle it\" is sufficient here.\n\nGood point. Will do.\n\n> [...]\n> \n> Just a single space would be fine without [bracket].  I think older\n> tests use (literally) HT and SP inside [], many of them may still\n> survive.\n\nNoted.\n\n> > @@ -709,7 +709,7 @@ test_dwim_orphan () {\n> >  \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n> >  \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n> >  \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n> > -\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n> > +\tlocal invalid_ref_regex=\"^fatal: invalid reference: .*\" &&\n> \n> Feeding \"<something>\\+\" to BRE (this pattern is later used with\n> 'grep' but not with 'egrep' or 'grep -E') and expecting it to mean 1\n> or more is a GNU extension, \n\nOh it is. I've really gotta reread the chapters of the POSIX standard\non regex again.\n\n> and in this case \"there must be a SP\n> after colon\" is much easier to see, which is what the updated one\n> uses.  Good.\n> \n> By the way, you can drop the \".*\" at the end of the pattern, because\n> the match is not anchored at the tail end.\n\nUnderstood.\n\n> >  \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n> \n> This should also be corrected, I think.\n> \n> \t\"fatal: '[a-z-]\\{1,\\}' and '[a-z-]\\{1,\\}' cannot be used together\"\n> \n> or even simpler,\n> \n> \t\"fatal: '[a-z-]*' and '[a-z-]*' cannot be used together\"\n> \n> to avoid \\+ in BRE (see above).  \n\nI definitely prefer the latter so I'll update it to use that one.\n\n> \"[-a-z]\" (to show '-' at the\n> beginning) may make it easier to read by letting the hyphen-minus\n> stand out more, as we know we are giving two command line option\n> names and in a command line option name, the first letter is always\n> hyphen-minus.  But that is more of personal taste, not correctness.\n\nCertainly a matter of personal preference but I can see why this\ncould be preferable so I'll update it to this.\n\n> [...]\n\n"},{"id":"479920","messageId":"20230726214202.15775-1-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230721044012.24360-1-jacobabel@nullpo.dev","subject":"[PATCH v4 0/3] t2400: Fix test failures when using grep 2.5","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-26T21:42:09Z","receivedAt":"2023-07-26T21:42:34Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"This patchset is in response to build failures on GGG's Cirrus CI \nfreebsd_12 build jobs[1] and was prompted by a discussion thread [2].\nThese failures seem to be caused by the behavior outlined in [3]. \n\nChanges from v3:\n  * Replace `[ ]` with ` ` in regex for `test_wt_add_orphan_hint()` [4][5].\n  * Drop trailing `.*` from `invalid_ref_regex` [4][5].\n  * Change `[a-z-]` to `[-a-z]` in `bad_combo_regex` to better portray\n    intent [4][5].\n  * Replace `\\+` with `*` in `bad_combo_regex` as `\\+` is not POSIX\n    BRE and is a GNU extension [4][5].\n  * Drop \"without PCRE support\" from commit message [4].\n  * Reword commit message to reflect changes.\n\n1. https://github.com/gitgitgadget/git/pull/1550/checks?check_run_id=14949695859\n2. https://lore.kernel.org/git/CALnO6CDryTsguLshcQxx97ZxyY42Twu2hC2y1bLOsS-9zbqXMA@mail.gmail.com/\n3. https://stackoverflow.com/questions/4233159/grep-regex-whitespace-behavior\n4. https://lore.kernel.org/git/axnxvnmo6ekhhccppinji73ivlandwuqs44epmq4pdefm7ukiv@ejz7bee5xjli/\n5. https://lore.kernel.org/git/xmqqiladw9h7.fsf@gitster.g/\n\nJacob Abel (3):\n  t2400: drop no-op `--sq` from rev-parse call\n  builtin/worktree.c: convert tab in advice to space\n  t2400: rewrite regex to avoid unintentional PCRE\n\n builtin/worktree.c      |  4 ++--\n t/t2400-worktree-add.sh | 14 +++++++-------\n 2 files changed, 9 insertions(+), 9 deletions(-)\n\nRange-diff against v3:\n1:  96c21c5bee = 1:  96c21c5bee t2400: drop no-op `--sq` from rev-parse call\n2:  ebfba2d602 = 2:  ebfba2d602 builtin/worktree.c: convert tab in advice to space\n3:  dee0c8f350 ! 3:  13f61cd15a t2400: rewrite regex to avoid unintentional PCRE\n    @@ Commit message\n         t2400: rewrite regex to avoid unintentional PCRE\n     \n         Replace all cases of `\\s` with ` ` as it is not part of POSIX BRE or ERE\n    -    and therefore not all versions of grep handle it without PCRE support.\n    +    and therefore not all versions of grep handle it.\n     \n    -    For the same reason all cases of `\\S` are replaced with `[^ ]`. It's not\n    -    an exact replacement but it is close enough for this use case.\n    +    For the same reason all cases of `\\S` are replaced with `[^ ]`. It is\n    +    not an exact replacement but it is close enough for this use case.\n    +\n    +    Also, do not write `\\+` in BRE and expect it to mean 1 or more;\n    +    it is a GNU extension that may not work everywhere.\n    +\n    +    Remove `.*` from the end of a pattern that is not right-anchored.\n     \n         Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n    +    Helped-by: Junio C Hamano <gitster@pobox.com>\n     \n      ## t/t2400-worktree-add.sh ##\n     @@ t/t2400-worktree-add.sh: test_wt_add_orphan_hint () {\n    @@ t/t2400-worktree-add.sh: test_wt_add_orphan_hint () {\n      \t\tif [ $use_branch -eq 1 ]\n      \t\tthen\n     -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n    -+\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n    ++\t\t\tgrep -E \"^hint: +git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n      \t\telse\n     -\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n    -+\t\t\tgrep -E \"^hint:[ ]+git worktree add --orphan [^ ]+$\" actual\n    ++\t\t\tgrep -E \"^hint: +git worktree add --orphan [^ ]+$\" actual\n      \t\tfi\n      \n      \t'\n    @@ t/t2400-worktree-add.sh: test_dwim_orphan () {\n      \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n      \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n     -\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n    -+\tlocal invalid_ref_regex=\"^fatal: invalid reference: .*\" &&\n    - \tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n    +-\tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n    ++\tlocal invalid_ref_regex=\"^fatal: invalid reference: \" &&\n    ++\tlocal bad_combo_regex=\"^fatal: '[-a-z]*' and '[-a-z]*' cannot be used together\" &&\n      \n      \tlocal git_ns=\"repo\" &&\n    + \tlocal dashc_args=\"-C $git_ns\" &&\n     @@ t/t2400-worktree-add.sh: test_dwim_orphan () {\n      \t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n      \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n-- \n2.41.0\n\n\n"},{"id":"479921","messageId":"20230726214202.15775-2-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230726214202.15775-1-jacobabel@nullpo.dev","subject":"[PATCH v4 1/3] t2400: drop no-op `--sq` from rev-parse call","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-26T21:42:18Z","receivedAt":"2023-07-26T21:42:37Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\n t/t2400-worktree-add.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 0ac468e69e..e106540c6d 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -995,7 +995,7 @@ test_dwim_orphan () {\n \t\t\t\t\tgrep \"$invalid_ref_regex\" actual &&\n \t\t\t\t\t! grep \"$orphan_hint\" actual\n \t\t\t\telse\n-\t\t\t\t\theadpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&\n+\t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n \t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n-- \n2.41.0\n\n\n"},{"id":"479922","messageId":"20230726214202.15775-3-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230726214202.15775-1-jacobabel@nullpo.dev","subject":"[PATCH v4 2/3] builtin/worktree.c: convert tab in advice to space","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-26T21:42:24Z","receivedAt":"2023-07-26T21:43:00Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\n builtin/worktree.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 7c114d56a3..3cdcb86cd4 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -54,14 +54,14 @@\n \t\"(branch with no commits) for this repository, you can do so\\n\" \\\n \t\"using the --orphan flag:\\n\" \\\n \t\"\\n\" \\\n-\t\"\tgit worktree add --orphan -b %s %s\\n\")\n+\t\"    git worktree add --orphan -b %s %s\\n\")\n \n #define WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT \\\n \t_(\"If you meant to create a worktree containing a new orphan branch\\n\" \\\n \t\"(branch with no commits) for this repository, you can do so\\n\" \\\n \t\"using the --orphan flag:\\n\" \\\n \t\"\\n\" \\\n-\t\"\tgit worktree add --orphan %s\\n\")\n+\t\"    git worktree add --orphan %s\\n\")\n \n static const char * const git_worktree_usage[] = {\n \tBUILTIN_WORKTREE_ADD_USAGE,\n-- \n2.41.0\n\n\n"},{"id":"479923","messageId":"20230726214202.15775-4-jacobabel@nullpo.dev","threadId":"59988","inReplyTo":"20230726214202.15775-1-jacobabel@nullpo.dev","subject":"[PATCH v4 3/3] t2400: rewrite regex to avoid unintentional PCRE","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-07-26T21:42:34Z","receivedAt":"2023-07-26T21:43:01Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Replace all cases of `\\s` with ` ` as it is not part of POSIX BRE or ERE\nand therefore not all versions of grep handle it.\n\nFor the same reason all cases of `\\S` are replaced with `[^ ]`. It is\nnot an exact replacement but it is close enough for this use case.\n\nAlso, do not write `\\+` in BRE and expect it to mean 1 or more;\nit is a GNU extension that may not work everywhere.\n\nRemove `.*` from the end of a pattern that is not right-anchored.\n\nSigned-off-by: Jacob Abel <jacobabel@nullpo.dev>\nHelped-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t2400-worktree-add.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex e106540c6d..051363acbb 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -417,9 +417,9 @@ test_wt_add_orphan_hint () {\n \t\tgrep \"hint: If you meant to create a worktree containing a new orphan branch\" actual &&\n \t\tif [ $use_branch -eq 1 ]\n \t\tthen\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan -b \\S+ \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint: +git worktree add --orphan -b [^ ]+ [^ ]+$\" actual\n \t\telse\n-\t\t\tgrep -E \"^hint:\\s+git worktree add --orphan \\S+\\s*$\" actual\n+\t\t\tgrep -E \"^hint: +git worktree add --orphan [^ ]+$\" actual\n \t\tfi\n \n \t'\n@@ -709,8 +709,8 @@ test_dwim_orphan () {\n \tlocal info_text=\"No possible source branch, inferring '--orphan'\" &&\n \tlocal fetch_error_text=\"fatal: No local or remote refs exist despite at least one remote\" &&\n \tlocal orphan_hint=\"hint: If you meant to create a worktree containing a new orphan branch\" &&\n-\tlocal invalid_ref_regex=\"^fatal: invalid reference:\\s\\+.*\" &&\n-\tlocal bad_combo_regex=\"^fatal: '[a-z-]\\+' and '[a-z-]\\+' cannot be used together\" &&\n+\tlocal invalid_ref_regex=\"^fatal: invalid reference: \" &&\n+\tlocal bad_combo_regex=\"^fatal: '[-a-z]*' and '[-a-z]*' cannot be used together\" &&\n \n \tlocal git_ns=\"repo\" &&\n \tlocal dashc_args=\"-C $git_ns\" &&\n@@ -998,8 +998,8 @@ test_dwim_orphan () {\n \t\t\t\t\theadpath=$(git $dashc_args rev-parse --path-format=absolute --git-path HEAD) &&\n \t\t\t\t\theadcontents=$(cat \"$headpath\") &&\n \t\t\t\t\tgrep \"HEAD points to an invalid (or orphaned) reference\" actual &&\n-\t\t\t\t\tgrep \"HEAD path:\\s*.$headpath.\" actual &&\n-\t\t\t\t\tgrep \"HEAD contents:\\s*.$headcontents.\" actual &&\n+\t\t\t\t\tgrep \"HEAD path: .$headpath.\" actual &&\n+\t\t\t\t\tgrep \"HEAD contents: .$headcontents.\" actual &&\n \t\t\t\t\tgrep \"$orphan_hint\" actual &&\n \t\t\t\t\t! grep \"$info_text\" actual\n \t\t\t\tfi &&\n-- \n2.41.0\n\n\n"},{"id":"479924","messageId":"xmqq4jlqcok9.fsf@gitster.g","threadId":"59988","inReplyTo":"20230726214202.15775-1-jacobabel@nullpo.dev","subject":"Re: [PATCH v4 0/3] t2400: Fix test failures when using grep 2.5","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-26T22:09:42Z","receivedAt":"2023-07-26T22:09:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Abel <jacobabel@nullpo.dev> writes:\n\n> This patchset is in response to build failures on GGG's Cirrus CI \n> freebsd_12 build jobs[1] and was prompted by a discussion thread [2].\n> These failures seem to be caused by the behavior outlined in [3]. \n\nLooking very good.\n\nWill queue.  Let's plan to merge it down to 'next' shortly.\n\nThanks.\n"},{"id":"479964","messageId":"70e35a52-17a0-2590-e94e-4fea70947777@gmail.com","threadId":"59988","inReplyTo":"xmqq4jlqcok9.fsf@gitster.g","subject":"Re: [PATCH v4 0/3] t2400: Fix test failures when using grep 2.5","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2023-07-28T13:09:21Z","receivedAt":"2023-07-28T13:09:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/07/2023 23:09, Junio C Hamano wrote:\n> Jacob Abel <jacobabel@nullpo.dev> writes:\n> \n>> This patchset is in response to build failures on GGG's Cirrus CI\n>> freebsd_12 build jobs[1] and was prompted by a discussion thread [2].\n>> These failures seem to be caused by the behavior outlined in [3].\n> \n> Looking very good.\n> \n> Will queue.  Let's plan to merge it down to 'next' shortly.\n> \n> Thanks.\n\nI read through the patches and agree they're looking good now, thanks Jacob.\n\nBest Wishes\n\nPhillip\n"}]}