From 1df9bf46d656970d0db254cb7faab0d0505802e0 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 10 Dec 2011 06:47:36 -0600 Subject: revert: give --continue handling its own function This makes pick_revisions() a little shorter and easier to read straight through. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 1ea525c10..9f6c85c1a 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -1038,6 +1038,21 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) return 0; } +static int sequencer_continue(struct replay_opts *opts) +{ + struct commit_list *todo_list = NULL; + + if (!file_exists(git_path(SEQ_TODO_FILE))) + return error(_("No %s in progress"), action_name(opts)); + read_populate_opts(&opts); + read_populate_todo(&todo_list, opts); + + /* Verify that the conflict has been resolved */ + if (!index_differs_from("HEAD", 0)) + todo_list = todo_list->next; + return pick_commits(todo_list, opts); +} + static int pick_revisions(struct replay_opts *opts) { struct commit_list *todo_list = NULL; @@ -1056,17 +1071,8 @@ static int pick_revisions(struct replay_opts *opts) } if (opts->subcommand == REPLAY_ROLLBACK) return sequencer_rollback(opts); - if (opts->subcommand == REPLAY_CONTINUE) { - if (!file_exists(git_path(SEQ_TODO_FILE))) - return error(_("No %s in progress"), action_name(opts)); - read_populate_opts(&opts); - read_populate_todo(&todo_list, opts); - - /* Verify that the conflict has been resolved */ - if (!index_differs_from("HEAD", 0)) - todo_list = todo_list->next; - return pick_commits(todo_list, opts); - } + if (opts->subcommand == REPLAY_CONTINUE) + return sequencer_continue(opts); /* * Start a new cherry-pick/ revert sequence; but -- cgit v1.2.1 From 093a309136c38eca0ea2dd5da3c68b483443d113 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 10 Dec 2011 06:49:25 -0600 Subject: revert: allow cherry-pick --continue to commit before resuming When "git cherry-pick ..bar" encounters conflicts, permit the operator to use cherry-pick --continue after resolving them as a shortcut for "git commit && git cherry-pick --continue" to record the resolution and carry on with the rest of the sequence. This improves the analogy with "git rebase" (in olden days --continue was the way to preserve authorship when a rebase encountered conflicts) and fits well with a general UI goal of making "git cmd --continue" save humans the trouble of deciding what to do next. Example: after encountering a conflict from running "git cherry-pick foo bar baz": CONFLICT (content): Merge conflict in main.c error: could not apply f78a8d98c... bar! hint: after resolving the conflicts, mark the corrected paths hint: with 'git add ' or 'git rm ' hint: and commit the result with 'git commit' We edit main.c to resolve the conflict, mark it acceptable with "git add main.c", and can run "cherry-pick --continue" to resume the sequence. $ git cherry-pick --continue [editor opens to confirm commit message] [master 78c8a8c98] bar! 1 files changed, 1 insertions(+), 1 deletions(-) [master 87ca8798c] baz! 1 files changed, 1 insertions(+), 1 deletions(-) This is done for both codepaths to pick multiple commits and a single commit. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 9f6c85c1a..a43b4d85f 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -1038,18 +1038,35 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) return 0; } +static int continue_single_pick(void) +{ + const char *argv[] = { "commit", NULL }; + + if (!file_exists(git_path("CHERRY_PICK_HEAD")) && + !file_exists(git_path("REVERT_HEAD"))) + return error(_("no cherry-pick or revert in progress")); + return run_command_v_opt(argv, RUN_GIT_CMD); +} + static int sequencer_continue(struct replay_opts *opts) { struct commit_list *todo_list = NULL; if (!file_exists(git_path(SEQ_TODO_FILE))) - return error(_("No %s in progress"), action_name(opts)); + return continue_single_pick(); read_populate_opts(&opts); read_populate_todo(&todo_list, opts); /* Verify that the conflict has been resolved */ - if (!index_differs_from("HEAD", 0)) - todo_list = todo_list->next; + if (file_exists(git_path("CHERRY_PICK_HEAD")) || + file_exists(git_path("REVERT_HEAD"))) { + int ret = continue_single_pick(); + if (ret) + return ret; + } + if (index_differs_from("HEAD", 0)) + return error_dirty_index(opts); + todo_list = todo_list->next; return pick_commits(todo_list, opts); } -- cgit v1.2.1 From 7f13334e074bb053eccd14787e416306bc4b413a Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 13 Aug 2011 12:06:23 -0500 Subject: revert: pass around rev-list args in already-parsed form Since 7e2bfd3f (revert: allow cherry-picking more than one commit, 2010-07-02), the pick/revert machinery has kept track of the set of commits to be cherry-picked or reverted using commit_argc and commit_argv variables, storing the corresponding command-line parameters. Future callers as other commands are built in (am, rebase, sequencer) may find it easier to pass rev-list options to this machinery in already-parsed form. Teach cmd_cherry_pick and cmd_revert to parse the rev-list arguments in advance and pass the commit set to pick_revisions() as a rev_info structure. Original patch by Jonathan, tweaks and test from Ram. Signed-off-by: Jonathan Nieder Improved-by: Ramkumar Ramachandra Signed-off-by: Junio C Hamano --- builtin/revert.c | 53 +++++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 24 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index a43b4d85f..f7f4bb357 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -60,13 +60,14 @@ struct replay_opts { int allow_rerere_auto; int mainline; - int commit_argc; - const char **commit_argv; /* Merge strategy */ const char *strategy; const char **xopts; size_t xopts_nr, xopts_alloc; + + /* Only used by REPLAY_NONE */ + struct rev_info *revs; }; #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -169,9 +170,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) die(_("program error")); } - opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str, - PARSE_OPT_KEEP_ARGV0 | - PARSE_OPT_KEEP_UNKNOWN); + argc = parse_options(argc, argv, NULL, options, usage_str, + PARSE_OPT_KEEP_ARGV0 | + PARSE_OPT_KEEP_UNKNOWN); /* Check for incompatible subcommands */ verify_opt_mutually_compatible(me, @@ -213,9 +214,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) NULL); } - else if (opts->commit_argc < 2) - usage_with_options(usage_str, options); - if (opts->allow_ff) verify_opt_compatible(me, "--ff", "--signoff", opts->signoff, @@ -223,7 +221,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts) "-x", opts->record_origin, "--edit", opts->edit, NULL); - opts->commit_argv = argv; + + if (opts->subcommand != REPLAY_NONE) { + opts->revs = NULL; + } else { + opts->revs = xmalloc(sizeof(*opts->revs)); + init_revisions(opts->revs, NULL); + opts->revs->no_walk = 1; + if (argc < 2) + usage_with_options(usage_str, options); + argc = setup_revisions(argc, argv, opts->revs, NULL); + } + + if (argc > 1) + usage_with_options(usage_str, options); } struct commit_message { @@ -631,23 +642,15 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts) return res; } -static void prepare_revs(struct rev_info *revs, struct replay_opts *opts) +static void prepare_revs(struct replay_opts *opts) { - int argc; - - init_revisions(revs, NULL); - revs->no_walk = 1; if (opts->action != REVERT) - revs->reverse = 1; + opts->revs->reverse ^= 1; - argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL); - if (argc > 1) - usage(*revert_or_cherry_pick_usage(opts)); - - if (prepare_revision_walk(revs)) + if (prepare_revision_walk(opts->revs)) die(_("revision walk setup failed")); - if (!revs->commits) + if (!opts->revs->commits) die(_("empty commit set passed")); } @@ -844,14 +847,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr) static void walk_revs_populate_todo(struct commit_list **todo_list, struct replay_opts *opts) { - struct rev_info revs; struct commit *commit; struct commit_list **next; - prepare_revs(&revs, opts); + prepare_revs(opts); next = todo_list; - while ((commit = get_revision(&revs))) + while ((commit = get_revision(opts->revs))) next = commit_list_append(commit, next); } @@ -1075,6 +1077,9 @@ static int pick_revisions(struct replay_opts *opts) struct commit_list *todo_list = NULL; unsigned char sha1[20]; + if (opts->subcommand == REPLAY_NONE) + assert(opts->revs); + read_and_refresh_cache(opts); /* -- cgit v1.2.1 From 7acaaac275a1d338f7b2540779b7ea60f3f0667c Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 10 Dec 2011 06:59:48 -0600 Subject: revert: allow single-pick in the middle of cherry-pick sequence After messing up a difficult conflict resolution in the middle of a cherry-pick sequence, it can be useful to be able to git checkout HEAD . && git cherry-pick that-one-commit to restart the conflict resolution. The current code however errors out saying that another cherry-pick is already in progress. Suggested-by: Johannes Sixt Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index f7f4bb357..5785ff994 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -1072,6 +1072,12 @@ static int sequencer_continue(struct replay_opts *opts) return pick_commits(todo_list, opts); } +static int single_pick(struct commit *cmit, struct replay_opts *opts) +{ + setenv(GIT_REFLOG_ACTION, action_name(opts), 0); + return do_pick_commit(cmit, opts); +} + static int pick_revisions(struct replay_opts *opts) { struct commit_list *todo_list = NULL; @@ -1096,6 +1102,26 @@ static int pick_revisions(struct replay_opts *opts) if (opts->subcommand == REPLAY_CONTINUE) return sequencer_continue(opts); + /* + * If we were called as "git cherry-pick ", just + * cherry-pick/revert it, set CHERRY_PICK_HEAD / + * REVERT_HEAD, and don't touch the sequencer state. + * This means it is possible to cherry-pick in the middle + * of a cherry-pick sequence. + */ + if (opts->revs->cmdline.nr == 1 && + opts->revs->cmdline.rev->whence == REV_CMD_REV && + opts->revs->no_walk && + !opts->revs->cmdline.rev->flags) { + struct commit *cmit; + if (prepare_revision_walk(opts->revs)) + die(_("revision walk setup failed")); + cmit = get_revision(opts->revs); + if (!cmit || get_revision(opts->revs)) + die("BUG: expected exactly one commit from walk"); + return single_pick(cmit, opts); + } + /* * Start a new cherry-pick/ revert sequence; but * first, make sure that an existing one isn't in -- cgit v1.2.1 From 218b65fbf9428517e739b8bc26680c29910cf1cd Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 10 Dec 2011 07:02:12 -0600 Subject: revert: do not remove state until sequence is finished As v1.7.8-rc0~141^2~4 (2011-08-04) explains, git cherry-pick removes the sequencer state just before applying the final patch. In the single-pick case, that was a good thing, since --abort and --continue work fine without access to such state and removing it provides a signal that git should not complain about the need to clobber it ("a cherry-pick or revert is already in progress") in sequences like the following: git cherry-pick foo git read-tree -m -u HEAD; # forget that; let's try a different one git cherry-pick bar After the recent patch "allow single-pick in the middle of cherry-pick sequence" we don't need that hack any more. In the new regime, a traditional "git cherry-pick " command never looks at .git/sequencer, so we do not need to cripple "git cherry-pick .." for it any more. So now you can run "git cherry-pick --abort" near the end of a multi-pick sequence and it will abort the entire sequence, instead of misbehaving and aborting just the final commit. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 5785ff994..5dcfa6ba6 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -1018,18 +1018,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) for (cur = todo_list; cur; cur = cur->next) { save_todo(cur, opts); res = do_pick_commit(cur->item, opts); - if (res) { - if (!cur->next) - /* - * An error was encountered while - * picking the last commit; the - * sequencer state is useless now -- - * the user simply needs to resolve - * the conflict and commit - */ - remove_sequencer_state(0); + if (res) return res; - } } /* -- cgit v1.2.1 From d596118d7a9b104db10e64b2680a30ea80f1439c Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 10 Dec 2011 07:06:12 -0600 Subject: revert: stop creating and removing sequencer-old directory Now that "git reset" no longer implicitly removes .git/sequencer that the operator may or may not have wanted to keep, the logic to write a backup copy of .git/sequencer and remove it when stale is not needed any more. Simplify the sequencer API and repository layout by dropping it. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 5dcfa6ba6..028bcbcd7 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -944,7 +944,7 @@ static int sequencer_rollback(struct replay_opts *opts) } if (reset_for_rollback(sha1)) goto fail; - remove_sequencer_state(1); + remove_sequencer_state(); strbuf_release(&buf); return 0; fail: @@ -1026,7 +1026,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts) * Sequence of picks finished successfully; cleanup by * removing the .git/sequencer directory */ - remove_sequencer_state(1); + remove_sequencer_state(); return 0; } @@ -1084,7 +1084,7 @@ static int pick_revisions(struct replay_opts *opts) * one that is being continued */ if (opts->subcommand == REPLAY_REMOVE_STATE) { - remove_sequencer_state(1); + remove_sequencer_state(); return 0; } if (opts->subcommand == REPLAY_ROLLBACK) -- cgit v1.2.1