Re: [PATCH] Update SVN.pm

[Stepan Kasal <kasal@ucw.cz>]

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@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,
	Stepan