On Sun, Apr 20, 2014 at 5:32 PM, Bert Huijben <b...@qqmail.nl> wrote:
> > > > -----Original Message----- > > From: rhuij...@apache.org [mailto:rhuij...@apache.org] > > Sent: zondag 20 april 2014 16:47 > > To: comm...@subversion.apache.org > > Subject: svn commit: r1588772 - in /subversion/trunk/subversion: > > mod_dav_svn/repos.c tests/cmdline/commit_tests.py > > > > Author: rhuijben > > Date: Sun Apr 20 14:46:42 2014 > > New Revision: 1588772 > > > > URL: http://svn.apache.org/r1588772 > > Log: > > Implement an initial fix for issue #4480. As noted in the code this patch > > might make this code report out of date in a few cases where we should > > allow > > a commit, but this is much safer than not reporting out of date where we > > should. > > > > As far as I can tell this check matches how we do things in the repos/fs > > commit api, so I hope somebody with more knowledge of these apis can > > confirm that this is the right fix to close issue #4480. > > Hi all, > > This issue has been bothering me for weeks, because I couldn't think of a > proper way to fix this. > > I think this patch (improved by r1588778) resolves the issue, but I would > welcome reviews. > Assuming that comb->priv.version_name is the BASE revision, then your patch will actually check whether there has been a commit to the respective path since BASE. If it is a directory, it will implicitly detect all changes to the whole sub-tree. So, apart from subtle typos, I'd say the patch does what it is supposed to do. - if (comb->priv.version_name < created_rev) + if (SVN_IS_VALID_REVNUM(created_rev)) { > Maybe, add a comment saying that you basically test for "node in txn" vs. "node not in txn, yet". + /* Issue #4480: With HTTPv2 we can receive the first change for a > + directory after it has been made mutable, because one of its > + descendants was changed before changing the directory. > That comment seems odd. Did you intend to say the opposite - first change *before* making a directory mutable? If possible I would like to see this issue fixed in the next 1.8 and 1.7 > releases as this issue allows breaking merge tracking by overwriting node > properties without a proper out of date check. > The relatedness check requires another indirection in 1.8- as you need to get the svn_fs_id_t for each node and then compare them. -- Stefan^2.
