subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kamesh Jayachandran <kam...@collab.net>
Subject 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

Mime
View raw message