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 e98c4269c8 (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 7c096b8d61 (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. 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@xxxxxxxxx> --- Documentation/git-cherry-pick.txt | 32 +++++++++++------ builtin/revert.c | 37 ++++++++++++++++++- sequencer.c | 6 ++++ t/t3505-cherry-pick-empty.sh | 23 +++++++++++- t/t3510-cherry-pick-sequence.sh | 40 +++++++++++++++++++++ t/t3515-cherry-pick-incompatible-options.sh | 14 ++++++++ 6 files changed, 140 insertions(+), 12 deletions(-) diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index c88bb88822..a444d960b1 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -132,23 +132,35 @@ 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|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. +`stop`;; + The cherry-pick will stop when the commit is applied, allowing + you to examine the commit. This is the default behavior. +-- ++ +Note that this option species 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 cause the cherry-pick to fail. To force the inclusion of +those commits, use `--allow-empty`. ++ + --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. diff --git a/builtin/revert.c b/builtin/revert.c index 48c426f277..27efb6284b 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -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 = 0, + 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); + } + if (cleanup_arg) { opts->default_msg_cleanup = get_cleanup_mode(cleanup_arg, 1); opts->explicit_cleanup = 1; @@ -164,6 +198,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix, "--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); } diff --git a/sequencer.c b/sequencer.c index 3f41863dae..509a5244d2 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2921,6 +2921,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); @@ -3465,6 +3468,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"); diff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh index 2709cfc677..669416c158 100755 --- a/t/t3505-cherry-pick-empty.sh +++ b/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-redundant nor --empty' ' git reset --hard && git checkout fork^0 && test_must_fail git cherry-pick main @@ -105,4 +105,25 @@ 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=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 diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index 72020a51c4..5f6c45dfe3 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -90,6 +90,46 @@ test_expect_success 'cherry-pick persists opts correctly' ' test_cmp expect actual ' +test_expect_success 'cherry-pick persists --empty=stop correctly' ' + pristine_detach initial && + # to make sure that the session to cherry-pick a sequence + # gets interrupted, use a high-enough number that is larger + # than the number of parents of any commit we have created + mainline=4 && + test_expect_code 128 git cherry-pick -s -m $mainline --empty=stop initial..anotherpick && + test_path_is_file .git/sequencer/opts && + test_must_fail git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits && + test_must_fail git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits +' + +test_expect_success 'cherry-pick persists --empty=drop correctly' ' + pristine_detach initial && + # to make sure that the session to cherry-pick a sequence + # gets interrupted, use a high-enough number that is larger + # than the number of parents of any commit we have created + mainline=4 && + test_expect_code 128 git cherry-pick -s -m $mainline --empty=drop initial..anotherpick && + test_path_is_file .git/sequencer/opts && + test_must_fail git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits && + echo "true" >expect && + git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits >actual && + test_cmp expect actual +' + +test_expect_success 'cherry-pick persists --empty=keep correctly' ' + pristine_detach initial && + # to make sure that the session to cherry-pick a sequence + # gets interrupted, use a high-enough number that is larger + # than the number of parents of any commit we have created + mainline=4 && + test_expect_code 128 git cherry-pick -s -m $mainline --empty=keep initial..anotherpick && + test_path_is_file .git/sequencer/opts && + echo "true" >expect && + git config --file=.git/sequencer/opts --get-all options.keep-redundant-commits >actual && + test_cmp expect actual && + test_must_fail git config --file=.git/sequencer/opts --get-all options.drop-redundant-commits +' + test_expect_success 'revert persists opts correctly' ' pristine_detach initial && # to make sure that the session to revert a sequence diff --git a/t/t3515-cherry-pick-incompatible-options.sh b/t/t3515-cherry-pick-incompatible-options.sh index 6100ab64fd..b2780fdbf3 100755 --- a/t/t3515-cherry-pick-incompatible-options.sh +++ b/t/t3515-cherry-pick-incompatible-options.sh @@ -31,4 +31,18 @@ test_expect_success '--keep-redundant-commits is incompatible with operations' ' 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_done -- 2.43.0