1
0
Fork 0
mirror of https://github.com/git/git.git synced 2024-04-28 14:35:10 +02:00

Merge branch 'bl/cherry-pick-empty' into next

Allow git-cherry-pick(1) to automatically drop redundant commits via
a new `--empty` option, similar to the `--empty` options for
git-rebase(1) and git-am(1). Includes a soft deprecation of
`--keep-redundant-commits` as well as some related docs changes and
sequencer code cleanup.

* bl/cherry-pick-empty:
  cherry-pick: add `--empty` for more robust redundant commit handling
  cherry-pick: enforce `--keep-redundant-commits` incompatibility
  sequencer: do not require `allow_empty` for redundant commit options
  sequencer: handle unborn branch with `--allow-empty`
  rebase: update `--empty=ask` to `--empty=stop`
  docs: clean up `--empty` formatting in git-rebase(1) and git-am(1)
  docs: address inaccurate `--empty` default with `--exec`
This commit is contained in:
Junio C Hamano 2024-03-28 14:20:12 -07:00
commit 22e8e4a68e
10 changed files with 285 additions and 67 deletions

View File

@ -66,13 +66,19 @@ OPTIONS
--quoted-cr=<action>::
This flag will be passed down to 'git mailinfo' (see linkgit:git-mailinfo[1]).
--empty=(stop|drop|keep)::
By default, or when the option is set to 'stop', the command
errors out on an input e-mail message lacking a patch
and stops in the middle of the current am session. When this
option is set to 'drop', skip such an e-mail message instead.
When this option is set to 'keep', create an empty commit,
recording the contents of the e-mail message as its log.
--empty=(drop|keep|stop)::
How to handle an e-mail message lacking a patch:
+
--
`drop`;;
The e-mail message will be skipped.
`keep`;;
An empty commit will be created, with the contents of the e-mail
message as its log.
`stop`;;
The command will fail, stopping in the middle of the current `am`
session. This is the default behavior.
--
-m::
--message-id::

View File

@ -131,20 +131,36 @@ effect to your index in a row.
even without this option. Note also, that use of this option only
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 are dropped. To force the inclusion of those commits
use `--keep-redundant-commits`.
previous commit will cause the cherry-pick to fail. To force the
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|stop)::
How to handle commits being cherry-picked that are redundant with
changes already in the current history.
+
--
`drop`;;
The commit will be dropped.
`keep`;;
The commit will be kept. Implies `--allow-empty`.
`stop`;;
The cherry-pick will stop when the commit is applied, allowing
you to examine the commit. This is the default behavior.
--
+
Note that `--empty=drop` and `--empty=stop` only specify how to handle a
commit that was not initially empty, but rather became empty due to a previous
commit. Commits that were initially empty will still cause the cherry-pick to
fail unless one of `--empty=keep` or `--allow-empty` are 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. Implies `--allow-empty`.
Deprecated synonym for `--empty=keep`.
--strategy=<strategy>::
Use the given merge strategy. Should only be used once.

View File

@ -289,17 +289,25 @@ See also INCOMPATIBLE OPTIONS below.
+
See also INCOMPATIBLE OPTIONS below.
--empty=(drop|keep|ask)::
--empty=(drop|keep|stop)::
How to handle commits that are not empty to start and are not
clean cherry-picks of any upstream commit, but which become
empty after rebasing (because they contain a subset of already
upstream changes). With drop (the default), commits that
become empty are dropped. With keep, such commits are kept.
With ask (implied by `--interactive`), the rebase will halt when
an empty commit is applied allowing you to choose whether to
drop it, edit files more, or just commit the empty changes.
Other options, like `--exec`, will use the default of drop unless
`-i`/`--interactive` is explicitly specified.
upstream changes):
+
--
`drop`;;
The commit will be dropped. This is the default behavior.
`keep`;;
The commit will be kept. This option is implied when `--exec` is
specified unless `-i`/`--interactive` is also specified.
`stop`;;
`ask`;;
The rebase will halt when the commit is applied, allowing you to
choose whether to drop it, edit files more, or just commit the empty
changes. This option is implied when `-i`/`--interactive` is
specified. `ask` is a deprecated synonym of `stop`.
--
+
Note that commits which start empty are kept (unless `--no-keep-empty`
is specified), and commits which are clean cherry-picks (as determined
@ -704,7 +712,7 @@ be dropped automatically with `--no-keep-empty`).
Similar to the apply backend, by default the merge backend drops
commits that become empty unless `-i`/`--interactive` is specified (in
which case it stops and asks the user what to do). The merge backend
also has an `--empty=(drop|keep|ask)` option for changing the behavior
also has an `--empty=(drop|keep|stop)` option for changing the behavior
of handling commits that become empty.
Directory rename detection

