Re: [PATCH v3 15/15] rebase: change the default backend from "am" to "merge"

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 10, 2020, 23:14 UTC
Message-ID: <20200110231436.GA24315@google.com>
In-Reply-To: <044853fd612ee8cf6928bb7c4ebb1eacaa172aa3.1577217299.git.gitgitgadget@gmail.com>

Hi,

Elijah Newren via GitGitGadget wrote:

Show 29 quoted lines

> The am-backend drops information and thus limits what we can do:
>
>   * lack of full tree information from the original commits means we
>     cannot do directory rename detection and warn users that they might
>     want to move some of their new files that they placed in old
>     directories to prevent their becoming orphaned.[1]
>   * reduction in context from only having a few lines beyond those
>     changed means that when context lines are non-unique we can apply
>     patches incorrectly.[2]
>   * lack of access to original commits means that conflict marker
>     annotation has less information available.
>
> Also, the merge/interactive backend have far more abilities, appear to
> currently have a slight performance advantage[3] and have room for more
> optimizations than the am backend[4] (and work is underway to take
> advantage of some of those possibilities).
>
> [1] https://lore.kernel.org/git/xmqqh8jeh1id.fsf@gitster-ct.c.googlers.com/
> [2] https://lore.kernel.org/git/CABPp-BGiu2nVMQY_t-rnFR5GQUz_ipyEE8oDocKeO+h+t4Mn4A@mail.gmail.com/
> [3] https://public-inbox.org/git/CABPp-BF=ev03WgODk6TMQmuNoatg2kiEe5DR__gJ0OTVqHSnfQ@mail.gmail.com/
> [4] https://lore.kernel.org/git/CABPp-BGh7yW69QwxQb13K0HM38NKmQif3A6C6UULEKYnkEJ5vA@mail.gmail.com/
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-rebase.txt           |  2 +-
>  builtin/rebase.c                       |  4 ++--
>  t/t5520-pull.sh                        | 10 ++++++----
>  t/t9106-git-svn-commit-diff-clobber.sh |  3 ++-
>  4 files changed, 11 insertions(+), 8 deletions(-)

Thanks for writing this. We finally rolled this out to our internal population at $DAYJOB and ran into a couple of issues:

 1. "git rebase --am" does not invoke the post-commit hook, but "git
    rebase --merge" does.  Is this behavior change intended?

    Noticed because jiri[1] installs a post-commit hook that warns
    about commits on detached HEAD, so this change makes rebases more
    noisy in repositories that were set up using jiri.

 2. GIT_REFLOG_ACTION contains "rebase -i" even though the rebase is
    not interactive.

 3. In circumstances I haven't pinned down yet, we get the error
    message "invalid date format: @@2592000 +0000":

	$ git rebase --committer-date-is-author-date --onto branch_K branch_L~1 branch_L
	$ git checkout --theirs file
	$ git add file
	$ git rebase --continue
	fatal: invalid date format: @@2592000 +0000
	error: could not commit staged changes.

    This isn't reproducible without --committer-date-is-author-date.
    More context (the test where it happens) is in [2].

 4. I suspect the exit status in the "you need to resolve conflicts"
    case has changed.  With rebase --am, [3] would automatically
    invoke rebase --abort when conflicts are present, but with rebase
    --merge it does not.

Known?

Thanks, Jonathan

[1] https://fuchsia.googlesource.com/jiri/+/60436c301224231cb99be41ce937dfc223bee272/project/manifest.go#1347 [2] https://source.chromium.org/chromium/chromium/tools/depot_tools/+/master:tests/git_common_test.py;l=721;drc=6b52dc21e166c46707b4c8eb26c74c70d4f9977e;bpv=1;bpt=0 [3] https://fuchsia.googlesource.com/jiri/+/60436c301224231cb99be41ce937dfc223bee272/project/project.go#1664