cherry-pick: Add --empty for more robust redundant commit handling

As with a `git-rebase`, a `git-cherry-pick` can result in a commit being
made redundant if the content from the picked commit is already present
in the target history. However, `git-cherry-pick` does not have the same
options available that `git-rebase` has.

There are three things that can be done with these redundant commits:
drop the, keep them, or have the cherry-pick fail and ask the user to
take an action. `git-rebase` has the `--empty` option added in commit
e98c426 (rebase (interactive-backend): fix handling of commits that
become empty, 2020-02-15), which handles all three of these scenarios.
`git-cherry-pick` has only two of them: Keep the redundant commits via
`--keep-redundant-commits`, or have the cherry-pick fail by not
specifying that option.

In order to bring `git-cherry-pick` more in-line with `git-rebase`, this
commit adds a `--empty` option to `git-cherry-pick`. It has the same
three options (keep, drop, and ask), and largely behaves the same. The
notable difference is that for `git-cherry-pick`, the default will be
`ask`, which maintains the current behavior when the option is not
specified.

The `--keep-redundant-commits` option will be documented as a deprecated
synonym of `--empty=keep`, and will be supported for backwards
compatibility for the time being.

Documentation/git-cherry-pick.txt

@@ -132,23 +132,36 @@ effect to your index in a row.
 	keeps commits that were initially empty (i.e. the commit recorded the
 	same tree as its parent).  Commits which are made empty due to a
 	previous commit will cause the cherry-pick to fail.  To force the
-	inclusion of those commits use `--keep-redundant-commits`.
+	inclusion of those commits use `--empty=keep`.
 
 --allow-empty-message::
 	By default, cherry-picking a commit with an empty message will fail.
 	This option overrides that behavior, allowing commits with empty
 	messages to be cherry picked.
 
+--empty=(drop|keep|ask)::
+	If a commit being cherry picked duplicates changes already in the
+	current history, it will become empty.
++
+This option determines how these empty commits are handled:
++
+* `drop`: These empty commits are dropped.
++
+* `keep`: These empty commits are kept. Note that use of this option only
+  results in an empty commit when the commit was not initially empty, but
+  rather became empty due to a previous commit. Commits that were initially
+  empty will cause the cherry-pick to fail. To force the inclusion of those
+  commits use `--allow-empty`.
++
+* `ask`: These empty commits cause `cherry-pick` to stop so the user can examine
+  the commit. This is the default behavior.
++
+Note that commits which start empty will cause the cherry-pick to fail (unless
+`--allow-empty` is specified).
++
+
 --keep-redundant-commits::
-	If a commit being cherry picked duplicates a commit already in the
-	current history, it will become empty.  By default these
-	redundant commits cause `cherry-pick` to stop so the user can
-	examine the commit. This option overrides that behavior and
-	creates an empty commit object. Note that use of this option only
-	results in an empty commit when the commit was not initially empty,
-	but rather became empty due to a previous commit. Commits that were
-	initially empty will cause the cherry-pick to fail. To force the
-	inclusion of those commits use `--allow-empty`.
+	Deprecated synonym for `--empty=keep`.
 
 --strategy=<strategy>::
 	Use the given merge strategy.  Should only be used once.

builtin/revert.c

@@ -1,3 +1,6 @@
+//TODO: How to properly handle?
+//Should previous empty rebase code have not used strcasecmp?
+#include <strings.h>
 #include "git-compat-util.h"
 #include "config.h"
 #include "builtin.h"
@@ -45,6 +48,31 @@ static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
 	return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
 }
 
