Greg Stein <gst...@gmail.com> writes: > On Fri, Apr 27, 2012 at 11:20, Philip Martin <philip.mar...@wandisco.com> > wrote: >> Greg Stein <gst...@gmail.com> writes: >> >>> On Wed, Apr 25, 2012 at 16:43, Philip Martin <philip.mar...@wandisco.com> >>> wrote: >>>> Perhaps we should keep the error and toss the path? >>> >>> mod_dav_svn uses that path to construct a "nice" error message based >>> on the context of the commit. >> >> mod_dav_svn isn't the only user of this API. > > WELL aware of that fact. I was simply giving a data point for how the > path was used. > > What's your point?
I don't understand why the fact that mod_dav_svn uses the path is a reason to discard the error. Particularly when svnserve currently discards the path and returns the error to the client. >>>> So currently reporting "conflict on path 'foo'" is probably sufficient >>>> but it's odd to limit ourselves. We have an error reporting mechanism >>>> why would we choose not to use it? >>> >>> Well... we return that conflict_path so that callers can make >>> nicer/contextual error messages with the path. We don't transfer the >>> whole chain over the wire, so if the path is on the inner error (say, >>> if we just wrapped context and relied on the path to be in the inner >>> error), then the user would never see the path. >>> >>> If we could/would marshal the whole error chain, then yes: we could >>> rely on our error reporting mechanism. But we don't, so we can't. >> >> svnserve returns an error chain, see svn_ra_svn_write_cmd_failure. > > And other RA layers do not. You seem to be suggesting that because mod_dav_svn doesn't return the error chain the error should not be available to other RA layers. What happens if an FS layer wants to return more information about a conflict? Does it return SVN_ERR_FS_CONFLICT_RICH to bypass the discarding of SVN_ERR_FS_CONFLICT? Perhaps we should keep the current behaviour and return both an error and a path and let the caller decide which to one use. -- uberSVN: Apache Subversion Made Easy http://www.uberSVN.com
