Re: [PATCH v1 09/19] rebase -i: commit only once when rewriting picks

[Fabian Ruch <bafain@gmail.com>]

Hi Jeff,

Jeff King writes:
> On Tue, Jul 29, 2014 at 01:18:09AM +0200, Fabian Ruch wrote:
>> The options passed to `do_pick` determine whether the picked commit
>> will be rewritten or not. If the commit gets rewritten, because the
>> user requested to edit the commit message for instance, let
>> `pick_one` merely apply the changes introduced by the commit and do
>> not commit the resulting tree yet. If the commit is replayed as is,
>> leave it to `pick_one` to recreate the commit (possibly by
>> fast-forwarding the head). This makes it easier to combine git-commit
>> options like `--edit` and `--amend` in `do_pick` because
>> git-cherry-pick does not support `--amend`.
>>
>> In the case of `--edit`, do not `exit_with_patch` but assign
>> `rewrite` to pick the changes with `-n`. If the pick conflicts, no
>> commit is created which we would have to amend when continuing the
>> rebase. To complete the pick after the conflicts are resolved the
>> user just resumes with `git rebase --continue`.
> 
> Hmm. So does this mean the user will actually see a different state
> during such a conflict?
> 
> E.g., if I have instructions like:
> 
>   pick A
>   squash B
>   squash C
> 
> and there is a conflict picking C, then what state do I see? Right now I
> see a commit with the A+B squash prepared. But your description sounds
> to me like we would avoid the squash for "B", and the user would see a
> different state.

The squash state will not be different. squash sequences are still taken
care of one line after the other: committing A, committing A+B,
committing A+B+C. If there is a conflict picking C, HEAD will point to
A+B and the index will record the conflicting changes.

> However, I couldn't trigger that behavior in a few experiments. Am I
> misunderstanding, or is there some case where the user-visible state
> will be different?

The user-visible state will be different for rewords. For instance,
let's consider

    pick A
    reword B.

The verbose log used to show two commits for B (with ff disabled), one
after picking and one after editing. Now the log will show a single
commit in connection with 'reword B' which might be less confusing.

Thanks for raising your eyebrows. I noticed now that the last paragraph
is plainly wrong. The described amend situation did not arise "if the
pick conflicted" but "if the edited commit did not verify". There will
be no "after the conflicts are resolved" but the user can either commit
manually and circumvent log message verification if she knows what she's
doing, or provide a corrected log message in the editor launched by 'git
rebase --continue'. The _incomplete_ 'git commit --amend' tip which used
to be displayed after a failed verification hook could become
unnecessary and this would possibly spare us including correct GPG sign
options for instance.

However, this patch is mostly motivated by the unification of how
commits are rewritten. Before, rewords and squashes went about this
differently, now both fail with an uncommitted index if there are
conflicts or the log message is ill-formatted.

The log message must be corrected and the following bug, which was
noticed after PATCH v2, must be fixed.

    cat >.git/hooks/commit-msg <<-EOF
    #!/bin/sh
    exit 1
    EOF
    chmod +x .git/hooks/commit-msg
    test_must_fail env FAKE_LINES="reword 1" git rebase -i
    test_must_fail git rebase --continue
    # the last command succeeds because --continue does not verify

   Fabian