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

Hi Alban,

On Sat, 26 Jan 2019, Alban Gruin wrote:

Le 18/01/2019 à 16:09, Johannes Schindelin via GitGitGadget a écrit :

quoted

-%<-
+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:

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