From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 24 Aug 2016 15:53:50 GMT
Subject: Re: [PATCH 02/15] sequencer: lib'ify do_recursive_merge()
Message-ID: <alpine.DEB.2.20.1608241753150.4924@virtualbox>
In-Reply-To: <CAPig+cTCotTDT83x9=q5ORR1ZWR9oewvXiun9sjvEbTV8OuChA@mail.gmail.com>

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:06 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -303,7 +303,8 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
> >         if (active_cache_changed &&
> >             write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
> >                 /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
> > -               die(_("%s: Unable to write new index file"), action_name(opts));
> > +               return error(_("%s: Unable to write new index file"),
> > +                       action_name(opts));
> 
> Does this need to rollback the lockfile before returning?
> 
> A cursory scan of read-cache.c:do_write_locked_index() seems to
> indicate that lockfile disposition is not handled automatically in
> case of error (unless I'm misreading).

As mentioned elsewhere in this thread: an atexit() handler is tasked with
rolling back uncommitted lockfiles.

Ciao,
Dscho