Skip to content

Commit ec79d76

Browse files
zivarahgitster
zivarah
authored and
gitster
committed
cherry-pick: add --empty for more robust redundant commit handling
As with git-rebase(1) and git-am(1), git-cherry-pick(1) 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(1) does not have the same options available that git-rebase(1) and git-am(1) have. There are three things that can be done with these redundant commits: drop them, keep them, or have the cherry-pick stop and wait for the user to take an action. git-rebase(1) 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. Similarly, git-am(1) got its own `--empty` in 7c096b8 (am: support --empty=<option> to handle empty patches, 2021-12-09). git-cherry-pick(1), on the other hand, only supports two of the three possiblities: Keep the redundant commits via `--keep-redundant-commits`, or have the cherry-pick fail by not specifying that option. There is no way to automatically drop redundant commits. In order to bring git-cherry-pick(1) more in-line with git-rebase(1) and git-am(1), this commit adds an `--empty` option to git-cherry-pick(1). It has the same three options (keep, drop, and stop), and largely behaves the same. The notable difference is that for git-cherry-pick(1), the default will be `stop`, which maintains the current behavior when the option is not specified. Like the existing `--keep-redundant-commits`, `--empty=keep` will imply `--allow-empty`. 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. Signed-off-by: Brian Lyles <brianmlyles@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent bd2f9fd commit ec79d76

File tree

5 files changed

+133
-9
lines changed

5 files changed

+133
-9
lines changed

‎Documentation/git-cherry-pick.txt‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,20 +131,36 @@ effect to your index in a row.
131131
even without this option. Note also, that use of this option only
132132
keeps commits that were initially empty (i.e. the commit recorded the
133133
same tree as its parent). Commits which are made empty due to a
134-
previous commit are dropped. To force the inclusion of those commits
135-
use `--keep-redundant-commits`.
134+
previous commit will cause the cherry-pick to fail. To force the
135+
inclusion of those commits, use `--empty=keep`.
136136

137137
--allow-empty-message::
138138
By default, cherry-picking a commit with an empty message will fail.
139139
This option overrides that behavior, allowing commits with empty
140140
messages to be cherry picked.
141141

142+
--empty=(drop|keep|stop)::
143+
How to handle commits being cherry-picked that are redundant with
144+
changes already in the current history.
145+
+
146+
--
147+
`drop`;;
148+
The commit will be dropped.
149+
`keep`;;
150+
The commit will be kept. Implies `--allow-empty`.
151+
`stop`;;
152+
The cherry-pick will stop when the commit is applied, allowing
153+
you to examine the commit. This is the default behavior.
154+
--
155+
+
156+
Note that `--empty=drop` and `--empty=stop` only specify how to handle a
157+
commit that was not initially empty, but rather became empty due to a previous
158+
commit. Commits that were initially empty will still cause the cherry-pick to
159+
fail unless one of `--empty=keep` or `--allow-empty` are specified.
160+
+
161+
142162
--keep-redundant-commits::
143-
If a commit being cherry picked duplicates a commit already in the
144-
current history, it will become empty. By default these
145-
redundant commits cause `cherry-pick` to stop so the user can
146-
examine the commit. This option overrides that behavior and
147-
creates an empty commit object. Implies `--allow-empty`.
163+
Deprecated synonym for `--empty=keep`.
148164

149165
--strategy=<strategy>::
150166
Use the given merge strategy. Should only be used once.

‎builtin/revert.c‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,31 @@ static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
4343
return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
4444
}
4545

