[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r966822 - in /subversion/trunk/subversion: libsvn_client/repos_diff.c tests/cmdline/merge_tests.py

From: Paul Burba <ptburba_at_gmail.com>
Date: Tue, 27 Jul 2010 11:50:55 -0400

On Fri, Jul 23, 2010 at 2:30 PM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> On 07/22/2010 05:56 PM, Paul Burba wrote:
>> On Thu, Jul 22, 2010 at 5:04 PM, Bert Huijben <bert_at_qqmail.nl> wrote:
>>> Shouldn't this be fixed in the ra layer implementations then?
>>
>> From what Mike told me in
>> http://subversion.tigris.org/issues/show_bug.cgi?id=3657#desc9 I
>> didn't think so.  I assumed that some users of the svn_delta_editor_t
>> rely on this behavior, since we are quite purposeful about it...but
>> looking again at the docstring for change_file_prop, it seems clear
>> that if anybody is relying on this behavior, they are wrong:
>>
>>   /** Change the value of a file's property.
>>    * - @a file_baton specifies the file whose property should change.
>>    * - @a name is the name of the property to change.
>>    * - @a value is the new (final) value of the property, or @c NULL if the
>>    *   property should be removed altogether.
>>    *
>>    * The callback is guaranteed to be called exactly once for each property
>>    * whose value differs between the start and the end of the edit.
>>    *
>>    * All allocations should be performed in @a pool.
>>    */
>>   svn_error_t *(*change_file_prop)(void *file_baton,
>>                                    const char *name,
>>                                    const svn_string_t *value,
>>                                    apr_pool_t *pool);
>>
>> I'll revisit this tomorrow.
>
> Thanks to wonders of English grammar, the promise above can be restated like so:
>
>   For each property whose value differs between the start and the end
>   of the edit, invoke the callback exactly once per property.  But
>   for each property whose value does NOT differ between the start and
>   the end of the edit, you can call this sucker all day long in any way
>   you want!
>
> So, it's *technically* fine for an editor driver to slam unchanged
> properties through the interface.  :-)
>
> (But no, I highly doubt that was the original intent of the promise.)

...Toto, I've a feeling we're not in libsvn_client any more...

I've had no luck finding a way to implement this in the RA layer yet.
From what Mike said in
http://subversion.tigris.org/issues/show_bug.cgi?id=3657#desc9 the RA
layer is getting all the properties, so I'm not sure how it can
determine if it has a legitimate prop change and thus whether it
should call svn_delta_editor_t:change_file_prop(). The
report_dir_t's props* member in lib_ra_serf:update.c's does contain
the DAV properties for the revision at the start of the edit.
Possibly we can also get the svn: and custom props and stash those
here too and use them to detect legitimate prop diffs. Not quite sure
how to do this or if it is even safe to do so.

I'm not quite waving the white flag yet, but if anybody with some
familiarity in this space wants to take a look/offer an opinion, well
then, I'd be right grateful.

Paul

* If you happen to look, I'll save you some trouble since this struct
is undocumented, it's a (svn_revnum_t) key mapped to a hash which maps
a (const char *) url (or path) to a hash, which maps a (const char *)
namespace to a hash, which maps a (const char *) prop name to an
(svn_string_t *) prop val.
Received on 2010-07-27 17:51:32 CEST

This is an archived mail posted to the Subversion Dev mailing list.