On Mon, Sep 22, 2008 at 8:04 AM, Danil Shopyrin
<danil.shopyrin_at_gmail.com> wrote:
>> Let's assume your target branch looks like this (these other paths
>> under trunk help better illustrate what I'm going to propose):
>>
>> /branch
>> /branch/bar.c
>> /branch/subdir
>>
>> We tweak the merge logic to set non-inheritable mergeinfo on the merge
>> target describing the merge, and normal mergeinfo on all the target's
>> present children, like this:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2,4* <--- Non-inheritable mergeinfo.
>> Properties on 'branch/bar.c':
>> svn:mergeinfo
>> /trunk/bar.c:2,4
>> Properties on 'branch/subdir':
>> svn:mergeinfo
>> /trunk/subdir:2,4
>>
>> Of course if the skipped path was a deeper subtree and not an
>> immediate child of our merge target we would adjust accordingly and
>> set the non-inheritable mergeinfo on the skipped path's immediate
>> parent and the normal mergeinfo on the skipped path's siblings.
>>
>> If we commit the above merge and then merge -c3 into branch now foo.c
>> is added but it only has mergeinfo for r2-3:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2-3,4*
>> Properties on 'branch/foo.c':
>> svn:mergeinfo
>> /trunk/foo.c:2-3
>> Properties on 'branch/bar.c':
>> svn:mergeinfo
>> /trunk/bar.c:2-4
>> Properties on 'branch/subdir':
>> svn:mergeinfo
>> /trunk/subdir:2-4
>>
>> Note: In my ad hoc testing foo.c isn't getting r2 in it's mergeinfo,
>> it should, but that can be fixed. Assuming that we fix this, if we
>> re-merge -r4 to branch then foo.c gets that change and all the
>> mergeinfo elides* up to branch and we are left with is this:
>>
>> Properties on 'branch':
>> svn:mergeinfo
>> /trunk:2-4
>>
>> I think this change is fairly simple, we already catch similar cases -
>> see libsvn_client/merge.c:get_mergeinfo_paths(). The only difference
>> is that we can't identify these paths at the start of the merge but
>> must catch them once we realize they are skipped. The
>> notification_receiver_baton_t struct keeps track of these already in
>> its skipped_paths member so this shouldn't be too hard.
>>
>> I can crank out a patch that does what I propose in fairly short order
>> if this idea solves your problem.
>>
>> * In the interests of full disclosure, in another thread I am
>> proposing the elimination of automatic mergeinfo elision as a fairly
>> useless feature...ironically here is a case where it is nice to have
>> :-\
>>
>>> XXX big ugly hack idea: Could we extend the mergeinfo
>>> format so that parent directories can record "negative"
>>> mergeinfo for direct children which were skipped?
>>
>> Befitting a "big ugly hack" there's a problem :-) What if, in your
>> example, r4 also affected trunk directly (e.g. we added a svn:ignore
>> property). It could get a bit ugly if we want a way to record that r4
>> affected trunk, but not one of its (currently) non-existent children.
>
> We've discussed this informally at VisualSVN and have found
> alternative solution.
>
> Let's assume that our target branch looks like this:
> /branch
> /branch/bar.c
> /branch/subdir
>
> And we have /trunk/foo.c added in r3. Files /trunk/bar.c and
> /trunk/foo.c are both changed in r4.
>
> We are merging r4 to the branch. Since r4 contains changes for an
> absent file (i.e. foo.c will be skipped) this revision can't be
> treated as regularly merged into branch. So the merge should produce
> the following mergeinfo in the trunk:
>
> Properties on 'branch':
> svn:mergeinfo
> <empty> //r4 isn't merged completely so we can't claim that it's merged!
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:4
> Properties on 'branch/subdir':
> svn:mergeinfo
> <empty>
>
> From our point of view, it's far more consistent, because r4 isn't
> actually merged.
There is one serious problem with this solution: It won't work if r4
affects 'trunk' directly (e.g. a prop change on 'trunk').
Because in that case the prop change in r4 will be merged into
'branch' so we need to record it in 'branch's mergeinfo. But if we
add '/trunk:4' to 'branch's mergeinfo that implies that
'/branch/foo.c' got r4, which of course it didn't since 'foo.c' isn't
present yet. I mentioned this problem earlier in this thread - see
"Befitting a "big ugly hack" there's a problem".
Do you understand why this is a problem? If not let's resolve that first!
> Only part of this revision is merged. It's better
> than write that r4 is merged "non-inheritable", because
> 'non-inheritable' merged revisions make no sense for an average user.
So normal 'inheritable' mergeinfo makes sense to users, but
non-inheritable mergeinfo doesn't? Heck, I'm not very fond of
mergeinfo as an inheritable property, but it's what we have at the
moment and I don't see how non-inheritable properties are
fundamentally more complex.
Also, am I to take it that you are proposing removing non-inheritable
mergeinfo? Unless we are planning on disabling merge-tracking when
performing shallow merges (i.e. with depths other than infinity), when
merging into shallow working copies, working copies with switched
subtrees, or WCs with subtrees missing due to authorization
restrictions, then we need non-inheritable mergeinfo. These problems
and the problem we are discussing in this thread are all subsets of
the same basic issue: How do we account for subtrees that are
(potentially) affected by a merge but that are missing from the
working copy. The solution to this class of problems is currently
non-inheritable mergeinfo. I'd like whatever solution is settled on
for this problem to the solution we use for this whole class of
problems.
> Note that we record that /trunk/bar.c has merged changes from r4. So
> there will be no conflicts in the future.
>
> Then we are merging r3 and get the following mergeinfo:
> Properties on 'branch':
> svn:mergeinfo
> /trunk:3
> Properties on 'branch/bar.c':
> svn:mergeinfo
> /trunk/bar.c:4
> Properties on 'branch/foo.c':
> svn:mergeinfo
> <empty>
> Properties on 'branch/subdir':
> svn:mergeinfo
> <empty>
Note: This example assumes the ideas proposed in
http://svn.haxx.se/dev/archive-2008-09/0443.shtml have been
implemented (but that is hardly a done deal). Specifically, r3
doesn't affect 'bar.c' so that revision is not recorded in 'bar.c's
mergeinfo. Not a big deal, but worth noting.
> Now user can find out that r4 is still not merged completely
Seriously? Can you state the general steps one would use to determine
this? Before you do, keep in mind what "real" mergeinfo looks like;
go run svn pg svn:mergeinfo -vR on any of the Subversion branches to
see what I mean. Then tell me how easy it will be to spot a single
revision or revision range that exists on one subtree but not on the
others.
I'd go as far as to say that the non-inheritable marker '*' makes it
somewhat easier to spot what revisions haven't been merged completely.
> on branch and merge it once again.
>
> We will get the following mergeinfo:
>
> Properties on 'branch':
> svn:mergeinfo
> /trunk:3,4
> Properties on 'branch/bar.c':
> svn:mergeinfo
> <empty>
> Properties on 'branch/foo.c':
> svn:mergeinfo
> <empty>
> Properties on 'branch/subdir':
> svn:mergeinfo
> <empty>
>
> Note than now we can elide mergeinfo on 'branch/bar.c'.
Actually no, until we rewrite the elision code this won't work.
Because when you perform the merge above, at the end of the merge,
before elision is attempted you will have mergeinfo that looks like
this:
Properties on 'branch':
svn:mergeinfo
/trunk:3,4
Properties on 'branch/bar.c':
svn:mergeinfo
/trunk/bar.c:4
The mergeinfo on '/branch/bar.c' will not elide to 'branch'. The
elision code needs to be rewritten to contact the server to check that
r3 doesn't impact '/branch/bar.c' and can thus be ignored for purposes
of eliding. Remember that "real" world mergeinfo is not going to be
this simple and thus we might contact the repository a *lot* for this
purpose and pay steep performance penalty for doing so.
> Advantages:
> * user understand that some revisions are merged incompletely;
As stated above I couldn't disagree more with this statement.
> * less mergeinfo records generated;
Agreed, less explicit mergeinfo may be generated. It's not correct in
all situations, but there is less of it.
> * user can easily figure our why r4 mergeinfo is written on /branch/bar.c
I don't want to sound like a broken record, but this behavior doesn't
seem any clearer than what I am proposing.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-22 17:05:59 CEST