46+
enum empty_action {
47+
EMPTY_COMMIT_UNSPECIFIED = -1,
48+
STOP_ON_EMPTY_COMMIT, /* output errors and stop in the middle of a cherry-pick */
49+
DROP_EMPTY_COMMIT, /* skip with a notice message */
50+
KEEP_EMPTY_COMMIT, /* keep recording as empty commits */
51+
};
52+
53+
static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
54+
{
55+
int *opt_value = opt->value;
56+
57+
BUG_ON_OPT_NEG(unset);
58+
59+
if (!strcmp(arg, "stop"))
60+
*opt_value = STOP_ON_EMPTY_COMMIT;
61+
else if (!strcmp(arg, "drop"))
62+
*opt_value = DROP_EMPTY_COMMIT;
63+
else if (!strcmp(arg, "keep"))
64+
*opt_value = KEEP_EMPTY_COMMIT;
65+
else
66+
return error(_("invalid value for '%s': '%s'"), "--empty", arg);
67+
68+
return 0;
69+
}
70+
4671
static int option_parse_m(const struct option *opt,
4772
const char *arg, int unset)
4873
{
@@ -85,6 +110,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
85110
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
86111
const char *me = action_name(opts);
87112
const char *cleanup_arg = NULL;
113+
enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED;
88114
int cmd = 0;
89115
struct option base_options[] = {
90116
OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
@@ -114,7 +140,10 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
114140
OPT_BOOL(0, "ff", &opts->allow_ff, N_("allow fast-forward")),
115141
OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
116142
OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
117-
OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
143+
OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("deprecated: use --empty=keep instead")),
144+
OPT_CALLBACK_F(0, "empty", &empty_opt, "(stop|drop|keep)",
145+
N_("how to handle commits that become empty"),
146+
PARSE_OPT_NONEG, parse_opt_empty),
118147
OPT_END(),
119148
};
120149
options = parse_options_concat(options, cp_extra);
@@ -134,6 +163,11 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
134163
prepare_repo_settings(the_repository);
135164
the_repository->settings.command_requires_full_index = 0;
136165

166+
if (opts->action == REPLAY_PICK) {
167+
opts->drop_redundant_commits = (empty_opt == DROP_EMPTY_COMMIT);
168+
opts->keep_redundant_commits = opts->keep_redundant_commits || (empty_opt == KEEP_EMPTY_COMMIT);
169+
}
170+
137171
/* implies allow_empty */
138172
if (opts->keep_redundant_commits)
139173
opts->allow_empty = 1;
@@ -168,6 +202,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
168202
"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
169203
"--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
170204
"--keep-redundant-commits", opts->keep_redundant_commits,
205+
"--empty", empty_opt != EMPTY_COMMIT_UNSPECIFIED,
171206
NULL);
172207
}
173208

