You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2006/07/09 17:54:51 UTC
[PATCH] merge-tracking first merge being revert does not record 'svn:mergeinfo'
Hi All,
Find the attached patch.
With regards
Kamesh Jayachandran
[[[
Patch by: Kamesh Jayachandran <ka...@collab.net>
If the first merge is revert, 'svn:mergeinfo' is not getting recorded.
* subversion/libsvn_client/diff.c
(update_wc_merge_info):
If there is no 'svn:mergeinfo' recorded already and the merge is a revert
make use of the passed in 'ranges' for 'svn:mergeinfo' calculation.
]]]
Re: [PATCH] merge-tracking first merge being revert does not record
'svn:mergeinfo'
Posted by Kamesh Jayachandran <ka...@collab.net>.
While trying to fix merge_tests.py:simple_property_merges, I came across
this case.
Extended the case exhibited by this testcase as mentioned in my earlier
post.
Apart from theory I don't have any strong case to track that kind of
reverse mergeinfo.
With regards
Kamesh Jayachandran
Madan U Sreenivasan wrote:
> On Mon, 10 Jul 2006 22:52:20 +0530, Daniel Berlin
> <db...@dberlin.org> wrote:
>
>> Daniel Berlin wrote:
>>> Kamesh Jayachandran wrote:
>>>> Daniel Rall wrote:
>
> [snip]
>
>>>> Why not recording the reverse merges?
>
> [snip]
>
>> Reverse merges should probably be recorded somewhere.
>
> My personal opinion is that a reverse merge should effectively only
> remove one or more revisions from the existing merge-info.
>
> What purpose does it serve (even reporting wise) to record the history
> of a reverse merge (like 50-47)? I think it would only complicate our
> calculation of future merges with no apparent benefit. The probable
> questions that the user is going to ask to a merge-tracking system are:
> - What revisions of what branch do I have in this branch?
> - What are the pending revisions to bring a given branch in sync with
> another given branch?
> - What are the changesets (revisions) that came from branch1 to branch2?
>
> I can't imagine him asking:
> - What revisions did I unmerge given a particular source and target?
> I cant imagine it because, it does not solve any real life problem
> AFAICT.
>
> Having said that, reverse merge is only a theoretical problem. It does
> not manifest in real life. Or should I say we could avoid it if our
> forward-merge implementation is intelligent enough.
>
> Let me explain:
> There are two scenarios possible at the time a reverse merge happens
> wrt a given source.
>
> 1) There is a merge history (due to either an earlier merge or cp)
> from the source branch into the target.
> In this case, we only have to remove the appropriate revisions from
> the target mergeinfo.
>
> 2) There is no merge history whatsover between the source and the target.
> Imagine this case in a little bit of detail: Two codes bases for
> totally different tools (which have no historical relationships), and
> the user deems it necessary to take a diffset from the first codebase
> to apply to the second codebase. This is as good as applying a patch
> of a changeset from the first codebase into the second codebase. This
> IMHO does NOT count as a reverse merge (not even a merge - in the
> literal sense).
>
>
>> Blocking revisions from future merges (which seems to be your usecase)
>> is a completely different beast entirely.
>
> Agree.
>
> [snip]
>
> Regards,
> Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Re: [PATCH] merge-tracking first merge being revert does not record 'svn:mergeinfo'
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Mon, 10 Jul 2006 22:52:20 +0530, Daniel Berlin <db...@dberlin.org>
wrote:
> Daniel Berlin wrote:
>> Kamesh Jayachandran wrote:
>>> Daniel Rall wrote:
[snip]
>>> Why not recording the reverse merges?
[snip]
> Reverse merges should probably be recorded somewhere.
My personal opinion is that a reverse merge should effectively only remove
one or more revisions from the existing merge-info.
What purpose does it serve (even reporting wise) to record the history of
a reverse merge (like 50-47)? I think it would only complicate our
calculation of future merges with no apparent benefit. The probable
questions that the user is going to ask to a merge-tracking system are:
- What revisions of what branch do I have in this branch?
- What are the pending revisions to bring a given branch in sync with
another given branch?
- What are the changesets (revisions) that came from branch1 to
branch2?
I can't imagine him asking:
- What revisions did I unmerge given a particular source and target?
I cant imagine it because, it does not solve any real life problem
AFAICT.
Having said that, reverse merge is only a theoretical problem. It does not
manifest in real life. Or should I say we could avoid it if our
forward-merge implementation is intelligent enough.
Let me explain:
There are two scenarios possible at the time a reverse merge happens wrt a
given source.
1) There is a merge history (due to either an earlier merge or cp) from
the source branch into the target.
In this case, we only have to remove the appropriate revisions
from the target mergeinfo.
2) There is no merge history whatsover between the source and the target.
Imagine this case in a little bit of detail: Two codes bases for
totally different tools (which have no historical relationships), and the
user deems it necessary to take a diffset from the first codebase to apply
to the second codebase. This is as good as applying a patch of a changeset
from the first codebase into the second codebase. This IMHO does NOT count
as a reverse merge (not even a merge - in the literal sense).
> Blocking revisions from future merges (which seems to be your usecase)
> is a completely different beast entirely.
Agree.
[snip]
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] merge-tracking first merge being revert does not record
'svn:mergeinfo'
Posted by Daniel Berlin <db...@dberlin.org>.
Daniel Berlin wrote:
> Kamesh Jayachandran wrote:
>> Daniel Rall wrote:
>>> On Sun, 09 Jul 2006, Kamesh Jayachandran wrote:
>>> ...
>>>
>>>> If the first merge is revert, 'svn:mergeinfo' is not getting recorded.
>>>>
>>>> * subversion/libsvn_client/diff.c
>>>> (update_wc_merge_info):
>>>> If there is no 'svn:mergeinfo' recorded already and the merge is a revert
>>>> make use of the passed in 'ranges' for 'svn:mergeinfo' calculation.
>>>>
>>> ...
>>>
>>>> @@ -1751,9 +1751,14 @@
>>>>
>>>> if (is_revert)
>>>> {
>>>> - ranges = svn_rangelist_dup(ranges, pool);
>>>> - SVN_ERR(svn_rangelist_reverse(ranges, pool));
>>>> - SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
>>>> + if (rangelist->nelts == 0)
>>>> + rangelist = ranges;
>>>> + else
>>>> + {
>>>> + ranges = svn_rangelist_dup(ranges, pool);
>>>> + SVN_ERR(svn_rangelist_reverse(ranges, pool));
>>>> + SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
>>>> + }
>>>> }
>>>> else
>>>> SVN_ERR(svn_rangelist_merge(&rangelist, rangelist, ranges, pool));
>>>>
>>> This would record a reversed rangelist (e.g. "/trunk: 10-9, 7-5").
>>>
>>>
>> Yes, it is correct right?
>>> If a range list (e.g. "10-9, 7-5") for a merge source (e.g. "/trunk")
>>> is not recorded in a WC's merge info, it's implied that either there
>>> have been no merges from that source, or that merges haven't been
>>> recorded. Either way, I'm not really seeing why we should record the
>>> "reverted" range, as they either weren't already merged (likely a
>>> no-op?), or there's no history of them being merged (meaning we've no
>>> merge history to adjust).
>>>
>> Why not recording the reverse merges?
>> What about the following usecase?
>> Release_1 copied to Release_2
>> From Releases_2's Wc someone trying to cut off some
>> featureset(associated with a changeset 'ry:x' in Release_1) of Release_1
>> by 'merge -rx:y Release_1' where 'x>y'.
>
>> If this revert is not recorded, later readding(forward merges) same
>> reverted subset for featureset of Release_1 to Release_2, would be
>> recorded, which would look bit odd, given the fact that both 'Release_1'
>> and 'Release_2' share the history.
After talking with dlr and madan, we are pretty much all on the same page:
Reverse merges should probably be recorded somewhere.
Blocking revisions from future merges (which seems to be your usecase)
is a completely different beast entirely.
My *personal* feelings are:
reverse merges should go in svn:reversemergeinfo or something
blocking revisions should go in svn:blockedmergeinfo.
Neither blocking nor reverse merges should be mixed with svn:mergeinfo
as it stands now.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] merge-tracking first merge being revert does not record
'svn:mergeinfo'
Posted by Daniel Berlin <db...@dberlin.org>.
Kamesh Jayachandran wrote:
> Daniel Rall wrote:
>> On Sun, 09 Jul 2006, Kamesh Jayachandran wrote:
>> ...
>>
>>> If the first merge is revert, 'svn:mergeinfo' is not getting recorded.
>>>
>>> * subversion/libsvn_client/diff.c
>>> (update_wc_merge_info):
>>> If there is no 'svn:mergeinfo' recorded already and the merge is a revert
>>> make use of the passed in 'ranges' for 'svn:mergeinfo' calculation.
>>>
>> ...
>>
>>> @@ -1751,9 +1751,14 @@
>>>
>>> if (is_revert)
>>> {
>>> - ranges = svn_rangelist_dup(ranges, pool);
>>> - SVN_ERR(svn_rangelist_reverse(ranges, pool));
>>> - SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
>>> + if (rangelist->nelts == 0)
>>> + rangelist = ranges;
>>> + else
>>> + {
>>> + ranges = svn_rangelist_dup(ranges, pool);
>>> + SVN_ERR(svn_rangelist_reverse(ranges, pool));
>>> + SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
>>> + }
>>> }
>>> else
>>> SVN_ERR(svn_rangelist_merge(&rangelist, rangelist, ranges, pool));
>>>
>> This would record a reversed rangelist (e.g. "/trunk: 10-9, 7-5").
>>
>>
> Yes, it is correct right?
>> If a range list (e.g. "10-9, 7-5") for a merge source (e.g. "/trunk")
>> is not recorded in a WC's merge info, it's implied that either there
>> have been no merges from that source, or that merges haven't been
>> recorded. Either way, I'm not really seeing why we should record the
>> "reverted" range, as they either weren't already merged (likely a
>> no-op?), or there's no history of them being merged (meaning we've no
>> merge history to adjust).
>>
>
> Why not recording the reverse merges?
> What about the following usecase?
> Release_1 copied to Release_2
> From Releases_2's Wc someone trying to cut off some
> featureset(associated with a changeset 'ry:x' in Release_1) of Release_1
> by 'merge -rx:y Release_1' where 'x>y'.
> If this revert is not recorded, later readding(forward merges) same
> reverted subset for featureset of Release_1 to Release_2, would be
> recorded, which would look bit odd, given the fact that both 'Release_1'
> and 'Release_2' share the history.
They don't share history, so i'm not sure what you mean.
In fact, unless they very specifically asked for that range of revs to
be merged in, it wouldn't be remerged by default.
My understanding is that our default merge if you don't specify a
revision range will be <highest revnum in mergeinfo for that path>:HEAD.
This would still avoid remerging your feature set, unless it was at the
very tip.
I'm very wary of recording reverse ranges in the same property, because
they really have the exact opposite meaning of everything in there right
now.
IMHO, it would significantly complicate merge combining, and most of the
rangelist operations.
If we want to record reverse merges like this, we should have
svn:antimergeinfo or something :)
Or maybe even "svn:blockedmerges", which is exactly how svnmerge does it.
At least, that's my initial thought.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] merge-tracking first merge being revert does not record
'svn:mergeinfo'
Posted by Kamesh Jayachandran <ka...@collab.net>.
Daniel Rall wrote:
> On Sun, 09 Jul 2006, Kamesh Jayachandran wrote:
> ...
>
>> If the first merge is revert, 'svn:mergeinfo' is not getting recorded.
>>
>> * subversion/libsvn_client/diff.c
>> (update_wc_merge_info):
>> If there is no 'svn:mergeinfo' recorded already and the merge is a revert
>> make use of the passed in 'ranges' for 'svn:mergeinfo' calculation.
>>
> ...
>
>> @@ -1751,9 +1751,14 @@
>>
>> if (is_revert)
>> {
>> - ranges = svn_rangelist_dup(ranges, pool);
>> - SVN_ERR(svn_rangelist_reverse(ranges, pool));
>> - SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
>> + if (rangelist->nelts == 0)
>> + rangelist = ranges;
>> + else
>> + {
>> + ranges = svn_rangelist_dup(ranges, pool);
>> + SVN_ERR(svn_rangelist_reverse(ranges, pool));
>> + SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
>> + }
>> }
>> else
>> SVN_ERR(svn_rangelist_merge(&rangelist, rangelist, ranges, pool));
>>
>
> This would record a reversed rangelist (e.g. "/trunk: 10-9, 7-5").
>
>
Yes, it is correct right?
> If a range list (e.g. "10-9, 7-5") for a merge source (e.g. "/trunk")
> is not recorded in a WC's merge info, it's implied that either there
> have been no merges from that source, or that merges haven't been
> recorded. Either way, I'm not really seeing why we should record the
> "reverted" range, as they either weren't already merged (likely a
> no-op?), or there's no history of them being merged (meaning we've no
> merge history to adjust).
>
Why not recording the reverse merges?
What about the following usecase?
Release_1 copied to Release_2
From Releases_2's Wc someone trying to cut off some
featureset(associated with a changeset 'ry:x' in Release_1) of Release_1
by 'merge -rx:y Release_1' where 'x>y'.
If this revert is not recorded, later readding(forward merges) same
reverted subset for featureset of Release_1 to Release_2, would be
recorded, which would look bit odd, given the fact that both 'Release_1'
and 'Release_2' share the history.
P.S Incase we record reverse merges we need to be careful about -r5:4
case, this revert gets recorded as '5', then there would be a confusion
whether it is 4-5 or 5-4.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Re: [PATCH] merge-tracking first merge being revert does not record 'svn:mergeinfo'
Posted by Madan U Sreenivasan <ma...@collab.net>.
On Mon, 10 Jul 2006 12:37:56 +0530, Daniel Rall <dl...@collab.net> wrote:
> On Sun, 09 Jul 2006, Kamesh Jayachandran wrote:
> ...
>> If the first merge is revert, 'svn:mergeinfo' is not getting recorded.
[snip]
> This would record a reversed rangelist (e.g. "/trunk: 10-9, 7-5").
>
> If a range list (e.g. "10-9, 7-5") for a merge source (e.g. "/trunk")
> is not recorded in a WC's merge info, it's implied that either there
> have been no merges from that source, or that merges haven't been
> recorded. Either way, I'm not really seeing why we should record the
> "reverted" range, as they either weren't already merged (likely a
> no-op?), or there's no history of them being merged (meaning we've no
> merge history to adjust).
Exactly... I think so too... this is just an 'apply of a selected diff'
not a merge in its true sense.
OTOH, I think every branching operation ('svn cp') should record the merge
info of the source branch and its current revision (we already copy over
the indirect merge information during 'svn cp'). If we do that, we can
truly eliminate the need for recording reverse merges.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] merge-tracking first merge being revert does not record 'svn:mergeinfo'
Posted by Daniel Rall <dl...@collab.net>.
On Sun, 09 Jul 2006, Kamesh Jayachandran wrote:
...
> If the first merge is revert, 'svn:mergeinfo' is not getting recorded.
>
> * subversion/libsvn_client/diff.c
> (update_wc_merge_info):
> If there is no 'svn:mergeinfo' recorded already and the merge is a revert
> make use of the passed in 'ranges' for 'svn:mergeinfo' calculation.
...
> @@ -1751,9 +1751,14 @@
>
> if (is_revert)
> {
> - ranges = svn_rangelist_dup(ranges, pool);
> - SVN_ERR(svn_rangelist_reverse(ranges, pool));
> - SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
> + if (rangelist->nelts == 0)
> + rangelist = ranges;
> + else
> + {
> + ranges = svn_rangelist_dup(ranges, pool);
> + SVN_ERR(svn_rangelist_reverse(ranges, pool));
> + SVN_ERR(svn_rangelist_remove(&rangelist, ranges, rangelist, pool));
> + }
> }
> else
> SVN_ERR(svn_rangelist_merge(&rangelist, rangelist, ranges, pool));
This would record a reversed rangelist (e.g. "/trunk: 10-9, 7-5").
If a range list (e.g. "10-9, 7-5") for a merge source (e.g. "/trunk")
is not recorded in a WC's merge info, it's implied that either there
have been no merges from that source, or that merges haven't been
recorded. Either way, I'm not really seeing why we should record the
"reverted" range, as they either weren't already merged (likely a
no-op?), or there's no history of them being merged (meaning we've no
merge history to adjust).