From bc84a7fbac4ce85bb93eeb57b5cb39548d286ad0 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 31 Mar 2010 21:22:04 +0200 Subject: [PATCH 1/5] revert: use strbuf to refactor the code that writes the merge message The code in this commit was written by Stephan Beyer for the sequencer GSoC project: git://repo.or.cz/git/sbeyer.git Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/revert.c | 70 +++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 1ddfac15b55..944c39a8ee9 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -157,28 +157,17 @@ static char *get_encoding(const char *message) return NULL; } -static struct lock_file msg_file; -static int msg_fd; - -static void add_to_msg(const char *string) -{ - int len = strlen(string); - if (write_in_full(msg_fd, string, len) < 0) - die_errno ("Could not write to MERGE_MSG"); -} - -static void add_message_to_msg(const char *message) +static void add_message_to_msg(struct strbuf *msgbuf, const char *message) { const char *p = message; while (*p && (*p != '\n' || p[1] != '\n')) p++; if (!*p) - add_to_msg(sha1_to_hex(commit->object.sha1)); + strbuf_addstr(msgbuf, sha1_to_hex(commit->object.sha1)); p += 2; - add_to_msg(p); - return; + strbuf_addstr(msgbuf, p); } static void set_author_ident_env(const char *message) @@ -254,6 +243,19 @@ static char *help_msg(const char *name) return strbuf_detach(&helpbuf, NULL); } +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) { struct tree *tree = xcalloc(1, sizeof(struct tree)); @@ -289,6 +291,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) struct merge_options o; struct tree *result, *next_tree, *base_tree, *head_tree; static struct lock_file index_lock; + struct strbuf msgbuf = STRBUF_INIT; git_config(git_default_config, NULL); me = action == REVERT ? "revert" : "cherry-pick"; @@ -363,35 +366,32 @@ static int revert_or_cherry_pick(int argc, const char **argv) * reverse of it if we are revert. */ - msg_fd = hold_lock_file_for_update(&msg_file, defmsg, - LOCK_DIE_ON_ERROR); - if (action == REVERT) { base = commit; base_label = msg.label; next = parent; next_label = msg.parent_label; - add_to_msg("Revert \""); - add_to_msg(msg.subject); - add_to_msg("\"\n\nThis reverts commit "); - add_to_msg(sha1_to_hex(commit->object.sha1)); + 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->next) { - add_to_msg(", reversing\nchanges made to "); - add_to_msg(sha1_to_hex(parent->object.sha1)); + strbuf_addstr(&msgbuf, ", reversing\nchanges made to "); + strbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1)); } - add_to_msg(".\n"); + strbuf_addstr(&msgbuf, ".\n"); } else { base = parent; base_label = msg.parent_label; next = commit; next_label = msg.label; set_author_ident_env(msg.message); - add_message_to_msg(msg.message); + add_message_to_msg(&msgbuf, msg.message); if (no_replay) { - add_to_msg("(cherry picked from commit "); - add_to_msg(sha1_to_hex(commit->object.sha1)); - add_to_msg(")\n"); + strbuf_addstr(&msgbuf, "(cherry picked from commit "); + strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1)); + strbuf_addstr(&msgbuf, ")\n"); } } @@ -416,27 +416,25 @@ static int revert_or_cherry_pick(int argc, const char **argv) rollback_lock_file(&index_lock); if (!clean) { - add_to_msg("\nConflicts:\n\n"); + strbuf_addstr(&msgbuf, "\nConflicts:\n\n"); for (i = 0; i < active_nr;) { struct cache_entry *ce = active_cache[i++]; if (ce_stage(ce)) { - add_to_msg("\t"); - add_to_msg(ce->name); - add_to_msg("\n"); + 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++; } } - if (commit_lock_file(&msg_file) < 0) - die ("Error wrapping up %s", defmsg); + write_message(&msgbuf, defmsg); fprintf(stderr, "Automatic %s failed.%s\n", me, help_msg(commit_name)); rerere(allow_rerere_auto); exit(1); } - if (commit_lock_file(&msg_file) < 0) - die ("Error wrapping up %s", defmsg); + write_message(&msgbuf, defmsg); fprintf(stderr, "Finished one %s.\n", me); /* From ae8c79fd8fe47bfebf56ae83461cbedb934cff92 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 31 Mar 2010 21:22:05 +0200 Subject: [PATCH 2/5] revert: refactor merge recursive code into its own function The code that is used to do a recursive merge is extracted from the revert_or_cherry_pick() function and put into a new do_recursive_merge() function. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/revert.c | 106 ++++++++++++++++++++++++++--------------------- 1 file changed, 58 insertions(+), 48 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index 944c39a8ee9..f4ad0fc971b 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -279,18 +279,69 @@ static NORETURN void die_dirty_index(const char *me) } } +static void do_recursive_merge(struct commit *base, struct commit *next, + const char *base_label, const char *next_label, + unsigned char *head, struct strbuf *msgbuf, + char *defmsg) +{ + struct merge_options o; + struct tree *result, *next_tree, *base_tree, *head_tree; + int clean, index_fd; + 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(); + + 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))) + die("%s: Unable to write new index file", me); + 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++; + } + } + write_message(msgbuf, defmsg); + fprintf(stderr, "Automatic %s failed.%s\n", + me, help_msg(commit_name)); + rerere(allow_rerere_auto); + exit(1); + } + write_message(msgbuf, defmsg); + fprintf(stderr, "Finished one %s.\n", me); +} + static int revert_or_cherry_pick(int argc, const char **argv) { unsigned char head[20]; struct commit *base, *next, *parent; const char *base_label, *next_label; - int i, index_fd, clean; struct commit_message msg = { NULL, NULL, NULL, NULL, NULL }; - char *defmsg = git_pathdup("MERGE_MSG"); - struct merge_options o; - struct tree *result, *next_tree, *base_tree, *head_tree; - static struct lock_file index_lock; struct strbuf msgbuf = STRBUF_INIT; git_config(git_default_config, NULL); @@ -321,8 +372,6 @@ static int revert_or_cherry_pick(int argc, const char **argv) } discard_cache(); - index_fd = hold_locked_index(&index_lock, 1); - if (!commit->parents) { if (action == REVERT) die ("Cannot revert a root commit"); @@ -395,47 +444,8 @@ static int revert_or_cherry_pick(int argc, const char **argv) } } - 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(); - - 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))) - die("%s: Unable to write new index file", me); - rollback_lock_file(&index_lock); - - if (!clean) { - 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++; - } - } - write_message(&msgbuf, defmsg); - fprintf(stderr, "Automatic %s failed.%s\n", - me, help_msg(commit_name)); - rerere(allow_rerere_auto); - exit(1); - } - write_message(&msgbuf, defmsg); - fprintf(stderr, "Finished one %s.\n", me); + do_recursive_merge(base, next, base_label, next_label, + head, &msgbuf, defmsg); /* * From 3f9083cde3b434155c274168f166ffce2bb243e7 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 31 Mar 2010 21:22:06 +0200 Subject: [PATCH 3/5] merge: refactor code that calls "git merge-STRATEGY" In the try_merge_strategy() function, when the strategy is "recursive" or "subtree", the merge_recursive() function is called. Otherwise we launch a "git merge-STRATEGY" process. To make it possible to reuse code that launches a "git merge-STRATEGY" process, this patch refactors this code into a new try_merge_command() function. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/merge.c | 81 +++++++++++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index 3aaec7bed76..ff875f1f050 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -548,13 +548,53 @@ static void write_tree_trivial(unsigned char *sha1) die("git write-tree failed to write a tree"); } -static int try_merge_strategy(const char *strategy, struct commit_list *common, - const char *head_arg) +static int try_merge_command(const char *strategy, struct commit_list *common, + const char *head_arg, struct commit_list *remotes) { const char **args; int i = 0, x = 0, ret; struct commit_list *j; struct strbuf buf = STRBUF_INIT; + + args = xmalloc((4 + xopts_nr + commit_list_count(common) + + commit_list_count(remotes)) * sizeof(char *)); + strbuf_addf(&buf, "merge-%s", strategy); + args[i++] = buf.buf; + for (x = 0; x < xopts_nr; x++) { + char *s = xmalloc(strlen(xopts[x])+2+1); + strcpy(s, "--"); + strcpy(s+2, xopts[x]); + args[i++] = s; + } + for (j = common; j; j = j->next) + args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1)); + args[i++] = "--"; + args[i++] = head_arg; + for (j = remotes; j; j = j->next) + args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1)); + args[i] = NULL; + ret = run_command_v_opt(args, RUN_GIT_CMD); + strbuf_release(&buf); + i = 1; + for (x = 0; x < xopts_nr; x++) + free((void *)args[i++]); + for (j = common; j; j = j->next) + free((void *)args[i++]); + i += 2; + for (j = remotes; j; j = j->next) + free((void *)args[i++]); + free(args); + discard_cache(); + if (read_cache() < 0) + die("failed to read the cache"); + resolve_undo_clear(); + + return ret; +} + +static int try_merge_strategy(const char *strategy, struct commit_list *common, + const char *head_arg) +{ int index_fd; struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); @@ -567,12 +607,13 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common, rollback_lock_file(lock); if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) { - int clean; + int clean, x; struct commit *result; struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int index_fd; struct commit_list *reversed = NULL; struct merge_options o; + struct commit_list *j; if (remoteheads->next) { error("Not handling anything other than two heads merge."); @@ -612,39 +653,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common, rollback_lock_file(lock); return clean ? 0 : 1; } else { - args = xmalloc((4 + xopts_nr + commit_list_count(common) + - commit_list_count(remoteheads)) * sizeof(char *)); - strbuf_addf(&buf, "merge-%s", strategy); - args[i++] = buf.buf; - for (x = 0; x < xopts_nr; x++) { - char *s = xmalloc(strlen(xopts[x])+2+1); - strcpy(s, "--"); - strcpy(s+2, xopts[x]); - args[i++] = s; - } - for (j = common; j; j = j->next) - args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1)); - args[i++] = "--"; - args[i++] = head_arg; - for (j = remoteheads; j; j = j->next) - args[i++] = xstrdup(sha1_to_hex(j->item->object.sha1)); - args[i] = NULL; - ret = run_command_v_opt(args, RUN_GIT_CMD); - strbuf_release(&buf); - i = 1; - for (x = 0; x < xopts_nr; x++) - free((void *)args[i++]); - for (j = common; j; j = j->next) - free((void *)args[i++]); - i += 2; - for (j = remoteheads; j; j = j->next) - free((void *)args[i++]); - free(args); - discard_cache(); - if (read_cache() < 0) - die("failed to read the cache"); - resolve_undo_clear(); - return ret; + return try_merge_command(strategy, common, head_arg, remoteheads); } } From c674d0527396e4100454e8992ed7381ee29dd54e Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 31 Mar 2010 21:22:07 +0200 Subject: [PATCH 4/5] merge: make function try_merge_command non static Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/merge.c | 4 ++-- merge-recursive.h | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index ff875f1f050..64c6c6f393f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -548,8 +548,8 @@ static void write_tree_trivial(unsigned char *sha1) die("git write-tree failed to write a tree"); } -static int try_merge_command(const char *strategy, struct commit_list *common, - const char *head_arg, struct commit_list *remotes) +int try_merge_command(const char *strategy, struct commit_list *common, + const char *head_arg, struct commit_list *remotes) { const char **args; int i = 0, x = 0, ret; diff --git a/merge-recursive.h b/merge-recursive.h index d1192f56d79..0cc465ec5d1 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -54,4 +54,7 @@ int merge_recursive_generic(struct merge_options *o, void init_merge_options(struct merge_options *o); struct tree *write_tree_from_memory(struct merge_options *o); +/* builtin/merge.c */ +int try_merge_command(const char *strategy, struct commit_list *common, const char *head_arg, struct commit_list *remotes); + #endif From 91e525989665d69ace11b8f39618b1e8004fe19a Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 31 Mar 2010 21:22:08 +0200 Subject: [PATCH 5/5] revert: add "--strategy" option to choose merge strategy This patch makes it possible to use a different merge strategy when cherry-picking. This is usefull mainly for debugging purposes as it allows to see if some failures are caused by the merge strategy used or not. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- builtin/revert.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/builtin/revert.c b/builtin/revert.c index f4ad0fc971b..b70f4b0af36 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -42,6 +42,7 @@ static const char *commit_name; static int allow_rerere_auto; static const char *me; +static const char *strategy; #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -61,6 +62,7 @@ static void parse_args(int argc, const char **argv) OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"), OPT_INTEGER('m', "mainline", &mainline, "parent number"), OPT_RERERE_AUTOUPDATE(&allow_rerere_auto), + OPT_STRING(0, "strategy", &strategy, "strategy", "merge strategy"), OPT_END(), }; @@ -444,8 +446,27 @@ static int revert_or_cherry_pick(int argc, const char **argv) } } - do_recursive_merge(base, next, base_label, next_label, - head, &msgbuf, defmsg); + if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) + do_recursive_merge(base, next, base_label, next_label, + head, &msgbuf, defmsg); + else { + int res; + 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(strategy, common, + sha1_to_hex(head), remotes); + free_commit_list(common); + free_commit_list(remotes); + if (res) { + fprintf(stderr, "Automatic %s with strategy %s failed.%s\n", + me, strategy, help_msg(commit_name)); + rerere(allow_rerere_auto); + exit(1); + } + } /* *