On Tue, 05 Dec 2006 00:42:39 +0530, Daniel Rall <dlr@collab.net> wrote:
> On Mon, 04 Dec 2006, Madan S. wrote:
[snip]
> I have a question/concern about the use of svn_client_commit_item2_t's
> wcprop_changes field (which references
> http://subversion.tigris.org/issues/show_bug.cgi?id=806).
>
> The doc string on libsvn_client/ra.c:push_wc_prop() mentions that it
> "implements the 'svn_ra_push_wc_prop_func_t' interface". This
> function does make use of wcprop_changes, which contains property data
> sent back from the repository for *post-commit* processing. That is,
> it appears that any data left in wcprop_changes after a commit occurs
> is intended to trigger some additional processing which sets its
> property data on the WC. See 'svn di -r 3632:3635', which shows
> wcprop_changes propogated down to
> libsvn_wc/adm_ops.c:process_committed_leaf(), by way of what's now the
> svn_wc_process_committed4() -- or _committed5, on the merge-tracking
> branch -- API.
>
> While I didn't notice anything in particular which would prevent us
> from using wcprop_changes as a pre-commit queue of additional property
> changes to send from the client to the repository, won't we need to
> clear out the wcprop_changes array after sending the data to the repos
> as part of the commit to avoid doing unnecessary and incorrect
> post-commit processing (which sets the WC's properties to what we sent
> to the repos)? Something along the lines of a
> "item->wcprop_changes->nelts = 0;" as the last statement at the end of
> your "if (item->wcprop_changes)" block might suffice.
You are correct. I didn't understand this logic earlier... but again, as
you say, this should not prevent us from using the member prior to commit
on the repos (maybe the name doesnt exactly match what we want to do ? -
as it says 'wcprop' in a different sense?).
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.
> 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?
Thanks for the review, Dan.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 5 10:11:55 2006