Stefan Sperling wrote on Fri, Sep 17, 2010 at 12:03:20 +0200: > 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). >
ra_svn already marshals error chains, and I believe cmike's fix was for the case where the error chain is headed by *two* SVN_ERR_RA_SVN_CMD_ERR elements (rather than just one). What I have to do now is make ra_dav preserve the error code in the first place. You're correct, though, in observing that I need to preserve just the topmost error code (as opposed to the whole chain). > > 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
