RE: [PATCH] Single file merge tracking implementation. /merge-tracking/subversion/libsvn_client/diff.c

-----Original Message-----
From: Daniel L. Rall [mailto:dlr@collab.net]
Sent: Thu 6/29/2006 11:42 AM
To: Kamesh Jayachandran
Cc: SVN Dev
Subject: Re: [PATCH] Single file merge tracking implementation. /merge-tracking/subversion/libsvn_client/diff.c
 
On Wed, 28 Jun 2006, Kamesh Jayachandran wrote:
...

...
> @@ -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).

Taken care.
...
> + /* 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?

Taken care.

...
> @@ -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?

Taken care.

> + /* 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.

Taken care now the single-file merge implementation reflects that of directory merges at r20359.

> + /* 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.

Taken care.

[[[
Patch by: Kamesh Jayachandran <kamesh@collab.net>

Single file merge tracking implementation.

* subversion/libsvn_fs_fs/tree.c
  (fs_change_node_prop):
    For file nodes under '/' we receive path as just the file names
    without a '/' at the begining. This happens for single file merges,
    so canonicalize the path to make sure 'mergeinfo.mergeto' is getting
    recorded as canonicalized path.

* subversion/libsvn_client/diff.c
  (ENSURE_VALID_REVISION_KINDS): New macro to check whether merge revision
   info are specified or not.
  (do_merge): uses ENSURE_VALID_REVISION_KINDS.
  (single_file_merge_get_file):
   Caller has to pass ra_session to reuse the ra_session.
   No need to translate 'svn_opt_revision_t' to 'svn_revnum_t', Caller
   has this info already, so no need of 'path' and 'revision' args.
  (do_single_file_merge):
   Added 'svn_boolean_t dry_run' as arg.
   Implemented merge tracking for single file merges the same way as
   directory merges.
   Remove the FIXME docstring for merge-tracking.
   Calls single_file_merge_get_file with the new signature.
  (svn_client_merge2):
   Calls do_single_file_merge with the new arg.
  (svn_client_merge_peg2):
   Calls do_single_file_merge with the new arg.
]]]

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org