View File

@ -58,7 +58,7 @@ enum empty_type {
EMPTY_UNSPECIFIED = -1,
EMPTY_DROP,
EMPTY_KEEP,
EMPTY_ASK
EMPTY_STOP
};
enum action {
@ -945,10 +945,14 @@ static enum empty_type parse_empty_value(const char *value)
return EMPTY_DROP;
else if (!strcasecmp(value, "keep"))
return EMPTY_KEEP;
else if (!strcasecmp(value, "ask"))
return EMPTY_ASK;
else if (!strcasecmp(value, "stop"))
return EMPTY_STOP;
else if (!strcasecmp(value, "ask")) {
warning(_("--empty=ask is deprecated; use '--empty=stop' instead."));
return EMPTY_STOP;
}
die(_("unrecognized empty type '%s'; valid values are \"drop\", \"keep\", and \"ask\"."), value);
die(_("unrecognized empty type '%s'; valid values are \"drop\", \"keep\", and \"stop\"."), value);
}
static int parse_opt_keep_empty(const struct option *opt, const char *arg,
@ -1127,7 +1131,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
"instead of ignoring them"),
1, PARSE_OPT_HIDDEN),
OPT_RERERE_AUTOUPDATE(&options.allow_rerere_autoupdate),
OPT_CALLBACK_F(0, "empty", &options, "(drop|keep|ask)",
OPT_CALLBACK_F(0, "empty", &options, "(drop|keep|stop)",
N_("how to handle commits that become empty"),
PARSE_OPT_NONEG, parse_opt_empty),
OPT_CALLBACK_F('k', "keep-empty", &options, NULL,
@ -1544,7 +1548,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (options.empty == EMPTY_UNSPECIFIED) {
if (options.flags & REBASE_INTERACTIVE_EXPLICIT)
options.empty = EMPTY_ASK;
options.empty = EMPTY_STOP;
else if (options.exec.nr > 0)
options.empty = EMPTY_KEEP;
else

View File

@ -43,6 +43,31 @@ static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
}
enum empty_action {
EMPTY_COMMIT_UNSPECIFIED = -1,
STOP_ON_EMPTY_COMMIT, /* output errors and stop in the middle of a cherry-pick */
DROP_EMPTY_COMMIT, /* skip with a notice message */
KEEP_EMPTY_COMMIT, /* keep recording as empty commits */
};
static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
{
int *opt_value = opt->value;
BUG_ON_OPT_NEG(unset);
if (!strcmp(arg, "stop"))
*opt_value = STOP_ON_EMPTY_COMMIT;
else if (!strcmp(arg, "drop"))
*opt_value = DROP_EMPTY_COMMIT;
else if (!strcmp(arg, "keep"))
*opt_value = KEEP_EMPTY_COMMIT;
else
return error(_("invalid value for '%s': '%s'"), "--empty", arg);
return 0;
}
static int option_parse_m(const struct option *opt,
const char *arg, int unset)
{
@ -85,6 +110,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_action empty_opt = EMPTY_COMMIT_UNSPECIFIED;
int cmd = 0;
struct option base_options[] = {
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,
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")),
OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("deprecated: use --empty=keep instead")),
OPT_CALLBACK_F(0, "empty", &empty_opt, "(stop|drop|keep)",
N_("how to handle commits that become empty"),
PARSE_OPT_NONEG, parse_opt_empty),
OPT_END(),
};
options = parse_options_concat(options, cp_extra);
@ -134,6 +163,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 == DROP_EMPTY_COMMIT);
opts->keep_redundant_commits = opts->keep_redundant_commits || (empty_opt == KEEP_EMPTY_COMMIT);
}
/* implies allow_empty */
if (opts->keep_redundant_commits)
opts->allow_empty = 1;
@ -167,6 +201,8 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
"--ff", opts->allow_ff,
"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
"--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
"--keep-redundant-commits", opts->keep_redundant_commits,
"--empty", empty_opt != EMPTY_COMMIT_UNSPECIFIED,
NULL);
}