+//TODO: Centralize?
+enum empty_type {
+	EMPTY_UNSPECIFIED = -1,
+	EMPTY_DROP,
+	EMPTY_KEEP,
+	EMPTY_ASK
+};
+
+static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
+{
+	enum empty_type value = EMPTY_UNSPECIFIED;
+
+	if (unset || !strcasecmp(arg, "ask"))
+		value = EMPTY_ASK;
+	else if (!strcasecmp(arg, "drop"))
+		value = EMPTY_DROP;
+	else if (!strcasecmp(arg, "keep"))
+		value = EMPTY_KEEP;
+	else
+		return error(_("unrecognized empty type '%s'; valid values are \"drop\", \"keep\", and \"ask\"."), arg);
+
+	*(enum empty_type *)opt->value = value;
+	return 0;
+}
+
 static int option_parse_m(const struct option *opt,
 			  const char *arg, int unset)
 {
@@ -87,6 +115,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 	const char * const * usage_str = revert_or_cherry_pick_usage(opts);
 	const char *me = action_name(opts);
 	const char *cleanup_arg = NULL;
+	enum empty_type empty_opt = EMPTY_UNSPECIFIED;
 	int cmd = 0;
 	struct option base_options[] = {
 		OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
@@ -116,7 +145,12 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 			OPT_BOOL(0, "ff", &opts->allow_ff, N_("allow fast-forward")),
 			OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
 			OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
-			OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+			//TODO: anything to do for translation here?
+			OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("deprecated: use --empty=keep instead")),
+			//TODO: Same - translation?
+			OPT_CALLBACK_F(0, "empty", &empty_opt, "(drop|keep|ask)",
+				       N_("how to handle commits that become empty"),
+				       PARSE_OPT_NONEG, parse_opt_empty),
 			OPT_END(),
 		};
 		options = parse_options_concat(options, cp_extra);
@@ -136,6 +170,11 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 	prepare_repo_settings(the_repository);
 	the_repository->settings.command_requires_full_index = 0;
 
+	if (opts->action == REPLAY_PICK) {
+		opts->drop_redundant_commits = (empty_opt == EMPTY_DROP);
+		opts->keep_redundant_commits = opts->keep_redundant_commits || (empty_opt == EMPTY_KEEP);
+	}
+
 	if (cleanup_arg) {
 		opts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1);
 		opts->explicit_cleanup = 1;

sequencer.c

@@ -2934,6 +2934,9 @@ static int populate_opts_cb(const char *key, const char *value,
 	else if (!strcmp(key, "options.allow-empty-message"))
 		opts->allow_empty_message =
 			git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
+	else if (!strcmp(key, "options.drop-redundant-commits"))
+		opts->drop_redundant_commits =
+			git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
 	else if (!strcmp(key, "options.keep-redundant-commits"))
 		opts->keep_redundant_commits =
 			git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
@@ -3478,6 +3481,9 @@ static int save_opts(struct replay_opts *opts)
 	if (opts->allow_empty_message)
 		res |= git_config_set_in_file_gently(opts_file,
 				"options.allow-empty-message", "true");
+	if (opts->drop_redundant_commits)
+		res |= git_config_set_in_file_gently(opts_file,
+				"options.drop-redundant-commits", "true");
 	if (opts->keep_redundant_commits)
 		res |= git_config_set_in_file_gently(opts_file,
 				"options.keep-redundant-commits", "true");

t/t3505-cherry-pick-empty.sh

@@ -90,7 +90,7 @@ test_expect_success 'cherry-pick a commit that becomes no-op (prep)' '
 	git commit -m "add file2 on the side"
 '
 
-test_expect_success 'cherry-pick a no-op without --keep-redundant' '
+test_expect_success 'cherry-pick a no-op with neither --keep-redundante nor --empty' '
 	git reset --hard &&
 	git checkout fork^0 &&
 	test_must_fail git cherry-pick main
@@ -105,4 +105,28 @@ test_expect_success 'cherry-pick a no-op with --keep-redundant' '
 	test_cmp expect actual
 '
 
+test_expect_success 'cherry-pick a no-op with --empty=ask' '
+	git reset --hard &&
+	git checkout fork^0 &&
+	test_must_fail git cherry-pick --empty=ask main
+'
+
+test_expect_success 'cherry-pick a no-op with --empty=drop' '
+	git reset --hard &&
+	git checkout fork^0 &&
+	git cherry-pick --empty=drop main &&
+	git show -s --format=%s >actual &&
+	echo "add file2 on the side" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick a no-op with --empty=keep' '
+	git reset --hard &&
+	git checkout fork^0 &&
+	git cherry-pick --empty=keep main &&
+	git show -s --format=%s >actual &&
+	echo "add file2 on main" >expect &&
+	test_cmp expect actual
+'
+
 test_done