Re: [PATCH v2 4/4] built-in rebase: call `git am` directly

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 29, 2019, 09:46 UTC
Message-ID: <nycvar.QRO.7.76.6.1901291033180.41@tvgsbejvaqbjf.bet>
In-Reply-To: <dc6830ab-d31c-fb9b-4f8d-24a79a978764@gmail.com>

Hi Alban,

On Sat, 26 Jan 2019, Alban Gruin wrote:

Show 21 quoted lines

> Le 18/01/2019 à 16:09, Johannes Schindelin via GitGitGadget a écrit :
> > -%<-
> > +static int run_am(struct rebase_options *opts)
> > +{
> > -%<-
> > +
> > +	if (!status) {
> > +		return move_to_original_branch(opts);
> > +	}
> > +
> > +	if (is_directory(opts->state_dir))
> > +		write_basic_state(opts);
> > +
> > +	return status;
> > +}
> > +
> 
> This means that the state directory will be created only if there is a
> problem (ie. a conflict), not if the user cancels the rebase by hitting
> ^C.  In this case, the user will be left in a corrupted state where the
> builtin rebase cannot abort the rebase properly.

This is an almost literal translation of https://github.com/git/git/blob/v2.20.1/git-rebase--am.sh#L77-L83:

```sh
if test 0 != $ret
then
        test -d "$state_dir" && write_basic_state
        return $ret
fi

move_to_original_branch ```

All I did was to switch the order to handle the simple (and common) case first.

So I do not think that this changes the behavior of the shell-scripted `am` backend.

Looking at the code, for a moment I thought that I had introduced a bug where the state_dir is not cleaned up after move_to_original_branch() is called, but I call run_am() in run_specific_rebase() and skip directly to the finished_rebase label after that, which does take care of calling finish_rebase() (which removes the state_dir, among other things such as applying the autostash).

Ciao, Dscho