On Tue, Mar 10, 2020 at 10:40 AM Jeff King <peff@xxxxxxxx> wrote:
>
> On Tue, Mar 10, 2020 at 07:57:11AM -0700, Junio C Hamano wrote:
>
> > * "git rebase" has learned to use the merge backend (i.e. the
> > machinery that drives "rebase -i") by default, while allowing
> > "--apply" option to use the "apply" backend (e.g. the moral
> > equivalent of "format-patch piped to am"). The rebase.backend
> > configuration variable can be set to customize.
>
> I noticed a few behavior changes that I think are related to this
> switch. I'm not sure to what degree we'd consider these a problem
> (though the second I think may be an existing bug in the merge backend
> that's just getting more attention), but it seems like the time to raise
> the issue is now, before the release. :)
>
> The first change is that we'll now open an editor when continuing a
> conflicted rebase. You can see it by running this:
>
> for backend in apply merge; do
> echo "==> Running with rebase.backend=$backend"
>
> # new repo
> rm -rf repo
> git init -q repo
> cd repo
>
> # create two conflicting branches
> echo base >base && git add base && git commit -qm base
> echo master >file && git add file && git commit -qm master
> git checkout -q -b side HEAD^
> echo side >file && git add file && git commit -qm side
>
> # rebase on master, hit the conflict, then resolve it
> git -c rebase.backend=$backend rebase master
> echo resolved >file
> git add file
>
> # continue the rebase, noting whether we used the editor
> GIT_EDITOR='echo >&2 running editor on:' git rebase --continue
> done
>
> We won't run the editor the "apply" backend, but do for "merge". I'm
> not sure how big a deal this is. It bit me because I have a script which
> runs rebase in a "while read" shell loop, with stdin coming from a pipe
> feeding the loop. It auto-continues when rerere is able to fix up the
> conflicts, but the editor complains about stdin not being a tty and
> dies.
>
> I'd imagine that's a pretty rare situation, and it was easy enough to
> fix up. But I wonder if we should be more careful about making sure the
> behavior is more identical. On the other hand, I imagine this is the way
> the merge backend has always behaved, so it would be a change in
> behavior for people who had been using it already. I guess the _most_
> compatible thing would be a merge-that-behaves-more-like-apply backend,
> but I'm not sure if we want to support that forever.
This behavior did not always exist with the merge backend, it began
with commit 68aa495b59 ("rebase: implement --merge via the interactive
machinery", 2018-12-11). Before that, there was not too much in
sequencer.c or its predecessors for non-interactive rebases, so I
wouldn't give much weight to historical precedent for how the merge
backend behaves here.
However, as Junio argues elsewhere in this thread, the apply backend
should probably be the one that is considered to be buggy here.
Although fixing the apply backend is probably best, and adding an
escape hatch such as --no-edit as Junio suggests for the merge backend
makes sense, given where we are in the 2.26 cycle, I only sent off a
patch to document this difference for now.
> The second thing I noticed is more clearly a bug, I think. If we have a
> patch that is skipped because it was already applied, we get stuck in
> "cherry-picking" mode. Try:
>
> for backend in apply merge; do
> echo "==> Running with rebase.backend=$backend"
>
> # new repo
> rm -rf repo
> git init -q repo
> cd repo
>
> echo base >file && git add file && git commit -qm base
> # do this in two steps so we don't match patch-id
> echo one >file && git commit -qam master-1
> echo two >file && git commit -qam master-2
>
> git checkout -q -b side HEAD~2
> echo two >file && git commit -qam side
>
> # start a rebase, which should realize that the patch is a noop
> git -c rebase.backend=$backend rebase master
>
> # see what state "status" reports us in
> git status
> done
>
> For the "apply" case, I get:
>
> ==> Running with rebase.backend=apply
> First, rewinding head to replay your work on top of it...
> Applying: side
> Using index info to reconstruct a base tree...
> M file
> Falling back to patching base and 3-way merge...
> No changes -- Patch already applied.
> On branch side
> nothing to commit, working tree clean
>
> So we complete the rebase, and git-status shows nothing. But for the
> merge backend:
>
> ==> Running with rebase.backend=merge
> dropping f8b25a2cd2a3a0e64d820efe2fcbae81dec98616 side -- patch contents already upstream
> Successfully rebased and updated refs/heads/side.
> On branch side
> You are currently cherry-picking commit f8b25a2.
>
> nothing to commit, working tree clean
>
> Oops. If I "git rebase --continue" from there, I get "No rebase in
> progress?". Doing "git cherry-pick --skip" clears it. I guess the issue
> is the continued presence of .git/CHERRY_PICK_HEAD.
Yes, definitely a bug, and new as of "the eighth batch for
git-2.26.0", a few days before 2.26.0-rc0. I found a one-liner fix.
> As you can see from the output above (and the earlier snippet, if you
> run it), there are also a bunch of minor stderr output changes. I think
> these probably aren't worth caring about.
Agreed; also, these output differences are already documented in
"Behavioral Differences" of the git-rebase manpage.
Thanks for the careful reports; new series up at
https://lore.kernel.org/git/pull.722.git.git.1583903621.gitgitgadget@xxxxxxxxx/
to address the issues raised.
