From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 17 Jun 2019 17:15:17 +0000 (-0700)
Subject: Merge branch 'vv/merge-squash-with-explicit-commit'
X-Git-Tag: v2.23.0-rc0~118
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/6e0b1c60ad3e6067d5cae51a7dc36e58184accd5?hp=-c

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

"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
---

6e0b1c60ad3e6067d5cae51a7dc36e58184accd5
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 5c83f89cc6,18b90913d8..6e99aead46
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@@ -37,9 -37,7 +37,9 @@@
  #include "packfile.h"
  #include "tag.h"
  #include "alias.h"
 +#include "branch.h"
  #include "commit-reach.h"
 +#include "wt-status.h"
  
  #define DEFAULT_TWOHEAD (1<<0)
  #define DEFAULT_OCTOPUS (1<<1)
@@@ -59,7 -57,7 +59,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;
@@@ -74,7 -72,6 +74,7 @@@ static int option_renormalize
  static int verbosity;
  static int allow_rerere_auto;
  static int abort_current_merge;
 +static int quit_current_merge;
  static int continue_current_merge;
  static int allow_unrelated_histories;
  static int show_progress = -1;
@@@ -101,9 -98,6 +101,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)
  {
@@@ -119,15 -113,12 +119,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");
  
@@@ -255,7 -246,6 +255,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"),
@@@ -272,12 -262,10 +272,12 @@@
  		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")),
 +	OPT_BOOL(0, "quit", &quit_current_merge,
 +		N_("--abort but leave index and working tree alone")),
  	OPT_BOOL(0, "continue", &continue_current_merge,
  		N_("continue the current in-progress merge")),
  	OPT_BOOL(0, "allow-unrelated-histories", &allow_unrelated_histories,
@@@ -291,6 -279,14 +291,6 @@@
  	OPT_END()
  };
  
 -/* Cleans up metadata that is uninteresting after a succeeded merge. */
 -static void drop_save(void)
 -{
 -	unlink(git_path_merge_head(the_repository));
 -	unlink(git_path_merge_msg(the_repository));
 -	unlink(git_path_merge_mode(the_repository));
 -}
 -
  static int save_state(struct object_id *stash)
  {
  	int len;
@@@ -384,7 -380,7 +384,7 @@@ static void finish_up_to_date(const cha
  {
  	if (verbosity >= 0)
  		printf("%s%s\n", squash ? _(" (nothing to squash)") : "", msg);
 -	drop_save();
 +	remove_merge_branch_state(the_repository);
  }
  
  static void squash_message(struct commit *commit, struct commit_list *remoteheads)
@@@ -613,8 -609,6 +613,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")) {
@@@ -803,13 -797,8 +803,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 *);
@@@ -817,19 -806,11 +817,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);
@@@ -848,7 -829,7 +848,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);
@@@ -877,7 -858,7 +877,7 @@@ static int merge_trivial(struct commit 
  			&result_commit, NULL, sign_commit))
  		die(_("failed to write commit object"));
  	finish(head, remoteheads, &result_commit, "In-index merge");
 -	drop_save();
 +	remove_merge_branch_state(the_repository);
  	return 0;
  }
  
@@@ -896,6 -877,7 +896,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))
@@@ -903,7 -885,7 +903,7 @@@
  	strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
  	finish(head, remoteheads, &result_commit, buf.buf);
  	strbuf_release(&buf);
 -	drop_save();
 +	remove_merge_branch_state(the_repository);
  	return 0;
  }
  
@@@ -916,15 -898,7 +916,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);
@@@ -1285,16 -1259,6 +1285,16 @@@ int cmd_merge(int argc, const char **ar
  		goto done;
  	}
  
 +	if (quit_current_merge) {
 +		if (orig_argc != 2)
 +			usage_msg_opt(_("--quit expects no arguments"),
 +				      builtin_merge_usage,
 +				      builtin_merge_options);
 +
 +		remove_merge_branch_state(the_repository);
 +		goto done;
 +	}
 +
  	if (continue_current_merge) {
  		int nargc = 1;
  		const char *nargv[] = {"commit", NULL};
@@@ -1334,20 -1298,25 +1334,30 @@@
  	}
  	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);
@@@ -1424,6 -1393,9 +1434,6 @@@
  			fast_forward = FF_NO;
  	}
  
 -	if (option_edit < 0)
 -		option_edit = default_edit_option();
 -
  	if (!use_strategies) {
  		if (!remoteheads)
  			; /* already up-to-date */
@@@ -1501,7 -1473,7 +1511,7 @@@
  		}
  
  		finish(head_commit, remoteheads, &commit->object.oid, msg.buf);
 -		drop_save();
 +		remove_merge_branch_state(the_repository);
  		goto done;
  	} else if (!remoteheads->next && common->next)
  		;
diff --combined t/t7600-merge.sh
index 612ebe7d82,eeb2222b4a..132608879a
--- 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'\'' &&
@@@ -867,50 -828,4 +873,50 @@@
  	verify_parents $c0 $c1
  '
  
 +test_expect_success 'merge --quit' '
 +	git init merge-quit &&
 +	(
 +		cd merge-quit &&
 +		test_commit base &&
 +		echo one >>base.t &&
 +		git commit -am one &&
 +		git branch one &&
 +		git checkout base &&
 +		echo two >>base.t &&
 +		git commit -am two &&
 +		test_must_fail git -c rerere.enabled=true merge one &&
 +		test_path_is_file .git/MERGE_HEAD &&
 +		test_path_is_file .git/MERGE_MODE &&
 +		test_path_is_file .git/MERGE_MSG &&
 +		git rerere status >rerere.before &&
 +		git merge --quit &&
 +		test_path_is_missing .git/MERGE_HEAD &&
 +		test_path_is_missing .git/MERGE_MODE &&
 +		test_path_is_missing .git/MERGE_MSG &&
 +		git rerere status >rerere.after &&
 +		test_must_be_empty rerere.after &&
 +		! test_cmp rerere.after rerere.before
 +	)
 +'
 +
 +test_expect_success 'merge suggests matching remote refname' '
 +	git commit --allow-empty -m not-local &&
 +	git update-ref refs/remotes/origin/not-local HEAD &&
 +	git reset --hard HEAD^ &&
 +
 +	# This is white-box testing hackery; we happen to know
 +	# that reading packed refs is more picky about the memory
 +	# ownership of strings we pass to for_each_ref() callbacks.
 +	git pack-refs --all --prune &&
 +
 +	test_must_fail git merge not-local 2>stderr &&
 +	grep origin/not-local stderr
 +'
 +
 +test_expect_success 'suggested names are not ambiguous' '
 +	git update-ref refs/heads/origin/not-local HEAD &&
 +	test_must_fail git merge not-local 2>stderr &&
 +	grep remotes/origin/not-local stderr
 +'
 +
  test_done