Hello, cc'ing Roman, the original author. (I should have done that in the first post, sorry. I have also forwarded him another mail from this thread, asking him for author's sign off.) On Thu, Apr 17, 2014 at 10:39:49AM -0700, Junio C Hamano wrote: > Stepan Kasal <kasal@xxxxxx> writes: > > > On Wed, Apr 16, 2014 at 12:13:21PM -0700, Junio C Hamano wrote: > >> Interesting. What other strange forms can they record in their > >> repositories, I have to wonder. Can they do > >> 2014-01-07T5:8:6.048176Z > >> for example? > > > > Roman Belinsky, the author of this fix, witnessed after large scale > > conversion that the problem happens with the hour part only. > > Is this "large scale conversion" done from a SVN repository that is > created by bog standard SVN, or something else? I don't know. Roman? > How certain are we that this "hour part is broken" is the only kind > of breakage in timestamps we would encouter? I would say we can be certain, as Roman said that the same PC that inserts the timestamp with one-digit hours does not misformat minutes. (Still cited from the same discussion https://github.com/msysgit/git/pull/126#discussion_r9661916 ) We do not have code review for that bug, as far as I know, but this is a natural bug: a reasonably looking time "5:08:09.048176" is used in format "%sT%s" > [...] and by being slightly more lenient than necessary to cover > one observed case that triggered the patch, we can cover SVN > repositories broken in a similar but slightly different way. I second that, in general. But my guess is that this particular "similar but slightly different" breakage will never appear, so the self-documenting original fix wins for me. > Especially given that this regexp matching is not used for finding a > timestamp from random places [...] I agree that the broader regexp is not dangerous in this context. So it seems to be no big issue either way. Thanks for taking this so carefully, Stepan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html