When we allow mixing "revert" and "pick" instructions in the same sheet in the next patch, the following workflow would be perfectly valid: $ git cherry-pick base..latercommit [conflict occurs] $ edit problematicfile $ git add problematicfile $ git revert --continue [finishes successfully] This is confusing to the operator, because the sequencer is an implementation detail hidden behind the 'git cherry-pick' and 'git revert' builtins. So, disallow this workflow by keeping track of the builtin command executed (either "revert" or "cherry-pick") in '.git/sequencer/cmd'. Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx> --- builtin/revert.c | 57 +++++++++++++++++++++++++++++++++++++++ sequencer.h | 1 + t/t3510-cherry-pick-sequence.sh | 11 +++++++ 3 files changed, 69 insertions(+), 0 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 3ac6da0..52fa115 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -871,6 +871,50 @@ static int create_seq_dir(void) return 0; } +static enum replay_command read_cmd(void) +{ + const char *cmd_file = git_path(SEQ_CMD_FILE); + struct strbuf buf = STRBUF_INIT; + enum replay_command res; + int fd; + + fd = open(cmd_file, O_RDONLY); + if (fd < 0) + die_errno(_("Could not open %s"), cmd_file); + if (strbuf_read(&buf, fd, 0) < 0) { + close(fd); + strbuf_release(&buf); + die(_("Could not read %s."), cmd_file); + } + close(fd); + + if (!strcmp(buf.buf, "revert\n")) + res = REPLAY_CMD_REVERT; + else if (!strcmp(buf.buf, "cherry-pick\n")) + res = REPLAY_CMD_CHERRY_PICK; + else { + strbuf_release(&buf); + die(_("Malformed command file: %s"), cmd_file); + } + strbuf_release(&buf); + return res; +} + +static void save_cmd(struct replay_opts *opts) +{ + const char *cmd_file = git_path(SEQ_CMD_FILE); + static struct lock_file cmd_lock; + struct strbuf buf = STRBUF_INIT; + int fd; + + fd = hold_lock_file_for_update(&cmd_lock, cmd_file, LOCK_DIE_ON_ERROR); + strbuf_addf(&buf, "%s\n", command_name(opts)); + if (write_in_full(fd, buf.buf, buf.len) < 0) + die_errno(_("Could not write to %s"), cmd_file); + if (commit_lock_file(&cmd_lock) < 0) + die(_("Error wrapping up %s."), cmd_file); +} + static void save_head(const char *head) { const char *head_file = git_path(SEQ_HEAD_FILE); @@ -1043,9 +1087,21 @@ static int continue_single_pick(void) static int sequencer_continue(struct replay_opts *opts) { struct commit_list *todo_list = NULL; + enum replay_command cmd; if (!file_exists(git_path(SEQ_TODO_FILE))) return continue_single_pick(); + + /* + * Disallow continuing a cherry-pick with 'git revert + * --continue' and viceversa + */ + cmd = read_cmd(); + if (cmd != opts->command) + return error(_("cannot %s: a %s is in progress."), + command_name(opts), + cmd == REPLAY_CMD_REVERT ? "revert" : "cherry-pick"); + read_populate_opts(&opts); read_populate_todo(&todo_list, opts); @@ -1126,6 +1182,7 @@ static int pick_revisions(struct replay_opts *opts) return error(_("Can't revert as initial commit")); return error(_("Can't cherry-pick into empty head")); } + save_cmd(opts); save_head(sha1_to_hex(sha1)); save_opts(opts); return pick_commits(todo_list, opts); diff --git a/sequencer.h b/sequencer.h index 07e0639..00ab685 100644 --- a/sequencer.h +++ b/sequencer.h @@ -2,6 +2,7 @@ #define SEQUENCER_H #define SEQ_DIR "sequencer" +#define SEQ_CMD_FILE "sequencer/cmd" #define SEQ_HEAD_FILE "sequencer/head" #define SEQ_TODO_FILE "sequencer/todo" #define SEQ_OPTS_FILE "sequencer/opts" diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index 97f3710..73298cf 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -48,6 +48,7 @@ test_expect_success 'cherry-pick persists data on failure' ' pristine_detach initial && test_expect_code 1 git cherry-pick -s base..anotherpick && test_path_is_dir .git/sequencer && + test_path_is_file .git/sequencer/cmd && test_path_is_file .git/sequencer/head && test_path_is_file .git/sequencer/todo && test_path_is_file .git/sequencer/opts @@ -69,6 +70,7 @@ test_expect_success 'cherry-pick persists opts correctly' ' pristine_detach initial && test_expect_code 128 git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours initial..anotherpick && test_path_is_dir .git/sequencer && + test_path_is_file .git/sequencer/cmd && test_path_is_file .git/sequencer/head && test_path_is_file .git/sequencer/todo && test_path_is_file .git/sequencer/opts && @@ -517,4 +519,13 @@ test_expect_success 'commit descriptions in insn sheet are optional' ' test_line_count = 4 commits ' +test_expect_success 'revert --continue refuses to follow cherry-pick' ' + pristine_detach initial && + test_expect_code 1 git cherry-pick base..anotherpick && + echo "c" >foo && + git add foo && + git commit && + test_expect_code 128 git revert --continue +' + test_done -- 1.7.8.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html