Re: svn commit: r1335639 - /subversion/trunk/subversion/libsvn_wc/adm_ops.c

On 05/08/2012 05:47 PM, Ivan Zhakov wrote:
> On Wed, May 9, 2012 at 1:34 AM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>> On Wed, May 9, 2012 at 12:49 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>>> On 05/08/2012 04:39 PM, Mark Phippard wrote:
>>>> On Tue, May 8, 2012 at 4:20 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>>>>> On Wed, May 9, 2012 at 12:09 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>>>>>> On 05/08/2012 03:35 PM, Greg Stein wrote:
>>>>>>> One question: the ordering of PROPFIND and GET. Do you know if that is
>>>>>>> a requirement, or simply that you were preserving prior behavior?
>>>>>>
>>>>>> Upon reflection, it's probably not a hard requirement. In general, I
>>>>>> suppose it's easier (and more efficient) to cache properties and stream
>>>>>> contents while we drive an editor than the reverse, so that's probably why
>>>>>> that ordering was chosen prior.
>>>>>>
>>>>> Another option is to include properties in REPORT even in skelta-mode
>>>>> if they are small. With defining small something like 0.5-1k.
>>>>
>>>> Agreed. I had forgotten that we would still need these roundtrips to
>>>> get the properties. Maybe the REPORT request could at least indicate
>>>> which items have properties. It would be better for performance if
>>>> things like svn:eol-style and svn:mimetype were included in the
>>>> request so we then only had to go back to the server for custom props
>>>> (and we knew which files have them).
>>>
>>> The REPORT request does include a <fetch-props/> type of indicator which
>>> says "there's something worth fetching here".
>>>
>>> I'm quite in favor of including, say, the "svn:" class of properties in the
>>> REPORT response proper.
>>>
>> We could include all properties if we know there are small. It seems
>> to be possible implement this on server side, but I'm not sure that
>> current client code is ready for mixing embedded and external
>> properties in REPORT response.
>>
> Well, it seems things are more complicated: current mod_dav_svn
> implementation never sends <fetch-props /> tag and ra_serf always asks
> for properties, even if there is no properties.

I double-checked this claim, Ivan, and don't believe it to be accurate. I
started with code inspection, and found things to be as I'd hoped.

I then created an empty repository, and made these changes:

   r1: add a file, with properties set on it.
   r2: modified the file's contents only
   r3: modified the file's svn:eol-style property value (which also
        tweaked its content)
   r4; delete the file's svn:eol-style property.

Then, I backdated my working copy to r0, and started updating to each
successive revision.

$ svn up -r1:

   "OPTIONS /tests/repos HTTP/1.1"
   "OPTIONS /tests/repos HTTP/1.1"
   "REPORT /tests/repos/!svn/me HTTP/1.1"
   "PROPFIND /tests/repos/!svn/rvr/1/file.py HTTP/1.1"
   "HEAD /tests/repos/!svn/rvr/1/file.py HTTP/1.1"

This update adds the new file back to the working copy. I get a PROPFIND to
fetch its properties (the server never transmits props for added files/dirs)
and a HEAD for the contents (because they are cached in the WC pristine
store already ... else this would have been a GET).

$ svn up -r2:

   "OPTIONS /tests/repos HTTP/1.1"
   "OPTIONS /tests/repos HTTP/1.1"
   "REPORT /tests/repos/!svn/me HTTP/1.1"
   "HEAD /tests/repos/!svn/rvr/2/file.py HTTP/1.1"

No PROPFIND. HEAD to install the new contents.

$ svn up -r3:

   "OPTIONS /tests/repos HTTP/1.1"
   "OPTIONS /tests/repos HTTP/1.1"
   "REPORT /tests/repos/!svn/me HTTP/1.1"
   "HEAD /tests/repos/!svn/rvr/3/file.py HTTP/1.1"

Again no PROPFIND, because the <D:set-prop> REPORT response item carried the
svn:eol-style propchange. HEAD because the eol-style changed and new
contents were required.

$ svn up -r4:

   "OPTIONS /tests/repos HTTP/1.1"
   "OPTIONS /tests/repos HTTP/1.1"
   "REPORT /tests/repos/!svn/me HTTP/1.1"

Again, no PROPFIND, because the <D:remove-prop> REPORT response item carried
the propchange info. No HEAD because the contents didn't change.

So, it's only the added files and directories that seem to necessitate a
PROPFIND.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development