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

Re: issue-2897(reflective merge solution)

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Wed, 06 Aug 2008 21:59:29 +0530

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

>
> Thanks, Kamesh. I have not followed the development of merge tracking
> very closely, so I am one of the "general audience" trying to understand
> this now.
>

Welcome.

>
>> Problem:
>> - ---------------------
>> - - Create /feature_branch from /trunk.
>> - - Synch up change(rX) from /trunk to /feature_branch and commit at rY
>> along with some other local changes 'Z'.
>
> To keep things simple and concrete, I will assume that, before this
> merge, /feature_branch had already been synchronised with all of the
> changes up to r(X-1) from /trunk.

Fine. By the above example I meant that 'final reflective merge
solution' should support cherry picking also.

>
> If the user is following best practices, then the extra change 'Z' is
> what is needed to resolve conflicts.
>

May be, some times people may do logical conflict resolution(i.e not SVN
conflict), for example merge may be successful but logically incorrect
with respect to the state of things in feature branch, user may try to
fix this inconsistency along with the merge and commit to ensure sanity
of his commit.

>
>> Available solution:
>> - ------------------------
>
> Solution 0: If you were to use Subversion 1.5's plain "svn
> merge /feature_branch /trunk" command, then Subversion would omit rY
> entirely, which is wrong because we need the conflict-resolution part of
> the change. Without that part, the merge would throw up those same (or
> similar) conflicts again, which the user would have to resolve again.
>

I believe you mean 'svn merge trunk_wc ^/feature_branch'
This would *not* leave rY rather it would do a 'mad' merge
possibly(mostly) causing repeat merge problem.

Only instance it would not cause conflict is when there is enough
distance between 'trunk' and 'feature_branch' hunks.

>> Solution 1: One can use '--reintegrate' switch to 'merge' command, but
>> it has lots of expectation on merge-target, which makes it unusable on a
>> feature_branch with renamed subtrees which is common when someone does a
>> refactoring there.(issue-3128).
>
> The restriction that makes "--reintegrate" unusable with sub-tree
> mergeinfo is something we could fix. It should check whether the subtree
> mergeinfo meets the criteria for what situations the command can handle.
> Currently it doesn't attempt to check this, and just assumes the
> merrgeinfo doesn't meet the criteria.

OK.

>
> I know solution 2 can handle many more general cases, but, in the simple
> cases that "--reintegrate" is intended to handle, does your solution 2
> achieve the same result, or does it have a different concept of what is
> the right result?

It has no special cases, it just segregates a range of revisions to
merge as
[ordinary_range-1]...[reflective_rev-1]....[reflective-rev-2]...[ordinary_range-N]...

Existing merge call backs are used for 'ordinary_ranges.

Reflective call backs are used for 'reflective revs'.

Purpose of Reflective call backs is 'extract non-reflective change and
merge the same'.

>
>> Solution 2: issue-2897 branch 'extract non-reflective changes from a
>> reflective revision and apply'.
>> This branch relies on availability of 'what revisions+merge_sources
>> got merged in a commit'. It had this information implemented in 'sqlite'
>> our old 'mergeinfo' store.
>> In January we have changed this 'sqlite' based mergeinfo storage to a
>> 'fs' based 'storage', which this implementation has not caught up with yet.
>> Given the issue-3128 in 'Solution 1' it seems 'Solution 2' is worth a
>> try.
>>
>> Solution 2 explained:
>>
>> Part 1: Store mergeinfo_added in a commit in the back ground(Currently I
>> have kind of local patch implementing the same for bdb, I will initiate
>> a separate discussion once my local patch becomes clean and mature).
>
> By "mergeinfo_added" do you mean the change in mergeinfo from r(X-1) to
> r(X), for each revision X? So this is a cache to improve the speed of
> mergeinfo calculations.

Yes.

