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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alban,

On Sat, 26 Jan 2019, Alban Gruin wrote:

> 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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux