From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 25 Jul 2019 21:27:10 +0000 (-0700)
Subject: Merge branch 'vv/merge-squash-with-explicit-commit' into maint
X-Git-Tag: v2.23.0-rc0~6^2~16
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/33f2790ecafd29b8551a290e566003b975807c61?hp=-c

Merge branch 'vv/merge-squash-with-explicit-commit' into maint

"git merge --squash" is designed to update the working tree and the
index without creating the commit, and this cannot be countermanded
by adding the "--commit" option; the command now refuses to work
when both options are given.

* vv/merge-squash-with-explicit-commit:
merge: refuse --commit with --squash
---

33f2790ecafd29b8551a290e566003b975807c61
diff --combined Documentation/merge-options.txt
index 61876dbc33,a147d436ed..79a00d2a4a
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@@ -3,14 -3,9 +3,14 @@@
  	Perform the merge and commit the result. This option can
  	be used to override --no-commit.
  +
 -With --no-commit perform the merge but pretend the merge
 -failed and do not autocommit, to give the user a chance to
 -inspect and further tweak the merge result before committing.
 +With --no-commit perform the merge and stop just before creating
 +a merge commit, to give the user a chance to inspect and further
 +tweak the merge result before committing.
 ++
 +Note that fast-forward updates do not create a merge commit and
 +therefore there is no way to stop those merges with --no-commit.
 +Thus, if you want to ensure your branch is not changed or updated
 +by the merge command, use --no-ff with --no-commit.
  
  --edit::
  -e::
@@@ -32,13 -27,6 +32,13 @@@ they run `git merge`. To make it easie
  updated behaviour, the environment variable `GIT_MERGE_AUTOEDIT` can be
  set to `no` at the beginning of them.
  
 +--cleanup=<mode>::
 +	This option determines how the merge message will be cleaned up before
 +	commiting. See linkgit:git-commit[1] for more details. In addition, if
 +	the '<mode>' is given a value of `scissors`, scissors will be appended
 +	to `MERGE_MSG` before being passed on to the commit machinery in the
 +	case of a merge conflict.
 +
  --ff::
  	When the merge resolves as a fast-forward, only update the branch
  	pointer, without creating a merge commit.  This is the default
@@@ -102,6 -90,8 +102,8 @@@ merge
  +
  With --no-squash perform the merge and commit the result. This
  option can be used to override --squash.
+ +
+ With --squash, --commit is not allowed, and will fail.
  
  -s <strategy>::
  --strategy=<strategy>::
diff --combined builtin/merge.c
index e96f72af80,18b90913d8..57c2a24f6d
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@@ -38,7 -38,6 +38,7 @@@
  #include "tag.h"
  #include "alias.h"
  #include "commit-reach.h"
 +#include "wt-status.h"
  
  #define DEFAULT_TWOHEAD (1<<0)
  #define DEFAULT_OCTOPUS (1<<1)
@@@ -58,7 -57,7 +58,7 @@@ static const char * const builtin_merge
  };
  
  static int show_diffstat = 1, shortlog_len = -1, squash;
- static int option_commit = 1;
+ static int option_commit = -1;
  static int option_edit = -1;
  static int allow_trivial = 1, have_message, verify_signatures;
  static int overwrite_ignore = 1;
@@@ -99,9 -98,6 +99,9 @@@ enum ff_type 
  
  static enum ff_type fast_forward = FF_ALLOW;
  
 +static const char *cleanup_arg;
 +static enum commit_msg_cleanup_mode cleanup_mode;
 +
  static int option_parse_message(const struct option *opt,
  				const char *arg, int unset)
  {
@@@ -117,15 -113,12 +117,15 @@@
  	return 0;
  }
  
 -static int option_read_message(struct parse_opt_ctx_t *ctx,
 -			       const struct option *opt, int unset)
 +static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
 +						 const struct option *opt,
 +						 const char *arg_not_used,
 +						 int unset)
  {
  	struct strbuf *buf = opt->value;
  	const char *arg;
  
 +	BUG_ON_OPT_ARG(arg_not_used);
  	if (unset)
  		BUG("-F cannot be negated");
  
@@@ -253,7 -246,6 +253,7 @@@ static struct option builtin_merge_opti
  		N_("perform a commit if the merge succeeds (default)")),
  	OPT_BOOL('e', "edit", &option_edit,
  		N_("edit message before committing")),
 +	OPT_CLEANUP(&cleanup_arg),
  	OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"), FF_ALLOW),
  	OPT_SET_INT_F(0, "ff-only", &fast_forward,
  		      N_("abort if fast-forward is not possible"),
@@@ -270,7 -262,7 +270,7 @@@
  		option_parse_message),
  	{ OPTION_LOWLEVEL_CALLBACK, 'F', "file", &merge_msg, N_("path"),
  		N_("read message from file"), PARSE_OPT_NONEG,
 -		(parse_opt_cb *) option_read_message },
 +		NULL, 0, option_read_message },
  	OPT__VERBOSITY(&verbosity),
  	OPT_BOOL(0, "abort", &abort_current_merge,
  		N_("abort the current in-progress merge")),
