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 <ka...@ucw.cz> 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,
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to