RE: svnsync crash on empty repo

> -----Original Message-----
> From: Julian Foad [mailto:julianfoad_at_gmail.com]
> Sent: zaterdag 5 september 2015 15:00
> To: Bert Huijben <bert_at_qqmail.nl>
> Cc: dev <dev_at_subversion.apache.org>
> 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
Received on 2015-09-05 18:45:33 CEST