subversion-dev mailing list archives

Re: [PATCH] Make svn clients indicate their operation name to backend(right now only to DAV) Version 2

Date

Wed, 13 Jan 2010 11:37:29 GMT

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.1070400@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