On Fri, Sep 17, 2010 at 08:17:22AM +0100, Daniel Shahaf wrote: > https://svn.apache.org/repos/asf/subversion/branches/atomic-revprop/BRANCH-README > > The atomic-revprop branch, today, implements marshalling of the > OLD_VALUE_P parameter over all RA layers to the repos and fs layers. > (See svn_fs_change_rev_prop2() in trunk for a description of that parameter.) > The branch does not yet promise a specific error code will be returned if > the revprop change fails /due to the atomicity requirement/. Ensuring > a specific error code is the bulk of the remaining work on the branch; > implementing it requires teaching ra_dav how to marshal svn_error_t chains > over the wire. > > In other words, the branch implements the API correctly and (from an > FS-oriented point of view) completely, and only the ability to signal to > the caller what sort of error condition occurred is absent. That > absence means a caller cannot tell[*] whether the propchange failure was > due to requesting an atomic change or due to some other reason.
Why do we need to return multiple errors when setting the revprop fails due to non-atomicity? Can't the server just select the proper error code from the chain and return that to the client? In a recent commit by cmike something similar was done (r997466). > Is it fine to merge the branch to trunk, given this state? Is the > branch, in its current state, fit for release (in 1.7), or is additional > work (possibly including the abovementioned errorcode work) required > prior to releasing it? I don't really like the error string comparison. I'd still be happy to see this in trunk, with comments indicating how the error handling should be done instead. Stefan