@@@ -617,8 -609,6 +617,8 @@@ static int git_merge_config(const char 
  		return git_config_string(&pull_twohead, k, v);
  	else if (!strcmp(k, "pull.octopus"))
  		return git_config_string(&pull_octopus, k, v);
 +	else if (!strcmp(k, "commit.cleanup"))
 +		return git_config_string(&cleanup_arg, k, v);
  	else if (!strcmp(k, "merge.renormalize"))
  		option_renormalize = git_config_bool(k, v);
  	else if (!strcmp(k, "merge.ff")) {
@@@ -807,13 -797,8 +807,13 @@@ static void abort_commit(struct commit_
  static const char merge_editor_comment[] =
  N_("Please enter a commit message to explain why this merge is necessary,\n"
     "especially if it merges an updated upstream into a topic branch.\n"
 -   "\n"
 -   "Lines starting with '%c' will be ignored, and an empty message aborts\n"
 +   "\n");
 +
 +static const char scissors_editor_comment[] =
 +N_("An empty message aborts the commit.\n");
 +
 +static const char no_scissors_editor_comment[] =
 +N_("Lines starting with '%c' will be ignored, and an empty message aborts\n"
     "the commit.\n");
  
  static void write_merge_heads(struct commit_list *);
@@@ -821,19 -806,11 +821,19 @@@ static void prepare_to_commit(struct co
  {
  	struct strbuf msg = STRBUF_INIT;
  	strbuf_addbuf(&msg, &merge_msg);
 -	strbuf_addch(&msg, '\n');
  	if (squash)
  		BUG("the control must not reach here under --squash");
 -	if (0 < option_edit)
 -		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
 +	if (0 < option_edit) {
 +		strbuf_addch(&msg, '\n');
 +		if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {
 +			wt_status_append_cut_line(&msg);
 +			strbuf_commented_addf(&msg, "\n");
 +		}
 +		strbuf_commented_addf(&msg, _(merge_editor_comment));
 +		strbuf_commented_addf(&msg, _(cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS ?
 +			scissors_editor_comment :
 +			no_scissors_editor_comment), comment_line_char);
 +	}
  	if (signoff)
  		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
  	write_merge_heads(remoteheads);
@@@ -852,7 -829,7 +852,7 @@@
  		abort_commit(remoteheads, NULL);
  
  	read_merge_msg(&msg);
 -	strbuf_stripspace(&msg, 0 < option_edit);
 +	cleanup_message(&msg, cleanup_mode, 0);
  	if (!msg.len)
  		abort_commit(remoteheads, _("Empty commit message."));
  	strbuf_release(&merge_msg);
@@@ -900,6 -877,7 +900,6 @@@ static int finish_automerge(struct comm
  	parents = remoteheads;
  	if (!head_subsumed || fast_forward == FF_NO)
  		commit_list_insert(head, &parents);
 -	strbuf_addch(&merge_msg, '\n');
  	prepare_to_commit(remoteheads);
  	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
  			&result_commit, NULL, sign_commit))
@@@ -920,15 -898,7 +920,15 @@@ static int suggest_conflicts(void
  	filename = git_path_merge_msg(the_repository);
  	fp = xfopen(filename, "a");
  
 -	append_conflicts_hint(&the_index, &msgbuf);
 +	/*
 +	 * We can't use cleanup_mode because if we're not using the editor,
 +	 * get_cleanup_mode will return COMMIT_MSG_CLEANUP_SPACE instead, even
 +	 * though the message is meant to be processed later by git-commit.
 +	 * Thus, we will get the cleanup mode which is returned when we _are_
 +	 * using an editor.
 +	 */
 +	append_conflicts_hint(&the_index, &msgbuf,
 +			      get_cleanup_mode(cleanup_arg, 1));
  	fputs(msgbuf.buf, fp);
  	strbuf_release(&msgbuf);
  	fclose(fp);
@@@ -1328,20 -1298,25 +1328,30 @@@ int cmd_merge(int argc, const char **ar
  	}
  	resolve_undo_clear();
  
 +	if (option_edit < 0)
 +		option_edit = default_edit_option();
 +
 +	cleanup_mode = get_cleanup_mode(cleanup_arg, 0 < option_edit);
 +
  	if (verbosity < 0)
  		show_diffstat = 0;
  
  	if (squash) {
  		if (fast_forward == FF_NO)
  			die(_("You cannot combine --squash with --no-ff."));
+ 		if (option_commit > 0)
+ 			die(_("You cannot combine --squash with --commit."));
+ 		/*
+ 		 * squash can now silently disable option_commit - this is not
+ 		 * a problem as it is only overriding the default, not a user
+ 		 * supplied option.
+ 		 */
  		option_commit = 0;
  	}
  
+ 	if (option_commit < 0)
+ 		option_commit = 1;
+ 
  	if (!argc) {
  		if (default_to_upstream)
  			argc = setup_with_upstream(&argv);
@@@ -1418,6 -1393,9 +1428,6 @@@
  			fast_forward = FF_NO;
  	}
  
 -	if (option_edit < 0)
 -		option_edit = default_edit_option();
 -
  	if (!use_strategies) {
  		if (!remoteheads)
  			; /* already up-to-date */
diff --combined t/t7600-merge.sh
index 7f9c68cbe7,eeb2222b4a..4ec5d9ec79
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@@ -233,65 -233,20 +233,65 @@@ test_expect_success 'merge --squash c3 
  	cat result.9z >file &&
  	git commit --no-edit -a &&
  
 -	{
 -		cat <<-EOF
 -		Squashed commit of the following:
 +	cat >expect <<-EOF &&
 +	Squashed commit of the following:
  
 -		$(git show -s c7)
 +	$(git show -s c7)
  
 -		# Conflicts:
 -		#	file
 -		EOF
 -	} >expect &&
 -	git cat-file commit HEAD | sed -e '1,/^$/d' >actual &&
 +	# Conflicts:
 +	#	file
 +	EOF
 +	git cat-file commit HEAD >raw &&
 +	sed -e '1,/^$/d' raw >actual &&
  	test_cmp expect actual
  '
  
 +test_expect_success 'merge c3 with c7 with commit.cleanup = scissors' '
 +	git config commit.cleanup scissors &&
 +	git reset --hard c3 &&
 +	test_must_fail git merge c7 &&
 +	cat result.9z >file &&
 +	git commit --no-edit -a &&
 +
 +	cat >expect <<-\EOF &&
 +	Merge tag '"'"'c7'"'"'
 +
 +	# ------------------------ >8 ------------------------
 +	# Do not modify or remove the line above.
 +	# Everything below it will be ignored.
 +	#
 +	# Conflicts:
 +	#	file
 +	EOF
 +	git cat-file commit HEAD >raw &&
 +	sed -e '1,/^$/d' raw >actual &&
 +	test_i18ncmp expect actual
 +'
 +
 +test_expect_success 'merge c3 with c7 with --squash commit.cleanup = scissors' '
 +	git config commit.cleanup scissors &&
 +	git reset --hard c3 &&
 +	test_must_fail git merge --squash c7 &&
 +	cat result.9z >file &&
 +	git commit --no-edit -a &&
 +
 +	cat >expect <<-EOF &&
 +	Squashed commit of the following:
 +
 +	$(git show -s c7)
 +
 +	# ------------------------ >8 ------------------------
 +	# Do not modify or remove the line above.
 +	# Everything below it will be ignored.
 +	#
 +	# Conflicts:
 +	#	file
 +	EOF
 +	git cat-file commit HEAD >raw &&
 +	sed -e '1,/^$/d' raw >actual &&
 +	test_i18ncmp expect actual
 +'
 +
  test_debug 'git log --graph --decorate --oneline --all'
  
  test_expect_success 'merge c1 with c2 and c3' '
@@@ -570,6 -525,12 +570,12 @@@ test_expect_success 'combining --squas
  	test_must_fail git merge --no-ff --squash c1
  '
  
+ test_expect_success 'combining --squash and --commit is refused' '
+ 	git reset --hard c0 &&
+ 	test_must_fail git merge --squash --commit c1 &&
+ 	test_must_fail git merge --commit --squash c1
+ '
+ 
  test_expect_success 'option --ff-only overwrites --no-ff' '
  	git merge --no-ff --ff-only c1 &&
  	test_must_fail git merge --no-ff --ff-only c2
@@@ -725,10 -686,10 +731,10 @@@ cat >editor <<\EO
  (
  	echo "Merge work done on the side branch c1"
  	echo
 -	cat <"$1"
 +	cat "$1"
  ) >"$1.tmp" && mv "$1.tmp" "$1"
  # strip comments and blank lines from end of message
 -sed -e '/^#/d' < "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' > expected
 +sed -e '/^#/d' "$1" | sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' >expected
  EOF
  chmod 755 editor
  
@@@ -813,14 -774,14 +819,14 @@@ test_expect_success 'set up mod-256 con
  	git commit -m base &&
  
  	# one side changes the first line of each to "master"
 -	sed s/-1/-master/ <file >tmp &&
 +	sed s/-1/-master/ file >tmp &&
  	mv tmp file &&
  	git commit -am master &&
  
  	# and the other to "side"; merging the two will
  	# yield 256 separate conflicts
  	git checkout -b side HEAD^ &&
 -	sed s/-1/-side/ <file >tmp &&
 +	sed s/-1/-side/ file >tmp &&
  	mv tmp file &&
  	git commit -am side
  '
@@@ -859,7 -820,7 +865,7 @@@ EO
  test_expect_success EXECKEEPSPID 'killed merge can be completed with --continue' '
  	git reset --hard c0 &&
  	! "$SHELL_PATH" -c '\''
 -	  echo kill -TERM $$ >> .git/FAKE_EDITOR
 +	  echo kill -TERM $$ >>.git/FAKE_EDITOR
  	  GIT_EDITOR=.git/FAKE_EDITOR
  	  export GIT_EDITOR
  	  exec git merge --no-ff --edit c1'\'' &&