> -----Original Message-----
> From: Julian Foad [mailto:[email protected]]
> Sent: zaterdag 5 september 2015 15:00
> To: Bert Huijben <[email protected]>
> Cc: dev <[email protected]>
> Subject: Re: svnsync crash on empty repo
>
> Bert wrote:
> > I don't think we should fix this with a 'revision+1' to explicitly allow
> > many bad ranges,
>
> Why?
>
> An empty range isn't inherently "bad". Allowing an empty range isn't
> bad. Being able to specify an empty range in many different ways isn't
> bad. Changing a released API to make a previously no-op case now throw
> an assertion failure breaks our compatibility guarantees. That is bad.
An empty range isn't bad, but your patch appears to also explicitly allow
undefined ranges, if I read it correctly.
I would call A range of r1 upto r0 defined (empty), but I would call a range or
r4 upto r3 with current HEAD r3 undefined.
We use r1 and r0 as special in quite a few places where we really just want to
point at the initial revision, I see no problem with that but I don't want to
introduce a definition of 'HEAD+1' as there is no we can declare stable
behavior on that... It may even change partway through an operation, the moment
that revision is committed.
(It would be nicer if we used SVN_INVALID_REVNUM in more cases as the magic MIN
value, but at least ra_svn will assert if we would try to do that, as it
doesn't allow transferring r-1 as a normal serialized revision. So fixing that
requires a timemachine)
Svnrdump had a bug where it actually passed HEAD+1 as base revision of a commit
in a specific case... It is better to catch these errors on the client than to
rely on not having concurrent commits to define behavior of our tools.
Bet
