Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Feb 5, 2008, 20:38 UTC
Message-ID: <alpine.LNX.1.00.0802051439200.13593@iabervon.org>
In-Reply-To: <alpine.LSU.1.00.0802050112380.8543@racer.site>

On Tue, 5 Feb 2008, Johannes Schindelin wrote:

Show 21 quoted lines

> Hi,
> 
> On Mon, 4 Feb 2008, Daniel Barkalow wrote:
> 
> > This will allow builtin-checkout to suppress merge errors if it's going 
> > to try more merging methods.
> 
> I saw one error() in twoway_merge(), one in bind_merge(), and one in 
> onway_merge() that were not guarded by o->gently.
> 
> Also, I'd call it "silent", not "gently".
> 
> > Additionally, if unpack_trees() returns with an error, but without 
> > printing anything, it will roll back any changes to the index (by 
> > rereading the index, currently). This obviously could be done by the 
> > caller, but chances are that the caller would forget and debugging this 
> > is difficult.
> 
> Granted, it is easy to forget.  But maybe the caller does not need the 
> index?  Or maybe it wants a different one?  I'd prefer the caller to clean 
> up, if necessary.

That's what makes it "gently" instead of just "silent"; it has no effect if it doesn't succeed. Longer term, I'd like to have unpack_trees() unpack into a separate index, which should actually be faster (since it doesn't have to keep shifting the entries in the index it's working on) and make this moot. In any case, it only rolls it back with the option that's only used by a caller that wants the index unchanged on failure, currently. If a caller turns out to just want a return code and not care about the index or the error message, and the code hasn't been reworked, we can add a separate flag then.

I'd done some analysis at the time that suggested that, if you didn't want to give a message on failure, you must want to do something else to the index to replace what hadn't worked, so you must want the index reset, but I've forgotten why I was so sure at the time, aside from that nobody's wanted it before now.

	-Daniel
*This .sig left intentionally blank*