[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2007-01-31 15:17:32 CET

Daniel Rall wrote:
> 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.
>
>
The check is exactly same as do_*_merge.(URL1=initial_URL1,
URL2=initial_URL2 no changes at all since
peg_revision.kind=svn_opt_revision_unspecified).

The check is important as we need to make a call to
'discover_and_merge_children' only if both the URLs are same.
>
>>>>> 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.
>
>
Only if you have merged(or atleast recorded) 'N' already.
First merge being reverse is not supported.

Refer the thread at,
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=117551
>> 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
>

Will post a patch.

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 31 15:16:59 2007

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.