Re: [PATCH 02/15] sequencer: lib'ify do_recursive_merge()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 24, 2016, 07:16 UTC
Message-ID: <CAPig+cTCotTDT83x9=q5ORR1ZWR9oewvXiun9sjvEbTV8OuChA@mail.gmail.com>
In-Reply-To: <e00df1449af0d8c55000a93c734d8a241b1cb5f0.1471968378.git.johannes.schindelin@gmx.de>

On Tue, Aug 23, 2016 at 12:06 PM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 13 quoted lines

> 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).

>         rollback_lock_file(&index_lock);
>
>         if (opts->signoff)