On Wed, May 9, 2012 at 8:51 PM, C. Michael Pilato <cmpil...@collab.net> wrote:
> On 05/08/2012 05:47 PM, Ivan Zhakov wrote:
>> On Wed, May 9, 2012 at 1:34 AM, Ivan Zhakov <i...@visualsvn.com> wrote:
>>> On Wed, May 9, 2012 at 12:49 AM, C. Michael Pilato <cmpil...@collab.net>
>>> wrote:
>>>> On 05/08/2012 04:39 PM, Mark Phippard wrote:
>>>>> On Tue, May 8, 2012 at 4:20 PM, Ivan Zhakov <i...@visualsvn.com> wrote:
>>>>>> On Wed, May 9, 2012 at 12:09 AM, C. Michael Pilato <cmpil...@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.
>
Hi Mike,
I've only checked that mod_dav_svn never sends <fetch-props /> tags
while you mentioned that it does. Actually I checked it only by
analyzing the server-side code, not by capturing real transfer. Sorry
for confusion.
It seems that ra_serf unconditionally retrieves properties using
PROPFIND for *all* added files:
subversion\libsvn_ra_serf\update.c:1633 (start_report)
[[[
else if ((state == OPEN_DIR || state == ADD_DIR) &&
strcmp(name.name, "add-file") == 0)
{
const char *file_name, *cf, *cr;
report_info_t *info;
file_name = svn_xml_get_attr_value("name", attrs);
cf = svn_xml_get_attr_value("copyfrom-path", attrs);
cr = svn_xml_get_attr_value("copyfrom-rev", attrs);
if (!file_name)
{
return svn_error_create(
SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
_("Missing name attr in add-file element"));
}
info = push_state(parser, ctx, ADD_FILE);
info->base_rev = SVN_INVALID_REVNUM;
info->fetch_props = TRUE;
info->fetch_file = TRUE;
]]]
Do you know why?
--
Ivan Zhakov
