On Mon, 04 Dec 2006, Madan S. wrote:
> Please find attached a preparatory step to record copyfrom info on a wc
> to repos copy. This patch makes use of the currently unused wcprop_changes
> member of the svn_client_commit_item2_t structure, to push props
> explicitly set in wcprop_changes to the repository editor.
This looks like the right spot to push the additional "svn:mergeinfo"
property information from the client to the repository. I like the
direct editor drive here in libsvn_client's do_item_commit(), rather
than in svn_wc_transmit_prop_deltas(), because in WC -> repos
copy/move, we don't intend to set this merge info on the WC -- we only
set it on the repository on the copy/move destination.
> The next step would be to calculate the merge info and fill the
> wcprop_changes member in libsvn_client/copy.c:wc_to_repos_copy().
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.
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.
> I have tested this code for functionality with a simple script I wrote.
> I have run 'make check' on local to test for regression, which I beleive
> is sufficient given the change is only on the client side. Please let me
> know if theres anything else I should be doing.
Changes like this which drive a RA editor should be tested over all RA
mechanisms.
...
> Preparatory step for recording copyfrom info on wc to repos copy.
>
> On the merge-tracking branch:
>
> * subversion/libsvn_client/commit_util.c
> (do_item_commit): Take into account the item's wcprop_changes value when
> the item's state flag has SVN_CLIENT_COMMIT_ITEM_PROP_MODS set.
...
> --- subversion/libsvn_client/commit_util.c (revision 22554)
> +++ subversion/libsvn_client/commit_util.c (working copy)
> @@ -1180,6 +1180,28 @@
> tempfile = apr_pstrdup(apr_hash_pool_get(tempfiles), tempfile);
> apr_hash_set(tempfiles, tempfile, APR_HASH_KEY_STRING, (void *)1);
> }
> +
> + /* Set other prop-changes, if available in the baton */
> + if (item->wcprop_changes)
> + {
> + svn_prop_t *prop;
> + apr_array_header_t *prop_changes = item->wcprop_changes;
> + int ctr;
> + for (ctr = 0; ctr < prop_changes->nelts; ctr++)
> + {
> + prop = APR_ARRAY_IDX(prop_changes, ctr, svn_prop_t *);
> + if (kind == svn_node_file)
> + {
> + editor->change_file_prop(file_baton, prop->name,
> + prop->value, pool);
> + }
> + else
> + {
> + editor->change_dir_prop(*dir_baton, prop->name,
> + prop->value, pool);
> + }
> + }
> + }
> }
>
> /* Finally, handle text mods (in that we need to open a file if it
- application/pgp-signature attachment: stored
Received on Mon Dec 4 20:07:28 2006