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

Re: O(n**3) behaviour in reintegrate merge

From: Stefan Sperling <stsp_at_elego.de>
Date: Thu, 29 Sep 2011 11:41:45 +0200

On Wed, Sep 28, 2011 at 01:28:15PM -0400, Paul Burba wrote:
> The issue to be decided here is what error message to provide when a
> reintegrate merge fails because the source is not fully synced with
> the target:
>
> 1) Current behavior: Issue an error message describing the unsynced
> revs and which
>
> >svn merge ^^/branch-X A --reintegrate
> ..\..\..\subversion\svn\merge-cmd.c:382: (apr_err=195016)
> ..\..\..\subversion\libsvn_client\merge.c:10904: (apr_err=195016)
> ..\..\..\subversion\libsvn_client\merge.c:10873: (apr_err=195016)
> ..\..\..\subversion\libsvn_client\merge.c:10873: (apr_err=195016)
> ..\..\..\subversion\libsvn_client\merge.c:10811: (apr_err=195016)
> svn: E195016: Reintegrate can only be used if revisions 2 through 12
> were previously merged from
> file:///C:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work/repositories/merge_tests-98/A
> to the reintegrate source, but this is not the case:
> branch-X
> Missing ranges: /A:3-4
>
> 2) Same as #1 but truncate the output after N revisions and/or M sources
>
> 3) No detailed error message, instead suggest that a sync merge be run first.
>
> 4) No detailed error message, instead suggest the specific 'svn
> mergeinfo' command that must be run to find any unsynced revisions.
>
> Personally I'm happy with the current solution (once the memory bloat
> has been fixed of course). Daniel's original example in this thread,
>
> "After cherry-picking r1167546 from trunk to the fs-successor-ids branch
> in r1167550, reintegrating that branch to trunk takes a long time
> (sufficiently long for the server to assume the client has disconnected)."
>
> seems to me to be a purposeful abuse of what reintegrate is intended
> to support. The fs-successor-ids branch was copied from trunk_at_880535
> in r880536 and *never* synced with trunk. The only change from trunk
> brought to fs-successor-ids was the single cherry-pick of r1167546.
> So obviously the reintegrate merge is going to fail and in this case
> the intervening 287010 missing revisions result in a rather unwieldy
> error message.
>
> I think the far more common use case (based on customer questions I
> have dealt with) is where a user has been keeping a branch in sync
> with "trunk" but somewhere along the line did a subtree merge, or is
> otherwise missing some "reasonably" small part of trunk. In those
> cases the error messages pinpoint the problem down the the specific
> subtree with mergeinfo.
>
> Keep in mind this error has been around since 1.5.5 and I don't see a
> lot of complaints about it.

I agree that the error message is useful in the case you describe.

However, I am not convinced that the approach 1) is smart in general.
It doesn't matter how common the problematic case is. It does exist,
and in case people run into it svn is not being helpful. It wastes
time constructing an error message too large for anyone to read.
I think that displaying the list of missing ranges should be an
optional step.

My favourite solution is 4) because:
 - It is easy to implement and shaves a small bit of code from the
   implementation of merge --reintegrate.
 - It allows svn to be more responsive. As soon as we find one gap,
   we error out. There is no need to stream the list of missing ranges
   from the server to the client before an error is printed informing
   the user that something is wrong.
 - It covers all cases. Users who want to know about missing ranges are
   told how to get at it. They can run the 'svn mergeinfo' command (or
   equivalent) to kick off the process of dowloading the list of missing
   ranges. Other users may decide to just run a sync merge right away,
   regardless of what is missing.
 - It allows for more flexible client behaviour.
   The library should use a special error code to signal that there are
   missing ranges (the existing SVN_ERR_CLIENT_NOT_READY_TO_MERGE is not
   specific enough since it is also used for problems such as
   mixed-revision working copies). The CLI client should wrap it with a
   message that suggests to run 'svn mergeinfo' to see which ranges are
   missing, and to run a sync merge to merge the missing ranges.
   Other clients, such as GUI clients, can point users at their equivalents
   of 'svn mergeinfo' and 'svn merge', and/or offer to download the list
   of missing ranges for display, and/or offer the option to run a sync merge.

The only downside is that a new error code is an API change we cannot
backport to 1.7 unless it still makes the .0 release (I have no
intention to hold the release over this). We could try to come up with
some way of saying 'run the svn mergeinfo command' in a client-independent
way. Or have the 1.7 svn client always wrap SVN_ERR_CLIENT_NOT_READY_TO_MERGE
with a message such as:
  svn: If there are missing ranges in the reintegrate source, run
  'svn mergeinfo --show-revs=eligible ^/trunk ^/branches/foo' to see them
Received on 2011-09-29 11:42:29 CEST

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