View File

@ -787,29 +787,42 @@ static struct object_id *get_cache_tree_oid(struct index_state *istate)
static int is_index_unchanged(struct repository *r)
{
struct object_id head_oid, *cache_tree_oid;
const struct object_id *head_tree_oid;
struct commit *head_commit;
struct index_state *istate = r->index;
const char *head_name;
if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL))
return error(_("could not resolve HEAD commit"));
if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, &head_oid, NULL)) {
/* Check to see if this is an unborn branch */
head_name = resolve_ref_unsafe("HEAD",
RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
&head_oid, NULL);
if (!head_name ||
!starts_with(head_name, "refs/heads/") ||
!is_null_oid(&head_oid))
return error(_("could not resolve HEAD commit"));
head_tree_oid = the_hash_algo->empty_tree;
} else {
head_commit = lookup_commit(r, &head_oid);
head_commit = lookup_commit(r, &head_oid);
/*
* If head_commit is NULL, check_commit, called from
* lookup_commit, would have indicated that head_commit is not
* a commit object already. repo_parse_commit() will return failure
* without further complaints in such a case. Otherwise, if
* the commit is invalid, repo_parse_commit() will complain. So
* there is nothing for us to say here. Just return failure.
*/
if (repo_parse_commit(r, head_commit))
return -1;
/*
* If head_commit is NULL, check_commit, called from
* lookup_commit, would have indicated that head_commit is not
* a commit object already. repo_parse_commit() will return failure
* without further complaints in such a case. Otherwise, if
* the commit is invalid, repo_parse_commit() will complain. So
* there is nothing for us to say here. Just return failure.
*/
if (repo_parse_commit(r, head_commit))
return -1;
head_tree_oid = get_commit_tree_oid(head_commit);
}
if (!(cache_tree_oid = get_cache_tree_oid(istate)))
return -1;
return oideq(cache_tree_oid, get_commit_tree_oid(head_commit));
return oideq(cache_tree_oid, head_tree_oid);
}
static int write_author_script(const char *message)
@ -1736,34 +1749,25 @@ static int allow_empty(struct repository *r,
int index_unchanged, originally_empty;
/*
* Four cases:
* For a commit that is initially empty, allow_empty determines if it
* should be kept or not
*
* (1) we do not allow empty at all and error out.
*
* (2) we allow ones that were initially empty, and
* just drop the ones that become empty
*
* (3) we allow ones that were initially empty, but
* halt for the ones that become empty;
*
* (4) we allow both.
* For a commit that becomes empty, keep_redundant_commits and
* drop_redundant_commits determine whether the commit should be kept or
* dropped. If neither is specified, halt.
*/
if (!opts->allow_empty)
return 0; /* let "git commit" barf as necessary */
index_unchanged = is_index_unchanged(r);
if (index_unchanged < 0)
return index_unchanged;
if (!index_unchanged)
return 0; /* we do not have to say --allow-empty */
if (opts->keep_redundant_commits)
return 1;
originally_empty = is_original_commit_empty(commit);
if (originally_empty < 0)
return originally_empty;
if (originally_empty)
return opts->allow_empty;
else if (opts->keep_redundant_commits)
return 1;
else if (opts->drop_redundant_commits)
return 2;
@ -2945,6 +2949,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);
@ -3489,6 +3496,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", NULL, "true");
if (opts->drop_redundant_commits)
res |= git_config_set_in_file_gently(opts_file,
"options.drop-redundant-commits", NULL, "true");
if (opts->keep_redundant_commits)
res |= git_config_set_in_file_gently(opts_file,
"options.keep-redundant-commits", NULL, "true");

