Re: [PATCH][merge-tracking]Fix repeat merge problem while subtree of merge target having overlapping merge already

On Thu, 25 Jan 2007, Kamesh Jayachandran wrote:
...
> >>>@@ -3547,6 +3692,25 @@
> >>> recursive diff-editor thing. */
> >>> else if (entry->kind == svn_node_dir)
> >>> {
> >>>+ if (strcmp(URL1, URL2) == 0)
> >>>
> >>>Inside of do_merge(), we perform some calculations on the two URLs
> >>>before using them to set the merge_baton.same_urls field. The
> >>>same_urls field is effectively what you're trying to use here to get
> >>>do_subtree_merges(). Why the difference? Is it important?
...
> Then we may need to change the do_merge, do_single_file_merge, to accept
> the 'notification_receiver_baton_t *'. So that we can do this check at
> single place. Am I correct in understanding this?

The key here is getting the final "URL1" and "URL2" from the
"initial_URL1" and "initial_URL2" parameters (which may change after
being passed on to the do_*merge implementation), so that the
"same_urls" boolean is set correctly.

To do this outside of the do_*merge implementation looks like it might
be messy, which is why I was asking if the difference I describe in
the previous paragraph is important.

> >>>In do_merge() and do_single_file_merge(), we also have:
> >>>
> >>> /* When only recording merge info, we don't perform an actual
> >>> merge for the specified range. */
> >>> if (merge_b->record_only)
> >>> {
...
> >>> /* ### Handle WC-local reverts which have modified our merge
> >>> ### info. */
> >>> apr_hash_t *merges;
> >>> SVN_ERR(determine_merges_performed(&merges, target_wcpath,
> >>> &range, &notify_b, pool));
> >>> return update_wc_merge_info(target_wcpath, entry, rel_path,
> >>> merges, is_revert, ra_session,
> >>> adm_access, ctx, pool);
> >>> }
...
> >I believe it's about an 'svn merge -r N:N-1'
> >which reverted changes in the WC which had previously been merged in.
> >In such a case, we need to remove merge info from the WC. I'm not
> >clear on whether determine_merges_performed() and
> >update_wc_merge_info() handles that case. It rather looks like it
> >does, but this is another area that we could use a test case for.
...
> I am still not clear here.
> <comment inside --record-only block>
> /* ### Handle WC-local reverts which have modified our merge
> ### info. */
> </comment>
>
> Why bother about reverts? That too in record-only block! The
> 'do_(merge|single_file_merge)' 's job is to append(with mergeinfo
> algebra) the merge range to existing mergeinfo. Why it should bother
> about earlier revert attempts?

'svn merge --record-only -c -N' can be used to revert the record of a
merge.

> Do you mean the scenario like the following
> * pristine WC with svn:mergeinfo '/branches/b1:87-96'
> * svn merge -r 88:87 url_to_branches/b1
> Then svn:mergeinfo will be '/branches/b1:87, 89-96'
> I fail to see the connection to above comment and the above scenario.

This scenario needs to be handled. Another scenario which needs to be
handled is that any revert(s) which occurred *before* the currently
running 'merge' operation should not be clobbered by the current
'merge'. The "is_revert" flag I added to update_wc_merge_info() looks
like it may already be doing the trick for the first case; I'm less
sure about the second.

We should add a test which covers both these cases (or enhance an
existing test).

- Dan