{"thread":{"id":"30666","subject":"Bug: rebase when an author uses accents in name on MacOSx","startedAt":"2012-05-30T22:16:53Z","lastAt":"2012-06-02T16:37:10Z","messageCount":21,"participants":["Lanny Ripple","Junio C Hamano","Jürgen Kreileder","Jeff King","Thomas Rast"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"192521","messageId":"06DD2F56-F956-46DF-84A4-3443D4702CDE@spotinfluence.com","threadId":"30666","inReplyTo":null,"subject":"Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-05-30T22:16:53Z","receivedAt":"2012-05-30T22:16:53Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"Hello,\n\nI've come across behavior I'd say is a bug.\n\nWe have a developer with an accent in his name, Rémi Leblond.  Very recently we stopped being able to rebase getting the error\n\n  lanny(master);<work/IdeaProjects/Piper> git rebase master rl-clean292\n  First, rewinding head to replay your work on top of it...\n  /sw/lib/git-core/git-am: line 692: Leblond: command not found\n  Patch does not have a valid e-mail address.\n  lanny((ae6c220...)|REBASE);<work/IdeaProjects/Piper> \n\nVersions 1.7.10.2 is (now?) exhibiting this behavior.  We've been rebasing fine for several month which is why I wonder if being on MacOSX is involved?  (I'm at 10.7.4)  Some digging shows the root cause to be in function get_author_ident_from_commit at line 210 of git-core/git-sh-setup, namely the sed with environment overrides LANG=C LC_ALL=C.  This causes git-am to incorrectly build the .git/rebase-apply/author-script.\n\n  lanny((ae6c220...)|REBASE);<work/IdeaProjects/Piper> cat .git/rebase-apply/author-script\n  GIT_AUTHOR_NAME='R'émi Leblond\n  GIT_AUTHOR_EMAIL='remi@spotinfluence.com'\n  GIT_AUTHOR_DATE='@1335301038 -0600'\n\nFrom the command-line\n\n  lanny;~> echo $LANG\n  en_US.UTF-8\n  lanny;~> echo $LC_ALL\n\n  lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n  GIT_AUTHOR_NAME='R'émi Leblond\n  lanny;~> echo \"Rémi Leblond\" | sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n  GIT_AUTHOR_NAME='Rémi Leblond'\n\nI can work around it easily enough by editing git-sh-setup to remove the locale overrides but thought a bug report might be useful.\n\nEnjoy,\n  -ljr\n\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\n"},{"id":"192527","messageId":"7vehq18c82.fsf@alter.siamese.dyndns.org","threadId":"30666","inReplyTo":"06DD2F56-F956-46DF-84A4-3443D4702CDE@spotinfluence.com","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-30T23:45:33Z","receivedAt":"2012-05-30T23:45:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lanny Ripple <lanny@spotinfluence.com> writes:\n\n>   lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n>   GIT_AUTHOR_NAME='R'émi Leblond\n\nSo in C locale where each byte is supposed to be a single character,\nthat implementation of \"sed\" refuses to match a byte with high-bit\nset when given a pattern '.'?\n\nThat is a surprising breakage, I would have to say.\n\nCan anybody with a more vanilla BSD try the above out and report\nwhat happens?  I am mostly interested to see if this was inherited\nfrom BSD or something MacOS introduced.\n"},{"id":"192528","messageId":"CAKD0UuwGptoOQOT9eBEpd_aXes76CdaijYvnvyWGU99ZNz=v2g@mail.gmail.com","threadId":"30666","inReplyTo":"7vehq18c82.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Jürgen Kreileder","fromEmail":"jk@blackdown.de","sentAt":"2012-05-30T23:57:52Z","receivedAt":"2012-05-30T23:57:52Z","isPatch":false,"sender":{"key":"jk@blackdown.de","avatar":"https://avatars.githubusercontent.com/u/4390544?v=4"},"body":"On Thu, May 31, 2012 at 1:45 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Lanny Ripple <lanny@spotinfluence.com> writes:\n>\n> >   lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n> >   GIT_AUTHOR_NAME='R'émi Leblond\n>\n> So in C locale where each byte is supposed to be a single character,\n> that implementation of \"sed\" refuses to match a byte with high-bit\n> set when given a pattern '.'?\n>\n> That is a surprising breakage, I would have to say.\n\nFWIW, the command above correctly returns GIT_AUTHOR_NAME='Rémi\nLeblond' for me on OS X 10.8.\n\n\nJürgen\n\n--\nhttps://blackdown.de/\nhttp://www.flickr.com/photos/jkreileder/\n"},{"id":"192530","messageId":"20120531011911.GC5488@sigill.intra.peff.net","threadId":"30666","inReplyTo":"7vehq18c82.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-31T01:19:11Z","receivedAt":"2012-05-31T01:19:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 30, 2012 at 04:45:33PM -0700, Junio C Hamano wrote:\n\n> Lanny Ripple <lanny@spotinfluence.com> writes:\n> \n> >   lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n> >   GIT_AUTHOR_NAME='R'émi Leblond\n> \n> So in C locale where each byte is supposed to be a single character,\n> that implementation of \"sed\" refuses to match a byte with high-bit\n> set when given a pattern '.'?\n> \n> That is a surprising breakage, I would have to say.\n\nIt should not be too surprising, since we discussed it a few months ago:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/192218\n\nThomas provided a gross but workable solution here:\n\n  http://article.gmane.org/gmane.comp.version-control.git/192237\n\nand we also talked about eventually having a shell-quoting mechanism for\npretty placeholders.  Then the discussion rambled into \"this sed is\nhorribly broken, and the user should get a better sed\" territory. Maybe\nwe need to revisit that decision, since this is now two bug reports.\n\n-Peff\n"},{"id":"192537","messageId":"7v62bc97w1.fsf@alter.siamese.dyndns.org","threadId":"30666","inReplyTo":"20120531011911.GC5488@sigill.intra.peff.net","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-31T06:33:50Z","receivedAt":"2012-05-31T06:33:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, May 30, 2012 at 04:45:33PM -0700, Junio C Hamano wrote:\n> ...\n>> So in C locale where each byte is supposed to be a single character,\n>> that implementation of \"sed\" refuses to match a byte with high-bit\n>> set when given a pattern '.'?\n>> \n>> That is a surprising breakage, I would have to say.\n>\n> It should not be too surprising, since we discussed it a few months ago:\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/192218\n\nHeh, no wonder I do not recall that one, as everything happened and\nconclusions reached while I was sleeping ;-)\n\nIf it is not a bug in platform-sanctioned sed, but a buggy\nthird-party build, then I wouldn't worry about it for this cycle,\nbut pre-release freeze might be a good time to start sketching\nThomas's --format=\"%'%an <%ae>%'\" approach, perhaps?\n"},{"id":"192551","messageId":"87pq9k66f4.fsf@thomas.inf.ethz.ch","threadId":"30666","inReplyTo":"20120531011911.GC5488@sigill.intra.peff.net","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-05-31T09:33:51Z","receivedAt":"2012-05-31T09:33:51Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, May 30, 2012 at 04:45:33PM -0700, Junio C Hamano wrote:\n>\n>> Lanny Ripple <lanny@spotinfluence.com> writes:\n>> \n>> >   lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n>> >   GIT_AUTHOR_NAME='R'émi Leblond\n>> \n>> So in C locale where each byte is supposed to be a single character,\n>> that implementation of \"sed\" refuses to match a byte with high-bit\n>> set when given a pattern '.'?\n>> \n>> That is a surprising breakage, I would have to say.\n>\n> It should not be too surprising, since we discussed it a few months ago:\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/192218\n>\n> Thomas provided a gross but workable solution here:\n>\n>   http://article.gmane.org/gmane.comp.version-control.git/192237\n>\n> and we also talked about eventually having a shell-quoting mechanism for\n> pretty placeholders.  Then the discussion rambled into \"this sed is\n> horribly broken, and the user should get a better sed\" territory. Maybe\n> we need to revisit that decision, since this is now two bug reports.\n\nThree actually, also counting Will Palmer (shruggar) who brought this up\non #git-devel shortly before the thread above.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"192571","messageId":"78E7FEAC-6587-4EB3-B78B-08D321F53BF9@spotinfluence.com","threadId":"30666","inReplyTo":"7v62bc97w1.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-05-31T13:36:16Z","receivedAt":"2012-05-31T13:36:16Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"Bingo.\n\n  lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C /usr/bin/sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n  GIT_AUTHOR_NAME='Rémi Leblond'\n\nJust occurred to me that I'm using fink and that git-am doesn't use /usr/bin/sed but just sed.  My suggestion is to be explicit on the path in git-am.\n\nSo it now stands at two bug-reports and one pebkac.\n\n  Thanks!,\n  -ljr\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\nOn May 31, 2012, at 1:33 AM, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n>> On Wed, May 30, 2012 at 04:45:33PM -0700, Junio C Hamano wrote:\n>> ...\n>>> So in C locale where each byte is supposed to be a single character,\n>>> that implementation of \"sed\" refuses to match a byte with high-bit\n>>> set when given a pattern '.'?\n>>> \n>>> That is a surprising breakage, I would have to say.\n>> \n>> It should not be too surprising, since we discussed it a few months ago:\n>> \n>>  http://thread.gmane.org/gmane.comp.version-control.git/192218\n> \n> Heh, no wonder I do not recall that one, as everything happened and\n> conclusions reached while I was sleeping ;-)\n> \n> If it is not a bug in platform-sanctioned sed, but a buggy\n> third-party build, then I wouldn't worry about it for this cycle,\n> but pre-release freeze might be a good time to start sketching\n> Thomas's --format=\"%'%an <%ae>%'\" approach, perhaps?\n\n"},{"id":"192582","messageId":"87bol4ifvn.fsf@thomas.inf.ethz.ch","threadId":"30666","inReplyTo":"78E7FEAC-6587-4EB3-B78B-08D321F53BF9@spotinfluence.com","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-05-31T14:28:44Z","receivedAt":"2012-05-31T14:28:44Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Lanny Ripple <lanny@spotinfluence.com> writes:\n\n> Bingo.\n>\n>   lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C /usr/bin/sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n>   GIT_AUTHOR_NAME='Rémi Leblond'\n>\n> Just occurred to me that I'm using fink and that git-am doesn't use\n> /usr/bin/sed but just sed.  My suggestion is to be explicit on the\n> path in git-am.\n\nThen how would you work around a platform sed that is broken?  You can't\njust install a new one in another directory if we use an absolute path.\nWhich apparently is what people have to do on Solaris, see the\nSANE_TOOL_PATH setting for the Makefile.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"192592","messageId":"938B14B8-EA46-492B-A8C1-15C5E93F22AC@spotinfluence.com","threadId":"30666","inReplyTo":"87bol4ifvn.fsf@thomas.inf.ethz.ch","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-05-31T14:56:45Z","receivedAt":"2012-05-31T14:56:45Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"A platform sed that is broken would be worked around by recognizing the platform.  If the bulk of platforms are sane then fixing the path for the platform is the easiest solution.  If the bulk of platforms don't have a sane sed then not using sed is probably the solution.  If you want to make the path variable then a platform config file built automatically figuring out default paths that could be changed by the host's admin comes to mind (much like, oh I don't know, building a list of shell variables named author-script to use during rebases).\n\nYou could modify the name, email, and timestamp so they didn't have to be parsed.  Separate lines in the header for name, email, and date would work.  A length encoding of the three fields at the beginning of the header line or in some other (new) location if backwards compatibility is a concern.  There's lots of rather straight-forward technical solutions.\n\nNow if what you are saying is the social cost of applying any of those solutions is too high that's a horse of a different color.\n\nAnyway thanks to everyone for the clue that got me working again,\n  -ljr\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\nOn May 31, 2012, at 9:28 AM, Thomas Rast wrote:\n\n> Lanny Ripple <lanny@spotinfluence.com> writes:\n> \n>> Bingo.\n>> \n>>  lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C /usr/bin/sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n>>  GIT_AUTHOR_NAME='Rémi Leblond'\n>> \n>> Just occurred to me that I'm using fink and that git-am doesn't use\n>> /usr/bin/sed but just sed.  My suggestion is to be explicit on the\n>> path in git-am.\n> \n> Then how would you work around a platform sed that is broken?  You can't\n> just install a new one in another directory if we use an absolute path.\n> Which apparently is what people have to do on Solaris, see the\n> SANE_TOOL_PATH setting for the Makefile.\n> \n> -- \n> Thomas Rast\n> trast@{inf,student}.ethz.ch\n\n"},{"id":"192602","messageId":"7vtxyw6ypx.fsf@alter.siamese.dyndns.org","threadId":"30666","inReplyTo":"78E7FEAC-6587-4EB3-B78B-08D321F53BF9@spotinfluence.com","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-31T17:34:50Z","receivedAt":"2012-05-31T17:34:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lanny Ripple <lanny@spotinfluence.com> writes:\n\n> Bingo.\n>\n>   lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C /usr/bin/sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n>   GIT_AUTHOR_NAME='Rémi Leblond'\n>\n> Just occurred to me that I'm using fink and that git-am doesn't use /usr/bin/sed but just sed.  My suggestion is to be explicit on the path in git-am.\n>\n> So it now stands at two bug-reports and one pebkac.\n\nMy impression from reading the older thread Peff mentioned is that\nthe other ones are also broken implementation of sed supplied by\nthird-party, so it probably is not two Xs and one Y.  It is three\nsomething.\n"},{"id":"192603","messageId":"69ED148F-BD10-4DE3-91F2-D31F83A0EAB7@spotinfluence.com","threadId":"30666","inReplyTo":"7vtxyw6ypx.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-05-31T17:49:16Z","receivedAt":"2012-05-31T17:49:16Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"Perhaps the error message in git-am could be modified to indicate sed is a suspect?.  E.g.,\n\n lanny(master);<work/IdeaProjects/Piper> git rebase master rl-clean292\n First, rewinding head to replay your work on top of it...\n /sw/lib/git-core/git-am: line 692: Leblond: command not found\n Patch does not have a valid e-mail address.  (Used /sw/bin/sed found on PATH).\n                                              ^  ^  ^  ^  ^  ^  ^  ^  ^  ^  ^\n lanny((ae6c220...)|REBASE);<work/IdeaProjects/Piper> \n\nAlternately in git-am a comment at the error emit point explaining that third-party seds are often the problem for this error and a suggestion to modify the PATH so the platform sed occurs first.\n\n  Regards,\n  -ljr\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\nOn May 31, 2012, at 12:34 PM, Junio C Hamano wrote:\n\n> Lanny Ripple <lanny@spotinfluence.com> writes:\n> \n>> Bingo.\n>> \n>>  lanny;~> echo \"Rémi Leblond\" | LANG=C LC_ALL=C /usr/bin/sed -ne 's/.*/GIT_AUTHOR_NAME='\\''&'\\''/p'\n>>  GIT_AUTHOR_NAME='Rémi Leblond'\n>> \n>> Just occurred to me that I'm using fink and that git-am doesn't use /usr/bin/sed but just sed.  My suggestion is to be explicit on the path in git-am.\n>> \n>> So it now stands at two bug-reports and one pebkac.\n> \n> My impression from reading the older thread Peff mentioned is that\n> the other ones are also broken implementation of sed supplied by\n> third-party, so it probably is not two Xs and one Y.  It is three\n> something.\n\n"},{"id":"192612","messageId":"7vd35k6w0i.fsf@alter.siamese.dyndns.org","threadId":"30666","inReplyTo":"69ED148F-BD10-4DE3-91F2-D31F83A0EAB7@spotinfluence.com","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-31T18:33:17Z","receivedAt":"2012-05-31T18:33:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lanny Ripple <lanny@spotinfluence.com> writes:\n\n> Perhaps the error message in git-am could be modified to indicate\n> sed is a suspect?.  E.g.,\n>\n>  lanny(master);<work/IdeaProjects/Piper> git rebase master rl-clean292\n>  First, rewinding head to replay your work on top of it...\n>  /sw/lib/git-core/git-am: line 692: Leblond: command not found\n>  Patch does not have a valid e-mail address.  (Used /sw/bin/sed found on PATH).\n>                                               ^  ^  ^  ^  ^  ^  ^  ^  ^  ^  ^\n>  lanny((ae6c220...)|REBASE);<work/IdeaProjects/Piper> \n\nHrm, that does not sound an attractive way going forward.\n\nDo we have to suspect any and all uses of POSIX tools, just in case\nsomebody installs a broken implementation from random places?  Is\nsed the only thing that is possibly broken?\n\nBy the way, have you filed a bug report to whoever supplied your /sw/bin/sed?\n"},{"id":"192614","messageId":"168277BB-0E71-4987-A2BE-6202034A96F1@spotinfluence.com","threadId":"30666","inReplyTo":"7vd35k6w0i.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-05-31T19:21:16Z","receivedAt":"2012-05-31T19:21:16Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"You have three recent instances where people have bumped into this with sed.  (And yes on reporting it to the packaging project.)  It seems to me leaving a breadcrumb so that folks can figure out what's going on without having to bother the list would be a win for everyone.  And yes, if any and all uses of POSIX tools started showing up with some frequency then I think the same breadcrumb win/win logic would apply.\n\n  Enjoy,\n  -ljr\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\nOn May 31, 2012, at 1:33 PM, Junio C Hamano wrote:\n\n> Lanny Ripple <lanny@spotinfluence.com> writes:\n> \n>> Perhaps the error message in git-am could be modified to indicate\n>> sed is a suspect?.  E.g.,\n>> \n>> lanny(master);<work/IdeaProjects/Piper> git rebase master rl-clean292\n>> First, rewinding head to replay your work on top of it...\n>> /sw/lib/git-core/git-am: line 692: Leblond: command not found\n>> Patch does not have a valid e-mail address.  (Used /sw/bin/sed found on PATH).\n>>                                              ^  ^  ^  ^  ^  ^  ^  ^  ^  ^  ^\n>> lanny((ae6c220...)|REBASE);<work/IdeaProjects/Piper> \n> \n> Hrm, that does not sound an attractive way going forward.\n> \n> Do we have to suspect any and all uses of POSIX tools, just in case\n> somebody installs a broken implementation from random places?  Is\n> sed the only thing that is possibly broken?\n> \n> By the way, have you filed a bug report to whoever supplied your /sw/bin/sed?\n> \n> \n\n"},{"id":"192642","messageId":"20120601093039.GD32340@sigill.intra.peff.net","threadId":"30666","inReplyTo":"168277BB-0E71-4987-A2BE-6202034A96F1@spotinfluence.com","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-01T09:30:39Z","receivedAt":"2012-06-01T09:30:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[Please don't top-post.]\n\nOn Thu, May 31, 2012 at 02:21:16PM -0500, Lanny Ripple wrote:\n\n> >> Perhaps the error message in git-am could be modified to indicate\n> >> sed is a suspect?.  E.g.,\n> [...]\n> > Hrm, that does not sound an attractive way going forward.\n> [...]\n> You have three recent instances where people have bumped into this\n> with sed.  (And yes on reporting it to the packaging project.)  It\n> seems to me leaving a breadcrumb so that folks can figure out what's\n> going on without having to bother the list would be a win for\n> everyone.\n\nBut you have to keep in mind all of the people who will be led down the\nwrong path by your breadcrumb when the failure is caused by a\n_different_ problem. What is the probability that it is helpful versus\nnot helpful?  If you are going to give advice that sed might be broken,\nyou should at least test to see if it is broken and report it.\n\nBut really, I'd rather just see the broken sed fixed. Where would the\nbreadcrumb lead people at this point, anyway? We don't actually have a\nsolution besides \"uninstall this other, crappy sed\". Has the sed bug\nactually been fixed?\n\n-Peff\n"},{"id":"192662","messageId":"B0BB0BD8-EFB5-4A3C-A6F7-00A1C7DE0509@spotinfluence.com","threadId":"30666","inReplyTo":"20120601093039.GD32340@sigill.intra.peff.net","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-06-01T13:56:01Z","receivedAt":"2012-06-01T13:56:01Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"I did show that sed was broken and have provided a minimal, reproducible test.\n\nI have reported it to the sed maintainers and they are working on it.\n\nA message or comment in the code that seds not properly handling utf8 characters have been shown to be the cause of the problem and that git selects sed from the PATH would have been 100% effective in at least one case.  I don't know the troubleshooting skills of the other two people that bumped into the problem so can't comment.  Of the billions of people that have not (if it existed) looked at the breadcrumb and weren't led astray it's (would have) also been 100% effective.  Can you in turn posit any reasonable way that get_author_ident_from_commit would improperly build author-script short of a bad sed?  I guess you could pull out transient or systematic disk error.\n\nYou do, in fact, have several solutions.  I won't reiterate since they are in the thread earlier.  You also have in many cases the valid concern that the solutions would not be backwards compatible.  And yes, this sed will get fixed but what then?  The next person that gets a sed they don't expect earlier in their PATH will have to go through the same steps.\n\nI do appreciate the assistance that led to the solution to my problem.  Thanks for maintaining and making available such a great piece of software.\n\nRegards,\n  -ljr\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\nOn Jun 1, 2012, at 4:30 AM, Jeff King wrote:\n\n> [Please don't top-post.]\n> \n> On Thu, May 31, 2012 at 02:21:16PM -0500, Lanny Ripple wrote:\n> \n>>>> Perhaps the error message in git-am could be modified to indicate\n>>>> sed is a suspect?.  E.g.,\n>> [...]\n>>> Hrm, that does not sound an attractive way going forward.\n>> [...]\n>> You have three recent instances where people have bumped into this\n>> with sed.  (And yes on reporting it to the packaging project.)  It\n>> seems to me leaving a breadcrumb so that folks can figure out what's\n>> going on without having to bother the list would be a win for\n>> everyone.\n> \n> But you have to keep in mind all of the people who will be led down the\n> wrong path by your breadcrumb when the failure is caused by a\n> _different_ problem. What is the probability that it is helpful versus\n> not helpful?  If you are going to give advice that sed might be broken,\n> you should at least test to see if it is broken and report it.\n> \n> But really, I'd rather just see the broken sed fixed. Where would the\n> breadcrumb lead people at this point, anyway? We don't actually have a\n> solution besides \"uninstall this other, crappy sed\". Has the sed bug\n> actually been fixed?\n> \n> -Peff\n\n"},{"id":"192671","messageId":"7vmx4n3sz5.fsf@alter.siamese.dyndns.org","threadId":"30666","inReplyTo":"20120601093039.GD32340@sigill.intra.peff.net","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-01T16:19:26Z","receivedAt":"2012-06-01T16:19:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n> [Please don't top-post.]\n> ...\n> But you have to keep in mind all of the people who will be led down the\n> wrong path by your breadcrumb when the failure is caused by a\n> _different_ problem. What is the probability that it is helpful versus\n> not helpful?  If you are going to give advice that sed might be broken,\n> you should at least test to see if it is broken and report it.\n\nEek, do that at runtime in the error code path?\n\nAdd something like\n\n\tsuspected_sed_breakage () {\n\t\txxxxx=$(printf \"\\370\\235\\204\\236\\n\" | LC_CTYPE=C sed 's/./x/g')\n                if test \"x$xxxxx\" != \"xxxxx\"\n                then\n\t\t\tdie \"Your sed is broken; cannot run $1\"\n\t\tfi\n\t}\n\nto git-sh-setup, and do something like:\n\n\t. \"$dotest/author-script\" || suspected_sed_breakage \"$0\"\n\nin git-am?\n\nThe problem I see is that at that point where we have to suspect\nsomething fundamental as sed broken on the platform, we cannot even\ntrust printf, test, or even the shell itself behaving sanely.\n\nSo I would say, although it is a fun thought-experiment, such a test\nand breadcrumb is not really worth it.\n"},{"id":"192679","messageId":"0A6ABBD7-01D5-4A24-BF0F-78A7F2C46938@spotinfluence.com","threadId":"30666","inReplyTo":"7vmx4n3sz5.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-06-01T17:05:36Z","receivedAt":"2012-06-01T17:05:36Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"I still think the best solution is figuring out if the platform sed is sane at build time and using a full path (via config setup if being able to change the sed used is a priority).  Short of that something as simple as\n\n(git-am:699+)\n\n   if test -z \"$GIT_AUTHOR_EMAIL\"\n   then\n           # Can occur when sed in PATH will not handle UTF8 under LC_ALL=C.\n           gettextln \"Patch does not have a valid e-mail address.\"\n           stop_here $this\n   fi\n\nwould give folks trying to troubleshoot the problem a clue to what was going on.  From the fink developers' list it seems Darwin and perhaps FreeBSD use US-ASCII for LC_ALL=C or POSIX which is why Gnu sed gets it wrong.\n\nMy problem is still fixed whatever is decided.  Enjoy,\n  -ljr\n\t\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\nOn Jun 1, 2012, at 11:19 AM, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n>> [Please don't top-post.]\n>> ...\n>> But you have to keep in mind all of the people who will be led down the\n>> wrong path by your breadcrumb when the failure is caused by a\n>> _different_ problem. What is the probability that it is helpful versus\n>> not helpful?  If you are going to give advice that sed might be broken,\n>> you should at least test to see if it is broken and report it.\n> \n> Eek, do that at runtime in the error code path?\n> \n> Add something like\n> \n> \tsuspected_sed_breakage () {\n> \t\txxxxx=$(printf \"\\370\\235\\204\\236\\n\" | LC_CTYPE=C sed 's/./x/g')\n>                if test \"x$xxxxx\" != \"xxxxx\"\n>                then\n> \t\t\tdie \"Your sed is broken; cannot run $1\"\n> \t\tfi\n> \t}\n> \n> to git-sh-setup, and do something like:\n> \n> \t. \"$dotest/author-script\" || suspected_sed_breakage \"$0\"\n> \n> in git-am?\n> \n> The problem I see is that at that point where we have to suspect\n> something fundamental as sed broken on the platform, we cannot even\n> trust printf, test, or even the shell itself behaving sanely.\n> \n> So I would say, although it is a fun thought-experiment, such a test\n> and breadcrumb is not really worth it.\n> \n\n"},{"id":"192681","messageId":"7vmx4m3og5.fsf@alter.siamese.dyndns.org","threadId":"30666","inReplyTo":"0A6ABBD7-01D5-4A24-BF0F-78A7F2C46938@spotinfluence.com","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-01T17:57:14Z","receivedAt":"2012-06-01T17:57:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lanny Ripple <lanny@spotinfluence.com> writes:\n\n> I still think the best solution is figuring out if the platform sed is sane at build time and using a full path (via config setup if being able to change the sed used is a priority).  Short of that something as simple as\n>\n> (git-am:699+)\n>\n>    if test -z \"$GIT_AUTHOR_EMAIL\"\n>    then\n>            # Can occur when sed in PATH will not handle UTF8 under LC_ALL=C.\n>            gettextln \"Patch does not have a valid e-mail address.\"\n>            stop_here $this\n>    fi\n\nThat is a real fix to the issue (can also happen to name, no?) and\nis worth checking.\n"},{"id":"192716","messageId":"20120602160904.GB15017@sigill.intra.peff.net","threadId":"30666","inReplyTo":"B0BB0BD8-EFB5-4A3C-A6F7-00A1C7DE0509@spotinfluence.com","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T16:09:04Z","receivedAt":"2012-06-02T16:09:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 01, 2012 at 08:56:01AM -0500, Lanny Ripple wrote:\n\n> I did show that sed was broken and have provided a minimal, reproducible test.\n> \n> I have reported it to the sed maintainers and they are working on it.\n\nGreat. Do we know yet which versions are affected?\n\n> A message or comment in the code that seds not properly handling utf8\n> characters have been shown to be the cause of the problem and that git\n> selects sed from the PATH would have been 100% effective in at least\n> one case.  I don't know the troubleshooting skills of the other two\n> people that bumped into the problem so can't comment.  Of the billions\n> of people that have not (if it existed) looked at the breadcrumb and\n> weren't led astray it's (would have) also been 100% effective.  Can\n> you in turn posit any reasonable way that get_author_ident_from_commit\n> would improperly build author-script short of a bad sed?  I guess you\n> could pull out transient or systematic disk error.\n\nI assume from bogus commit objects. But I admit I am just guessing, and\ndon't have data.\n\n> You do, in fact, have several solutions.  I won't reiterate since they\n> are in the thread earlier.  You also have in many cases the valid\n> concern that the solutions would not be backwards compatible.  And\n> yes, this sed will get fixed but what then?  The next person that gets\n> a sed they don't expect earlier in their PATH will have to go through\n> the same steps.\n\nWhen I said:\n\n> > But really, I'd rather just see the broken sed fixed. Where would the\n> > breadcrumb lead people at this point, anyway? We don't actually have a\n> > solution besides \"uninstall this other, crappy sed\". Has the sed bug\n> > actually been fixed?\n\nI meant that there is not a fix for the _user_ to perform at that point.\nThe point of a breadcrumb like that is that we are not going to put a\nfix into git, so we want to at least give the user a clue that their\nsystem has a problem. But what is their next step after being informed\nthat their system has a problem?\n\n-Peff\n"},{"id":"192717","messageId":"20120602162336.GC15017@sigill.intra.peff.net","threadId":"30666","inReplyTo":"7vmx4n3sz5.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-02T16:23:36Z","receivedAt":"2012-06-02T16:23:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 01, 2012 at 09:19:26AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> > [Please don't top-post.]\n> > ...\n> > But you have to keep in mind all of the people who will be led down the\n> > wrong path by your breadcrumb when the failure is caused by a\n> > _different_ problem. What is the probability that it is helpful versus\n> > not helpful?  If you are going to give advice that sed might be broken,\n> > you should at least test to see if it is broken and report it.\n> \n> Eek, do that at runtime in the error code path?\n\nYes, which I think is utterly gross, but at least it is on the error\ncode path, so most people will never run it.\n\nIt's slightly less gross to do it at build-time (or even as part of\nconfigure, I guess), but of course it is a run-time dependency, so there\nis nothing to stop the broken sed from being installed after git (or\neven a user with a different PATH than the builder triggering it).\n\nOne gross thing about doing it at run-time is that it only affects\n_this_ code path, which happens to have an easy-to-diagnose outcome from\nthe bug. But how many other code paths are using sed on data which might\ncontain utf-8 characters? And are they failing silently or otherwise\nsimply corrupting the output?\n\nOne other thing to think about: this particular manifestation of the\nbug is to cause our shell-quoting to fail. Are there are security\nimplications? That is, can I execute arbitrary code by having you run\nget_author_ident_from_commit on a specially-crafted commit?\n\n> The problem I see is that at that point where we have to suspect\n> something fundamental as sed broken on the platform, we cannot even\n> trust printf, test, or even the shell itself behaving sanely.\n\nI don't think that's true. We have to deal with this kind of portability\nnonsense all the time. We assume that the tools work until we get a\nreport that they don't, and then we fix it, work around it, or whatever.\nYes, the \"your sed is broken\" test would not work if \"test\" is also\ntotally broken. But we have not seen such a system in real life, or have\nreason to suspect that it exists. Whereas we do know there is a\ncommon[1] platform where the sed is broken in a specific way, but\nnothing else is. Dealing with that helps solve a real problem for some\npeople.\n\n-Peff\n\n[1] I am just guessing about how common it is. I still haven't seen an\n    indication of how common this version of sed is, or even which\n    versions are affected.\n"},{"id":"192719","messageId":"FB665BFF-9661-4624-A9D3-597AE5FF65AA@spotinfluence.com","threadId":"30666","inReplyTo":"20120602160904.GB15017@sigill.intra.peff.net","subject":"Re: Bug: rebase when an author uses accents in name on MacOSx","fromName":"Lanny Ripple","fromEmail":"lanny@spotinfluence.com","sentAt":"2012-06-02T16:37:10Z","receivedAt":"2012-06-02T16:37:10Z","isPatch":false,"sender":{"key":"lanny@spotinfluence.com","avatar":null},"body":"At a guess (I haven't tested) every Gnu sed from 4.2.1 down on Darwin and perhaps FreeBSD.  The fink developer looking at the sed giving problems (Gnu sed 4.2.1) says that it gets its charset idea from nl_langinfo() which reports US-ASCII for LC_ALL=C or POSIX (on Darwin, derived from FreeBSD).  Since US-ASCII is only 7-bit chars anything accented will break.\n\n  -ljr\n---\nLanny Ripple\nlanny@spotinfluence.com\n\n\nOn Jun 2, 2012, at 11:09 AM, Jeff King wrote:\n\n> On Fri, Jun 01, 2012 at 08:56:01AM -0500, Lanny Ripple wrote:\n> \n>> I did show that sed was broken and have provided a minimal, reproducible test.\n>> \n>> I have reported it to the sed maintainers and they are working on it.\n> \n> Great. Do we know yet which versions are affected?\n\n"}]}