‎sequencer.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2932,6 +2932,9 @@ static int populate_opts_cb(const char *key, const char *value,
29322932
else if (!strcmp(key, "options.allow-empty-message"))
29332933
opts->allow_empty_message =
29342934
git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
2935+
else if (!strcmp(key, "options.drop-redundant-commits"))
2936+
opts->drop_redundant_commits =
2937+
git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
29352938
else if (!strcmp(key, "options.keep-redundant-commits"))
29362939
opts->keep_redundant_commits =
29372940
git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
@@ -3476,6 +3479,9 @@ static int save_opts(struct replay_opts *opts)
34763479
if (opts->allow_empty_message)
34773480
res |= git_config_set_in_file_gently(opts_file,
34783481
"options.allow-empty-message", "true");
3482+
if (opts->drop_redundant_commits)
3483+
res |= git_config_set_in_file_gently(opts_file,
3484+
"options.drop-redundant-commits", "true");
34793485
if (opts->keep_redundant_commits)
34803486
res |= git_config_set_in_file_gently(opts_file,
34813487
"options.keep-redundant-commits", "true");

‎t/t3505-cherry-pick-empty.sh‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ test_expect_success 'cherry-pick a commit that becomes no-op (prep)' '
8484
git commit -m "add file2 on the side"
8585
'
8686

87-
test_expect_success 'cherry-pick a no-op without --keep-redundant' '
87+
test_expect_success 'cherry-pick a no-op with neither --keep-redundant nor --empty' '
8888
git reset --hard &&
8989
git checkout fork^0 &&
9090
test_must_fail git cherry-pick main
@@ -113,4 +113,39 @@ test_expect_success '--keep-redundant-commits is incompatible with operations' '
113113
git cherry-pick --abort
114114
'
115115

116+
test_expect_success '--empty is incompatible with operations' '
117+
test_must_fail git cherry-pick HEAD 2>output &&
118+
test_grep "The previous cherry-pick is now empty" output &&
119+
test_must_fail git cherry-pick --empty=stop --continue 2>output &&
120+
test_grep "fatal: cherry-pick: --empty cannot be used with --continue" output &&
121+
test_must_fail git cherry-pick --empty=stop --skip 2>output &&
122+
test_grep "fatal: cherry-pick: --empty cannot be used with --skip" output &&
123+
test_must_fail git cherry-pick --empty=stop --abort 2>output &&
124+
test_grep "fatal: cherry-pick: --empty cannot be used with --abort" output &&
125+
test_must_fail git cherry-pick --empty=stop --quit 2>output &&
126+
test_grep "fatal: cherry-pick: --empty cannot be used with --quit" output &&
127+
git cherry-pick --abort
128+
'
129+
130+
test_expect_success 'cherry-pick a no-op with --empty=stop' '
131+
git reset --hard &&
132+
git checkout fork^0 &&
133+
test_must_fail git cherry-pick --empty=stop main 2>output &&
134+
test_grep "The previous cherry-pick is now empty" output
135+
'
136+
137+
test_expect_success 'cherry-pick a no-op with --empty=drop' '
138+
git reset --hard &&
139+
git checkout fork^0 &&
140+
git cherry-pick --empty=drop main &&
141+
test_commit_message HEAD -m "add file2 on the side"
142+
'
143+
144+
test_expect_success 'cherry-pick a no-op with --empty=keep' '
145+
git reset --hard &&
146+
git checkout fork^0 &&
147+
git cherry-pick --empty=keep main &&
148+
test_commit_message HEAD -m "add file2 on main"
149+
'
150+
116151
test_done

‎t/t3510-cherry-pick-sequence.sh‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,38 @@ test_expect_success 'cherry-pick persists opts correctly' '
9090
test_cmp expect actual
9191
'
9292

93+
test_expect_success 'cherry-pick persists --empty=stop correctly' '
94+
pristine_detach yetanotherpick &&
95+
# Picking `anotherpick` forces a conflict so that we stop. That
96+
# commit is then skipped, after which we pick `yetanotherpick`
97+
# while already on `yetanotherpick` to cause an empty commit
98+
test_must_fail git cherry-pick --empty=stop anotherpick yetanotherpick &&
99+
test_must_fail git cherry-pick --skip 2>msg &&
100+
test_grep "The previous cherry-pick is now empty" msg &&
101+
rm msg &&
102+
git cherry-pick --abort
103+
'
104+
105+
test_expect_success 'cherry-pick persists --empty=drop correctly' '
106+
pristine_detach yetanotherpick &&
107+
# Picking `anotherpick` forces a conflict so that we stop. That
108+
# commit is then skipped, after which we pick `yetanotherpick`
109+
# while already on `yetanotherpick` to cause an empty commit
110+
test_must_fail git cherry-pick --empty=drop anotherpick yetanotherpick &&
111+
git cherry-pick --skip &&
112+
test_cmp_rev yetanotherpick HEAD
113+
'
114+
115+
test_expect_success 'cherry-pick persists --empty=keep correctly' '
116+
pristine_detach yetanotherpick &&
117+
# Picking `anotherpick` forces a conflict so that we stop. That
118+
# commit is then skipped, after which we pick `yetanotherpick`
119+
# while already on `yetanotherpick` to cause an empty commit
120+
test_must_fail git cherry-pick --empty=keep anotherpick yetanotherpick &&
121+
git cherry-pick --skip &&
122+
test_cmp_rev yetanotherpick HEAD^
123+
'
124+
93125
test_expect_success 'revert persists opts correctly' '
94126
pristine_detach initial &&
95127
# to make sure that the session to revert a sequence

0 commit comments

Comments
 (0)