Junio C Hamano venit, vidit, dixit 22.08.2017 02:38:
> Michael J Gruber <git@xxxxxxxxx> writes:
>
>> static void prepare_to_commit(struct commit_list *remoteheads)
>> {
>> struct strbuf msg = STRBUF_INIT;
>> @@ -767,6 +768,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
>> strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
>> if (signoff)
>> append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
>> + if (!squash)
>> + write_merge_heads(remoteheads);
>> write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
>> if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
>> git_path_merge_msg(), "merge", NULL))
>
> I can understand that you would never want to write out MERGE_HEAD
> while squashing, but I somehow think it would be a bug in the caller
> to call prepare_to_commit(), whose point is to prepare the merge
> message to be recorded in the resulting merge commit, when the user
> gave us the "--squash" option, which is an explicit instruction that
> the user does not want the merge commit the message is used.
That sounds reasonable. I vaguely remember a failing test for an earlier
version that I tried, but that was before the "split".
> Can squash ever be true in this function?
>
> This function has two callsites: merge_trivial() and
> finish_automerge().
>
> I think merge_trivial() will not be called under "--squash", which
> turns option_commit off and the only callsite of it is inside an
> else-if clause that requres option_commit to be true. You can do a
> similar deduction around the "automerge_was_ok" variable to see if
> finish_automerge() can be called when "--squash" is given; I suspect
> the answer may be no.
I'll go without the if, after more testing.
>> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
>> index 2ebda509ac..80194b79f9 100755
>> --- a/t/t7600-merge.sh
>> +++ b/t/t7600-merge.sh
>> @@ -774,4 +774,19 @@ test_expect_success 'merge can be completed with --continue' '
>> verify_parents $c0 $c1
>> '
>>
>> +write_script .git/FAKE_EDITOR <<EOF
>> +# kill -TERM command added below.
>> +EOF
>> +
>> +test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue' '
>> + git reset --hard c0 &&
>> + ! "$SHELL_PATH" -c '\''
>> + echo kill -TERM $$ >> .git/FAKE_EDITOR
>> + GIT_EDITOR=.git/FAKE_EDITOR
>> + export GIT_EDITOR
>> + exec git merge --no-ff --edit c1'\'' &&
>
> This is a tricky construct. You "reserve" a process ID by using a
> shell, arrange it to be killed and then using "exec" to make it the
> "git merge" program to be killed. I kind of like the convolutedness.
That is from t7502. Sorry for hiding that note in the cover letter, I
should put it into 3/3's message or a test comment.
When testing, I simply used "git merge... &" and "ps" or "jobs" to know
which process to kill. Apparantly, the above is the most portable way to
script that. t7502 went through a few iterations to ensure this.
>> + git merge --continue &&
>> + verify_parents $c0 $c1
>> +'
>> +
>> test_done
