Phillip Wood <phillip.w...@talktalk.net> writes:

> diff --git a/sequencer.c b/sequencer.c
> index 
> ae24405c23d021ed7916e5e2d9df6de27f867a2e..3e4c3bbb265db58df22cfcb5a321fb74d822327e
>  100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -477,9 +477,6 @@ static int do_recursive_merge(struct commit *base, struct 
> commit *next,
>                       _(action_name(opts)));
>       rollback_lock_file(&index_lock);
>  
> -     if (opts->signoff)
> -             append_signoff(msgbuf, 0, 0);
> -
>       if (!clean)
>               append_conflicts_hint(msgbuf);
>  

This function is called from only one place,  do_pick_commit(), and
then the message returned from here in msgbuf is written to
merge_msg(), even when the merge conflicted.

And when the merge conflicts, sequencer would stop and gives the
control back to you---the MERGE_MSG file would have had the sign-off
when you conclude the conflict resolution.

With the new code, we instead add the sign-off before calling the
function to compensate for the above change, so MERGE_MSG file would
have the sign-off as before, when the sequencer stops.

> @@ -657,8 +654,6 @@ static int run_git_commit(const char *defmsg, struct 
> replay_opts *opts,
>               argv_array_push(&cmd.args, "--amend");
>       if (opts->gpg_sign)
>               argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
> -     if (opts->signoff)
> -             argv_array_push(&cmd.args, "-s");
>       if (defmsg)
>               argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
>       if ((flags & CLEANUP_MSG))

This has two callers.  

The caller in do_pick_commit() is a bit curious; as we saw already,
the message file should already have the sign-off and then we used
to give another "-s" here.  Were we depending on "-s" to become
no-op when the last sign-off is by the same person, I wonder?  In
any case, the removal of "-s" from here won't hurt that caller.

The other caller is commit_staged_changes() which is called when
doing "rebase -i continue".  I am not quite sure where the contents
stored in the file rebase_path_message() comes from.  The function
error_failed_squash() moves SQUASH_MSG to it and then makes a copy
of it to MERGE_MSG, but that should only be relevant for squashed
commit and no other cases, so...?

I need to block a bit more time to read the relevant code to comment
on this step, especially on this removal.

Thanks for working on this, anyway.

> @@ -1347,6 +1342,9 @@ static int do_pick_commit(enum todo_command command, 
> struct commit *commit,
>               }
>       }
>  
> +     if (opts->signoff)
> +             append_signoff(&msgbuf, 0, 0);
> +
>       if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
>               res = -1;
>       else if (!opts->strategy || !strcmp(opts->strategy, "recursive") || 
> command == TODO_REVERT) {

Reply via email to