> 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...
>
>
Will take up svn_rangelist_intersect() and fixing tests in merge_tests.py.
> 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).
>
> ...
>
Incorporated.
>> + /* 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?
>
> ...
>
Introduced the new macro.
> 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?
>
I don't think so, from do_single_merge we call
svn_client__repos_locations with 'ra_session' as NULL and this makes
sense, as it is used for pegrevision parsing. We might have URLs from
two different repositories so single ra_session might not be enough.
single_file_merge_get_file internally creates own ra_session but again
here too we might have two different repositories.
>
> 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.
>
>
I have changed it to as that of r20287.
>> + /* 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.
>
Is this 'for' loop needed for single file merges. I believe if at all
needed I need to change the current rev1 and rev2 based on is_revert?
Not sure about it though will investigate it.
Thanks Dan for the Review.
Attaching the patch without this 'for loop'
With regards
Kamesh Jayachandran
[[[
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
numbers are specified or not.
(do_merge): uses ENSURE_VALID_REVISION_KINDS.
(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.
(svn_client_merge2):
calling do_single_file_merge with the new arg.
(svn_client_merge_peg2):
calling do_single_file_merge with the new arg.
]]]
Index: subversion/libsvn_fs_fs/tree.c
===================================================================
--- subversion/libsvn_fs_fs/tree.c (revision 20293)
+++ subversion/libsvn_fs_fs/tree.c (working copy)
@@ -1458,9 +1458,13 @@
directly. */
svn_fs_txn_t *txn;
+ /* 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 canon_path */
+ const char *canon_path = svn_fs_fs__canonicalize_abspath(path, pool);
SVN_ERR(svn_fs_open_txn(&txn, root->fs, txn_id, pool));
- SVN_ERR(svn_fs_fs__change_txn_mergeinfo(txn, path, value, pool));
+ SVN_ERR(svn_fs_fs__change_txn_mergeinfo(txn, canon_path, value, pool));
SVN_ERR(svn_fs_fs__change_txn_prop(txn,
SVN_FS_PROP_TXN_CONTAINS_MERGEINFO,
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 20293)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -44,6 +44,17 @@
#include "svn_private_config.h"
+
+/* Sanity check -- ensure that we have valid revisions to look at. */
+#define ENSURE_VALID_REVISION_KINDS(rev1_kind, rev2_kind) \
+ if ((rev1_kind == svn_opt_revision_unspecified) \
+ || (rev2_kind == svn_opt_revision_unspecified)) \
+ { \
+ return svn_error_create \
+ (SVN_ERR_CLIENT_BAD_REVISION, NULL, \
+ _("Not all required revisions are specified")); \
+ }
+
/*
* Constant separator strings
*/
@@ -1797,14 +1808,7 @@
svn_opt_revision_t *revision1, *revision2;
int i;
- /* 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"));
- }
+ ENSURE_VALID_REVISION_KINDS(initial_revision1->kind, initial_revision2->kind);
/* If we are performing a pegged merge, we need to find out what our
actual URLs will be. */
@@ -1968,7 +1972,6 @@
/* The single-file, simplified version of do_merge. */
-/* ### FIXME: To handle merge tracking, follow pattern from do_merge(). */
static svn_error_t *
do_single_file_merge(const char *initial_URL1,
const char *initial_path1,
@@ -1979,6 +1982,7 @@
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,
apr_pool_t *pool)
{
@@ -1990,10 +1994,20 @@
apr_array_header_t *propchanges;
svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
svn_wc_notify_state_t text_state = svn_wc_notify_state_unknown;
- const char *URL1, *path1, *URL2, *path2;
+ svn_client_ctx_t *ctx = merge_b->ctx;
+ const char *URL1, *path1, *URL2, *path2, *rel_path;
svn_opt_revision_t *revision1, *revision2;
svn_error_t *err;
+ svn_merge_range_t range;
+ svn_ra_session_t *ra_session;
+ svn_boolean_t is_revert;
+ apr_hash_t *target_mergeinfo;
+ apr_array_header_t *remaining_ranges;
+ const char *uuid1, *uuid2;
+
+ ENSURE_VALID_REVISION_KINDS(initial_revision1->kind, initial_revision2->kind);
+
/* If we are performing a pegged merge, we need to find out what our
actual URLs will be. */
if (peg_revision->kind != svn_opt_revision_unspecified)
@@ -2006,7 +2020,7 @@
peg_revision,
initial_revision1,
initial_revision2,
- merge_b->ctx, pool));
+ ctx, pool));
merge_b->url = URL2;
merge_b->path = NULL;
@@ -2024,7 +2038,40 @@
revision2 = apr_pcalloc(pool, sizeof(*revision2));
*revision2 = *initial_revision2;
}
-
+
+ SVN_ERR(svn_client_uuid_from_url(&uuid1, URL1, ctx, pool));
+ SVN_ERR(svn_client_uuid_from_url(&uuid2, URL2, ctx, pool));
+
+ if (strcmp(uuid1, uuid2) == 0)
+ {
+ /* Establish RA session to URL1. */
+ SVN_ERR(svn_client__open_ra_session_internal(&ra_session, URL1, NULL,
+ NULL, NULL, FALSE, TRUE,
+ ctx, pool));
+
+ /* 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));
+ SVN_ERR(svn_client__get_revision_number
+ (&range.end, ra_session, revision2, path2, pool));
+ is_revert = (range.start > range.end);
+ if (is_revert)
+ range.end += 1;
+ else
+ range.start += 1;
+
+ /* 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));
+ }
+
/* ### heh, funny. we could be fetching two fulltexts from two
*totally* different repositories here. :-) */
SVN_ERR(single_file_merge_get_file(&tmpfile1, &props1, &rev1,
@@ -2066,7 +2113,7 @@
return err;
svn_error_clear(err);
- if (merge_b->ctx->notify_func2)
+ if (ctx->notify_func2)
{
svn_wc_notify_t *notify
= svn_wc_create_notify(merge_b->target, svn_wc_notify_update_update,
@@ -2074,10 +2121,15 @@
notify->kind = svn_node_file;
notify->content_state = text_state;
notify->prop_state = prop_state;
- (*merge_b->ctx->notify_func2)(merge_b->ctx->notify_baton2, notify,
- pool);
+ (*ctx->notify_func2)(ctx->notify_baton2, notify, pool);
}
+ if ((!dry_run) && (strcmp(uuid1, uuid2) == 0))
+ {
+ SVN_ERR(update_wc_merge_info(target_wcpath, target_mergeinfo, rel_path,
+ remaining_ranges, is_revert, adm_access,
+ pool));
+ }
return SVN_NO_ERROR;
}
@@ -2928,6 +2980,7 @@
&peg_revision,
target_wcpath,
adm_access,
+ dry_run,
&merge_cmd_baton,
pool));
}
@@ -3056,6 +3109,7 @@
peg_revision,
target_wcpath,
adm_access,
+ dry_run,
&merge_cmd_baton,
pool));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jun 29 22:12:29 2006