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
Received on 2010-09-18 08:39:12 CEST