Re: [PATCH] am: handle stray $dotest directory case

Junio C Hamano wrote:

Hmph, when did ORIG_HEAD set, and what commit does it point at?

By some unrelated previous operation (eg. pull, rebase, merge).  The
point is that at any point in "normal operation", ORIG_HEAD exists,
and usually points to @~N, for some N.  If I rm .git/ORIG_HEAD by
hand, the am --abort obviously errors out:

  $ git am --abort
  fatal: Not a valid object name ORIG_HEAD
  fatal: ambiguous argument 'ORIG_HEAD': unknown revision or path not
in the working tree.
  Use '--' to separate paths from revisions, like this:
  'git <command> [<revision>...] -- [<file>...]'

As "git am" reading from stdin, waiting, hasn't moved HEAD yet at
all, I think two things need to happen to fix that:

 (1) at around the call to check_patch_format and split_patches,
     clear ORIG_HEAD (this may have to be done only !$rebasing,
     though).

 (2) safe_to_abort() should make sure ORIG_HEAD exists; otherwise it
     is unsafe.

But that is entirely an independent issue (I am going to agree with
you in the end).

Exactly.  It might be nice to fix those two things (are there any
observed bugs?), but it is entirely orthogonal to our issue.

That is a correct observation.  But it needed a bit of thinking to
reach your conclusion that special casing this and handling --abort
in a new different codepath is the right solution.

Yeah, the commit message is lacking.

How would "am --skip", "am --resolved", or "am anothermbox" behave
in this "we already have $dotest because the user started one
session but killed it" case, which used to be covered by -d $dotest
alone but now flows to the other side of the if/else/fi codepath?
Do they need a similar treatment, or would they naturally error out
as they should?

am --skip and am --resolved will error out, saying "Resolve operation
not in progress, we are not resuming".  The message needs to be
tweaked.

am anothermbox will work just fine, implicitly overwriting the
existing $dotest directory.  But yeah, we could explicitly remove the
directory and allow the mkdir -p to re-create it.

I'll probably work on these follow-ups in the morning.  Thanks for poking.