View File

@ -72,6 +72,17 @@ test_expect_success 'rebase --merge --empty=keep' '
test_cmp expect actual
'
test_expect_success 'rebase --merge --empty=stop' '
git checkout -B testing localmods &&
test_must_fail git rebase --merge --empty=stop upstream &&
git rebase --skip &&
test_write_lines D C B A >expect &&
git log --format=%s >actual &&
test_cmp expect actual
'
test_expect_success 'rebase --merge --empty=ask' '
git checkout -B testing localmods &&
test_must_fail git rebase --merge --empty=ask upstream &&
@ -101,9 +112,9 @@ test_expect_success 'rebase --interactive --empty=keep' '
test_cmp expect actual
'
test_expect_success 'rebase --interactive --empty=ask' '
test_expect_success 'rebase --interactive --empty=stop' '
git checkout -B testing localmods &&
test_must_fail git rebase --interactive --empty=ask upstream &&
test_must_fail git rebase --interactive --empty=stop upstream &&
git rebase --skip &&
@ -112,7 +123,7 @@ test_expect_success 'rebase --interactive --empty=ask' '
test_cmp expect actual
'
test_expect_success 'rebase --interactive uses default of --empty=ask' '
test_expect_success 'rebase --interactive uses default of --empty=stop' '
git checkout -B testing localmods &&
test_must_fail git rebase --interactive upstream &&
@ -167,4 +178,42 @@ test_expect_success 'rebase --merge does not leave state laying around' '
test_path_is_missing .git/MERGE_MSG
'
test_expect_success 'rebase --exec --empty=drop' '
git checkout -B testing localmods &&
git rebase --exec "true" --empty=drop upstream &&
test_write_lines D C B A >expect &&
git log --format=%s >actual &&
test_cmp expect actual
'
test_expect_success 'rebase --exec --empty=keep' '
git checkout -B testing localmods &&
git rebase --exec "true" --empty=keep upstream &&
test_write_lines D C2 C B A >expect &&
git log --format=%s >actual &&
test_cmp expect actual
'
test_expect_success 'rebase --exec uses default of --empty=keep' '
git checkout -B testing localmods &&
git rebase --exec "true" upstream &&
test_write_lines D C2 C B A >expect &&
git log --format=%s >actual &&
test_cmp expect actual
'
test_expect_success 'rebase --exec --empty=stop' '
git checkout -B testing localmods &&
test_must_fail git rebase --exec "true" --empty=stop upstream &&
git rebase --skip &&
test_write_lines D C B A >expect &&
git log --format=%s >actual &&
test_cmp expect actual
'
test_done

View File

@ -104,11 +104,19 @@ test_expect_success 'revert forbidden on dirty working tree' '
'
test_expect_success 'cherry-pick on unborn branch' '
git checkout --orphan unborn &&
git switch --orphan unborn &&
git rm --cached -r . &&
rm -rf * &&
git cherry-pick initial &&
git diff --quiet initial &&
git diff --exit-code initial &&
test_cmp_rev ! initial HEAD
'
test_expect_success 'cherry-pick on unborn branch with --allow-empty' '
git checkout --detach &&
git branch -D unborn &&
git switch --orphan unborn &&
git cherry-pick initial --allow-empty &&
git diff --exit-code initial &&
test_cmp_rev ! initial HEAD
'

View File

