You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2008/11/05 15:02:50 UTC
Merging the reintegrate-improvements branch back to trunk
The http://svn.collab.net/repos/svn/branches/reintegrate-improvements
branch is ready to be merged back to trunk.
This branch makes two significant changes:
1) Allow reintegrate to work when the reintegrate merge source has
explicit subtree mergeinfo, as long as that mergeinfo represents that
all of the target was previously merged to the root of the source.
Today reintegrate is largely useless because once you have subtree
mergeinfo on 'trunk', when you copy 'trunk' to 'some_branch' there is
explicit subtree mergeinfo on the latter, which stops subsequent
reintegration. But if all you have done is to make changes to
'some_branch' and synched all available revisions from 'trunk' to
'some_branch', then reintegrate's underlying 2-URL merge should be
allowed to work (i.e. the branch, edit branch, synch, reintegrate
workflow described in the Subversion book should work).
2) Improve the error message if a reintegrate merge is legitimately
thwarted by subtree mergeinfo on the source. By "legitimately" I mean
if you have done subtree merges from 'trunk' to 'some_branch' in such
a way that 'some_branch' has not had all of 'trunk' uniformly merged
to it. Previously if the reintegrate source had *any* explicit
mergeinfo we returned an error like this:
C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate --accept postpone
http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
.
svn: Cannot reintegrate from
'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees'
yet:
Some revisions have been merged under it that have not been merged
into the reintegration target; merge them first, then retry.
I changed text of this error on the reintegrate-improvements branch,
but also added a dump of the offending subtree paths and their
mergeinfo so now a user has some hope(?) of seeing what is wrong:
C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate
http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
.
svn: Reintegrate can only be used if the revisions previously merged
from the reintegrate target to
'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
are the same', but there are differences:
branches/issue-3067-deleted-subtrees
/trunk:33301-33960
branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
/trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
My hope is that a user could see the above and realize, "oh, I merged
r33301-33960 from trunk to my branch, but only
r33301-33923,33925-3396to win32_crypto.c, the latter needs r33924
too." Of course in doing so the user violated the reintegrate
workflow, maybe they reverse merged r33924 from trunk to
win32_crypto.c, but at least we give them an idea of where to look for
the problem.
Notice that only the subtree paths that differ from the root of the
source are displayed. In the above example there are many other paths
in branches/issue-3067-deleted-subtrees that have explicit mergeinfo,
but they all show that r33301-33960 have been merged from trunk. Only
the branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
subtree differs. Notice also that all mergeinfo from other branches
has been filtered out, only mergeinfo from trunk is shown as that is
all that matters here.
The one thing I dislike about this error is the size to which it can
grow (and become completely useless) if the reintegrate source is
vastly out of synch with the target -- see the attached output.
Does this error make sense? Should we keep it? Any suggestions for improvement?
~~~~~
So if anyone familiar with merge tracking, specifically the
reintegrate code, would like to take a look I'd appreciate it.
Alternatively, if you are going to merge a branch back to trunk please
try it with this branch and use --reintegrate. And finally, if you
have any thoughts on the new error message, have at it.
Thanks,
Paul
Re: Merging the reintegrate-improvements branch back to trunk
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
> > Alternative: don't print subtrees and rev-ranges at all, unless --verbose
> > (or a new --bikeshed flag...) is given.
>
> Daniel - I prefer we have the aforementioned detailed error message
> (whatever its final form may be) by default. My thinking is that if
> --reintegrate fails, and the user knows that --verbose will give them
> additional information to solve the problem, then they are *always*
> going to repeat the merge with --verbose no? So why not just give it
> to them the first time?
>
You said it: because the error might be several screenfuls tall.
> Also, if you are dealing with an extremely large WC with a lot of
> subtree mergeinfo it can still take some significant time to calculate
> all this (due to the tree walk required). I think it might be
> frustrating to do the merge, wait, get the error, then be forced to do
> it again so you can get the useful error.
>
For "extremely large" wc's, *anything* takes a significant amount of time.
> Lastly, I don't like tying the quality of the error message to an
> option. Do we do that anywhere else? We'd have to add the -v option
> to merge just for this.
>
Not that I could find.
> Paul
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging the reintegrate-improvements branch back to trunk
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Nov 5, 2008 at 12:08 PM, Julian Foad <ju...@btopenworld.com> wrote:
> On Wed, 2008-11-05 at 10:27 -0500, Mark Phippard wrote:
>> 2008/11/5 Paul Burba <pt...@gmail.com>:
>> > The http://svn.collab.net/repos/svn/branches/reintegrate-improvements
>> > branch is ready to be merged back to trunk.
>> >
>> > This branch makes two significant changes:
>> >
>> > 1) Allow reintegrate to work when the reintegrate merge source has
>> > explicit subtree mergeinfo, as long as that mergeinfo represents that
>> > all of the target was previously merged to the root of the source.
>>
>> Yay!
>
> That's fantasic, Paul.
>
>> > 2) Improve the error message if a reintegrate merge is legitimately
>> > thwarted by subtree mergeinfo on the source. By "legitimately" I mean
>> > if you have done subtree merges from 'trunk' to 'some_branch' in such
>> > a way that 'some_branch' has not had all of 'trunk' uniformly merged
>> > to it. Previously if the reintegrate source had *any* explicit
>> > mergeinfo we returned an error like this:
>> >
>> > C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate --accept postpone
>> > http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
>> > .
>> > svn: Cannot reintegrate from
>> > 'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees'
>> > yet:
>> > Some revisions have been merged under it that have not been merged
>> > into the reintegration target; merge them first, then retry.
>> >
>> > I changed text of this error on the reintegrate-improvements branch,
>> > but also added a dump of the offending subtree paths and their
>> > mergeinfo so now a user has some hope(?) of seeing what is wrong:
>>
>> Yay again!
>
> +1.
>
>> > C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate
>> > http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
>> > .
>> > svn: Reintegrate can only be used if the revisions previously merged
>> > from the reintegrate target to
>> > 'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
>> > are the same', but there are differences:
>> > branches/issue-3067-deleted-subtrees
>> > /trunk:33301-33960
>> > branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
>> > /trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
>> >
>> > My hope is that a user could see the above and realize, "oh, I merged
>> > r33301-33960 from trunk to my branch, but only
>> > r33301-33923,33925-3396to win32_crypto.c, the latter needs r33924
>> > too." Of course in doing so the user violated the reintegrate
>> > workflow, maybe they reverse merged r33924 from trunk to
>> > win32_crypto.c, but at least we give them an idea of where to look for
>> > the problem.
>>
>> Why does the part about trunk show up? I find that confusing because
>> it sounds like it is flagging it as part of the problem and I do not
>> think you mean to say that.
>
> You mean why print "/trunk:..." (the root path of the merge) as well as
> "/trunk/subversion/lib...:..." (a subtree)? It's so the user can see the
> differences.
That's exactly it, I want to show the target's relative mergeinfo so
the user can see the problem with the subtree in comparison to it.
> Of course that leads to a suggestion for improvement: show the
> difference between each subtree's mergeinfo and the root's mergeinfo.
I like that idea, how to show it might be a bit tricky, how about this:
C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate
http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
.
svn: Reintegrate can only be used if the revisions previously merged
from the reintegrate target to
'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
are the same', but there are differences:
branches/issue-3067-deleted-subtrees
MERGED: /trunk:33301-33960
branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
MERGED: /trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
MISSING: /trunk/subversion/libsvn_subr/win32_crypto.c:33924
>> > Notice that only the subtree paths that differ from the root of the
>> > source are displayed. In the above example there are many other paths
>> > in branches/issue-3067-deleted-subtrees that have explicit mergeinfo,
>> > but they all show that r33301-33960 have been merged from trunk. Only
>> > the branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
>> > subtree differs. Notice also that all mergeinfo from other branches
>> > has been filtered out, only mergeinfo from trunk is shown as that is
>> > all that matters here.
>> >
>> > The one thing I dislike about this error is the size to which it can
>> > grow (and become completely useless) if the reintegrate source is
>> > vastly out of synch with the target -- see the attached output.
>> >
>> > Does this error make sense? Should we keep it? Any suggestions for improvement?
>>
>> I do not see how we can do anything about the possibly large
>> differences. As long as the information is correct, I do not see a
>> problem with it. Is it possible for the error message to describe
>> what they need to do next? Are there a specific set of revisions that
>> need to be merged into the "root" to resolve this subtree mergeinfo
>> problem?
>
> I think it's worth doing something to prevent ridiculously long error
> messages, as they might fill the user's console or dialogue box chock
> full of revision numbers so that the actual text of the message can't be
> seen. There are simple solutions: e.g. print up to 5 subtrees, followed
> by "and N more subtrees", and for each subtree print up to 10 rev-ranges
> followed by "...".
>
> But not before merging to trunk.
Agreed, we can work out the details later. Merged back to trunk in r34091.
>> I think the error message is fine. Here is an attempt at an alternative:
>>
>> svn: Reintegrate cannot be used with specified merge source due to
>> differences in subtree mergeinfo. Here are the differences that were
>> found:
>> branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
>> /trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
>>
>> I do not like this suggestion as is. I am trying to make it more to
>> the point but do not think I achieved that. I put it out there to
>> advance the conversation. I repeat that I think this should just go
>> to trunk.
I'm totally open to improving the static text of the error. You might
be on to something with not printing the whole URL. Maybe this?
svn: Reintegrate cannot be used because the target was not uniformly
merged to the source. The following inconsistencies were found:
branches/issue-3067-deleted-subtrees
MERGED: /trunk:33301-33960
branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
MERGED: /trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
MISSING: /trunk/subversion/libsvn_subr/win32_crypto.c:33924
>> I think you should probably merge this to trunk as is. I mainly offer
>> that last comment as a suggestion for where this could possibly go
>> next.
>
> +1. It's a _major_ improvement. Ship it!
>
> - Julian
On Wed, Nov 5, 2008 at 12:27 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Alternative: don't print subtrees and rev-ranges at all, unless --verbose
> (or a new --bikeshed flag...) is given.
Daniel - I prefer we have the aforementioned detailed error message
(whatever its final form may be) by default. My thinking is that if
--reintegrate fails, and the user knows that --verbose will give them
additional information to solve the problem, then they are *always*
going to repeat the merge with --verbose no? So why not just give it
to them the first time?
Also, if you are dealing with an extremely large WC with a lot of
subtree mergeinfo it can still take some significant time to calculate
all this (due to the tree walk required). I think it might be
frustrating to do the merge, wait, get the error, then be forced to do
it again so you can get the useful error.
Lastly, I don't like tying the quality of the error message to an
option. Do we do that anywhere else? We'd have to add the -v option
to merge just for this.
Paul
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging the reintegrate-improvements branch back to trunk
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Wed, 5 Nov 2008 at 17:08 -0000:
> On Wed, 2008-11-05 at 10:27 -0500, Mark Phippard wrote:
> > 2008/11/5 Paul Burba <pt...@gmail.com>:
> Of course that leads to a suggestion for improvement: show the
> difference between each subtree's mergeinfo and the root's mergeinfo.
>
*nod*
> > > Does this error make sense? Should we keep it? Any suggestions for improvement?
> >
> > I do not see how we can do anything about the possibly large
> > differences. As long as the information is correct, I do not see a
> > problem with it. Is it possible for the error message to describe
> > what they need to do next? Are there a specific set of revisions that
> > need to be merged into the "root" to resolve this subtree mergeinfo
> > problem?
>
> I think it's worth doing something to prevent ridiculously long error
> messages, as they might fill the user's console or dialogue box chock
> full of revision numbers so that the actual text of the message can't be
> seen. There are simple solutions: e.g. print up to 5 subtrees, followed
> by "and N more subtrees", and for each subtree print up to 10 rev-ranges
> followed by "...".
>
Alternative: don't print subtrees and rev-ranges at all, unless --verbose
(or a new --bikeshed flag...) is given.
Daniel
> But not before merging to trunk.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging the reintegrate-improvements branch back to trunk
Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2008-11-05 at 10:27 -0500, Mark Phippard wrote:
> 2008/11/5 Paul Burba <pt...@gmail.com>:
> > The http://svn.collab.net/repos/svn/branches/reintegrate-improvements
> > branch is ready to be merged back to trunk.
> >
> > This branch makes two significant changes:
> >
> > 1) Allow reintegrate to work when the reintegrate merge source has
> > explicit subtree mergeinfo, as long as that mergeinfo represents that
> > all of the target was previously merged to the root of the source.
>
> Yay!
That's fantasic, Paul.
> > 2) Improve the error message if a reintegrate merge is legitimately
> > thwarted by subtree mergeinfo on the source. By "legitimately" I mean
> > if you have done subtree merges from 'trunk' to 'some_branch' in such
> > a way that 'some_branch' has not had all of 'trunk' uniformly merged
> > to it. Previously if the reintegrate source had *any* explicit
> > mergeinfo we returned an error like this:
> >
> > C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate --accept postpone
> > http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
> > .
> > svn: Cannot reintegrate from
> > 'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees'
> > yet:
> > Some revisions have been merged under it that have not been merged
> > into the reintegration target; merge them first, then retry.
> >
> > I changed text of this error on the reintegrate-improvements branch,
> > but also added a dump of the offending subtree paths and their
> > mergeinfo so now a user has some hope(?) of seeing what is wrong:
>
> Yay again!
+1.
> > C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate
> > http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
> > .
> > svn: Reintegrate can only be used if the revisions previously merged
> > from the reintegrate target to
> > 'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
> > are the same', but there are differences:
> > branches/issue-3067-deleted-subtrees
> > /trunk:33301-33960
> > branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
> > /trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
> >
> > My hope is that a user could see the above and realize, "oh, I merged
> > r33301-33960 from trunk to my branch, but only
> > r33301-33923,33925-3396to win32_crypto.c, the latter needs r33924
> > too." Of course in doing so the user violated the reintegrate
> > workflow, maybe they reverse merged r33924 from trunk to
> > win32_crypto.c, but at least we give them an idea of where to look for
> > the problem.
>
> Why does the part about trunk show up? I find that confusing because
> it sounds like it is flagging it as part of the problem and I do not
> think you mean to say that.
You mean why print "/trunk:..." (the root path of the merge) as well as
"/trunk/subversion/lib...:..." (a subtree)? It's so the user can see the
differences.
Of course that leads to a suggestion for improvement: show the
difference between each subtree's mergeinfo and the root's mergeinfo.
> > Notice that only the subtree paths that differ from the root of the
> > source are displayed. In the above example there are many other paths
> > in branches/issue-3067-deleted-subtrees that have explicit mergeinfo,
> > but they all show that r33301-33960 have been merged from trunk. Only
> > the branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
> > subtree differs. Notice also that all mergeinfo from other branches
> > has been filtered out, only mergeinfo from trunk is shown as that is
> > all that matters here.
> >
> > The one thing I dislike about this error is the size to which it can
> > grow (and become completely useless) if the reintegrate source is
> > vastly out of synch with the target -- see the attached output.
> >
> > Does this error make sense? Should we keep it? Any suggestions for improvement?
>
> I do not see how we can do anything about the possibly large
> differences. As long as the information is correct, I do not see a
> problem with it. Is it possible for the error message to describe
> what they need to do next? Are there a specific set of revisions that
> need to be merged into the "root" to resolve this subtree mergeinfo
> problem?
I think it's worth doing something to prevent ridiculously long error
messages, as they might fill the user's console or dialogue box chock
full of revision numbers so that the actual text of the message can't be
seen. There are simple solutions: e.g. print up to 5 subtrees, followed
by "and N more subtrees", and for each subtree print up to 10 rev-ranges
followed by "...".
But not before merging to trunk.
> I think you should probably merge this to trunk as is. I mainly offer
> that last comment as a suggestion for where this could possibly go
> next.
+1. It's a _major_ improvement. Ship it!
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Merging the reintegrate-improvements branch back to trunk
Posted by Mark Phippard <ma...@gmail.com>.
2008/11/5 Paul Burba <pt...@gmail.com>:
> The http://svn.collab.net/repos/svn/branches/reintegrate-improvements
> branch is ready to be merged back to trunk.
>
> This branch makes two significant changes:
>
> 1) Allow reintegrate to work when the reintegrate merge source has
> explicit subtree mergeinfo, as long as that mergeinfo represents that
> all of the target was previously merged to the root of the source.
Yay!
> 2) Improve the error message if a reintegrate merge is legitimately
> thwarted by subtree mergeinfo on the source. By "legitimately" I mean
> if you have done subtree merges from 'trunk' to 'some_branch' in such
> a way that 'some_branch' has not had all of 'trunk' uniformly merged
> to it. Previously if the reintegrate source had *any* explicit
> mergeinfo we returned an error like this:
>
> C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate --accept postpone
> http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
> .
> svn: Cannot reintegrate from
> 'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees'
> yet:
> Some revisions have been merged under it that have not been merged
> into the reintegration target; merge them first, then retry.
>
> I changed text of this error on the reintegrate-improvements branch,
> but also added a dump of the offending subtree paths and their
> mergeinfo so now a user has some hope(?) of seeing what is wrong:
Yay again!
> C:\SVN\src-trunk-mirror-wc>svn merge --reintegrate
> http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
> .
> svn: Reintegrate can only be used if the revisions previously merged
> from the reintegrate target to
> 'http://localhost/local-trunk-mirror/trunk-mirror/branches/issue-3067-deleted-subtrees
> are the same', but there are differences:
> branches/issue-3067-deleted-subtrees
> /trunk:33301-33960
> branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
> /trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
>
> My hope is that a user could see the above and realize, "oh, I merged
> r33301-33960 from trunk to my branch, but only
> r33301-33923,33925-3396to win32_crypto.c, the latter needs r33924
> too." Of course in doing so the user violated the reintegrate
> workflow, maybe they reverse merged r33924 from trunk to
> win32_crypto.c, but at least we give them an idea of where to look for
> the problem.
Why does the part about trunk show up? I find that confusing because
it sounds like it is flagging it as part of the problem and I do not
think you mean to say that.
> Notice that only the subtree paths that differ from the root of the
> source are displayed. In the above example there are many other paths
> in branches/issue-3067-deleted-subtrees that have explicit mergeinfo,
> but they all show that r33301-33960 have been merged from trunk. Only
> the branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
> subtree differs. Notice also that all mergeinfo from other branches
> has been filtered out, only mergeinfo from trunk is shown as that is
> all that matters here.
>
> The one thing I dislike about this error is the size to which it can
> grow (and become completely useless) if the reintegrate source is
> vastly out of synch with the target -- see the attached output.
>
> Does this error make sense? Should we keep it? Any suggestions for improvement?
I do not see how we can do anything about the possibly large
differences. As long as the information is correct, I do not see a
problem with it. Is it possible for the error message to describe
what they need to do next? Are there a specific set of revisions that
need to be merged into the "root" to resolve this subtree mergeinfo
problem?
I think you should probably merge this to trunk as is. I mainly offer
that last comment as a suggestion for where this could possibly go
next.
I think the error message is fine. Here is an attempt at an alternative:
svn: Reintegrate cannot be used with specified merge source due to
differences in subtree mergeinfo. Here are the differences that were
found:
branches/issue-3067-deleted-subtrees/subversion/libsvn_subr/win32_crypto.c
/trunk/subversion/libsvn_subr/win32_crypto.c:33301-33923,33925-3396
I do not like this suggestion as is. I am trying to make it more to
the point but do not think I achieved that. I put it out there to
advance the conversation. I repeat that I think this should just go
to trunk.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org