Giuseppe BilottaThis allows cherry-picking a set of commits, some of which may be redundant, without stopping to ask for the user intervention.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com> --- Documentation/git-cherry-pick.txt | 4 ++++ builtin/revert.c | 1 + sequencer.c | 45 +++++++++++++++++++++++++++++++-------- sequencer.h | 1 + 4 files changed, 42 insertions(+), 9 deletions(-)
I would like to add to cherry-pick the ability to skip patches. To this end, I'm working on two options: a general `--skip-empty` option to handle redundant and empty commits by simply skipping them (no user intervention), and a `--skip` option as an alternative form to `--continue` to skip the ongoing (conflicting or empty) commit.
The patch here presents my implementation of the `--skip-empty` option, including documentation. Comments welcome.
However, it has been some time since I dwelved into git internals and I'm having issues with the `--skip` option. My idea was to rely on the existing implementation for `--continue`, and have it run the rollback instead of the commit. However, I'm finding that after the rollback is done (I'm using the existing rollback function in sequencer, which calls `git reset --merge`, but I've also tried a `reset --hard`) the sequencer does not continue because e.g. if the cherry-pick had stopped due to a conflict, it still finds unmerged paths, and I can't find a way to tell the sequencer to just re-read the index from scratch. Opinions welcome (I can provide my WIP patch to make discussion easier).
On the same note, I've noticed that `git reset` clears CHERRY_PICK_HEAD. I find this a little confusing, both for humans for the code itself (since it doesn't really abort an ongoing cherry-pick, yet the sequencer will think there is no running cherry-pick). This is particularly bad because `git reset` is one of the strategies proposed to solve conflicts in cherry-picking. So I was wondering if this was intentional or a side effect of something else that I might look into cleaning up during this patch series.
Show changes to 4 files +42 −9
Documentation/git-cherry-pick.txt, builtin/revert.c, sequencer.c, sequencer.h
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt index d35d771fc8..ffced816d6 100644 --- a/Documentation/git-cherry-pick.txt +++ b/Documentation/git-cherry-pick.txt @@ -138,6 +138,10 @@ effect to your index in a row. examine the commit. This option overrides that behavior and creates an empty commit object. Implies `--allow-empty`. +--skip-empty:: + This option causes empty and redundant cherry-picked commits to + be skipped without requesting the user intervention. + --strategy=<strategy>:: Use the given merge strategy. Should only be used once. See the MERGE STRATEGIES section in linkgit:git-merge[1] diff --git a/builtin/revert.c b/builtin/revert.c index 4ca5b51544..ffdd367f99 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -102,6 +102,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts) 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, "skip-empty", &opts->skip_empty, N_("skip redundant, empty commits")), OPT_END(), }; options = parse_options_concat(options, cp_extra); diff --git a/sequencer.c b/sequencer.c index 9adb7bbf1d..72962fd2fa 100644 --- a/sequencer.c +++ b/sequencer.c @@ -550,22 +550,32 @@ static int is_original_commit_empty(struct commit *commit) /* * Do we run "git commit" with "--allow-empty"? + * + * Or do we just skip this empty commit? + * + * Returns 1 if a commit should be done with --allow-empty, + * 0 if a commit should be done without --allow-empty, + * 2 if no commit should be done at all (skip empty commit) + * negative values in case of error + * */ -static int allow_empty(struct replay_opts *opts, struct commit *commit) +static int allow_or_skip_empty(struct replay_opts *opts, struct commit *commit) { int index_unchanged, empty_commit; /* - * Three cases: + * Four cases: * - * (1) we do not allow empty at all and error out. + * (1) we do not allow empty at all and error out; * - * (2) we allow ones that were initially empty, but + * (2) we skip empty commits altogether; + * + * (3) we allow ones that were initially empty, but * forbid the ones that become empty; * - * (3) we allow both. + * (4) we allow both. */ - if (!opts->allow_empty) + if (!opts->allow_empty && !opts->skip_empty) return 0; /* let "git commit" barf as necessary */ index_unchanged = is_index_unchanged(); @@ -574,6 +584,9 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit) if (!index_unchanged) return 0; /* we do not have to say --allow-empty */ + if (opts->skip_empty) + return 2; + if (opts->keep_redundant_commits) return 1; @@ -612,7 +625,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, const char *base_label, *next_label; struct commit_message msg = { NULL, NULL, NULL, NULL }; struct strbuf msgbuf = STRBUF_INIT; - int res, unborn = 0, allow; + int res = 0, unborn = 0, allow; if (opts->no_commit) { /* @@ -771,12 +784,13 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, goto leave; } - allow = allow_empty(opts, commit); + allow = allow_or_skip_empty(opts, commit); if (allow < 0) { res = allow; goto leave; } - if (!opts->no_commit) + /* allow == 2 means skip this commit */ + if (allow != 2 && !opts->no_commit) res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(), opts, allow, opts->edit, 0, 0); @@ -983,6 +997,8 @@ static int populate_opts_cb(const char *key, const char *value, void *data) opts->signoff = git_config_bool_or_int(key, value, &error_flag); else if (!strcmp(key, "options.record-origin")) opts->record_origin = git_config_bool_or_int(key, value, &error_flag); + else if (!strcmp(key, "options.skip-empty")) + opts->skip_empty = git_config_bool_or_int(key, value, &error_flag); else if (!strcmp(key, "options.allow-ff")) opts->allow_ff = git_config_bool_or_int(key, value, &error_flag); else if (!strcmp(key, "options.mainline")) @@ -1231,6 +1247,8 @@ static int save_opts(struct replay_opts *opts) res |= git_config_set_in_file_gently(opts_file, "options.signoff", "true"); if (opts->record_origin) res |= git_config_set_in_file_gently(opts_file, "options.record-origin", "true"); + if (opts->skip_empty) + res |= git_config_set_in_file_gently(opts_file, "options.skip-empty", "true"); if (opts->allow_ff) res |= git_config_set_in_file_gently(opts_file, "options.allow-ff", "true"); if (opts->mainline) { @@ -1306,6 +1324,14 @@ int sequencer_continue(struct replay_opts *opts) if ((res = read_populate_todo(&todo_list, opts))) goto release_todo_list; + /* check if there is something to commit */ + res = is_index_unchanged(); + if (res < 0) + goto release_todo_list; + + if (res && opts->skip_empty) + goto skip_this_commit; + /* Verify that the conflict has been resolved */ if (file_exists(git_path_cherry_pick_head()) || file_exists(git_path_revert_head())) { @@ -1317,6 +1343,7 @@ int sequencer_continue(struct replay_opts *opts) res = error_dirty_index(opts); goto release_todo_list; } +skip_this_commit: todo_list.current++; res = pick_commits(&todo_list, opts); release_todo_list: diff --git a/sequencer.h b/sequencer.h index 7a513c576b..c747cfcfc7 100644 --- a/sequencer.h +++ b/sequencer.h @@ -23,6 +23,7 @@ struct replay_opts { int allow_empty; int allow_empty_message; int keep_redundant_commits; + int skip_empty; int mainline;
-- 2.11.0.585.g56041942c3.dirty