>
>> Part 2: Retrieve the 'reflective rev and reflected merge ranges' within
>> a given merge range via the API. See API doc of
>> 'svn_fs_get_commit_and_merge_ranges'
>> http://svn.collab.net/repos/svn/branches/issue-2897/subversion/include/svn_fs.h
>>
>> Part 3: Treat the reflective revision specially as detailed below by
>> custom merge call back.
>>
>>
>> a) reflective text changes:
>> reflective_merge_file_changed(mine, older, yours)
>> mine = WC file.
>> older = file_at_reflective_rev-1
>> yours = file_at_reflective_rev
>>
>>
>> Normal merge_file_changed applies diff(older, yours) to mine.
>> This diff can contain the changes 'synch up from trunk' as well as the
>> local adhoc changes and conflict resolutions.
>>
>> Applying the diff directly is going to cause lots of headaches.
>>
>> So objective is to have a meaningful diff of *only local adhoc changes
>> done before committing the merge*.
>>
>> Let me take the example and explain,
>>
>>
>> Case1:
>>
>> We merge -r13, -r17, -r29 from /trunk to /feature_branch and commit at
>> r40 (Assume this is the only synch up)
>
> I'm trying to follow along. Is this cherry-picking, or a complete synch?
> I'll give a running commentary of how I interpret this.

Here it is 'cherry pick', it can be a cherry harvest too.

>
> I think you mean trunk was changed only in those three revisions, and
> we're doing a complete synch.
>

No, trunk may have other changes say in r20 and r22. Let us say while
working on feature branch I see some important changes in r13, r17 and
r29 from trunk of immediate interest to me and I am not prepared to
accept any other changes.

> Let's say no conflict resolution was necessary, and no other changes
> were made during this synch.
>
>> Now we merge /feature_branch -r1:40 back to /trunk.
>>
>> It does a merge of -r1:39 which is a normal merge.
>
> "It" is "Solution 2". A "normal merge" is Subversion's present
> merge-tracking "svn merge", which will therefore merge revision ranges
> 1:12, 13:16, 17:28, 29:39 from feature_branch to trunk, omitting the
> three revisions that it sees have come from trunk.
>

No, I meant 'normal present 1.5 merge'. To make the discussion more
complete let us say we cut '/feature_branch' from '/trunk_at_9' at r10.

Now 'svn-issue-2897-client merge trunk_wc ^/feature_branch' will merge
like the following,

To start with it has to merge 1:40 from ^/feature_branch, it will break
to following pieces based on 2 info.
info 1
- --------
What is already merged from ^/feature_branch to trunk_wc
Ans: implict /trunk:1-9

So effectively it has to merge r10:40 from ^/feature_branch

info 2(new ra call get_commit_and_merge_ranges)
- ---------------
This call will tell us

r40->[13, 17, 29]

Then we spilt the merge range [10:40] to [10:39] and [40]

[10:39] is normal svn 1.5 style normal merge.
[40] reflective merge.

>> It does a reflective merge of 40.
>
> A "reflective merge" is the new algorithm that calculates and applies
> the "Z" diff that you mentioned above.
>
> It is going to merge (r39:40 /feature_branch/test.c) to
> the /trunk/test.c working copy which has already got local scheduled
> changes in it from the r1:39 phase of this overall merge.

Yes.

>
>> Let us say /feature_branch/test.c got a change from a merge at r40(Our
>> first merge)
>>
>> To calculate the *meaningful diff*, We apply -r13 change to OLDER, and
>> r17 change to OLDER, r29 change to OLDER.
>
> For this "reflective merge" phase of just r40,
>
> MINE is /trunk/test.c working copy (BASE is r40)
> OLDER is /feature_branch/test.c_at_39
> YOURS is /feature_branch/test.c_at_40
>
> Applying diffs (-c13,17,29 /trunk/test.c) to OLDER is exactly what
> happened in r40.
>
>> Now in this case after the above 3 merges(r13, r17, r29) OLDER becomes
>> YOURS and hence no change is applied to MINE.
>
> Yes, if test.c has never been modified on the branch in this example.
>
>

YES.

>> Case2:
>>
>> We merge -r13, -r17, -r29 from /trunk to /feature_branch + local
>> non-conflicting change to /feature_branch/test.c and commit at r40
>> (Assume this is the only synch up)
>>
>> Now we merge /feature_branch -r1:40 back to /trunk.
>>
>> It does a merge of -r1:39 which is a normal merge.
>> It does a reflective merge of r40.
>> Let us say /feature_branch/test.c got a change from a merge at r40(Our
>> first merge)
>>
>> To calculate the *meaningful diff*, We apply -r13 change to OLDER, and
>> r17 change to OLDER, r29 change to OLDER.
>
> OK, the steps are the same so far.

