On 15 November 2017 at 11:41, Phillip Wood <[email protected]> wrote: > From: Phillip Wood <[email protected]> > > Return an error instead of dying if the index cannot be locked in > do_recursive_merge() as if the commit cannot be picked it needs to be > rescheduled when performing an interactive rebase. If the pick is not > rescheduled and the user runs 'git rebase --continue' rather than 'git > rebase --abort' then the commit gets silently dropped.
Makes sense. (Your analysis, not the current behavior. ;-) ) > Signed-off-by: Phillip Wood <[email protected]> > --- > sequencer.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/sequencer.c b/sequencer.c > index > 6d027b06c8d8dc69b14d05752637a65aa121ab24..8c10442b84068d3fb7ec809ef1faa0203cb83e60 > 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -438,7 +438,8 @@ static int do_recursive_merge(struct commit *base, struct > commit *next, > char **xopt; > static struct lock_file index_lock; > > - hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR); > + if (hold_locked_index(&index_lock, LOCK_REPORT_ON_ERROR)) > + return -1; > > read_cache(); >From the commit message, I would have expected the flags to be zero. This patch does not only turn off the die-ing, it also tells the lockfile-API to print an error message before returning. I don't have an opinion on whether that extra verboseness is good or bad, but if it's wanted, I think the commit message should mention this change. Also, don't you want to check "< 0" rather than "!= 0"? If all goes well, the return value will be a file descriptor. I think that it will always be non-zero, so I think you'll always return -1 here. Martin
