Hi Daniel. Thanks for reviewing...
(Dropping commits@ from the CC.) Daniel Shahaf wrote: > julianf...@apache.org wrote: >> >> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1303016&r1=1303015&r2=1303016&view=diff > ============================================================================== >> --- subversion/trunk/subversion/include/private/svn_client_private.h >> +++ subversion/trunk/subversion/include/private/svn_client_private.h >> +/* Perform a symmetric merge. >> + * >> + * Merge according to MERGE into the WC at TARGET_WCPATH. >> + */ >> +svn_error_t * >> +svn_client__do_symmetric_merge(const svn_client__symmetric_merge_t *merge, >> + const char *target_wcpath, >> + svn_depth_t depth, >> + svn_boolean_t ignore_ancestry, > > What does IGNORE_ANCESTRY mean in the context of symmetric merge? In > particular, is it meaningful for the second merge in a 'sync A->B, > sync A->B' scenario? Clearly I need to fill in the doc strings. IGNORE_ANCESTRY doesn't affect the high level operation of the merge, it only affects how file diffs are shown -- even if the source and target file are not historically related it will show a diff rather than a delete and an add of the file -- or something similar to that. From svn_client_merge4(): * Use @a ignore_ancestry to control whether or not items being * diffed will be checked for relatedness first. Unrelated items * are typically transmitted to the editor as a deletion of one thing * and the addition of another, but if this flag is TRUE, unrelated * items will be diffed as if they were related. >> + svn_boolean_t force, >> + svn_boolean_t record_only, >> + svn_boolean_t dry_run, >> + const apr_array_header_t *merge_options, >> + svn_client_ctx_t *ctx, >> + apr_pool_t *scratch_pool); >> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1303016&r1=1303015&r2=1303016&view=diff > ============================================================================== >> --- subversion/trunk/subversion/libsvn_client/merge.c >> +++ subversion/trunk/subversion/libsvn_client/merge.c >> @@ -10864,3 +10864,409 @@ >> +/* */ >> +static svn_error_t * >> +find_symmetric_merge(repo_location_t **yca_p, >> + repo_location_t **base_p, >> + repo_location_t **mid_p, >> + source_and_target_t *s_t, >> + svn_client_ctx_t *ctx, >> + apr_pool_t *result_pool, >> + apr_pool_t *scratch_pool) >> +{ >> + repo_location_t *yca, *base_on_source, *base_on_target, *mid; >> + >> + yca = apr_palloc(result_pool, sizeof(*yca)); >> + SVN_ERR(svn_client__get_youngest_common_ancestor( >> + NULL, &yca->url, &yca->rev, >> + s_t->source->url, s_t->source->rev, >> + s_t->target->loc.url, s_t->target->loc.rev, >> + ctx, result_pool)); >> + *yca_p = yca; >> + >> + /* Find the latest revision of A synced to B and the latest >> + * revision of B synced to A. >> + * >> + * base_on_source = youngest_complete_synced_point(source, target) >> + * base_on_target = youngest_complete_synced_point(target, source) >> + */ >> + SVN_ERR(find_base_on_source(&base_on_source, s_t, >> + ctx, scratch_pool, scratch_pool)); >> + SVN_ERR(find_base_on_target(&base_on_target, &mid, s_t, >> + ctx, scratch_pool, scratch_pool)); [...] >> + /* Choose a base. */ >> + if (base_on_source >> + && (! base_on_target || (base_on_source->rev > base_on_target->rev))) >> + { > > The last part of this condition seems arbitrary: in the criss-cross > scenario, the order in which the 'criss' and the 'cross' are > committed shouldn't affect the base the algorithm chooses. Yes, that's true for a criss-cross. However, it's not a problem for normal cases; criss-cross is a rare case. As I wrote in the criss-cross merge section of <http://wiki.apache.org/subversion/SymmetricMerge>, in that case we probably should consider the relative ages of A1, B1, A3, B3, and A2, but I haven't yet thought about what's the best way to compare them. >> + *base_p = base_on_source; >> + *mid_p = NULL; >> + } >> + else if (base_on_target) >> + { >> + *base_p = base_on_target; >> + *mid_p = mid; >> + } >> + else >> + { >> + /* No previous merge was found, so this is the simple case where >> + * the base is the youngest common ancestor of the branches. We'll >> + * set MID=NULL; in theory the end result should be the same if we >> + * set MID=YCA instead. */ >> + *base_p = yca; >> + *mid_p = NULL; >> + } >> + >> + return SVN_NO_ERROR; >> +} Thanks. - Julian