YES.

>
>> Now in this case after the above 3 merges(r13, r17, r29) OLDER becomes
>> (YOURS-'*local non conflicting changes*') and hence
>
> Just to clarify, the "-" here is a "minus" or "subtraction" sign.

Yes you can think it of something like 'All of YOURS except *local non
conflicting changes*'

>
>> (YOURS-YOURS +'*local non conflicting changes*') is applied to MINE. i.e
>> '*local non conflicting changes*'
>
> OK, great: this is where Solution 2 wins over the previous solutions.
>
>
>> Case3:
>>
>> We merge -r13, -r17(conflicting), -r29 from /trunk to /feature_branch +
>> local non-conflicting change + conflict resolution to r17 to
>> /feature_branch/test.c and commit at r40
>> (Assume this is the only synch up)
>>
>> Now we merge /feature_branch -r1:40 back to /trunk.
>>
>> It does a merge of -r1:39 which is a normal merge.
>> It does a reflective merge of 40.
>> Let us say /feature_branch/test.c got a change from a merge at r40(Our
>> first merge)
>>
>> To calculate the *meaningful diff*, We apply -r13 change to OLDER, and
>> r29 change to OLDER. (SEE WE DONT APPLY r17 as it is a conflicting one)/
>
> How does Subversion know that r17 was a conflicting one? Normally when
> Subversion wants to merge three successive changes from /trunk it
> applies a single diff (r1:39). So here you're saying Subversion tries to
> merge each individual source revision in turn, and notices which ones
> conflict?
>

The moment we identify 'r40' as reflective we have 'r13, r17 and r29' as
 merge_cmd_baton->reflected_ranges list.

Algo looks like this
for each range in merge_cmd_baton->reflected_ranges:
  copy OLDER to TMP
  retval = merge change represented by range from /trunk to TMP
  if retval == success:
    rename TMP OLDER

> Oh, I see: you must mean r13, r17, r29 were merged in three separate
> phases in the original merge. (This could be because some changes
> on /trunk in both of the ranges r13:16 and r17:28 had already been
> merged to the branch.) Is that right?
>

Yes they can be three independent merge or one merge with comma
separated revnum as supported by 1.5.

>> Now in this case after the above 2 merges(r13, r29) OLDER becomes
>> (YOURS-local non conflicting changes - r17 - conflict resolution to r17)
>> and hence
>> (YOURS-YOURS +local non conflicting changes + r17 + conflict resolution
>> to r17) is applied to MINE. i.e local non conflicting changes + r17 +
>> conflict resolution to r17. Moral: If synch up gave a conflict reverse
>> also would give.
>
> So, this works well for non-conflicting changes, and for conflicting
> changes it produces conflicts by merging the same change again.
>
> In summary:
>
> * Each source change that originally merged cleanly
> into /feature_branch will correctly be omitted from the re-integration
> to /trunk.
>
> * Each independent change made to /feature_branch, including changes
> made in the same revision as a merge, will correctly be re-integrated
> to /trunk.
>
> * Each conflict resolution performed on /feature_branch will correctly
> be included in the re-integration.

 Yes, after annoying with the user with the conflict again as mentioned
above.
>
> * Each revision from /trunk that conflicted when merged
> into /feature_branch will wrongly be included in the re-integration,
> creating new conflicts.
>
Yes. May be we can be bit more smart here once we have this stuff working.
>
> Have I understood this well enough?
>

Yes, you understood well.

Sorry for the delay, it needs some quality undisturbed length of time to
think about 'reflective merges'.

Thanks for the review.

With regards
Kamesh Jayachandran
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFImdFo3WHvyO0YTCwRAsajAJ99f+BWNfbipZwLM509JAzqJVH0zQCggldN
vtcrKj35I4HaZXd/WSe66WQ=
=XuVU
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-06 18:30:38 CEST

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.