Stefan Sperling wrote on Tue, Sep 21, 2010 at 18:25:10 +0200: > On Tue, Sep 21, 2010 at 05:07:49PM +0100, Jon Foster wrote: > > So... what do we do if a 1.7 svnsync connects to a <=1.6 mirror server? > > Some options are: > >
Right now, the code uses option (1). > > 1) It works like 1.6 - i.e. it does buggy locking that works most of the > > time, then one day it corrupts the mirror repo. > > > > 2) It runs without even trying to do locking. This is likely to be more > > obvious to the sysadmin, so they might notice it in testing. > > > > 3) It errors out with an informative message. If the user has control > > of the mirror server, they can upgrade it to 1.7. Alternatively, if the > > user doesn't need locking (e.g. they have set up external locking), they > > can pass --disable-locking to svnsync. > > > > > > I really don't like option 1. I think Subversion should be reliable, > > and random corruption is not good. I'd like to get rid the old, buggy > > locking code. > > > > I'm not keen on option 2 either. Silently disabling a > > required-for-correctness feature seems like a really bad idea. And the > > locking has always been a documented feature (even if it's never > > worked). > > > > So I guess that leaves option 3. That's not fully backwards-compatible > > - you can't just drop in a 1.7 svnsync to replace a 1.6 svnsync, unless > > you update the server at the same time. But I feel that it's the least > > bad option. > > I agree that option 3 is best. There's no point in hiding the problem > from the user in the name of backwards compatibility. > How about combining (1) and (3) --- that is, using the old locking mode (with its known race condition) but print a warning that the mirror server should be upgraded? That seems better than a fatal error for a condition that needs to be fixed on the server end. While here, there is a similar issue in svn_client_revprop_set2(). On the branch, it tries to be atomic if possible; but on trunk, it has always used a racy implementation. What do you think should be done in that case? > Daniel, this can easily be done on trunk, too, if you decide to merge > the branch back soon. > > Stefan
