[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-25 11:34:04 CET

Hi Dan,

Sorry for responding late on this.

> Will you think about the best way to go, and send a doc or naming
> patch for this one?
>

Sent.

> ...
>
>>> @@ -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?
>>>
>> It is just to do away with Three merge scenarios where these 'Subtree
>> exclusion' does not make sense atleast currently.
>>
>
> Sure, the reasoning for the checks is the same in both places. But
> the code used to implement the checks is different -- why? Is it
> important that it is different?
>

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?

>
>>> 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)
>>> {
>>> /* ### TODO: Support sub-tree merge info. */
>>> /* ### 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);
>>> }
>>>
>>> Is this TODO comment about sub-tree merge info still relevant? If so,
>>> what needs to be changed?
>>>
>> Yes it can be removed.
>>
>
> Okay, include that in the next patch. Can you also include another
> test for sub-tree merge info, especially tests in which one of the
> "internal" merges on a child path adds and/or substracts additional
> merge info.
>

Will do.

>
>> I have a less clue about what that second comment line means.
>>
>
> It's a separate TODO comment. I believe it's about an 'svn merge -r N:N-1'
>
> which reverted changes in the WC which had previous 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.
>
> - Dan
>
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?

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.

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 Thu Jan 25 11:33:34 2007

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