Re: [PATCH v2 3/3] merge: save merge state earlier
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 22, 2017, 00:38 UTC
- Message-ID
- <xmqqk21w4et2.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <407f205e2b824d56c652411256eea3c0047576ef.1503319519.git.git@grubix.eu>
Michael J Gruber <git@grubix.eu> writes:
Show 12 quoted lines
> 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.
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.
Show 19 quoted lines
> 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.
Show 5 quoted lines
> + git merge --continue && > + verify_parents $c0 $c1 > +' > + > test_done