On Wed, 28 Jun 2006, Kamesh Jayachandran wrote:
...
> Single file merge tracking implementation.
>
> * subversion/libsvn_client/diff.c
> (do_single_file_merge):
> Added 'svn_boolean_t dry_run' and 'svn_client_ctx_t *ctx' as args.
> Implemented merge tracking for single file merges the same way as
> directory merges.
> Remove the FIXME docstring for merge-tracking.
> (svn_client_merge2):
> calling do_single_file_merge with the new set of args.
> (svn_client_merge_peg2):
> calling do_single_file_merge with the new set of args.
Kamesh, thanks for taking a stab at the single-file merge portion of
the problem. I actually have a similar patch already sitting in my
WC, but was trying to shake the rest of the bugs out of
directory/multi-file merge (the do_merge() function) before applying
the pattern to single-file merging. I sent a mail to Madan yesterday
describing two areas where help would be most useful
(svn_rangelist_intersect(), which needs to be called from do_merge(),
and fixing the failing tests in merge_tests.py, which are at least in
part due to lack of handling for our new "svn:mergeinfo" housekeeping
property).
A few comments about the patch inline below...
...
> @@ -1972,7 +1971,9 @@
> const svn_opt_revision_t *peg_revision,
> const char *target_wcpath,
> svn_wc_adm_access_t *adm_access,
> + svn_boolean_t dry_run,
> struct merge_cmd_baton *merge_b,
> + svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> apr_hash_t *props1, *props2;
A svn_client_ctx_t * is already available via merge_b->ctx. I'm not
sure why we pass this in separately for do_merge() -- it looks like an
oversight (WRT to the baton data type), which I've fixed on trunk in
r20286, and integrated into the merge-tracking branch (using
svnmerge.py).
...
> + /* Sanity check -- ensure that we have valid revisions to look at. */
> + if ((initial_revision1->kind == svn_opt_revision_unspecified)
> + || (initial_revision2->kind == svn_opt_revision_unspecified))
> + {
> + return svn_error_create
> + (SVN_ERR_CLIENT_BAD_REVISION, NULL,
> + _("Not all required revisions are specified"));
> + }
This block is duplicated in do_merge() -- should we stick it into a
macro?
...
> @@ -2018,6 +2033,30 @@
> *revision2 = *initial_revision2;
> }
>
> + /* Establish first RA session to URL1. */
> + SVN_ERR(svn_client__open_ra_session_internal(&ra_session, URL1, NULL,
> + NULL, NULL, FALSE, TRUE,
> + ctx, pool));
In do_single_file_merge(), we call some other functions which take a
RA session, that we pass NULL into. Would it be possible to re-use
this RA session instead across those calls?
> + /* Resolve the revision numbers, and store them as a merge range.
> + Note that the "start" of a merge range is inclusive. */
> + SVN_ERR(svn_client__get_revision_number
> + (&range.start, ra_session, revision1, path1, pool));
> + range.start += 1;
> + SVN_ERR(svn_client__get_revision_number
> + (&range.end, ra_session, revision2, path2, pool));
> + is_revert = (range.start > range.end);
is_revert will be wrong here because range.start was already modified
-- you copied the bad example I committed incrementally while trying
to handle the logic for reverts. See r20280 and r20285.
> + /* Look at the merge info prop of the WC target to see what's
> + already been merged into it. */
> + SVN_ERR(parse_merge_info(&target_mergeinfo, target_wcpath, adm_access, ctx,
> + pool));
> +
> + SVN_ERR(svn_client__path_relative_to_root(&rel_path, URL1, NULL,
> + ra_session, adm_access, pool));
> + SVN_ERR(calculate_merge_ranges(&remaining_ranges, rel_path, target_mergeinfo,
> + &range, is_revert, pool));
> +
...
You appear to be missing a loop around the merging of the revision
ranges and cleanup.
- application/pgp-signature attachment: stored
Received on Thu Jun 29 08:14:34 2006