Re: Dangerous "git am --abort" behavior

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Dec 21, 2010, 18:46 UTC
Message-ID: <AANLkTimqxCBpF2tCqjsPMnc11nh4MZx2bh0gD7Q=duG+@mail.gmail.com>
In-Reply-To: <7vsjxr7zdn.fsf@alter.siamese.dyndns.org>

On Tue, Dec 21, 2010 at 10:29 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 6 quoted lines

>
> So here is the first step in that direction.  I suspect that stop_here
> should also record what the current branch is, and safe_to_abort should
> check it (the potentially risky sequence is "after a failed am, check out
> a different branch and then realize you need to 'am --abort'"), but that
> is left to interested others ;-) or a later round.

Yeah, this patch looks good to me.

And if you've switched branches, and do a "git am --abort" which still sees the expected commit, I actually think your patch does the right thing: we will rewind that new branch to ORIG_HEAD, and I think that is actually the semantics we want.

So what you can do with this is:

 - "git am <mbox-file>" fails in the middle

 - you go "hmm. I'm happy with what we did so far, but let's go back
to check what's up"

 - "git checkout -b test-branch ; git am --abort"

 - work on the original base and maybe try to re-apply the mbox with
soem manual editing or whatever...

and that's exactly the semantics that your patch allows, which seems to be very flexible and useful. No?

So the only thing it disallows is having "git am --abort" actually abort some unrelated commit, which is I think the exact behavior we want. In fact, if somebody has done a "git pull" or something, then "ORIG_HEAD" really doesn't mean what git am thinks it means. So I wonder if we should check ORIG_HEAD against "beginning of 'git am'" too, the way you check HEAD against the "abort-safely" point?

Again, if ORIG_HEAD doesn't match (for whatever reason - maybe somebody switched branches and did a 'git reset --hard" in that other branch, and then switched back?), then "git am --abort" shouldn't abort to some random point that came from some non-am workflow, no?

But with the HEAD check, you'd really have to _work_ at screwing up, so the ORIG_HEAD check seems to be much less important.

                              Linus