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

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:06 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > 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(_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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Eric Sunshine]

On Tue, Aug 23, 2016 at 12:06 PM, Johannes Schindelin
 wrote:
> To be truly useful, the sequencer should never die() but always return
> an error.
>
> Signed-off-by: Johannes Schindelin 
> ---
> 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(_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(_lock);
>
> if (opts->signoff)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Schindelin]

To be truly useful, the sequencer should never die() but always return
an error.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index c75296c..0c8c955 100644
--- 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(_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));
rollback_lock_file(_lock);
 
if (opts->signoff)
-- 
2.10.0.rc1.99.gcd66998


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html