@ -84,7 +84,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-redundant nor --empty' '
git reset --hard &&
git checkout fork^0 &&
test_must_fail git cherry-pick main
@ -99,4 +99,53 @@ test_expect_success 'cherry-pick a no-op with --keep-redundant' '
test_cmp expect actual
'
test_expect_success '--keep-redundant-commits is incompatible with operations' '
test_must_fail git cherry-pick HEAD 2>output &&
test_grep "The previous cherry-pick is now empty" output &&
test_must_fail git cherry-pick --keep-redundant-commits --continue 2>output &&
test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --continue" output &&
test_must_fail git cherry-pick --keep-redundant-commits --skip 2>output &&
test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --skip" output &&
test_must_fail git cherry-pick --keep-redundant-commits --abort 2>output &&
test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --abort" output &&
test_must_fail git cherry-pick --keep-redundant-commits --quit 2>output &&
test_grep "fatal: cherry-pick: --keep-redundant-commits cannot be used with --quit" output &&
git cherry-pick --abort
'
test_expect_success '--empty is incompatible with operations' '
test_must_fail git cherry-pick HEAD 2>output &&
test_grep "The previous cherry-pick is now empty" output &&
test_must_fail git cherry-pick --empty=stop --continue 2>output &&
test_grep "fatal: cherry-pick: --empty cannot be used with --continue" output &&
test_must_fail git cherry-pick --empty=stop --skip 2>output &&
test_grep "fatal: cherry-pick: --empty cannot be used with --skip" output &&
test_must_fail git cherry-pick --empty=stop --abort 2>output &&
test_grep "fatal: cherry-pick: --empty cannot be used with --abort" output &&
test_must_fail git cherry-pick --empty=stop --quit 2>output &&
test_grep "fatal: cherry-pick: --empty cannot be used with --quit" output &&
git cherry-pick --abort
'
test_expect_success 'cherry-pick a no-op with --empty=stop' '
git reset --hard &&
git checkout fork^0 &&
test_must_fail git cherry-pick --empty=stop main 2>output &&
test_grep "The previous cherry-pick is now empty" output
'
test_expect_success 'cherry-pick a no-op with --empty=drop' '
git reset --hard &&
git checkout fork^0 &&
git cherry-pick --empty=drop main &&
test_commit_message HEAD -m "add file2 on the side"
'
test_expect_success 'cherry-pick a no-op with --empty=keep' '
git reset --hard &&
git checkout fork^0 &&
git cherry-pick --empty=keep main &&
test_commit_message HEAD -m "add file2 on main"
'
test_done

View File

@ -90,6 +90,38 @@ test_expect_success 'cherry-pick persists opts correctly' '
test_cmp expect actual
'
test_expect_success 'cherry-pick persists --empty=stop correctly' '
pristine_detach yetanotherpick &&
# Picking `anotherpick` forces a conflict so that we stop. That
# commit is then skipped, after which we pick `yetanotherpick`
# while already on `yetanotherpick` to cause an empty commit
test_must_fail git cherry-pick --empty=stop anotherpick yetanotherpick &&
test_must_fail git cherry-pick --skip 2>msg &&
test_grep "The previous cherry-pick is now empty" msg &&
rm msg &&
git cherry-pick --abort
'
test_expect_success 'cherry-pick persists --empty=drop correctly' '
pristine_detach yetanotherpick &&
# Picking `anotherpick` forces a conflict so that we stop. That
# commit is then skipped, after which we pick `yetanotherpick`
# while already on `yetanotherpick` to cause an empty commit
test_must_fail git cherry-pick --empty=drop anotherpick yetanotherpick &&
git cherry-pick --skip &&
test_cmp_rev yetanotherpick HEAD
'
test_expect_success 'cherry-pick persists --empty=keep correctly' '
pristine_detach yetanotherpick &&
# Picking `anotherpick` forces a conflict so that we stop. That
# commit is then skipped, after which we pick `yetanotherpick`
# while already on `yetanotherpick` to cause an empty commit
test_must_fail git cherry-pick --empty=keep anotherpick yetanotherpick &&
git cherry-pick --skip &&
test_cmp_rev yetanotherpick HEAD^
'
test_expect_success 'revert persists opts correctly' '
pristine_detach initial &&
# to make sure that the session to revert a sequence