* [PATCH 0/7] New sequencer workflow!
@ 2011-11-13 10:46 Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 1/7] sequencer: factor code out of revert builtin Ramkumar Ramachandra
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
Hi,
After days of bending my mind around the backward-compatibility
problem, I'm happy to announce that we finally have an implementation!
This series preserves the old workflow:
$ git cherry-pick foo
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
$ git cherry-pick foo..bar
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
$ git revert --continue # No, there are no typos
And introduces a brand new workflow:
$ git cherry-pick foo
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
$ git cherry-pick foo..bar
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
$ git revert moo..baz
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
In other words, it just doesn't matter how you started what sequencing
operation: you can always continue a it with "git sequencer
--continue" and reset it with "git sequencer --reset".
This series is reasonably well-polished and based on
rr/revert-cherry-pick (since the topic hasn't made it to 'master'
yet). The only downside is that it's very complicated, and I've tried
my best to explain the motivation for everything in the commit
messages. Needless to say, it requires a huge amount of testing to
catch unpleasant corner cases.
Part 1 moves a lot of code from builtin/revert.c to sequencer.c.
Parts 2, 3, 4 carefully work around some complex issues related to
'.git/CHERRY_PICK_HEAD'.
Part 5 introduces a new git-sequencer builtin. Although it's
empty'ish now, I'd argue that adding it now helps think about the
unified interface. Any suggestions on what concrete function it
should implement? I'm thinking of something along the lines of
'rebase -i'.
Part 6 is the patch that tells the sequencer "how to continue". As
more commands grow sequencer functionalities, it will become clear
that committing isn't the only way to continue, but I think it's good
for the moment.
Part 7 is the icing on the cake. Enjoy.
I'd sent out $gmane/184859 last week as a sanity check: thanks to
Junio and Jonathan for looking at it. It's always comforting to know
that I haven't lost my head yet :)
If you're looking to get involved before this series hits 'pu', use
the "sequencer" branch of my Git fork on Github:
http://github.com/artagnon/git
Thanks.
-- Ram
Ramkumar Ramachandra (7):
sequencer: factor code out of revert builtin
sequencer: invalidate sequencer state without todo
sequencer: handle single-commit pick as special case
sequencer: handle cherry-pick conflict in last commit
sequencer: introduce git-sequencer builtin
sequencer: teach '--continue' how to commit
sequencer: teach parser about CHERRY_PICK_HEAD
.gitignore | 1 +
Documentation/git-sequencer.txt | 33 +
Makefile | 1 +
builtin.h | 1 +
builtin/revert.c | 821 +-------------------
builtin/sequencer.c | 52 ++
git.c | 1 +
sequencer.c | 842 +++++++++++++++++++-
sequencer.h | 26 +
...-cherry-pick-sequence.sh => t3510-sequencer.sh} | 140 +---
...-sequence.sh => t3511-cherry-pick-sequencer.sh} | 124 +---
11 files changed, 1002 insertions(+), 1040 deletions(-)
create mode 100644 Documentation/git-sequencer.txt
create mode 100644 builtin/sequencer.c
copy t/{t3510-cherry-pick-sequence.sh => t3510-sequencer.sh} (60%)
rename t/{t3510-cherry-pick-sequence.sh => t3511-cherry-pick-sequencer.sh} (59%)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/7] sequencer: factor code out of revert builtin
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
@ 2011-11-13 10:46 ` Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 2/7] sequencer: invalidate sequencer state without todo Ramkumar Ramachandra
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
To build a generalized sequencer of which cherry-picking and reverting
are special cases, we must first expose the cherry-picking machinery
through a public API. Move code from revert.c into sequencer.c so as
to expose pick_revisions() as sequencer_pick_revisions() in
sequencer.h. Consequently, make the cherry-pick builtin a thin
wrapper around sequencer_pick_revisions() that additionally does
command-line argument parsing. In the future, we can write a new
"foo" builtin that calls into the sequencer like:
memset(&opts, 0, sizeof(opts));
opts.action = REPLAY_FOO;
opts.revisions = xmalloc(sizeof(*opts.revs));
parse_args_populate_opts(argc, argv, &opts);
init_revisions(opts.revs);
sequencer_pick_revisions(&opts);
This is intended to be almost a pure code movement patch with no
functional changes. Check with:
$ git blame -s -CCC HEAD^..HEAD -- sequencer.c | grep -C3 '^[^^]'
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 821 +-----------------------------------------------------
sequencer.c | 802 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
sequencer.h | 26 ++
3 files changed, 828 insertions(+), 821 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index df9459b..c272920 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,19 +1,9 @@
#include "cache.h"
#include "builtin.h"
-#include "object.h"
-#include "commit.h"
-#include "tag.h"
-#include "run-command.h"
-#include "exec_cmd.h"
-#include "utf8.h"
#include "parse-options.h"
-#include "cache-tree.h"
#include "diff.h"
#include "revision.h"
#include "rerere.h"
-#include "merge-recursive.h"
-#include "refs.h"
-#include "dir.h"
#include "sequencer.h"
/*
@@ -39,40 +29,11 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
-
-struct replay_opts {
- enum replay_action action;
- enum replay_subcommand subcommand;
-
- /* Boolean options */
- int edit;
- int record_origin;
- int no_commit;
- int signoff;
- int allow_ff;
- int allow_rerere_auto;
-
- int mainline;
-
- /* 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"
-
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
}
-static char *get_encoding(const char *message);
-
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
@@ -222,784 +183,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
usage_with_options(usage_str, options);
}
-struct commit_message {
- char *parent_label;
- const char *label;
- const char *subject;
- char *reencoded_message;
- const char *message;
-};
-
-static int get_message(struct commit *commit, struct commit_message *out)
-{
- const char *encoding;
- const char *abbrev, *subject;
- int abbrev_len, subject_len;
- char *q;
-
- if (!commit->buffer)
- return -1;
- encoding = get_encoding(commit->buffer);
- if (!encoding)
- encoding = "UTF-8";
- if (!git_commit_encoding)
- git_commit_encoding = "UTF-8";
-
- out->reencoded_message = NULL;
- out->message = commit->buffer;
- if (strcmp(encoding, git_commit_encoding))
- out->reencoded_message = reencode_string(commit->buffer,
- git_commit_encoding, encoding);
- if (out->reencoded_message)
- out->message = out->reencoded_message;
-
- abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
- abbrev_len = strlen(abbrev);
-
- subject_len = find_commit_subject(out->message, &subject);
-
- out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
- strlen("... ") + subject_len + 1);
- q = out->parent_label;
- q = mempcpy(q, "parent of ", strlen("parent of "));
- out->label = q;
- q = mempcpy(q, abbrev, abbrev_len);
- q = mempcpy(q, "... ", strlen("... "));
- out->subject = q;
- q = mempcpy(q, subject, subject_len);
- *q = '\0';
- return 0;
-}
-
-static void free_message(struct commit_message *msg)
-{
- free(msg->parent_label);
- free(msg->reencoded_message);
-}
-
-static char *get_encoding(const char *message)
-{
- const char *p = message, *eol;
-
- while (*p && *p != '\n') {
- for (eol = p + 1; *eol && *eol != '\n'; eol++)
- ; /* do nothing */
- if (!prefixcmp(p, "encoding ")) {
- char *result = xmalloc(eol - 8 - p);
- strlcpy(result, p + 9, eol - 8 - p);
- return result;
- }
- p = eol;
- if (*p == '\n')
- p++;
- }
- return NULL;
-}
-
-static void write_cherry_pick_head(struct commit *commit)
-{
- int fd;
- struct strbuf buf = STRBUF_INIT;
-
- strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
-
- fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
- if (fd < 0)
- die_errno(_("Could not open '%s' for writing"),
- git_path("CHERRY_PICK_HEAD"));
- if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
- die_errno(_("Could not write to '%s'"), git_path("CHERRY_PICK_HEAD"));
- strbuf_release(&buf);
-}
-
-static void print_advice(int show_hint)
-{
- char *msg = getenv("GIT_CHERRY_PICK_HELP");
-
- if (msg) {
- fprintf(stderr, "%s\n", msg);
- /*
- * A conflict has occured but the porcelain
- * (typically rebase --interactive) wants to take care
- * of the commit itself so remove CHERRY_PICK_HEAD
- */
- unlink(git_path("CHERRY_PICK_HEAD"));
- return;
- }
-
- if (show_hint) {
- advise("after resolving the conflicts, mark the corrected paths");
- advise("with 'git add <paths>' or 'git rm <paths>'");
- advise("and commit the result with 'git commit'");
- }
-}
-
-static void write_message(struct strbuf *msgbuf, const char *filename)
-{
- static struct lock_file msg_file;
-
- int msg_fd = hold_lock_file_for_update(&msg_file, filename,
- LOCK_DIE_ON_ERROR);
- if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
- die_errno(_("Could not write to %s."), filename);
- strbuf_release(msgbuf);
- if (commit_lock_file(&msg_file) < 0)
- die(_("Error wrapping up %s"), filename);
-}
-
-static struct tree *empty_tree(void)
-{
- return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
-}
-
-static int error_dirty_index(struct replay_opts *opts)
-{
- if (read_cache_unmerged())
- return error_resolve_conflict(action_name(opts));
-
- /* Different translation strings for cherry-pick and revert */
- if (opts->action == REPLAY_PICK)
- error(_("Your local changes would be overwritten by cherry-pick."));
- else
- error(_("Your local changes would be overwritten by revert."));
-
- if (advice_commit_before_merge)
- advise(_("Commit your changes or stash them to proceed."));
- return -1;
-}
-
-static int fast_forward_to(const unsigned char *to, const unsigned char *from)
-{
- struct ref_lock *ref_lock;
-
- read_cache();
- if (checkout_fast_forward(from, to))
- exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update("HEAD", from, 0);
- return write_ref_sha1(ref_lock, to, "cherry-pick");
-}
-
-static int do_recursive_merge(struct commit *base, struct commit *next,
- const char *base_label, const char *next_label,
- unsigned char *head, struct strbuf *msgbuf,
- struct replay_opts *opts)
-{
- struct merge_options o;
- struct tree *result, *next_tree, *base_tree, *head_tree;
- int clean, index_fd;
- const char **xopt;
- static struct lock_file index_lock;
-
- index_fd = hold_locked_index(&index_lock, 1);
-
- read_cache();
-
- init_merge_options(&o);
- o.ancestor = base ? base_label : "(empty tree)";
- o.branch1 = "HEAD";
- o.branch2 = next ? next_label : "(empty tree)";
-
- head_tree = parse_tree_indirect(head);
- next_tree = next ? next->tree : empty_tree();
- base_tree = base ? base->tree : empty_tree();
-
- for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
- parse_merge_opt(&o, *xopt);
-
- clean = merge_trees(&o,
- head_tree,
- next_tree, base_tree, &result);
-
- if (active_cache_changed &&
- (write_cache(index_fd, active_cache, active_nr) ||
- commit_locked_index(&index_lock)))
- /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
- die(_("%s: Unable to write new index file"), action_name(opts));
- rollback_lock_file(&index_lock);
-
- if (!clean) {
- int i;
- strbuf_addstr(msgbuf, "\nConflicts:\n\n");
- for (i = 0; i < active_nr;) {
- struct cache_entry *ce = active_cache[i++];
- if (ce_stage(ce)) {
- strbuf_addch(msgbuf, '\t');
- strbuf_addstr(msgbuf, ce->name);
- strbuf_addch(msgbuf, '\n');
- while (i < active_nr && !strcmp(ce->name,
- active_cache[i]->name))
- i++;
- }
- }
- }
-
- return !clean;
-}
-
-/*
- * If we are cherry-pick, and if the merge did not result in
- * hand-editing, we will hit this commit and inherit the original
- * author date and name.
- * If we are revert, or if our cherry-pick results in a hand merge,
- * we had better say that the current user is responsible for that.
- */
-static int run_git_commit(const char *defmsg, struct replay_opts *opts)
-{
- /* 6 is max possible length of our args array including NULL */
- const char *args[6];
- int i = 0;
-
- args[i++] = "commit";
- args[i++] = "-n";
- if (opts->signoff)
- args[i++] = "-s";
- if (!opts->edit) {
- args[i++] = "-F";
- args[i++] = defmsg;
- }
- args[i] = NULL;
-
- return run_command_v_opt(args, RUN_GIT_CMD);
-}
-
-static int do_pick_commit(struct commit *commit, enum replay_action action,
- struct replay_opts *opts)
-{
- unsigned char head[20];
- struct commit *base, *next, *parent;
- const char *base_label, *next_label;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
- char *defmsg = NULL;
- struct strbuf msgbuf = STRBUF_INIT;
- int res;
-
- if (opts->no_commit) {
- /*
- * We do not intend to commit immediately. We just want to
- * merge the differences in, so let's compute the tree
- * that represents the "current" state for merge-recursive
- * to work on.
- */
- if (write_cache_as_tree(head, 0, NULL))
- die (_("Your index file is unmerged."));
- } else {
- if (get_sha1("HEAD", head))
- return error(_("You do not have a valid HEAD"));
- if (index_differs_from("HEAD", 0))
- return error_dirty_index(opts);
- }
- discard_cache();
-
- if (!commit->parents) {
- parent = NULL;
- }
- else if (commit->parents->next) {
- /* Reverting or cherry-picking a merge commit */
- int cnt;
- struct commit_list *p;
-
- if (!opts->mainline)
- return error(_("Commit %s is a merge but no -m option was given."),
- sha1_to_hex(commit->object.sha1));
-
- for (cnt = 1, p = commit->parents;
- cnt != opts->mainline && p;
- cnt++)
- p = p->next;
- if (cnt != opts->mainline || !p)
- return error(_("Commit %s does not have parent %d"),
- sha1_to_hex(commit->object.sha1), opts->mainline);
- parent = p->item;
- } else if (0 < opts->mainline)
- return error(_("Mainline was specified but commit %s is not a merge."),
- sha1_to_hex(commit->object.sha1));
- else
- parent = commit->parents->item;
-
- if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
- return fast_forward_to(commit->object.sha1, head);
-
- if (parent && parse_commit(parent) < 0)
- /* TRANSLATORS: The first %s will be "revert" or
- "cherry-pick", the second %s a SHA1 */
- return error(_("%s: cannot parse parent commit %s"),
- action_name(opts), sha1_to_hex(parent->object.sha1));
-
- if (get_message(commit, &msg) != 0)
- return error(_("Cannot get commit message for %s"),
- sha1_to_hex(commit->object.sha1));
-
- /*
- * "commit" is an existing commit. We would want to apply
- * the difference it introduces since its first parent "prev"
- * on top of the current HEAD if we are cherry-pick. Or the
- * reverse of it if we are revert.
- */
-
- defmsg = git_pathdup("MERGE_MSG");
-
- if (action == REPLAY_REVERT) {
- base = commit;
- base_label = msg.label;
- next = parent;
- next_label = msg.parent_label;
- strbuf_addstr(&msgbuf, "Revert \"");
- strbuf_addstr(&msgbuf, msg.subject);
- strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
-
- if (commit->parents && commit->parents->next) {
- strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
- strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
- }
- strbuf_addstr(&msgbuf, ".\n");
- } else {
- const char *p;
-
- base = parent;
- base_label = msg.parent_label;
- next = commit;
- next_label = msg.label;
-
- /*
- * Append the commit log message to msgbuf; it starts
- * after the tree, parent, author, committer
- * information followed by "\n\n".
- */
- p = strstr(msg.message, "\n\n");
- if (p) {
- p += 2;
- strbuf_addstr(&msgbuf, p);
- }
-
- if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
- strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
- strbuf_addstr(&msgbuf, ")\n");
- }
- }
-
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
- res = do_recursive_merge(base, next, base_label, next_label,
- head, &msgbuf, opts);
- write_message(&msgbuf, defmsg);
- } else {
- struct commit_list *common = NULL;
- struct commit_list *remotes = NULL;
-
- write_message(&msgbuf, defmsg);
-
- commit_list_insert(base, &common);
- commit_list_insert(next, &remotes);
- res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
- common, sha1_to_hex(head), remotes);
- free_commit_list(common);
- free_commit_list(remotes);
- }
-
- /*
- * If the merge was clean or if it failed due to conflict, we write
- * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
- * However, if the merge did not even start, then we don't want to
- * write it at all.
- */
- if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
- write_cherry_pick_head(commit);
-
- if (res) {
- error(action == REPLAY_REVERT
- ? _("could not revert %s... %s")
- : _("could not apply %s... %s"),
- find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
- msg.subject);
- print_advice(res == 1);
- rerere(opts->allow_rerere_auto);
- } else {
- if (!opts->no_commit)
- res = run_git_commit(defmsg, opts);
- }
-
- free_message(&msg);
- free(defmsg);
-
- return res;
-}
-
-static void prepare_revs(struct replay_opts *opts)
-{
- if (opts->action != REPLAY_REVERT)
- opts->revs->reverse ^= 1;
-
- if (prepare_revision_walk(opts->revs))
- die(_("revision walk setup failed"));
-
- if (!opts->revs->commits)
- die(_("empty commit set passed"));
-}
-
-static void read_and_refresh_cache(struct replay_opts *opts)
-{
- static struct lock_file index_lock;
- int index_fd = hold_locked_index(&index_lock, 0);
- if (read_index_preload(&the_index, NULL) < 0)
- die(_("git %s: failed to read the index"), action_name(opts));
- refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
- if (the_index.cache_changed) {
- if (write_index(&the_index, index_fd) ||
- commit_locked_index(&index_lock))
- die(_("git %s: failed to refresh the index"), action_name(opts));
- }
- rollback_lock_file(&index_lock);
-}
-
-/*
- * Append a commit to the end of the commit_list.
- *
- * next starts by pointing to the variable that holds the head of an
- * empty commit_list, and is updated to point to the "next" field of
- * the last item on the list as new commits are appended.
- *
- * Usage example:
- *
- * struct commit_list *list;
- * struct commit_list **next = &list;
- *
- * next = commit_list_append(c1, next);
- * next = commit_list_append(c2, next);
- * assert(commit_list_count(list) == 2);
- * return list;
- */
-static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
- struct commit *operand,
- struct replay_insn_list **next)
-{
- struct replay_insn_list *new = xmalloc(sizeof(*new));
- new->action = action;
- new->operand = operand;
- *next = new;
- new->next = NULL;
- return &new->next;
-}
-
-static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
-{
- struct replay_insn_list *cur;
-
- for (cur = todo_list; cur; cur = cur->next) {
- const char *sha1_abbrev, *action_str, *subject;
- int subject_len;
-
- action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
- sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
- subject_len = find_commit_subject(cur->operand->buffer, &subject);
- strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
- subject_len, subject);
- }
- return 0;
-}
-
-static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
-{
- unsigned char commit_sha1[20];
- char *end_of_object_name;
- int saved, status;
-
- if (!prefixcmp(bol, "pick ")) {
- item->action = REPLAY_PICK;
- bol += strlen("pick ");
- } else if (!prefixcmp(bol, "revert ")) {
- item->action = REPLAY_REVERT;
- bol += strlen("revert ");
- } else {
- size_t len = strchrnul(bol, '\n') - bol;
- if (len > 255)
- len = 255;
- return error(_("Unrecognized action: %.*s"), (int)len, bol);
- }
-
- end_of_object_name = bol + strcspn(bol, " \n");
- saved = *end_of_object_name;
- *end_of_object_name = '\0';
- status = get_sha1(bol, commit_sha1);
- *end_of_object_name = saved;
-
- if (status < 0)
- return error(_("Malformed object name: %s"), bol);
-
- item->operand = lookup_commit_reference(commit_sha1);
- if (!item->operand)
- return error(_("Not a valid commit: %s"), bol);
-
- item->next = NULL;
- return 0;
-}
-
-static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
-{
- struct replay_insn_list **next = todo_list;
- struct replay_insn_list item = {0, NULL, NULL};
- char *p = buf;
- int i;
-
- for (i = 1; *p; i++) {
- char *eol = strchrnul(p, '\n');
- if (parse_insn_line(p, eol, &item) < 0)
- return error(_("on line %d."), i);
- next = replay_insn_list_append(item.action, item.operand, next);
- p = *eol ? eol + 1 : eol;
- }
- if (!*todo_list)
- return error(_("No commits parsed."));
- return 0;
-}
-
-static void read_populate_todo(struct replay_insn_list **todo_list)
-{
- const char *todo_file = git_path(SEQ_TODO_FILE);
- struct strbuf buf = STRBUF_INIT;
- int fd, res;
-
- fd = open(todo_file, O_RDONLY);
- if (fd < 0)
- die_errno(_("Could not open %s."), todo_file);
- if (strbuf_read(&buf, fd, 0) < 0) {
- close(fd);
- strbuf_release(&buf);
- die(_("Could not read %s."), todo_file);
- }
- close(fd);
-
- res = parse_insn_buffer(buf.buf, todo_list);
- strbuf_release(&buf);
- if (res)
- die(_("Unusable instruction sheet: %s"), todo_file);
-}
-
-static int populate_opts_cb(const char *key, const char *value, void *data)
-{
- struct replay_opts *opts = data;
- int error_flag = 1;
-
- if (!value)
- error_flag = 0;
- else if (!strcmp(key, "options.no-commit"))
- opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.edit"))
- opts->edit = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.signoff"))
- 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.allow-ff"))
- opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
- else if (!strcmp(key, "options.mainline"))
- opts->mainline = git_config_int(key, value);
- else if (!strcmp(key, "options.strategy"))
- git_config_string(&opts->strategy, key, value);
- else if (!strcmp(key, "options.strategy-option")) {
- ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
- opts->xopts[opts->xopts_nr++] = xstrdup(value);
- } else
- return error(_("Invalid key: %s"), key);
-
- if (!error_flag)
- return error(_("Invalid value for %s: %s"), key, value);
-
- return 0;
-}
-
-static void read_populate_opts(struct replay_opts **opts_ptr)
-{
- const char *opts_file = git_path(SEQ_OPTS_FILE);
-
- if (!file_exists(opts_file))
- return;
- if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
- die(_("Malformed options sheet: %s"), opts_file);
-}
-
-static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
- struct replay_opts *opts)
-{
- struct commit *commit;
- struct replay_insn_list **next;
-
- prepare_revs(opts);
-
- next = todo_list;
- while ((commit = get_revision(opts->revs)))
- next = replay_insn_list_append(opts->action, commit, next);
-}
-
-static int create_seq_dir(void)
-{
- const char *seq_dir = git_path(SEQ_DIR);
-
- if (file_exists(seq_dir))
- return error(_("%s already exists."), seq_dir);
- else if (mkdir(seq_dir, 0777) < 0)
- die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
- return 0;
-}
-
-static void save_head(const char *head)
-{
- const char *head_file = git_path(SEQ_HEAD_FILE);
- static struct lock_file head_lock;
- struct strbuf buf = STRBUF_INIT;
- int fd;
-
- fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
- strbuf_addf(&buf, "%s\n", head);
- if (write_in_full(fd, buf.buf, buf.len) < 0)
- die_errno(_("Could not write to %s."), head_file);
- if (commit_lock_file(&head_lock) < 0)
- die(_("Error wrapping up %s."), head_file);
-}
-
-static void save_todo(struct replay_insn_list *todo_list)
-{
- const char *todo_file = git_path(SEQ_TODO_FILE);
- static struct lock_file todo_lock;
- struct strbuf buf = STRBUF_INIT;
- int fd;
-
- fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
- if (format_todo(&buf, todo_list) < 0)
- die(_("Could not format %s."), todo_file);
- if (write_in_full(fd, buf.buf, buf.len) < 0) {
- strbuf_release(&buf);
- die_errno(_("Could not write to %s."), todo_file);
- }
- if (commit_lock_file(&todo_lock) < 0) {
- strbuf_release(&buf);
- die(_("Error wrapping up %s."), todo_file);
- }
- strbuf_release(&buf);
-}
-
-static void save_opts(struct replay_opts *opts)
-{
- const char *opts_file = git_path(SEQ_OPTS_FILE);
-
- if (opts->no_commit)
- git_config_set_in_file(opts_file, "options.no-commit", "true");
- if (opts->edit)
- git_config_set_in_file(opts_file, "options.edit", "true");
- if (opts->signoff)
- git_config_set_in_file(opts_file, "options.signoff", "true");
- if (opts->record_origin)
- git_config_set_in_file(opts_file, "options.record-origin", "true");
- if (opts->allow_ff)
- git_config_set_in_file(opts_file, "options.allow-ff", "true");
- if (opts->mainline) {
- struct strbuf buf = STRBUF_INIT;
- strbuf_addf(&buf, "%d", opts->mainline);
- git_config_set_in_file(opts_file, "options.mainline", buf.buf);
- strbuf_release(&buf);
- }
- if (opts->strategy)
- git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
- if (opts->xopts) {
- int i;
- for (i = 0; i < opts->xopts_nr; i++)
- git_config_set_multivar_in_file(opts_file,
- "options.strategy-option",
- opts->xopts[i], "^$", 0);
- }
-}
-
-static int pick_commits(struct replay_insn_list *todo_list,
- struct replay_opts *opts)
-{
- struct replay_insn_list *cur;
- int res;
-
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
- if (opts->allow_ff)
- assert(!(opts->signoff || opts->no_commit ||
- opts->record_origin || opts->edit));
- read_and_refresh_cache(opts);
-
- for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur);
- res = do_pick_commit(cur->operand, cur->action, 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);
- return res;
- }
- }
-
- /*
- * Sequence of picks finished successfully; cleanup by
- * removing the .git/sequencer directory
- */
- remove_sequencer_state(1);
- return 0;
-}
-
-static int pick_revisions(struct replay_opts *opts)
-{
- struct replay_insn_list *todo_list = NULL;
- unsigned char sha1[20];
-
- if (opts->subcommand == REPLAY_NONE)
- assert(opts->revs);
-
- read_and_refresh_cache(opts);
-
- /*
- * Decide what to do depending on the arguments; a fresh
- * cherry-pick should be handled differently from an existing
- * one that is being continued
- */
- if (opts->subcommand == REPLAY_RESET) {
- remove_sequencer_state(1);
- return 0;
- } else if (opts->subcommand == REPLAY_CONTINUE) {
- if (!file_exists(git_path(SEQ_TODO_FILE)))
- goto error;
- read_populate_opts(&opts);
- read_populate_todo(&todo_list);
-
- /* Verify that the conflict has been resolved */
- if (!index_differs_from("HEAD", 0))
- todo_list = todo_list->next;
- } else {
- /*
- * Start a new cherry-pick/ revert sequence; but
- * first, make sure that an existing one isn't in
- * progress
- */
-
- walk_revs_populate_todo(&todo_list, opts);
- if (create_seq_dir() < 0) {
- error(_("A cherry-pick or revert is in progress."));
- advise(_("Use --continue to continue the operation"));
- advise(_("or --reset to forget about it"));
- return -1;
- }
- if (get_sha1("HEAD", sha1)) {
- if (opts->action == REPLAY_REVERT)
- return error(_("Can't revert as initial commit"));
- return error(_("Can't cherry-pick into empty head"));
- }
- save_head(sha1_to_hex(sha1));
- save_opts(opts);
- }
- return pick_commits(todo_list, opts);
-error:
- return error(_("No %s in progress"), action_name(opts));
-}
-
int cmd_revert(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts;
@@ -1011,7 +194,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("revert failed"));
return res;
@@ -1026,7 +209,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
- res = pick_revisions(&opts);
+ res = sequencer_pick_revisions(&opts);
if (res < 0)
die(_("cherry-pick failed"));
return res;
diff --git a/sequencer.c b/sequencer.c
index bc2c046..87f146b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,7 +1,27 @@
#include "cache.h"
-#include "sequencer.h"
-#include "strbuf.h"
+#include "object.h"
+#include "commit.h"
+#include "tag.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+#include "utf8.h"
+#include "cache-tree.h"
+#include "diff.h"
+#include "revision.h"
+#include "rerere.h"
+#include "merge-recursive.h"
+#include "refs.h"
#include "dir.h"
+#include "sequencer.h"
+
+#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
+
+static const char *action_name(const struct replay_opts *opts)
+{
+ return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+}
+
+static char *get_encoding(const char *message);
void remove_sequencer_state(int aggressive)
{
@@ -17,3 +37,781 @@ void remove_sequencer_state(int aggressive)
strbuf_release(&seq_dir);
strbuf_release(&seq_old_dir);
}
+
+struct commit_message {
+ char *parent_label;
+ const char *label;
+ const char *subject;
+ char *reencoded_message;
+ const char *message;
+};
+
+static int get_message(struct commit *commit, struct commit_message *out)
+{
+ const char *encoding;
+ const char *abbrev, *subject;
+ int abbrev_len, subject_len;
+ char *q;
+
+ if (!commit->buffer)
+ return -1;
+ encoding = get_encoding(commit->buffer);
+ if (!encoding)
+ encoding = "UTF-8";
+ if (!git_commit_encoding)
+ git_commit_encoding = "UTF-8";
+
+ out->reencoded_message = NULL;
+ out->message = commit->buffer;
+ if (strcmp(encoding, git_commit_encoding))
+ out->reencoded_message = reencode_string(commit->buffer,
+ git_commit_encoding, encoding);
+ if (out->reencoded_message)
+ out->message = out->reencoded_message;
+
+ abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+ abbrev_len = strlen(abbrev);
+
+ subject_len = find_commit_subject(out->message, &subject);
+
+ out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
+ strlen("... ") + subject_len + 1);
+ q = out->parent_label;
+ q = mempcpy(q, "parent of ", strlen("parent of "));
+ out->label = q;
+ q = mempcpy(q, abbrev, abbrev_len);
+ q = mempcpy(q, "... ", strlen("... "));
+ out->subject = q;
+ q = mempcpy(q, subject, subject_len);
+ *q = '\0';
+ return 0;
+}
+
+static void free_message(struct commit_message *msg)
+{
+ free(msg->parent_label);
+ free(msg->reencoded_message);
+}
+
+static char *get_encoding(const char *message)
+{
+ const char *p = message, *eol;
+
+ while (*p && *p != '\n') {
+ for (eol = p + 1; *eol && *eol != '\n'; eol++)
+ ; /* do nothing */
+ if (!prefixcmp(p, "encoding ")) {
+ char *result = xmalloc(eol - 8 - p);
+ strlcpy(result, p + 9, eol - 8 - p);
+ return result;
+ }
+ p = eol;
+ if (*p == '\n')
+ p++;
+ }
+ return NULL;
+}
+
+static void write_cherry_pick_head(struct commit *commit)
+{
+ int fd;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
+
+ fd = open(git_path("CHERRY_PICK_HEAD"), O_WRONLY | O_CREAT, 0666);
+ if (fd < 0)
+ die_errno(_("Could not open '%s' for writing"),
+ git_path("CHERRY_PICK_HEAD"));
+ if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
+ die_errno(_("Could not write to '%s'"), git_path("CHERRY_PICK_HEAD"));
+ strbuf_release(&buf);
+}
+
+static void print_advice(int show_hint)
+{
+ char *msg = getenv("GIT_CHERRY_PICK_HELP");
+
+ if (msg) {
+ fprintf(stderr, "%s\n", msg);
+ /*
+ * A conflict has occured but the porcelain
+ * (typically rebase --interactive) wants to take care
+ * of the commit itself so remove CHERRY_PICK_HEAD
+ */
+ unlink(git_path("CHERRY_PICK_HEAD"));
+ return;
+ }
+
+ if (show_hint) {
+ advise("after resolving the conflicts, mark the corrected paths");
+ advise("with 'git add <paths>' or 'git rm <paths>'");
+ advise("and commit the result with 'git commit'");
+ }
+}
+
+static void write_message(struct strbuf *msgbuf, const char *filename)
+{
+ static struct lock_file msg_file;
+
+ int msg_fd = hold_lock_file_for_update(&msg_file, filename,
+ LOCK_DIE_ON_ERROR);
+ if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
+ die_errno(_("Could not write to %s."), filename);
+ strbuf_release(msgbuf);
+ if (commit_lock_file(&msg_file) < 0)
+ die(_("Error wrapping up %s"), filename);
+}
+
+static struct tree *empty_tree(void)
+{
+ return lookup_tree((const unsigned char *)EMPTY_TREE_SHA1_BIN);
+}
+
+static int error_dirty_index(struct replay_opts *opts)
+{
+ if (read_cache_unmerged())
+ return error_resolve_conflict(action_name(opts));
+
+ /* Different translation strings for cherry-pick and revert */
+ if (opts->action == REPLAY_PICK)
+ error(_("Your local changes would be overwritten by cherry-pick."));
+ else
+ error(_("Your local changes would be overwritten by revert."));
+
+ if (advice_commit_before_merge)
+ advise(_("Commit your changes or stash them to proceed."));
+ return -1;
+}
+
+static int fast_forward_to(const unsigned char *to, const unsigned char *from)
+{
+ struct ref_lock *ref_lock;
+
+ read_cache();
+ if (checkout_fast_forward(from, to))
+ exit(1); /* the callee should have complained already */
+ ref_lock = lock_any_ref_for_update("HEAD", from, 0);
+ return write_ref_sha1(ref_lock, to, "cherry-pick");
+}
+
+static int do_recursive_merge(struct commit *base, struct commit *next,
+ const char *base_label, const char *next_label,
+ unsigned char *head, struct strbuf *msgbuf,
+ struct replay_opts *opts)
+{
+ struct merge_options o;
+ struct tree *result, *next_tree, *base_tree, *head_tree;
+ int clean, index_fd;
+ const char **xopt;
+ static struct lock_file index_lock;
+
+ index_fd = hold_locked_index(&index_lock, 1);
+
+ read_cache();
+
+ init_merge_options(&o);
+ o.ancestor = base ? base_label : "(empty tree)";
+ o.branch1 = "HEAD";
+ o.branch2 = next ? next_label : "(empty tree)";
+
+ head_tree = parse_tree_indirect(head);
+ next_tree = next ? next->tree : empty_tree();
+ base_tree = base ? base->tree : empty_tree();
+
+ for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
+ parse_merge_opt(&o, *xopt);
+
+ clean = merge_trees(&o,
+ head_tree,
+ next_tree, base_tree, &result);
+
+ if (active_cache_changed &&
+ (write_cache(index_fd, active_cache, active_nr) ||
+ commit_locked_index(&index_lock)))
+ /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
+ die(_("%s: Unable to write new index file"), action_name(opts));
+ rollback_lock_file(&index_lock);
+
+ if (!clean) {
+ int i;
+ strbuf_addstr(msgbuf, "\nConflicts:\n\n");
+ for (i = 0; i < active_nr;) {
+ struct cache_entry *ce = active_cache[i++];
+ if (ce_stage(ce)) {
+ strbuf_addch(msgbuf, '\t');
+ strbuf_addstr(msgbuf, ce->name);
+ strbuf_addch(msgbuf, '\n');
+ while (i < active_nr && !strcmp(ce->name,
+ active_cache[i]->name))
+ i++;
+ }
+ }
+ }
+
+ return !clean;
+}
+
+/*
+ * If we are cherry-pick, and if the merge did not result in
+ * hand-editing, we will hit this commit and inherit the original
+ * author date and name.
+ * If we are revert, or if our cherry-pick results in a hand merge,
+ * we had better say that the current user is responsible for that.
+ */
+static int run_git_commit(const char *defmsg, struct replay_opts *opts)
+{
+ /* 6 is max possible length of our args array including NULL */
+ const char *args[6];
+ int i = 0;
+
+ args[i++] = "commit";
+ args[i++] = "-n";
+ if (opts->signoff)
+ args[i++] = "-s";
+ if (!opts->edit) {
+ args[i++] = "-F";
+ args[i++] = defmsg;
+ }
+ args[i] = NULL;
+
+ return run_command_v_opt(args, RUN_GIT_CMD);
+}
+
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+ struct replay_opts *opts)
+{
+ unsigned char head[20];
+ struct commit *base, *next, *parent;
+ const char *base_label, *next_label;
+ struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+ char *defmsg = NULL;
+ struct strbuf msgbuf = STRBUF_INIT;
+ int res;
+
+ if (opts->no_commit) {
+ /*
+ * We do not intend to commit immediately. We just want to
+ * merge the differences in, so let's compute the tree
+ * that represents the "current" state for merge-recursive
+ * to work on.
+ */
+ if (write_cache_as_tree(head, 0, NULL))
+ die (_("Your index file is unmerged."));
+ } else {
+ if (get_sha1("HEAD", head))
+ return error(_("You do not have a valid HEAD"));
+ if (index_differs_from("HEAD", 0))
+ return error_dirty_index(opts);
+ }
+ discard_cache();
+
+ if (!commit->parents) {
+ parent = NULL;
+ }
+ else if (commit->parents->next) {
+ /* Reverting or cherry-picking a merge commit */
+ int cnt;
+ struct commit_list *p;
+
+ if (!opts->mainline)
+ return error(_("Commit %s is a merge but no -m option was given."),
+ sha1_to_hex(commit->object.sha1));
+
+ for (cnt = 1, p = commit->parents;
+ cnt != opts->mainline && p;
+ cnt++)
+ p = p->next;
+ if (cnt != opts->mainline || !p)
+ return error(_("Commit %s does not have parent %d"),
+ sha1_to_hex(commit->object.sha1), opts->mainline);
+ parent = p->item;
+ } else if (0 < opts->mainline)
+ return error(_("Mainline was specified but commit %s is not a merge."),
+ sha1_to_hex(commit->object.sha1));
+ else
+ parent = commit->parents->item;
+
+ if (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))
+ return fast_forward_to(commit->object.sha1, head);
+
+ if (parent && parse_commit(parent) < 0)
+ /* TRANSLATORS: The first %s will be "revert" or
+ "cherry-pick", the second %s a SHA1 */
+ return error(_("%s: cannot parse parent commit %s"),
+ action_name(opts), sha1_to_hex(parent->object.sha1));
+
+ if (get_message(commit, &msg) != 0)
+ return error(_("Cannot get commit message for %s"),
+ sha1_to_hex(commit->object.sha1));
+
+ /*
+ * "commit" is an existing commit. We would want to apply
+ * the difference it introduces since its first parent "prev"
+ * on top of the current HEAD if we are cherry-pick. Or the
+ * reverse of it if we are revert.
+ */
+
+ defmsg = git_pathdup("MERGE_MSG");
+
+ if (action == REPLAY_REVERT) {
+ base = commit;
+ base_label = msg.label;
+ next = parent;
+ next_label = msg.parent_label;
+ strbuf_addstr(&msgbuf, "Revert \"");
+ strbuf_addstr(&msgbuf, msg.subject);
+ strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+
+ if (commit->parents && commit->parents->next) {
+ strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));
+ }
+ strbuf_addstr(&msgbuf, ".\n");
+ } else {
+ const char *p;
+
+ base = parent;
+ base_label = msg.parent_label;
+ next = commit;
+ next_label = msg.label;
+
+ /*
+ * Append the commit log message to msgbuf; it starts
+ * after the tree, parent, author, committer
+ * information followed by "\n\n".
+ */
+ p = strstr(msg.message, "\n\n");
+ if (p) {
+ p += 2;
+ strbuf_addstr(&msgbuf, p);
+ }
+
+ if (opts->record_origin) {
+ strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
+ strbuf_addstr(&msgbuf, ")\n");
+ }
+ }
+
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
+ res = do_recursive_merge(base, next, base_label, next_label,
+ head, &msgbuf, opts);
+ write_message(&msgbuf, defmsg);
+ } else {
+ struct commit_list *common = NULL;
+ struct commit_list *remotes = NULL;
+
+ write_message(&msgbuf, defmsg);
+
+ commit_list_insert(base, &common);
+ commit_list_insert(next, &remotes);
+ res = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ common, sha1_to_hex(head), remotes);
+ free_commit_list(common);
+ free_commit_list(remotes);
+ }
+
+ /*
+ * If the merge was clean or if it failed due to conflict, we write
+ * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.
+ * However, if the merge did not even start, then we don't want to
+ * write it at all.
+ */
+ if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ write_cherry_pick_head(commit);
+
+ if (res) {
+ error(action == REPLAY_REVERT
+ ? _("could not revert %s... %s")
+ : _("could not apply %s... %s"),
+ find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+ msg.subject);
+ print_advice(res == 1);
+ rerere(opts->allow_rerere_auto);
+ } else {
+ if (!opts->no_commit)
+ res = run_git_commit(defmsg, opts);
+ }
+
+ free_message(&msg);
+ free(defmsg);
+
+ return res;
+}
+
+static void prepare_revs(struct replay_opts *opts)
+{
+ if (opts->action != REPLAY_REVERT)
+ opts->revs->reverse ^= 1;
+
+ if (prepare_revision_walk(opts->revs))
+ die(_("revision walk setup failed"));
+
+ if (!opts->revs->commits)
+ die(_("empty commit set passed"));
+}
+
+static void read_and_refresh_cache(struct replay_opts *opts)
+{
+ static struct lock_file index_lock;
+ int index_fd = hold_locked_index(&index_lock, 0);
+ if (read_index_preload(&the_index, NULL) < 0)
+ die(_("git %s: failed to read the index"), action_name(opts));
+ refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
+ if (the_index.cache_changed) {
+ if (write_index(&the_index, index_fd) ||
+ commit_locked_index(&index_lock))
+ die(_("git %s: failed to refresh the index"), action_name(opts));
+ }
+ rollback_lock_file(&index_lock);
+}
+
+/*
+ * Append a commit to the end of the commit_list.
+ *
+ * next starts by pointing to the variable that holds the head of an
+ * empty commit_list, and is updated to point to the "next" field of
+ * the last item on the list as new commits are appended.
+ *
+ * Usage example:
+ *
+ * struct commit_list *list;
+ * struct commit_list **next = &list;
+ *
+ * next = commit_list_append(c1, next);
+ * next = commit_list_append(c2, next);
+ * assert(commit_list_count(list) == 2);
+ * return list;
+ */
+static struct replay_insn_list **replay_insn_list_append(enum replay_action action,
+ struct commit *operand,
+ struct replay_insn_list **next)
+{
+ struct replay_insn_list *new = xmalloc(sizeof(*new));
+ new->action = action;
+ new->operand = operand;
+ *next = new;
+ new->next = NULL;
+ return &new->next;
+}
+
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
+{
+ struct replay_insn_list *cur;
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ const char *sha1_abbrev, *action_str, *subject;
+ int subject_len;
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ subject_len = find_commit_subject(cur->operand->buffer, &subject);
+ strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+ subject_len, subject);
+ }
+ return 0;
+}
+
+static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
+{
+ unsigned char commit_sha1[20];
+ char *end_of_object_name;
+ int saved, status;
+
+ if (!prefixcmp(bol, "pick ")) {
+ item->action = REPLAY_PICK;
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ")) {
+ item->action = REPLAY_REVERT;
+ bol += strlen("revert ");
+ } else {
+ size_t len = strchrnul(bol, '\n') - bol;
+ if (len > 255)
+ len = 255;
+ return error(_("Unrecognized action: %.*s"), (int)len, bol);
+ }
+
+ end_of_object_name = bol + strcspn(bol, " \n");
+ saved = *end_of_object_name;
+ *end_of_object_name = '\0';
+ status = get_sha1(bol, commit_sha1);
+ *end_of_object_name = saved;
+
+ if (status < 0)
+ return error(_("Malformed object name: %s"), bol);
+
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return error(_("Not a valid commit: %s"), bol);
+
+ item->next = NULL;
+ return 0;
+}
+
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
+{
+ struct replay_insn_list **next = todo_list;
+ struct replay_insn_list item = {0, NULL, NULL};
+ char *p = buf;
+ int i;
+
+ for (i = 1; *p; i++) {
+ char *eol = strchrnul(p, '\n');
+ if (parse_insn_line(p, eol, &item) < 0)
+ return error(_("on line %d."), i);
+ next = replay_insn_list_append(item.action, item.operand, next);
+ p = *eol ? eol + 1 : eol;
+ }
+ if (!*todo_list)
+ return error(_("No commits parsed."));
+ return 0;
+}
+
+static void read_populate_todo(struct replay_insn_list **todo_list)
+{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
+ struct strbuf buf = STRBUF_INIT;
+ int fd, res;
+
+ fd = open(todo_file, O_RDONLY);
+ if (fd < 0)
+ die_errno(_("Could not open %s."), todo_file);
+ if (strbuf_read(&buf, fd, 0) < 0) {
+ close(fd);
+ strbuf_release(&buf);
+ die(_("Could not read %s."), todo_file);
+ }
+ close(fd);
+
+ res = parse_insn_buffer(buf.buf, todo_list);
+ strbuf_release(&buf);
+ if (res)
+ die(_("Unusable instruction sheet: %s"), todo_file);
+}
+
+static int populate_opts_cb(const char *key, const char *value, void *data)
+{
+ struct replay_opts *opts = data;
+ int error_flag = 1;
+
+ if (!value)
+ error_flag = 0;
+ else if (!strcmp(key, "options.no-commit"))
+ opts->no_commit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.edit"))
+ opts->edit = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.signoff"))
+ 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.allow-ff"))
+ opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
+ else if (!strcmp(key, "options.mainline"))
+ opts->mainline = git_config_int(key, value);
+ else if (!strcmp(key, "options.strategy"))
+ git_config_string(&opts->strategy, key, value);
+ else if (!strcmp(key, "options.strategy-option")) {
+ ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
+ opts->xopts[opts->xopts_nr++] = xstrdup(value);
+ } else
+ return error(_("Invalid key: %s"), key);
+
+ if (!error_flag)
+ return error(_("Invalid value for %s: %s"), key, value);
+
+ return 0;
+}
+
+static void read_populate_opts(struct replay_opts **opts_ptr)
+{
+ const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+ if (!file_exists(opts_file))
+ return;
+ if (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)
+ die(_("Malformed options sheet: %s"), opts_file);
+}
+
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
+ struct replay_opts *opts)
+{
+ struct commit *commit;
+ struct replay_insn_list **next;
+
+ prepare_revs(opts);
+
+ next = todo_list;
+ while ((commit = get_revision(opts->revs)))
+ next = replay_insn_list_append(opts->action, commit, next);
+}
+
+static int create_seq_dir(void)
+{
+ const char *seq_dir = git_path(SEQ_DIR);
+
+ if (file_exists(seq_dir))
+ return error(_("%s already exists."), seq_dir);
+ else if (mkdir(seq_dir, 0777) < 0)
+ die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
+ return 0;
+}
+
+static void save_head(const char *head)
+{
+ const char *head_file = git_path(SEQ_HEAD_FILE);
+ static struct lock_file head_lock;
+ struct strbuf buf = STRBUF_INIT;
+ int fd;
+
+ fd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);
+ strbuf_addf(&buf, "%s\n", head);
+ if (write_in_full(fd, buf.buf, buf.len) < 0)
+ die_errno(_("Could not write to %s."), head_file);
+ if (commit_lock_file(&head_lock) < 0)
+ die(_("Error wrapping up %s."), head_file);
+}
+
+static void save_todo(struct replay_insn_list *todo_list)
+{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
+ static struct lock_file todo_lock;
+ struct strbuf buf = STRBUF_INIT;
+ int fd;
+
+ fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
+ if (format_todo(&buf, todo_list) < 0)
+ die(_("Could not format %s."), todo_file);
+ if (write_in_full(fd, buf.buf, buf.len) < 0) {
+ strbuf_release(&buf);
+ die_errno(_("Could not write to %s."), todo_file);
+ }
+ if (commit_lock_file(&todo_lock) < 0) {
+ strbuf_release(&buf);
+ die(_("Error wrapping up %s."), todo_file);
+ }
+ strbuf_release(&buf);
+}
+
+static void save_opts(struct replay_opts *opts)
+{
+ const char *opts_file = git_path(SEQ_OPTS_FILE);
+
+ if (opts->no_commit)
+ git_config_set_in_file(opts_file, "options.no-commit", "true");
+ if (opts->edit)
+ git_config_set_in_file(opts_file, "options.edit", "true");
+ if (opts->signoff)
+ git_config_set_in_file(opts_file, "options.signoff", "true");
+ if (opts->record_origin)
+ git_config_set_in_file(opts_file, "options.record-origin", "true");
+ if (opts->allow_ff)
+ git_config_set_in_file(opts_file, "options.allow-ff", "true");
+ if (opts->mainline) {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_addf(&buf, "%d", opts->mainline);
+ git_config_set_in_file(opts_file, "options.mainline", buf.buf);
+ strbuf_release(&buf);
+ }
+ if (opts->strategy)
+ git_config_set_in_file(opts_file, "options.strategy", opts->strategy);
+ if (opts->xopts) {
+ int i;
+ for (i = 0; i < opts->xopts_nr; i++)
+ git_config_set_multivar_in_file(opts_file,
+ "options.strategy-option",
+ opts->xopts[i], "^$", 0);
+ }
+}
+
+static int pick_commits(struct replay_insn_list *todo_list,
+ struct replay_opts *opts)
+{
+ struct replay_insn_list *cur;
+ int res;
+
+ setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ if (opts->allow_ff)
+ assert(!(opts->signoff || opts->no_commit ||
+ opts->record_origin || opts->edit));
+ read_and_refresh_cache(opts);
+
+ for (cur = todo_list; cur; cur = cur->next) {
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, 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);
+ return res;
+ }
+ }
+
+ /*
+ * Sequence of picks finished successfully; cleanup by
+ * removing the .git/sequencer directory
+ */
+ remove_sequencer_state(1);
+ return 0;
+}
+
+int sequencer_pick_revisions(struct replay_opts *opts)
+{
+ struct replay_insn_list *todo_list = NULL;
+ unsigned char sha1[20];
+
+ if (opts->subcommand == REPLAY_NONE)
+ assert(opts->revs);
+
+ read_and_refresh_cache(opts);
+
+ /*
+ * Decide what to do depending on the arguments; a fresh
+ * cherry-pick should be handled differently from an existing
+ * one that is being continued
+ */
+ if (opts->subcommand == REPLAY_RESET) {
+ remove_sequencer_state(1);
+ return 0;
+ } else if (opts->subcommand == REPLAY_CONTINUE) {
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ goto error;
+ read_populate_opts(&opts);
+ read_populate_todo(&todo_list);
+
+ /* Verify that the conflict has been resolved */
+ if (!index_differs_from("HEAD", 0))
+ todo_list = todo_list->next;
+ } else {
+ /*
+ * Start a new cherry-pick/ revert sequence; but
+ * first, make sure that an existing one isn't in
+ * progress
+ */
+
+ walk_revs_populate_todo(&todo_list, opts);
+ if (create_seq_dir() < 0) {
+ error(_("A cherry-pick or revert is in progress."));
+ advise(_("Use --continue to continue the operation"));
+ advise(_("or --reset to forget about it"));
+ return -1;
+ }
+ if (get_sha1("HEAD", sha1)) {
+ if (opts->action == REPLAY_REVERT)
+ return error(_("Can't revert as initial commit"));
+ return error(_("Can't cherry-pick into empty head"));
+ }
+ save_head(sha1_to_hex(sha1));
+ save_opts(opts);
+ }
+ return pick_commits(todo_list, opts);
+error:
+ return error(_("No %s in progress"), action_name(opts));
+}
diff --git a/sequencer.h b/sequencer.h
index f4db257..92b2d63 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -8,6 +8,30 @@
#define SEQ_OPTS_FILE "sequencer/opts"
enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
+
+struct replay_opts {
+ enum replay_action action;
+ enum replay_subcommand subcommand;
+
+ /* Boolean options */
+ int edit;
+ int record_origin;
+ int no_commit;
+ int signoff;
+ int allow_ff;
+ int allow_rerere_auto;
+
+ int mainline;
+
+ /* Merge strategy */
+ const char *strategy;
+ const char **xopts;
+ size_t xopts_nr, xopts_alloc;
+
+ /* Only used by REPLAY_NONE */
+ struct rev_info *revs;
+};
struct replay_insn_list {
enum replay_action action;
@@ -25,4 +49,6 @@ struct replay_insn_list {
*/
void remove_sequencer_state(int aggressive);
+int sequencer_pick_revisions(struct replay_opts *opts);
+
#endif
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/7] sequencer: invalidate sequencer state without todo
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 1/7] sequencer: factor code out of revert builtin Ramkumar Ramachandra
@ 2011-11-13 10:46 ` Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 3/7] sequencer: handle single-commit pick as special case Ramkumar Ramachandra
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
To check whether an existing sequencer operation is in progress and
error out, we currently check for the existence of the
'.git/sequencer' directory.
$ git cherry-pick foo..bar
... conflict in bar~1 ...
... .git/sequencer is created ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit # Success!
$ git cherry-pick moo # .git/sequencer exists
error: A cherry-pick or revert is in progress
Although the sequencer state is useless without '.git/sequencer/todo',
this case never occurs. However, in the light of the next patch,
where will handle single-commit picks as a special case by not
persisting '.git/sequencer/todo' in the first place, we are forced to
reconsider. So, when starting a fresh sequencer invocation, remove
the sequencer state carried over from a previous invocation if it's
missing '.git/sequencer/todo'.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 87f146b..8b2518c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -649,11 +649,15 @@ static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
static int create_seq_dir(void)
{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
const char *seq_dir = git_path(SEQ_DIR);
- if (file_exists(seq_dir))
- return error(_("%s already exists."), seq_dir);
- else if (mkdir(seq_dir, 0777) < 0)
+ if (file_exists(todo_file))
+ return error(_("%s already exists."), todo_file);
+
+ /* If todo_file doesn't exist, discard sequencer state */
+ remove_sequencer_state(1);
+ if (mkdir(seq_dir, 0777) < 0)
die_errno(_("Could not create sequencer directory '%s'."), seq_dir);
return 0;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/7] sequencer: handle single-commit pick as special case
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 1/7] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 2/7] sequencer: invalidate sequencer state without todo Ramkumar Ramachandra
@ 2011-11-13 10:46 ` Ramkumar Ramachandra
2011-11-13 23:23 ` Junio C Hamano
2011-11-13 10:46 ` [PATCH 4/7] sequencer: handle cherry-pick conflict in last commit Ramkumar Ramachandra
` (5 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
Prior to v1.7.2-rc1~4^2~7 (revert: allow cherry-picking more than one
commit, 2010-06-02), 'git cherry-pick' could only pick one commit at a
time, and it used '.git/CHERRY_PICK_HEAD' to pass on information to a
subsequent invocation in case of a conflict. While
'.git/CHERRY_PICK_HEAD' can only keep information about one commit,
the sequencer uses '.git/sequencer' to persist information in the
general case.
A problem arises because a single-commit cherry-pick operation can be
completed successfully using 'git commit'. This removes
'.git/CHERRY_PICK_HEAD' without informing the sequencer, leaving
behind a stale sequencer state as a result. We have worked around
this problem already by prematurely removing the sequencer state in
d3f4628e (revert: Remove sequencer state when no commits are pending,
2011-06-06). However, this gets in the way of our future plan to
eliminate a glaring workflow inconsistency:
$ git cherry-pick foo
... .git/sequencer is created ....
... .git/CHERRY_PICK_HEAD is created ...
... conflict ...
.... .git/sequencer is prematurely removed ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
... .git/CHERRY_PICK_HEAD is removed ...
$ git cherry-pick --continue
error: No cherry-pick in progress
$ git cherry-pick foo..bar
... .git/sequencer is created ....
... CHERRY_PICK_HEAD is created ...
... conflict in bar~1 ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
... CHERRY_PICK_HEAD is removed ...
$ git cherry-pick --continue # Success!
To eliminate this inconsistency, we have decided to make '--continue'
continue any general sequencer operation bypassing 'git commit'
completely (although preserving the existing workflow for backward
compatibility). For '--continue' and '--reset' to work uniformly,
they must use the information in:
1. '.git/sequencer/head', '.git/sequencer/opts', '.git/sequencer/todo'
in the general case.
2. '.git/sequencer/head', '.git/sequencer/opts', '.git/CHERRY_PICK_HEAD'
in case of a single-commit cherry-pick.
As a start, handle cherry-picking a single commit as a special case by
not creating '.git/sequencer/todo' in the first place. This will
eliminate the need for prematurely removing it in d3f4628e.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 8b2518c..b35fcc7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -741,6 +741,14 @@ static int pick_commits(struct replay_insn_list *todo_list,
opts->record_origin || opts->edit));
read_and_refresh_cache(opts);
+ /*
+ * Backward compatibility hack: handle single-commit pick as a
+ * special case.
+ */
+ if (opts->subcommand == REPLAY_NONE &&
+ todo_list->next == NULL && todo_list->action == REPLAY_PICK)
+ return do_pick_commit(todo_list->operand, REPLAY_PICK, opts);
+
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur);
res = do_pick_commit(cur->operand, cur->action, opts);
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/7] sequencer: handle cherry-pick conflict in last commit
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
` (2 preceding siblings ...)
2011-11-13 10:46 ` [PATCH 3/7] sequencer: handle single-commit pick as special case Ramkumar Ramachandra
@ 2011-11-13 10:46 ` Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 5/7] sequencer: introduce git-sequencer builtin Ramkumar Ramachandra
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
The previous two commits in the series implement special handling for
the single-commit cherry-pick case. Although we can technically
revert the changes made by d3f4628e (revert: Remove sequencer state
when no commits are pending, 2011-06-06) without breaking any existing
tests now, there is one pending corner case: when a cherry-pick is
invoked on a commit range, and a conflict that occurs in the last
commit is concluded with a 'git commit'. Without d3f4628e, we'd have
the following unpleasant case:
$ git cherry-pick foo..bar
... .git/sequencer is created ...
... .git/CHERRY_PICK_HEAD is created ...
... conflict in bar ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git commit
... .git/CHERRY_PICK_HEAD is removed ...
$ git cherry-pick moo
error: An existing cherry-pick or revert is in progress
$ git cherry-pick --continue
... .git/sequencer is removed ...
# We're in pristine shape now
While prematurely removing the entire sequencer state is an overkill,
we can revise our plan: prematurely remove only '.git/sequencer/todo'
in the 'REPLAY_PICK' case, because this is the exact case where the
information in '.git/sequencer/todo' can be inferred from
'.git/CHERRY_PICK_HEAD'. This will be compatible with our future plan
to implement '--continue' and '--reset' consistently.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 8 +++-----
t/t3510-cherry-pick-sequence.sh | 4 ++--
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index b35fcc7..23fd3fe 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -753,15 +753,13 @@ static int pick_commits(struct replay_insn_list *todo_list,
save_todo(cur);
res = do_pick_commit(cur->operand, cur->action, opts);
if (res) {
- if (!cur->next)
+ if (!cur->next && opts->action == REPLAY_PICK)
/*
* 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
+ * sequencer todo is useless now.
*/
- remove_sequencer_state(0);
+ unlink(git_path(SEQ_TODO_FILE));
return res;
}
}
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 4b12244..09b9e65 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -85,10 +85,10 @@ test_expect_success '--reset cleans up sequencer state' '
test_path_is_missing .git/sequencer
'
-test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '
+test_expect_success 'cherry-pick cleans up sequencer todo when one commit is left' '
pristine_detach initial &&
test_must_fail git cherry-pick base..picked &&
- test_path_is_missing .git/sequencer &&
+ test_path_is_missing .git/sequencer/todo &&
echo "resolved" >foo &&
git add foo &&
git commit &&
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/7] sequencer: introduce git-sequencer builtin
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
` (3 preceding siblings ...)
2011-11-13 10:46 ` [PATCH 4/7] sequencer: handle cherry-pick conflict in last commit Ramkumar Ramachandra
@ 2011-11-13 10:46 ` Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 6/7] sequencer: teach '--continue' how to commit Ramkumar Ramachandra
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
Introduce a new builtin 'git sequencer' that implements only
'--continue' and '--reset' features of the sequencer. As a result, it
can only be used to continue an existing sequencer operation, not to
start a new one. Now you can do:
$ git cherry-pick foo..bar
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
So, irrespective of the concrete implementation of the sequencer
invoked earlier, any sequencer operation can be continued with 'git
sequencer --continue' and reset with 'git sequencer --reset'. Also,
we plan to make the 'git sequencer' builtin implement a concrete
operation similar to 'git rebase -i' in the future.
While at it, change the advice printed by the sequencer library and
re-organize 't3510-cherry-pick-sequence.sh'. There are now two
separate sets of tests:
t3510-sequencer.sh is meant to test the continuation features of the
sequencer, using 'git cherry-pick' to initiate an operation.
t3511-cherry-pick-sequencer.sh is meant to test 'git cherry-pick' as
a concrete implementation of the sequencer, stressing on many of the
backward compatibility features specific to it.
In the future, when the sequencer grows more functionality that
doesn't directly impact the concrete implementation 'git cherry-pick',
the corresponding tests should go into 't3510-sequencer.sh'.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
.gitignore | 1 +
Documentation/git-sequencer.txt | 33 +++++
Makefile | 1 +
builtin.h | 1 +
builtin/sequencer.c | 52 +++++++
git.c | 1 +
sequencer.c | 6 +-
t/t3510-sequencer.sh | 153 ++++++++++++++++++++
...-sequence.sh => t3511-cherry-pick-sequencer.sh} | 120 +---------------
9 files changed, 251 insertions(+), 117 deletions(-)
create mode 100644 Documentation/git-sequencer.txt
create mode 100644 builtin/sequencer.c
create mode 100755 t/t3510-sequencer.sh
rename t/{t3510-cherry-pick-sequence.sh => t3511-cherry-pick-sequencer.sh} (61%)
diff --git a/.gitignore b/.gitignore
index 8572c8c..74ea408 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,6 +128,7 @@
/git-rm
/git-send-email
/git-send-pack
+/git-sequencer
/git-sh-i18n
/git-sh-i18n--envsubst
/git-sh-setup
diff --git a/Documentation/git-sequencer.txt b/Documentation/git-sequencer.txt
new file mode 100644
index 0000000..aa8a80b
--- /dev/null
+++ b/Documentation/git-sequencer.txt
@@ -0,0 +1,33 @@
+git-sequencer(1)
+================
+
+NAME
+----
+git-sequencer - Finish an existing sequencer operation
+
+
+SYNOPSIS
+--------
+[verse]
+'git sequencer' --reset
+'git sequencer' --continue
+
+DESCRIPTION
+-----------
+Given any arbitrary sequencer operation started using 'git
+cherry-pick' or 'git revert', facilitate continuing or resetting it
+via a uniform interface.
+
+
+OPTIONS
+-------
+include::sequencer.txt[]
+
+
+SEE ALSO
+--------
+linkgit:git-cherry-pick[1]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 17404c4..e367310 100644
--- a/Makefile
+++ b/Makefile
@@ -777,6 +777,7 @@ BUILTIN_OBJS += builtin/rev-parse.o
BUILTIN_OBJS += builtin/revert.o
BUILTIN_OBJS += builtin/rm.o
BUILTIN_OBJS += builtin/send-pack.o
+BUILTIN_OBJS += builtin/sequencer.o
BUILTIN_OBJS += builtin/shortlog.o
BUILTIN_OBJS += builtin/show-branch.o
BUILTIN_OBJS += builtin/show-ref.o
diff --git a/builtin.h b/builtin.h
index 0e9da90..0335163 100644
--- a/builtin.h
+++ b/builtin.h
@@ -119,6 +119,7 @@ extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);
extern int cmd_revert(int argc, const char **argv, const char *prefix);
extern int cmd_rm(int argc, const char **argv, const char *prefix);
extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
+extern int cmd_sequencer(int argc, const char **argv, const char *prefix);
extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
diff --git a/builtin/sequencer.c b/builtin/sequencer.c
new file mode 100644
index 0000000..7364a52
--- /dev/null
+++ b/builtin/sequencer.c
@@ -0,0 +1,52 @@
+#include "cache.h"
+#include "builtin.h"
+#include "parse-options.h"
+#include "sequencer.h"
+
+static const char *const sequencer_usage[] = {
+ "git sequencer --continue",
+ "git sequencer --reset",
+ NULL
+};
+
+static void parse_args(int argc, const char **argv, struct replay_opts *opts)
+{
+ int reset = 0;
+ int contin = 0;
+ struct option options[] = {
+ OPT_BOOLEAN(0, "reset", &reset, "forget the current operation"),
+ OPT_BOOLEAN(0, "continue", &contin, "continue the current operation"),
+ OPT_END(),
+ };
+
+ argc = parse_options(argc, argv, NULL, options, sequencer_usage,
+ PARSE_OPT_KEEP_ARGV0 |
+ PARSE_OPT_KEEP_UNKNOWN);
+
+ /* Set the subcommand */
+ if (reset)
+ opts->subcommand = REPLAY_RESET;
+ else if (contin)
+ opts->subcommand = REPLAY_CONTINUE;
+ else
+ opts->subcommand = REPLAY_NONE;
+
+ /* Forbid REPLAY_NONE and stray command-line arguments */
+ if (opts->subcommand == REPLAY_NONE || argc > 1)
+ usage_with_options(sequencer_usage, options);
+}
+
+int cmd_sequencer(int argc, const char **argv, const char *prefix)
+{
+ struct replay_opts opts;
+ int res;
+
+ memset(&opts, 0, sizeof(opts));
+ opts.action = REPLAY_REVERT;
+ git_config(git_default_config, NULL);
+ parse_args(argc, argv, &opts);
+ res = sequencer_pick_revisions(&opts);
+ if (res < 0)
+ die(_("sequencer failed"));
+ return res;
+}
diff --git a/git.c b/git.c
index 8e34903..5262c9b 100644
--- a/git.c
+++ b/git.c
@@ -418,6 +418,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
{ "rm", cmd_rm, RUN_SETUP },
{ "send-pack", cmd_send_pack, RUN_SETUP },
+ { "sequencer", cmd_sequencer, RUN_SETUP | NEED_WORK_TREE },
{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
{ "show", cmd_show, RUN_SETUP },
{ "show-branch", cmd_show_branch, RUN_SETUP },
diff --git a/sequencer.c b/sequencer.c
index 23fd3fe..012d531 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -808,9 +808,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
walk_revs_populate_todo(&todo_list, opts);
if (create_seq_dir() < 0) {
- error(_("A cherry-pick or revert is in progress."));
- advise(_("Use --continue to continue the operation"));
- advise(_("or --reset to forget about it"));
+ error(_("A sequencer operation is in progress."));
+ advise(_("Use git sequencer --continue to it"));
+ advise(_("or git sequencer --reset to forget about it"));
return -1;
}
if (get_sha1("HEAD", sha1)) {
diff --git a/t/t3510-sequencer.sh b/t/t3510-sequencer.sh
new file mode 100755
index 0000000..6b2e712
--- /dev/null
+++ b/t/t3510-sequencer.sh
@@ -0,0 +1,153 @@
+#!/bin/sh
+
+test_description='Test sequencer continuation features
+
+ + anotherpick: rewrites foo to d
+ + picked: rewrites foo to c
+ + unrelatedpick: rewrites unrelated to reallyunrelated
+ + base: rewrites foo to b
+ + initial: writes foo as a, unrelated as unrelated
+
+'
+
+. ./test-lib.sh
+
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
+pristine_detach () {
+ git cherry-pick --reset &&
+ git checkout -f "$1^0" &&
+ git read-tree -u --reset HEAD &&
+ git clean -d -f -f -q -x
+}
+
+test_expect_success setup '
+ echo unrelated >unrelated &&
+ git add unrelated &&
+ test_commit initial foo a &&
+ test_commit base foo b &&
+ test_commit unrelatedpick unrelated reallyunrelated &&
+ test_commit picked foo c &&
+ test_commit anotherpick foo d &&
+ git config advice.detachedhead false
+
+'
+
+test_expect_success '--continue complains when no sequencer operation is in progress' '
+ pristine_detach initial &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success '--continue complains when there are unresolved conflicts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'malformed instruction sheet 1' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'malformed instruction sheet 2' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'malformed instruction sheet 3' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git sequencer --continue
+'
+
+test_expect_success 'commit descriptions in insn sheet are optional' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ git rev-list HEAD >commits
+ test_line_count = 4 commits
+'
+
+test_expect_success 'revert --continue continues after cherry-pick' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ git revert --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'mixed pick and revert instructions' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ oldsha=`git rev-parse --short HEAD~1` &&
+ echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_done
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3511-cherry-pick-sequencer.sh
similarity index 61%
rename from t/t3510-cherry-pick-sequence.sh
rename to t/t3511-cherry-pick-sequencer.sh
index 09b9e65..a9c6ac1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3511-cherry-pick-sequencer.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='Test cherry-pick continuation features
+test_description='Test cherry-pick as a sequencer implementation
+ anotherpick: rewrites foo to d
+ picked: rewrites foo to c
@@ -12,9 +12,6 @@ test_description='Test cherry-pick continuation features
. ./test-lib.sh
-# Repeat first match 10 times
-_r10='\1\1\1\1\1\1\1\1\1\1'
-
pristine_detach () {
git cherry-pick --reset &&
git checkout -f "$1^0" &&
@@ -34,7 +31,7 @@ test_expect_success setup '
'
-test_expect_success 'cherry-pick persists data on failure' '
+test_expect_success 'sequencer persists data on failure' '
pristine_detach initial &&
test_must_fail git cherry-pick -s base..anotherpick &&
test_path_is_dir .git/sequencer &&
@@ -43,7 +40,7 @@ test_expect_success 'cherry-pick persists data on failure' '
test_path_is_file .git/sequencer/opts
'
-test_expect_success 'cherry-pick persists opts correctly' '
+test_expect_success 'sequencer persists opts correctly' '
pristine_detach initial &&
test_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&
test_path_is_dir .git/sequencer &&
@@ -67,7 +64,7 @@ test_expect_success 'cherry-pick persists opts correctly' '
test_cmp expect actual
'
-test_expect_success 'cherry-pick cleans up sequencer state upon success' '
+test_expect_success 'sequencer cleans up state upon success' '
pristine_detach initial &&
git cherry-pick initial..picked &&
test_path_is_missing .git/sequencer
@@ -109,7 +106,7 @@ test_expect_success 'cherry-pick cleans up sequencer todo when one commit is lef
test_cmp expect actual
'
-test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '
+test_expect_success 'sequencer does not implicitly stomp an existing operation' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
test-chmtime -v +0 .git/sequencer >expect &&
@@ -118,7 +115,7 @@ test_expect_success 'cherry-pick does not implicitly stomp an existing operation
test_cmp expect actual
'
-test_expect_success '--continue complains when no cherry-pick is in progress' '
+test_expect_success '--continue complains when no sequencer operation is in progress' '
pristine_detach initial &&
test_must_fail git cherry-pick --continue
'
@@ -192,109 +189,4 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
grep "Signed-off-by:" anotherpick_msg
'
-test_expect_success 'malformed instruction sheet 1' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "resolved" >foo &&
- git add foo &&
- git commit &&
- sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
-'
-
-test_expect_success 'malformed instruction sheet 2' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "resolved" >foo &&
- git add foo &&
- git commit &&
- sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
-'
-
-test_expect_success 'malformed instruction sheet 3' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "resolved" >foo &&
- git add foo &&
- git commit &&
- sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- test_must_fail git cherry-pick --continue
-'
-
-test_expect_success 'commit descriptions in insn sheet are optional' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "c" >foo &&
- git add foo &&
- git commit &&
- cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- git cherry-pick --continue &&
- test_path_is_missing .git/sequencer &&
- git rev-list HEAD >commits
- test_line_count = 4 commits
-'
-
-test_expect_success 'revert --continue continues after cherry-pick' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "c" >foo &&
- git add foo &&
- git commit &&
- git revert --continue &&
- test_path_is_missing .git/sequencer &&
- {
- git rev-list HEAD |
- git diff-tree --root --stdin |
- sed "s/$_x40/OBJID/g"
- } >actual &&
- cat >expect <<-\EOF &&
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M unrelated
- OBJID
- :000000 100644 OBJID OBJID A foo
- :000000 100644 OBJID OBJID A unrelated
- EOF
- test_cmp expect actual
-'
-
-test_expect_success 'mixed pick and revert instructions' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- echo "c" >foo &&
- git add foo &&
- git commit &&
- oldsha=`git rev-parse --short HEAD~1` &&
- echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
- git cherry-pick --continue &&
- test_path_is_missing .git/sequencer &&
- {
- git rev-list HEAD |
- git diff-tree --root --stdin |
- sed "s/$_x40/OBJID/g"
- } >actual &&
- cat >expect <<-\EOF &&
- OBJID
- :100644 100644 OBJID OBJID M unrelated
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M foo
- OBJID
- :100644 100644 OBJID OBJID M unrelated
- OBJID
- :000000 100644 OBJID OBJID A foo
- :000000 100644 OBJID OBJID A unrelated
- EOF
- test_cmp expect actual
-'
-
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/7] sequencer: teach '--continue' how to commit
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
` (4 preceding siblings ...)
2011-11-13 10:46 ` [PATCH 5/7] sequencer: introduce git-sequencer builtin Ramkumar Ramachandra
@ 2011-11-13 10:46 ` Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 7/7] sequencer: teach parser about CHERRY_PICK_HEAD Ramkumar Ramachandra
` (2 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
As discussed earlier in the series, the commit-before-continue
convention has caused a lot of workflow inconsistencies. Teach
'--continue' how to commit so that you can now do:
$ git cherry-pick foo..bar
... conflict occured in bar~1 ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
Modify existing tests in 't3510-sequencer.sh' to exclude the 'git
commit' step, and port relevant tests from
't3511-cherry-pick-sequence.sh' after removing the 'git commit' step.
Note that the tests in 't3511-cherry-pick-sequence.sh' (with the 'git
commit' step) still need to respect backward compatibility.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 10 +++++++-
t/t3510-sequencer.sh | 51 +++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 012d531..7b10b7b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -269,7 +269,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
args[i++] = "-n";
if (opts->signoff)
args[i++] = "-s";
- if (!opts->edit) {
+ if (!opts->edit && defmsg) {
args[i++] = "-F";
args[i++] = defmsg;
}
@@ -776,6 +776,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
{
struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
+ int res;
if (opts->subcommand == REPLAY_NONE)
assert(opts->revs);
@@ -796,9 +797,14 @@ int sequencer_pick_revisions(struct replay_opts *opts)
read_populate_opts(&opts);
read_populate_todo(&todo_list);
- /* Verify that the conflict has been resolved */
if (!index_differs_from("HEAD", 0))
todo_list = todo_list->next;
+ else if (!opts->no_commit && !read_cache_unmerged()) {
+ res = run_git_commit(NULL, opts);
+ if (res)
+ return res;
+ todo_list = todo_list->next;
+ }
} else {
/*
* Start a new cherry-pick/ revert sequence; but
diff --git a/t/t3510-sequencer.sh b/t/t3510-sequencer.sh
index 6b2e712..65f2724 100755
--- a/t/t3510-sequencer.sh
+++ b/t/t3510-sequencer.sh
@@ -45,12 +45,54 @@ test_expect_success '--continue complains when there are unresolved conflicts' '
test_must_fail git sequencer --continue
'
+test_expect_success '--continue continues after conflicts are resolved' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--continue respects opts' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -x base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD >anotherpick_msg &&
+ git cat-file commit HEAD~1 >picked_msg &&
+ git cat-file commit HEAD~2 >unrelatedpick_msg &&
+ git cat-file commit HEAD~3 >initial_msg &&
+ test_must_fail grep "cherry picked from" initial_msg &&
+ grep "cherry picked from" unrelatedpick_msg &&
+ grep "cherry picked from" picked_msg &&
+ grep "cherry picked from" anotherpick_msg
+'
+
test_expect_success 'malformed instruction sheet 1' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
- git commit &&
sed "s/pick /pick/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
test_must_fail git sequencer --continue
@@ -61,7 +103,6 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
- git commit &&
sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
test_must_fail git sequencer --continue
@@ -72,7 +113,6 @@ test_expect_success 'malformed instruction sheet 3' '
test_must_fail git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
git add foo &&
- git commit &&
sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
test_must_fail git sequencer --continue
@@ -83,7 +123,6 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_must_fail git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
- git commit &&
cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
cp new_sheet .git/sequencer/todo &&
git sequencer --continue &&
@@ -97,7 +136,6 @@ test_expect_success 'revert --continue continues after cherry-pick' '
test_must_fail git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
- git commit &&
git revert --continue &&
test_path_is_missing .git/sequencer &&
{
@@ -120,12 +158,11 @@ test_expect_success 'revert --continue continues after cherry-pick' '
'
test_expect_success 'mixed pick and revert instructions' '
+ oldsha=`git rev-parse --short HEAD~2` &&
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
echo "c" >foo &&
git add foo &&
- git commit &&
- oldsha=`git rev-parse --short HEAD~1` &&
echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
git sequencer --continue &&
test_path_is_missing .git/sequencer &&
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/7] sequencer: teach parser about CHERRY_PICK_HEAD
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
` (5 preceding siblings ...)
2011-11-13 10:46 ` [PATCH 6/7] sequencer: teach '--continue' how to commit Ramkumar Ramachandra
@ 2011-11-13 10:46 ` Ramkumar Ramachandra
2011-11-13 20:56 ` [PATCH 0/7] New sequencer workflow! Junio C Hamano
2011-11-15 15:46 ` Phil Hord
8 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-13 10:46 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
The final step in unifying the sequencer interface involves making
sure that a single-commit pick can be concluded with a '--continue'.
$ git cherry-pick foo
... conflict ...
$ echo "resolved" >problematicfile
$ git add problematicfile
$ git sequencer --continue
To do this, we have to update our parser that normally reads
'.git/sequencer/todo' to read '.git/CHERRY_PICK_HEAD' as a special
case before proceeding as usual. Add a new test in
't3510-sequencer.sh' to make sure that this works. Although we have
added a similar test for revert in the same patch for symmetry, the
test doesn't depend on this patch to work. A single-commit revert
operation does not create a '.git/CHERRY_PICK_HEAD' at all: it uses
the information in '.git/sequencer' as usual.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sequencer.c | 32 ++++++++++++++++++++++++++++----
t/t3510-sequencer.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 7b10b7b..84df926 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -569,12 +569,32 @@ static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
return 0;
}
-static void read_populate_todo(struct replay_insn_list **todo_list)
+static void read_populate_todo(struct replay_insn_list **todo_list, int cph_flag)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
struct strbuf buf = STRBUF_INIT;
int fd, res;
+ if (cph_flag) {
+ struct replay_insn_list item = {0, NULL, NULL};
+ const char *name = "CHERRY_PICK_HEAD";
+ const char *CHERRY_PICK_HEAD = git_path(name);
+
+ fd = open(CHERRY_PICK_HEAD, O_RDONLY);
+ if (fd < 0)
+ die_errno(_("Could not open %s."), CHERRY_PICK_HEAD);
+
+ item.action = REPLAY_PICK;
+ item.operand = lookup_commit_reference_by_name(name);
+
+ if (!item.operand)
+ die(_("could not lookup commit %s"), name);
+ replay_insn_list_append(item.action, item.operand, todo_list);
+ close(fd);
+ strbuf_release(&buf);
+ return;
+ }
+
fd = open(todo_file, O_RDONLY);
if (fd < 0)
die_errno(_("Could not open %s."), todo_file);
@@ -792,10 +812,14 @@ int sequencer_pick_revisions(struct replay_opts *opts)
remove_sequencer_state(1);
return 0;
} else if (opts->subcommand == REPLAY_CONTINUE) {
- if (!file_exists(git_path(SEQ_TODO_FILE)))
- goto error;
+ if (!file_exists(git_path(SEQ_TODO_FILE))) {
+ if (!file_exists(git_path("CHERRY_PICK_HEAD")))
+ goto error;
+ read_populate_todo(&todo_list, 1);
+ }
+ else
+ read_populate_todo(&todo_list, 0);
read_populate_opts(&opts);
- read_populate_todo(&todo_list);
if (!index_differs_from("HEAD", 0))
todo_list = todo_list->next;
diff --git a/t/t3510-sequencer.sh b/t/t3510-sequencer.sh
index 65f2724..f7c3a37 100755
--- a/t/t3510-sequencer.sh
+++ b/t/t3510-sequencer.sh
@@ -45,6 +45,50 @@ test_expect_success '--continue complains when there are unresolved conflicts' '
test_must_fail git sequencer --continue
'
+test_expect_success '--continue continues single-commit cherry-pick' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--continue continues single-commit revert' '
+ pristine_detach initial &&
+ test_must_fail git revert anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git sequencer --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success '--continue continues after conflicts are resolved' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] New sequencer workflow!
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
` (6 preceding siblings ...)
2011-11-13 10:46 ` [PATCH 7/7] sequencer: teach parser about CHERRY_PICK_HEAD Ramkumar Ramachandra
@ 2011-11-13 20:56 ` Junio C Hamano
2011-11-14 0:04 ` Junio C Hamano
2011-11-15 15:46 ` Phil Hord
8 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2011-11-13 20:56 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jonathan Nieder, Christian Couder
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> This series preserves the old workflow:
>
> $ git cherry-pick foo
> ... conflict ...
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git commit
>
> $ git cherry-pick foo..bar
> ... conflict ...
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git commit
> $ git revert --continue # No, there are no typos
Hmm, doesn't "git add && git cherry-pick --continue" (i.e. no "git commit"
in between) trigger the "commit the resolution first and then go on"?
> And introduces a brand new workflow:
>
> $ git cherry-pick foo
> ... conflict ...
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git sequencer --continue
>
> $ git cherry-pick foo..bar
> ... conflict ...
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git sequencer --continue
>
> $ git revert moo..baz
> ... conflict ...
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git sequencer --continue
Almost. If these are replaced with "git cherry-pick --continue" and "git
revert --continue" that internally triggers "git sequencer --continue"
*and* errors out if a wrong command was used to "--continue", it would be
perfect.
The longer-term ultimate goal would be to make it "git continue" that
covers more than the sequencer based workflow, i.e. allow "git merge" that
conflicted to be concluded with "edit && git add && git continue".
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/7] sequencer: handle single-commit pick as special case
2011-11-13 10:46 ` [PATCH 3/7] sequencer: handle single-commit pick as special case Ramkumar Ramachandra
@ 2011-11-13 23:23 ` Junio C Hamano
2011-11-15 8:47 ` Ramkumar Ramachandra
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2011-11-13 23:23 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jonathan Nieder, Christian Couder
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Prior to v1.7.2-rc1~4^2~7 (revert: allow cherry-picking more than one
> commit, 2010-06-02), 'git cherry-pick' could only pick one commit at a
> time, and it used '.git/CHERRY_PICK_HEAD' to pass on information to a
> subsequent invocation in case of a conflict. While
> '.git/CHERRY_PICK_HEAD' can only keep information about one commit,
> the sequencer uses '.git/sequencer' to persist information in the
> general case.
>
> A problem arises because a single-commit cherry-pick operation can be
> completed successfully using 'git commit'. This removes
> '.git/CHERRY_PICK_HEAD' without informing the sequencer, leaving
> behind a stale sequencer state as a result. We have worked around
> this problem already by prematurely removing the sequencer state in
> d3f4628e (revert: Remove sequencer state when no commits are pending,
> 2011-06-06). However, this gets in the way of our future plan to
> eliminate a glaring workflow inconsistency:
>
> $ git cherry-pick foo
> ... .git/sequencer is created ....
> ... .git/CHERRY_PICK_HEAD is created ...
> ... conflict ...
> .... .git/sequencer is prematurely removed ...
Isn't the real problem that .git/sequencer is created in the first place,
when this form of the command knows it will use CHERRY_PICK_HEAD?
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git commit
> ... .git/CHERRY_PICK_HEAD is removed ...
> $ git cherry-pick --continue
> error: No cherry-pick in progress
This is the right thing to happen, no? There is no in-progress cherry-pick
anymore after that commit. The user said "I want to replay this commit",
the command couldn't finish it by itself and asked the user to help
resolving the conflict, and the user resolved and recorded the result.
It is a different story if the sequence were like this:
$ git cherry-pick foo
... conflict happens
$ edit problematicfile
$ git add problematicfile
$ git cherry-pick --continue
... This should notice CHERRY_PICK_HEAD and record it.
... After that, there is nothing remaining to be done.
In other words, the user said "I want to replay this commit", the command
couldn't finish it by itself and asked the user to help resolving the
conflict, the user resolved and told the command to continue. The command
should continue recording the result.
And if "continue" does not work in this sequence, that is a bug worth
fixing.
> $ git cherry-pick foo..bar
> ... .git/sequencer is created ....
> ... CHERRY_PICK_HEAD is created ...
> ... conflict in bar~1 ...
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git commit
> ... CHERRY_PICK_HEAD is removed ...
> $ git cherry-pick --continue # Success!
This again is the right thing to happen, as the user had to help the
command while replaying bar~1 and then told it to continue, which
successfully replayed bar.
I do not see an inconsistency here, let alone any "glaring" one.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] New sequencer workflow!
2011-11-13 20:56 ` [PATCH 0/7] New sequencer workflow! Junio C Hamano
@ 2011-11-14 0:04 ` Junio C Hamano
2011-11-15 8:33 ` Ramkumar Ramachandra
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2011-11-14 0:04 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jonathan Nieder, Christian Couder
Junio C Hamano <gitster@pobox.com> writes:
> Almost. If these are replaced with "git cherry-pick --continue" and "git
> revert --continue" that internally triggers "git sequencer --continue"
> *and* errors out if a wrong command was used to "--continue", it would be
> perfect.
>
> The longer-term ultimate goal would be to make it "git continue" that
> covers more than the sequencer based workflow, i.e. allow "git merge" that
> conflicted to be concluded with "edit && git add && git continue".
To clarify a bit.
While I do not mind "sequencer --continue" as a step to get us closer to a
more pleasant user experience at the implementation level, exposing the
name "sequencer" or having the user to type "sequencer --continue" is going
in a very wrong direction at the UI level.
There are many commands in the Git suite that take an order from the user,
attempt to perform the necessary operation but stops in the middle to ask
for help from the user. "cherry-pick" and "revert" are two of them, and
there are many others: e.g. am, merge, pull, rebase, "rebase -i". They use
different mechanisms to keep track of their states before giving the
control back to the user when they ask for help.
The original workflow was "pull; edit && add && commit", and it is very
unlikely this will change while we are still in Git 1.X series. The
original single commit variants of "cherry-pick" and "revert" are in the
same category. We would need to keep supporting "cherry-pick/revert; edit
&& add && commit" as a workflow for a while.
Others that deal with more than one stop-point follow a different pattern
from necessity. The user tells the same command to continue after the
command asks for help in "am; edit && add && am --continue" and "rebase
[-i]; edit && add && rebase --continue" sequence. Multi-commit variants of
"cherry-pick" and "revert" take the same approach for consistency.
In the shorter term, a person new to Git, after learning "run command X, X
gives back control asking for help, help X by editing and adding, and
telling X to continue" pattern from these commands, will eventually find
that the commands in single stop-point category (i.e. "pull", "merge" and
single-commit "cherry-pick" and "revert") inconsistent from others that
take "--continue" to resume. For this reason, "git cherry-pick --continue"
that would work even when picking a single commit like this would be
beneficial:
$ git cherry-pick X
... conflicts
$ edit && add
$ git cherry-pick --continue
That is, no "commit" by the user. The "helping" part is literally that;
the user helps Git because Git is too stupid to read the user's mind to
come up with a perfect conflict resolution. Git knows, given a correct
resolution, how to make a commit out of it perfectly fine and does not
need help from the user to commit the result.
In the medium term, extending the above "shorter term" goal, it would make
sense to support (but not necessarily require) the following flow for
consistency:
$ git pull ;# or "git merge branch"
... conflicts
$ edit && add ;# again, no "commit"
$ git pull --continue ;# or "git merge --continue"
Now, if you rename "cherry-pick --continue" and "revert --continue" to
"sequencer --continue", what message are you sending to the users? They
now need to learn that only these two commands are based on something
called "sequencer", can be resumed with "sequencer --continue", but other
commands need to be continued with "X --continue".
That is totally backwards, no? You are _adding_ mental burden to your
users by introducing another thing or two they need to learn about.
In the longer term (now we are talking about Git 2.X version bump), it
would be ideal if all the "git X --continue" can be consolidated to a
single "git continue" command (and "git abort" to give up the sequence
of operations).
Given that bash-completion script can tell in what state the repository
is, I think this is doable. "git continue" may invoke your implementation
of "git sequencer --continue" internally when it detects that the state is
something the "sequencer" machinery knows how to continue, and if it is in
the middle of conflicted "am -3" or rejected "am", the command may invoke
"git am --continue" instead.
That way, the user does not have to learn which command can be resumed
with "sequencer --continue" and which other command needs to be called
with "--continue" to resume. The user does not even need to know there is
a mechanism called sequencer, some commands are already using it while
others are not yet using it, and these other commands are in the process
of being rewritten to use the sequencer machinery.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] New sequencer workflow!
2011-11-14 0:04 ` Junio C Hamano
@ 2011-11-15 8:33 ` Ramkumar Ramachandra
0 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-15 8:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Jonathan Nieder
Hi Junio,
Thanks for the fantastic review. My lack of foresight is very disturbing.
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> This series preserves the old workflow:
>> [...]
> Hmm, doesn't "git add && git cherry-pick --continue" (i.e. no "git commit"
> in between) trigger the "commit the resolution first and then go on"?
Ugh, right. I can't possibly preserve the old workflow fully- what I
meant is that users can still "git commit" before continuing (in the
case of a multi-commit operation). It's not so much about preserving
a workflow, as it is about not breaking existing tests.
> Almost. If these are replaced with "git cherry-pick --continue" and "git
> revert --continue" that internally triggers "git sequencer --continue"
> *and* errors out if a wrong command was used to "--continue", it would be
> perfect.
Okay, there are two ways to fix this:
1. If the first instruction in the instruction sheet is a "revert" and
"git cherry-pick --continue" is called or viceversa, error out. The
downside is that there's really no sane way to handle mixed "pick" and
"revert" instructions in the sheet using this approach: parsing the
whole sheet in advance is a total overkill.
2. The key issue is that the sequencer doesn't keep track of which
command was actually executed (argv[0]): fix this by creating a new
'.git/sequencer/cmd' to keep this information.
I think we should pick the latter option. Also, why should "git
<cherry-pick|revert> --continue" invoke "git sequencer --continue"
when it can just call into the corresponding function in the sequencer
library (isn't lib'ification one of the issues we're trying to
address)?
> While I do not mind "sequencer --continue" as a step to get us closer to a
> more pleasant user experience at the implementation level, exposing the
> name "sequencer" or having the user to type "sequencer --continue" is going
> in a very wrong direction at the UI level.
> [...]
> Now, if you rename "cherry-pick --continue" and "revert --continue" to
> "sequencer --continue", what message are you sending to the users? They
> now need to learn that only these two commands are based on something
> called "sequencer", can be resumed with "sequencer --continue", but other
> commands need to be continued with "X --continue".
Thanks for the superb explanation- it's plain as day now. In a
nutshell: it makes little sense to have a grand plan about an
all-encompassing "git sequencer --continue"; even if that is the plan
in the super-long term (although "git continue" is a better idea), it
makes no sense to burden the user now with additional UI
inconsistencies.
I'm still a little confused about what to do with the "git sequencer"
builtin though: how do we prevent users from being confused by it- by
not advertising it?
> The original workflow was "pull; edit && add && commit", and it is very
> unlikely this will change while we are still in Git 1.X series. The
> original single commit variants of "cherry-pick" and "revert" are in the
> same category. We would need to keep supporting "cherry-pick/revert; edit
> && add && commit" as a workflow for a while.
True. I wrote this pretending that only "git <cherry-pick|revert>"
was stuck with this UI inconsistency between one-commit picks and
many-commit picks.
> In the shorter term, a person new to Git, after learning "run command X, X
> gives back control asking for help, help X by editing and adding, and
> telling X to continue" pattern from these commands, will eventually find
> that the commands in single stop-point category (i.e. "pull", "merge" and
> single-commit "cherry-pick" and "revert") inconsistent from others that
> take "--continue" to resume. For this reason, "git cherry-pick --continue"
> that would work even when picking a single commit like this would be
> beneficial:
> [...]
Interesting aside: this final detail is fixed only in the last patch
in the series; that's how deep the problem is.
> In the longer term (now we are talking about Git 2.X version bump), it
> would be ideal if all the "git X --continue" can be consolidated to a
> single "git continue" command (and "git abort" to give up the sequence
> of operations).
>
> Given that bash-completion script can tell in what state the repository
> is, I think this is doable. "git continue" may invoke your implementation
> of "git sequencer --continue" internally when it detects that the state is
> something the "sequencer" machinery knows how to continue, and if it is in
> the middle of conflicted "am -3" or rejected "am", the command may invoke
> "git am --continue" instead.
Makes perfect sense.
Thanks.
-- Ram
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/7] sequencer: handle single-commit pick as special case
2011-11-13 23:23 ` Junio C Hamano
@ 2011-11-15 8:47 ` Ramkumar Ramachandra
2011-11-15 21:58 ` Junio C Hamano
0 siblings, 1 reply; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-15 8:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Jonathan Nieder
Hi Junio,
Junio C Hamano writes:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> [...]
> [...]
> I do not see an inconsistency here, let alone any "glaring" one.
Yeah, my commit message is totally misleading and unclear. Yes, all
the operations make sense to you and me; for a new user who doesn't
understand how Git works, it's completely inconsistent at a very
superficial level, no? The sequence of operations he has to execute
depends on:
1. If he literally specified a single commit or a commit range on the
command-line.
2. In the case of multi-commit picking, there's an additional layer of
decision making: did the conflict occur in the last commit in the
range?
Anyway, I'll rewrite this commit message in the next iteration.
Thanks.
-- Ram
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] New sequencer workflow!
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
` (7 preceding siblings ...)
2011-11-13 20:56 ` [PATCH 0/7] New sequencer workflow! Junio C Hamano
@ 2011-11-15 15:46 ` Phil Hord
2011-11-15 16:12 ` Ramkumar Ramachandra
8 siblings, 1 reply; 17+ messages in thread
From: Phil Hord @ 2011-11-15 15:46 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Jonathan Nieder, Christian Couder
On Sun, Nov 13, 2011 at 5:46 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> $ git revert moo..baz
> ... conflict ...
> $ echo "resolved" >problematicfile
> $ git add problematicfile
> $ git sequencer --continue
>
> In other words, it just doesn't matter how you started what sequencing
> operation: you can always continue a it with "git sequencer
> --continue" and reset it with "git sequencer --reset".
I see that --reset was added to cherry-pick, revert and sequencer
around the same time back in August. Shouldn't it be spelled
"--abort" instead?
This would align better with other commands that do the same thing
(am, merge, notes-merge, rebase) since they all appear to spell it
"--abort". Also, it will make it easier in git 2.x when we begin
using "git continue" and "git abort" rather than "git reset".
Phil
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] New sequencer workflow!
2011-11-15 15:46 ` Phil Hord
@ 2011-11-15 16:12 ` Ramkumar Ramachandra
0 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-15 16:12 UTC (permalink / raw)
To: Phil Hord; +Cc: Git List, Junio C Hamano, Jonathan Nieder, Christian Couder
Hi Phil,
Phil Hord wrote:
> I see that --reset was added to cherry-pick, revert and sequencer
> around the same time back in August. Shouldn't it be spelled
> "--abort" instead?
"reset" is actually different from "abort": it simply removes the
sequencer state without touching the worktree, index or HEAD. We
decided that this would be a nice low-level command to implement
(since I find myself doing `rm -rf .git/rebase-todo` sometimes).
Sure, "abort" and a lot of other porcelain would be nice- we can
always implement them carefully later :)
Thanks.
-- Ram
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/7] sequencer: handle single-commit pick as special case
2011-11-15 8:47 ` Ramkumar Ramachandra
@ 2011-11-15 21:58 ` Junio C Hamano
2011-11-16 6:30 ` Ramkumar Ramachandra
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2011-11-15 21:58 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jonathan Nieder
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Hi Junio,
>
> Junio C Hamano writes:
>> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>> [...]
>> [...]
>> I do not see an inconsistency here, let alone any "glaring" one.
>
> Yeah, my commit message is totally misleading and unclear. Yes, all
> the operations make sense to you and me; for a new user who doesn't
> understand how Git works, it's completely inconsistent at a very
> superficial level, no? The sequence of operations he has to execute
> depends on:
> 1. If he literally specified a single commit or a commit range on the
> command-line.
But we are already in agreement on this point, I think. Didn't I say:
It is a different story if the sequence were like this:
$ git cherry-pick foo
... conflict happens
$ edit problematicfile
$ git add problematicfile
$ git cherry-pick --continue
... This should notice CHERRY_PICK_HEAD and record it.
... After that, there is nothing remaining to be done.
In other words, the user said "I want to replay this commit", the command
couldn't finish it by itself and asked the user to help resolving the
conflict, the user resolved and told the command to continue. The command
should continue recording the result.
And if "continue" does not work in this sequence, that is a bug worth
fixing.
in the message you are responding to?
> 2. In the case of multi-commit picking, there's an additional layer of
> decision making: did the conflict occur in the last commit in the
> range?
Again, it would be the same thing. If the implementation forces that
decision, that would be a bug, no?
My understanding is that multi-commit form of cherry-pick and revert
intended to allow two forms of "user done helping and telling the command
to continue" at any stage, be it the first, in the middle, or the last
operation in a series:
- User resolves, adds and commits, and then tells the command to
continue. The command notices that the user has committed, and goes on
to the next task; or
- User resolves, adds, and then tells the command to continue. The
command notices that the user has not committed, makes a commit and
goes on to the next task.
I think "rebase -i" works the same way and both are valid ways to tell the
command to continue.
Now, multi-commit form of cherry-pick/revert _may_ be buggy (I do not use
that form often myself to know if this is the case, though) and may only
allow one or the other depending on which commit in the series the user is
asked to help, but if that is the case it is a bug in the implementation
of the multi-commit form done back in v1.7.2 timeframe.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/7] sequencer: handle single-commit pick as special case
2011-11-15 21:58 ` Junio C Hamano
@ 2011-11-16 6:30 ` Ramkumar Ramachandra
0 siblings, 0 replies; 17+ messages in thread
From: Ramkumar Ramachandra @ 2011-11-16 6:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Jonathan Nieder
Hi Junio,
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> [...]
> But we are already in agreement on this point, I think. Didn't I say:
Yes, yes. I was just thinking out loud so it'll help me write a new
commit message. Sorry for the noise.
>> 2. In the case of multi-commit picking, there's an additional layer of
>> decision making: did the conflict occur in the last commit in the
>> range?
>
> Again, it would be the same thing. If the implementation forces that
> decision, that would be a bug, no?
>
> My understanding is that multi-commit form of cherry-pick and revert
> intended to allow two forms of "user done helping and telling the command
> to continue" at any stage, be it the first, in the middle, or the last
> operation in a series:
>
> - User resolves, adds and commits, and then tells the command to
> continue. The command notices that the user has committed, and goes on
> to the next task; or
>
> - User resolves, adds, and then tells the command to continue. The
> command notices that the user has not committed, makes a commit and
> goes on to the next task.
Yep, this is exactly how it'll work after the series :D
Thanks.
-- Ram
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-11-16 6:30 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-13 10:46 [PATCH 0/7] New sequencer workflow! Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 1/7] sequencer: factor code out of revert builtin Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 2/7] sequencer: invalidate sequencer state without todo Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 3/7] sequencer: handle single-commit pick as special case Ramkumar Ramachandra
2011-11-13 23:23 ` Junio C Hamano
2011-11-15 8:47 ` Ramkumar Ramachandra
2011-11-15 21:58 ` Junio C Hamano
2011-11-16 6:30 ` Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 4/7] sequencer: handle cherry-pick conflict in last commit Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 5/7] sequencer: introduce git-sequencer builtin Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 6/7] sequencer: teach '--continue' how to commit Ramkumar Ramachandra
2011-11-13 10:46 ` [PATCH 7/7] sequencer: teach parser about CHERRY_PICK_HEAD Ramkumar Ramachandra
2011-11-13 20:56 ` [PATCH 0/7] New sequencer workflow! Junio C Hamano
2011-11-14 0:04 ` Junio C Hamano
2011-11-15 8:33 ` Ramkumar Ramachandra
2011-11-15 15:46 ` Phil Hord
2011-11-15 16:12 ` Ramkumar Ramachandra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).