On 01/13/2010 12:08 AM, C. Michael Pilato wrote:
Kamesh Jayachandran wrote:
Hi All,

Last week I posted the patch to implement 'svn client' to identify the
svn operation they are about to do with a given ra_session.

Following thread has the detailed discussion.

http://mail-archives.apache.org/mod_mbox/subversion-dev/201001.mbox/%3c4b448959.1070...@collab.net%3e



Below is the summary:

Concern/Suggestion 1:
Michael Pilato and Philip Martin were suggesting to tweak
libsvn_ra_(serf|neon) to detect the operation rather than making a
change across all layers from the simplicity point of view.

My answer to 1:
I feel it would be too hackish to tweak one general API inside these
modules to flag 'commit or write' operation to the server when the
solution I propose handles this in a transparent way.
I'm sorry, but did you say "transparent"?  What's transparent about bubbling
an RA-layer hack all the way up into the client?!  A "transparent" solution

This patch is *not* an RA layer hack rather a transparent generic feature so I see nothing wrong in propagating it to higher layers.

is one that preserves the existing transparency of the mirroring subsystem.

I do *not* like to expose the back-end internals to higher layers especially when it is not so common setup and not normally supposed to know(I mean why my client should know the repository it commits to is a mirror directly or indirectly).

  A "transparent" solution is one that doesn't allow ignorant third-party
consumers of the Subversion APIs to accidentally break their proxy setups
because they decide they wanted to pass "checkin" instead of "commit" as the
innocuous-appearing 'high_level_svn_operation' parameter.

Yes I agree. I was concerned about the *magical flag deep inside the code* for only one operation, now it looks like the only way to go.



There *must* be a better way to do this.

subversion/libsvn_ra_neon/commit.c:apply_revprops() has this comment:

   /* ### we should use DAV:apply-to-version on the CHECKOUT so we can skip
      ### retrieval of the baseline */

I looked a little bit into this.  IIUC, we can theoretically avoid the
problematic PROPFIND altogether by passing this special (yet
part-of-an-existing-standard) flag in the CHECKOUT request.  Currently,
though, mod_dav_svn doesn't handle the flag.  So there's still a server-side
change needed, but at least its one that would take us closer to better
WebDAV handling.  Maybe you could explore this option instead?


I will take a fresh look at this problem and a independent patch in this way.

Thanks
With regards
Kamesh Jayachandran

Reply via email to