Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: pass (athena.apache.org: local policy)
Date: Wed, 12 Oct 2011 11:54:44 +0200
From: Stefan Sperling <stsp@elego.de>
To: Paul Burba <ptburba@gmail.com>
Cc: dev@subversion.apache.org
Subject: Re: O(n**3) behaviour in reintegrate merge
Message-ID: <20111012095444.GC9845@ted.stsp.name>
Mail-Followup-To: Paul Burba <ptburba@gmail.com>, dev@subversion.apache.org
References: <20110910174850.GA31390@daniel3.local>
 <20110911081201.GK25324@ted.stsp.name>
 <20110911125336.GL25324@ted.stsp.name>
 <20110911125639.GA8009@daniel3.local>
 <20110912200515.GD21279@jack.stsp.name>
 <CAO16vNQp7jw2rbxc1U+F5LraoSUjRezw5MavBmTX=omjijpqvg@mail.gmail.com>
 <20110929094145.GB8773@jack.stsp.name>
 <20111011130511.GG10081@ted.stsp.name>
 <20111011133656.GH10081@ted.stsp.name>
 <CAO16vNTVTpQ9_j46hOUnZr69m82p8vijxadogYAqZ6GGO2X7yw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAO16vNTVTpQ9_j46hOUnZr69m82p8vijxadogYAqZ6GGO2X7yw@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)

On Tue, Oct 11, 2011 at 11:21:16AM -0700, Paul Burba wrote:
> On Tue, Oct 11, 2011 at 6:36 AM, Stefan Sperling <stsp@elego.de> wrote:
> > On Tue, Oct 11, 2011 at 03:05:11PM +0200, Stefan Sperling wrote:
> >> So while I think your fixes should be backported to 1.7.1 ASAP,
> >> I don't think the status quo is acceptable. How do we want to move
> >> forward?
> >>
> >> For reference, here's the error message I'm getting:
> 
> Hi Stefan,
> 
> Yes, the error message is rather long.  But as I've already said, this
> reintegrate merge is a complete and total abuse of the reintegrate
> feature.  I'm not sure we can save every user from themselves if they
> insist on doing strange things....but I've already made that point,
> and it appears I'm in the minority, so I wont belabor it any further
> :-)

I don't agree with this. I wouldn't call it "abuse" of this feature.

The user is clearly intending to reintegrate the branch. But one of the
preconditions for reintegration isn't met. Much like trying to merge into
a mixed-revision working copy, or a working copy with local modifications.
Would you also call that "abuse"? I doubt that :)

The user error is definitely not on purpose, and I don't see a point
in punishing users for this error by stealing 3 or more minutes of
their time gathering information of little value to them.
Note that the user performing the reintegrate merge is not necessarily
the same person who performed the cherry-picking merge which makes
--reintegrate impossible. They might simply be unaware of what happened.

> So while I'm a weak -0 on the basic premise of your patch[1], I won't
> object to it.
> 
> Paul
> 
> [1] Re your present patch, the error message is created such that it
> appears we have to sync the source branch (/fs-successor-ids) up to
> HEAD:

> But we need only catch up the branch to to youngest sync from trunk
> (r1167546) for the reintegrate to be successful:

Right. That should be fixed by printing the last revision of the last
range which has already been merged.

Of course, the patch isn't finished as it is. I just wanted to see how
much of a difference it would make to stop downloading the log immediately
when one missing operative revision was found.

How about we meet in the middle?
We could have the log message callback keep track of the number of
missing operative revisions it found, and return
SVN_ERR_CLIENT_NOT_READY_TO_MERGE if a certain limit is exceeded.
In which case the error will end with " (one or more additional missing
revisions not shown, for brevity)".
Let's say we set the limit to a maximum of 42 missing operative revisions.
Because 42 is the answer to life, the universe, and everything,
and because this amount of missing revivions should fit well into the use
case you have in mind, where only a few revisions are missing and the
user would like to see them all at once without running the 'svn
mergeinfo' command. What do you think?
In my patch, the current limit is 1. It would be easy to extend this to
42 (or some other value) and adjust the error message accordingly.