{"thread":{"id":"30186","subject":"[PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","startedAt":"2012-04-09T01:36:54Z","lastAt":"2012-04-13T00:48:15Z","messageCount":17,"participants":["Ben Walton","Andreas Schwab","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"188790","messageId":"1333935414-10389-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"30186","inReplyTo":null,"subject":"[PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-09T01:36:54Z","receivedAt":"2012-04-09T01:36:54Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"The sed provided by Solaris in /usr/xpg4/bin has a bug whereby an\nunanchored regex using * for zero or more repetitions sees two\nseparate matches fed to the substitution engine in some cases.\n\nThis is evidenced by:\n\n$ for sed in /usr/xpg4/bin/sed /usr/bin/sed /opt/csw/gnu/sed; do \\\necho 'ab' | $sed -e 's|[a]*|X|g'; \\\ndone\nXXbX\nXbX\nXbX\n\nThis bug was triggered during a git submodule clone operation as\nexercised in the setup stage of t5526-fetch-submodules when using the\ndefault SANE_TOOL_PATH for Solaris.  It led to paths such as\n..../.. being used in the submodule .git gitdir reference.\n\nUsing the expression 's|\\([^/]*\\(/*\\)\\)|..\\2|g' provides the desired\nresult with all three three tested sed implementations but is harder\nto read.  Instead, use an additional -e script to clean up after the\nbug on Solaris.  The second script will be a functional no-op on most\nsed implementations.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n git-submodule.sh |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex efc86ad..0adad22 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -167,10 +167,12 @@ module_clone()\n \ta=${a%/}\n \tb=${b%/}\n \n-\trel=$(echo $b | sed -e 's|[^/]*|..|g')\n+\t# Note: The second -e is to work around a bug in Solairs'\n+\t# xpg4/sed.  It will be a no-op in a working implementation.\n+\trel=$(echo $b | sed -e 's|[^/]*|..|g' -e 's|\\.\\{4\\}|..|g')\n \techo \"gitdir: $rel/$a\" >\"$path/.git\"\n \n-\trel=$(echo $a | sed -e 's|[^/]*|..|g')\n+\trel=$(echo $a | sed -e 's|[^/]*|..|g' -e 's|\\.\\{4\\}|..|g')\n \t(clear_local_git_env; cd \"$path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n }\n \n-- \n1.7.9\n"},{"id":"188791","messageId":"m2sjgde7cs.fsf@linux-m68k.org","threadId":"30186","inReplyTo":"1333935414-10389-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-09T06:40:03Z","receivedAt":"2012-04-09T06:40:03Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Ben Walton <bwalton@artsci.utoronto.ca> writes:\n\n> The sed provided by Solaris in /usr/xpg4/bin has a bug whereby an\n> unanchored regex using * for zero or more repetitions sees two\n> separate matches fed to the substitution engine in some cases.\n>\n> This is evidenced by:\n>\n> $ for sed in /usr/xpg4/bin/sed /usr/bin/sed /opt/csw/gnu/sed; do \\\n> echo 'ab' | $sed -e 's|[a]*|X|g'; \\\n> done\n> XXbX\n> XbX\n> XbX\n>\n> This bug was triggered during a git submodule clone operation as\n> exercised in the setup stage of t5526-fetch-submodules when using the\n> default SANE_TOOL_PATH for Solaris.  It led to paths such as\n> ..../.. being used in the submodule .git gitdir reference.\n>\n> Using the expression 's|\\([^/]*\\(/*\\)\\)|..\\2|g' provides the desired\n\nHow about using 's|[^/][^/]*|..|g' instead, which should avoid the bug\nas well.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"188795","messageId":"1333977812-sup-4111@pinkfloyd.chass.utoronto.ca","threadId":"30186","inReplyTo":"m2sjgde7cs.fsf@linux-m68k.org","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-09T13:30:37Z","receivedAt":"2012-04-09T13:30:37Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Andreas Schwab's message of Mon Apr 09 02:40:03 -0400 2012:\n\n> How about using 's|[^/][^/]*|..|g' instead, which should avoid the bug\n> as well.\n\nI'd be ok with that change if the changed semantics of the regex are\nok in this application.  It's essentially the same as s|[^/]+|..|g,\nwhich requires at least one character.\n\nIn the current code, if you do:\n\necho '/' | sed -e 's|[^]*|..|g'\n\nyou get: ../.. (from a working implementation).\n\nYour regex would see the result  be: /\n\nI don't think we'd ever be passing a plain /, but we might pass a\nfully qualified path /path/to/foo, which would see the result change\nfrom ../../../.. to /../../.. and that could have unintended impact.\n\nI'd need to look more closely to see if this is a problem in reality.\nOthers more familiar with this code likely know the answer already.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"188797","messageId":"m2fwcdc57q.fsf@igel.home","threadId":"30186","inReplyTo":"1333977812-sup-4111@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-09T15:09:13Z","receivedAt":"2012-04-09T15:09:13Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Ben Walton <bwalton@artsci.utoronto.ca> writes:\n\n> Excerpts from Andreas Schwab's message of Mon Apr 09 02:40:03 -0400 2012:\n>\n>> How about using 's|[^/][^/]*|..|g' instead, which should avoid the bug\n>> as well.\n>\n> I'd be ok with that change if the changed semantics of the regex are\n> ok in this application.  It's essentially the same as s|[^/]+|..|g,\n> which requires at least one character.\n>\n> In the current code, if you do:\n>\n> echo '/' | sed -e 's|[^]*|..|g'\n>\n> you get: ../.. (from a working implementation).\n>\n> Your regex would see the result  be: /\n>\n> I don't think we'd ever be passing a plain /, but we might pass a\n> fully qualified path /path/to/foo, which would see the result change\n> from ../../../.. to /../../.. and that could have unintended impact.\n\nAFAICS the variables at this point never contain a value with a leading\nslash.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"188805","messageId":"7vbon0wxqs.fsf@alter.siamese.dyndns.org","threadId":"30186","inReplyTo":"m2fwcdc57q.fsf@igel.home","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-09T18:44:59Z","receivedAt":"2012-04-09T18:44:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab <schwab@linux-m68k.org> writes:\n\n> Ben Walton <bwalton@artsci.utoronto.ca> writes:\n>\n>> Excerpts from Andreas Schwab's message of Mon Apr 09 02:40:03 -0400 2012:\n>>\n>>> How about using 's|[^/][^/]*|..|g' instead, which should avoid the bug\n>>> as well.\n>>\n>> I'd be ok with that change if the changed semantics of the regex are\n>> ok in this application.  It's essentially the same as s|[^/]+|..|g,\n>> which requires at least one character.\n>>\n>> In the current code, if you do:\n>>\n>> echo '/' | sed -e 's|[^]*|..|g'\n>>\n>> you get: ../.. (from a working implementation).\n>>\n>> Your regex would see the result  be: /\n>>\n>> I don't think we'd ever be passing a plain /, but we might pass a\n>> fully qualified path /path/to/foo, which would see the result change\n>> from ../../../.. to /../../.. and that could have unintended impact.\n>\n> AFAICS the variables at this point never contain a value with a leading\n> slash.\n\nCorrect.  After $a and $b are initialized to \"$(cd somewhere ; pwd)/\",\nthere is a loop:\n\n\twhile test \"${a%%/*}\" = \"${b%%/*}\"\n\tdo\n\t\ta=${a#*/}\n\t\tb=${b#*/}\n\tdone\n\nThe test for the first iteration will see that they both start with '/' so\nstripping maximal trailing substring that match \"/*\" from them will yield\ntwo empty strings, and we always enter the body of the loop.  The body\nduring the first iteration will strip the leading '/'.\n\nWhen pwd computed for both $a and $b were at '/', this loop may do\nsomething funky ($b will become empty), but that is covered by an earlier\n\"is $a inside $b or vice versa?\" check, so we should be OK.\n"},{"id":"188806","messageId":"1333997276-sup-2760@pinkfloyd.chass.utoronto.ca","threadId":"30186","inReplyTo":"m2fwcdc57q.fsf@igel.home","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-09T18:47:58Z","receivedAt":"2012-04-09T18:47:58Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Andreas Schwab's message of Mon Apr 09 11:09:13 -0400 2012:\n\n> >> How about using 's|[^/][^/]*|..|g' instead, which should avoid the bug\n> >> as well.\n>\n> AFAICS the variables at this point never contain a value with a leading\n> slash.\n\nOk.  Junio also agrees with this and the test suite passes too. I'll\nsend in an updated patch shortly.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"188816","messageId":"1334002082-5375-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"30186","inReplyTo":"1333997276-sup-2760@pinkfloyd.chass.utoronto.ca","subject":"[PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-09T20:08:02Z","receivedAt":"2012-04-09T20:08:02Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"The sed provided by Solaris in /usr/xpg4/bin has a bug whereby an\nunanchored regex using * for zero or more repetitions sees two\nseparate matches fed to the substitution engine in some cases.\n\nThis is evidenced by:\n\n$ for sed in /usr/xpg4/bin/sed /usr/bin/sed /opt/csw/gnu/sed; do \\\necho 'ab' | $sed -e 's|[a]*|X|g'; \\\ndone\nXXbX\nXbX\nXbX\n\nThis bug was triggered during a git submodule clone operation as\nexercised in the setup stage of t5526-fetch-submodules when using the\ndefault SANE_TOOL_PATH for Solaris.  It led to paths such as\n..../.. being used in the submodule .git gitdir reference.\n\nUsing the expression 's|\\([^/]*\\(/*\\)\\)|..\\2|g' provides the desired\nresult with all three three tested sed implementations but is harder\nto read.  As we do not need to handle fully qualified paths though,\nthe expression could actually be [^/]+ which isn't properly handled\neither.  Instead, use [^/][^/]*, as suggested by Andreas Schwab, which\nworks on all three tested sed implementations.\n\nThe new expression is semantically different than the original one.\nIt will not place a leading '..' on a fully qualified path as the\noriginal expression did.  All of the paths being passed through this\nregex are relative and did not rely on this behaviour so it's a safe\nchange.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n git-submodule.sh |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex efc86ad..de972e7 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -167,10 +167,12 @@ module_clone()\n \ta=${a%/}\n \tb=${b%/}\n \n-\trel=$(echo $b | sed -e 's|[^/]*|..|g')\n+\t# Note: The extra [^/] is to work around a bug in Solairs'\n+\t# xpg4/sed.\n+\trel=$(echo $b | sed -e 's|[^/][^/]*|..|g')\n \techo \"gitdir: $rel/$a\" >\"$path/.git\"\n \n-\trel=$(echo $a | sed -e 's|[^/]*|..|g')\n+\trel=$(echo $a | sed -e 's|[^/][^/]*|..|g')\n \t(clear_local_git_env; cd \"$path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n }\n \n-- \n1.7.4.1\n"},{"id":"188819","messageId":"m2vcl8bon4.fsf@igel.home","threadId":"30186","inReplyTo":"1334002082-5375-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-09T21:07:11Z","receivedAt":"2012-04-09T21:07:11Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Ben Walton <bwalton@artsci.utoronto.ca> writes:\n\n> +\t# Note: The extra [^/] is to work around a bug in Solairs'\n\ns/Solairs/Solaris/\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"188821","messageId":"7vy5q4tw3i.fsf@alter.siamese.dyndns.org","threadId":"30186","inReplyTo":"m2vcl8bon4.fsf@igel.home","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-09T21:48:49Z","receivedAt":"2012-04-09T21:48:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab <schwab@linux-m68k.org> writes:\n\n> Ben Walton <bwalton@artsci.utoronto.ca> writes:\n>\n>> +\t# Note: The extra [^/] is to work around a bug in Solairs'\n>\n> s/Solairs/Solaris/\n\nTrue, but we do not have to single out the bug.  The resulting code is\neasier to understand as it always expect at least one letter between\nslashes, which holds true because we are handling output from pwd.\n\nI'd rewrite it to:\n\n\t# Turn each leading \"*/\" component into \"../\"\n\nor something like that.\n"},{"id":"188828","messageId":"1334016800-11574-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"30186","inReplyTo":"7vy5q4tw3i.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-10T00:13:20Z","receivedAt":"2012-04-10T00:13:20Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"The sed provided by Solaris in /usr/xpg4/bin has a bug whereby an\nunanchored regex using * for zero or more repetitions sees two\nseparate matches fed to the substitution engine in some cases.\n\nThis is evidenced by:\n\n$ for sed in /usr/xpg4/bin/sed /usr/bin/sed /opt/csw/gnu/sed; do \\\necho 'ab' | $sed -e 's|[a]*|X|g'; \\\ndone\nXXbX\nXbX\nXbX\n\nThis bug was triggered during a git submodule clone operation as\nexercised in the setup stage of t5526-fetch-submodules when using the\ndefault SANE_TOOL_PATH for Solaris.  It led to paths such as\n..../.. being used in the submodule .git gitdir reference.\n\nAs we do not need to handle fully qualfied paths we can make the\nregex match 1 or more instead of 0 or more non-/ characters so use\n's|[^/]\\{1,\\}|..|g' instead, which is correctly handled by all tested\nsed implementations.  This expression is semantically different than\nthe original one.  It will not place leading '..' on a fully qualified\npath as the original expression did.  None of the paths passed to the\nregex relied on this behaviour so changing it shouldn't have negative\nimpact.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n\nFor some reason, I thought that using {x,y} range notation wasn't\nvalid in this scenario, but I think I was mistaken.  It seems to be\nwidely used elsewhere throughout the code.  I prefer that to the\n[^/][^/]* notation and as they're equivalent, I switched to it.\n\n git-submodule.sh |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex efc86ad..2c18e0c 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -167,10 +167,11 @@ module_clone()\n \ta=${a%/}\n \tb=${b%/}\n \n-\trel=$(echo $b | sed -e 's|[^/]*|..|g')\n+\t# Turn path components into .. \n+\trel=$(echo $b | sed -e 's|[^/]\\{1,\\}|..|g')\n \techo \"gitdir: $rel/$a\" >\"$path/.git\"\n \n-\trel=$(echo $a | sed -e 's|[^/]*|..|g')\n+\trel=$(echo $a | sed -e 's|[^/]\\{1,\\}|..|g')\n \t(clear_local_git_env; cd \"$path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n }\n \n-- \n1.7.9\n"},{"id":"188830","messageId":"7vwr5os9zr.fsf@alter.siamese.dyndns.org","threadId":"30186","inReplyTo":"1334016800-11574-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-10T00:31:36Z","receivedAt":"2012-04-10T00:31:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Walton <bwalton@artsci.utoronto.ca> writes:\n\n> For some reason, I thought that using {x,y} range notation wasn't\n> valid in this scenario, but I think I was mistaken.  It seems to be\n> widely used elsewhere throughout the code.\n\nWhere?  Only two test scripts seem to use it and worse yet, one of them\nuses it doubly incorrectly.\n"},{"id":"188831","messageId":"1334018233-sup-7389@pinkfloyd.chass.utoronto.ca","threadId":"30186","inReplyTo":"7vwr5os9zr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-10T00:40:57Z","receivedAt":"2012-04-10T00:40:57Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Junio C Hamano's message of Mon Apr 09 20:31:36 -0400 2012:\n\n> > For some reason, I thought that using {x,y} range notation wasn't\n> > valid in this scenario, but I think I was mistaken.  It seems to be\n> > widely used elsewhere throughout the code.\n> \n> Where?  Only two test scripts seem to use it and worse yet, one of them\n> uses it doubly incorrectly.\n\nI guess widely wasn't a good choice...My search wasn't narrow enough\nearlier.  Was I correct in my original thinking that the range\nnotation isn't to be used then?\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"188833","messageId":"7vobr0s8ej.fsf@alter.siamese.dyndns.org","threadId":"30186","inReplyTo":"1334018233-sup-7389@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-10T01:05:56Z","receivedAt":"2012-04-10T01:05:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Walton <bwalton@artsci.utoronto.ca> writes:\n\n> I guess widely wasn't a good choice...My search wasn't narrow enough\n> earlier.  Was I correct in my original thinking that the range\n> notation isn't to be used then?\n\nSo far we have tried to stay within a narrow subset of BRE (see\nDocumentation/CodingGuidelines) in fear of upsetting exotic and/or ancient\nsystems. I do not mind reviewing and revising the guidelines every couple\nof years, but for this particular case, I do not think [^/][^/]* is too\nbad.  FWIW, it bothers me a lot more that expression does not anchor\nmatches against path elements with explicit '/' than the issue your patch\naddresses, i.e.\n\n\tsed -e 's|[^/][^/]*/|../|g'\n"},{"id":"188855","messageId":"m262d8j8dv.fsf@igel.home","threadId":"30186","inReplyTo":"7vobr0s8ej.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-04-10T08:31:08Z","receivedAt":"2012-04-10T08:31:08Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> FWIW, it bothers me a lot more that expression does not anchor\n> matches against path elements with explicit '/' than the issue your patch\n> addresses, i.e.\n>\n> \tsed -e 's|[^/][^/]*/|../|g'\n\nNote that this requires readding a trailing slash to the path, which\ncomplicates the whole thing a bit.  The effect of * has always been to\nuse the longest match.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"188881","messageId":"7vhawrqzue.fsf@alter.siamese.dyndns.org","threadId":"30186","inReplyTo":"m262d8j8dv.fsf@igel.home","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-10T16:10:22Z","receivedAt":"2012-04-10T16:10:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab <schwab@linux-m68k.org> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> FWIW, it bothers me a lot more that expression does not anchor\n>> matches against path elements with explicit '/' than the issue your patch\n>> addresses, i.e.\n>>\n>> \tsed -e 's|[^/][^/]*/|../|g'\n>\n> Note that this requires readding a trailing slash to the path, which\n> complicates the whole thing a bit.\n\nOr simply swapping order of the stripping and sed invocation, which is not\na big deal.  I personally find the pattern with trailing '/' easier to grok\nbut it is also not a big deal.\n"},{"id":"189168","messageId":"1334274951-sup-6013@pinkfloyd.chass.utoronto.ca","threadId":"30186","inReplyTo":"7vhawrqzue.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-13T00:46:52Z","receivedAt":"2012-04-13T00:46:52Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Junio C Hamano's message of Tue Apr 10 12:10:22 -0400 2012:\n> Andreas Schwab <schwab@linux-m68k.org> writes:\n> \n> > Junio C Hamano <gitster@pobox.com> writes:\n> >\n> >> FWIW, it bothers me a lot more that expression does not anchor\n> >> matches against path elements with explicit '/' than the issue your patch\n> >> addresses, i.e.\n> >>\n> >>     sed -e 's|[^/][^/]*/|../|g'\n> >\n> > Note that this requires readding a trailing slash to the path, which\n> > complicates the whole thing a bit.\n> \n> Or simply swapping order of the stripping and sed invocation, which\n> is not a big deal.  I personally find the pattern with trailing '/'\n> easier to grok but it is also not a big deal.\n\nSorry for the delay here, I haven't had a chance to look at this in a\nfew days.  I tried the version with [^/][^/]*/ tonight and it was more\ninvasive than required for such a simple fix, I think.\n\nSwitching the order of the calls is ok, but then you need to use a\nrela and relb as the second substitution relies on $a being\n/-terminated and the creation of gitdir:... requires the opposite.  So\nnow, you're switching the order, moving other lines, renaming\nvariables and also altering the lines used to spit out the paths in\norder to avoid also stripping the / from the rel* variables.\n\nUpdated patch to follow, but please don't hesitate to request the\nalternate version if it really is what you'd like to see.  (I've done\nthe work already.)\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"189169","messageId":"1334278095-20212-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"30186","inReplyTo":"1334274951-sup-6013@pinkfloyd.chass.utoronto.ca","subject":"[PATCH] Avoid bug in Solaris xpg4/sed as used in submodule","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-13T00:48:15Z","receivedAt":"2012-04-13T00:48:15Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"The sed provided by Solaris in /usr/xpg4/bin has a bug whereby an\nunanchored regex using * for zero or more repetitions sees two\nseparate matches fed to the substitution engine in some cases.\n\nThis is evidenced by:\n\n$ for sed in /usr/xpg4/bin/sed /usr/bin/sed /opt/csw/gnu/sed; do \\\necho 'ab' | $sed -e 's|[a]*|X|g'; \\\ndone\nXXbX\nXbX\nXbX\n\nThis bug was triggered during a git submodule clone operation as\nexercised in the setup stage of t5526-fetch-submodules when using the\ndefault SANE_TOOL_PATH for Solaris.  It led to paths such as\n..../.. being used in the submodule .git gitdir reference.\n\nAs we do not need to handle fully qualfied paths we can make the regex\nmatch 1 or more instead of 0 or more non-/ characters so use\n's|[^/][^/]*|..|g' instead, which is correctly handled by all tested\nsed implementations.  This expression is semantically different than\nthe original one.  It will not place leading '..' on a fully qualified\npath as the original expression did.  None of the paths passed to the\nregex relied on this behaviour so changing it shouldn't have negative\nimpact.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n git-submodule.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex efc86ad..7aa9e95 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -167,10 +167,10 @@ module_clone()\n \ta=${a%/}\n \tb=${b%/}\n \n-\trel=$(echo $b | sed -e 's|[^/]*|..|g')\n+\trel=$(echo $b | sed -e 's|[^/][^/]*|..|g')\n \techo \"gitdir: $rel/$a\" >\"$path/.git\"\n \n-\trel=$(echo $a | sed -e 's|[^/]*|..|g')\n+\trel=$(echo $a | sed -e 's|[^/][^/]*|..|g')\n \t(clear_local_git_env; cd \"$path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n }\n \n-- \n1.7.9\n"}]}