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
Received on 2010-09-17 12:04:09 CEST