Subject: Re: "git am" and then "git am -3" regression?

From: Jeff King <[email-masked]>
Date: Fri, 24 Jul 2015 11:09:21 -0700

On Fri, Jul 24, 2015 at 10:48:18AM -0700, Junio C Hamano wrote:

> Hmm, there seems to be some glitches around running "am -3"
> after a failed "am" between 'maint' and 'master'.
> 
> When I try the following sequence, the 'am' from 'maint' succeeds,
> but 'am' in 'master' fails:
> 
>  * Save Eric's "minor documetation improvements" $gmane/274537
>    to a file.  
> 
>  * "git checkout e177995" (that's "next^0") and then apply them with
>    "git am" (no -3 necessary).
> 
>  * "git checkout 272be14" (that's "es/worktree-add-cleanup^0") and
>    then apply them with "git am" (without -3).
> 
>    This is expected to stop at 2/6, as the context has changed
>    between 272be14 and the tip of 'next'.
> 
>  * "git am -3".  This should restart and resolve cleanly.

Thanks for diagnosing. This bit me the other day, but I hadn't had time
to look at it yet (and I "am" a lot less than you do, I imagine).

> Reverting d96a275b91bae1800cd43be0651e886e7e042a17 seems to fix it,
> so that is what I'll do for 2.5 final.

Yeah, I think this hunk is to blame (though I just read the code and did not
test):

@@ -658,6 +665,8 @@ fi
 if test "$(cat "$dotest/threeway")" = t
 then
        threeway=t
+else
+       threeway=f
 fi

It comes after the command-line option parsing, so it overrides our option (I
think that running "git am -3" followed by "git am --no-3way" would have the
same problem). It cannot just check whether $threeway is unset, though, as it
may have come from the config. We'd need a separate variable, the way the code
is ordered now.

Ideally the code would just be ordered as:

  - load config from git-config

  - override that with defaults inherited from a previous run

  - override that with command-line parsing

but I don't know if there are other ordering gotchas that would break.
It does look like that is how Paul's builtin/am.c does it, which makes
me think it might not be broken. It's also possibly I've horribly
misdiagnosed the bug. ;)

-Peff