On Tue, 05 Dec 2006 23:45:32 +0530, Daniel Rall <dlr@collab.net> wrote:
> On Tue, 05 Dec 2006, Madan S. wrote:
>
>> On Tue, 05 Dec 2006 00:42:39 +0530, Daniel Rall <dlr@collab.net> wrote:
>>
>> >On Mon, 04 Dec 2006, Madan S. wrote:
[snip]
> Yes, the name doesn't match exactly what we want to do, as we're not
> actually changing any WC properties here. Too bad the field isn't
> named prop_changes. Hmmmmmm.
>
> All our options here appear to involving rev'ing the data structure
> and its callers. :-\
>
> a) Rename wcprop_changes to prop_changes, and use it for both extra
> client -> repos and repos -> WC property edits, with an API contract
> indicating that it must be cleared after the client -> repos prop
> edits. This could be confusing to developers using the structure for
> the first time, but might use less memory.
>
> b) Add a repos_prop_changes field for client -> repos prop edits.
>
> Either way, wcprop_changes could REALLY use some better documentation!
I would opt for (a) which would mean one more commit (to rename and change
all occurances of the old name) before we can check this into trunk.
>> But I guess like you said above, setting restting the wcprop_changes
>> with
>> a new array (not just setting nelts to 0) would not interfere with the
>> current implementation.
>
> Really? I didn't notice any place where it might interfere.
>
> The doc string on apr_array_header_t's nelts field says "The number of
> active elements in the array". It has a separate field, nalloc, which
> represents "The number of elements allocated in the array". My
> understanding of apr_array_header_t is that manipulating nelts is an
> acceptable way to shrink or clear the list (though there's also the
> apr_array_pop() API).
>
> process_committed_leaf() makes proper use of the nelts field:
>
> /* Do wcprops in the same log txn as revision, etc. */
> if (wcprop_changes && (wcprop_changes->nelts > 0))
> {
>
True, but this sanity should not be expected by default. Am not an expert
in APR, so you are most probably right... but I would prefer to err on the
right side :)
[snip]
>> >A change like this should be committed to trunk first, as it
>> >introduces a general non-WC property setting mechanism which is
>> >decoupled from Merge Tracking.
>>
>> Agree. Will submit a patch to trunk after testing on all ra_xxx's.
>> Does ra_serf need to be tested too?
>
> Yes, but if you get a patch working with the other three, I can give
> it a whirl over ra_serf.
hmm, I tested it over trunk, and it fails over ra_dav... will check out
the problem and let you know...
Thanks for offering to test over ra_serf.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Dec 7 14:14:09 2006