From a4b5e91c49238146f4cb85ff5f7f3bc97e0e51de Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 20 Mar 2010 19:35:18 -0500 Subject: xdl_merge(): move file1 and file2 labels to xmparam structure The labels for the three participants in a potential conflict are all optional arguments for the xdiff merge routine; if they are NULL, then xdl_merge() can cope by omitting the labels from its output. Move them to the xmparam structure to allow new callers to save some keystrokes where they are not needed. This also has the virtue of making the xdiff merge interface more similar to merge_trees, which might make it easier to learn. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/merge-file.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'builtin') diff --git a/builtin/merge-file.c b/builtin/merge-file.c index 69cc68333..65eb790fd 100644 --- a/builtin/merge-file.c +++ b/builtin/merge-file.c @@ -77,8 +77,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) argv[i]); } - ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2], - &xmp, &result); + xmp.file1 = names[0]; + xmp.file2 = names[2]; + ret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result); for (i = 0; i < 3; i++) free(mmfs[i].ptr); -- cgit v1.2.1 From 4bb0936206b046a4fe606e9b0c9fe6ecc7a0ff69 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 20 Mar 2010 19:37:33 -0500 Subject: merge-file --diff3: add a label for ancestor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git merge-file --diff3 can be used to present conflicts hunks including text from the common ancestor. The added information is helpful for resolving a merge by hand, and merge tools can usually grok it because it looks like output from diff3 -m. However, ‘diff3’ includes a label for the merge base on the ||||||| line and some tools cannot parse conflict hunks without such a label. Write the base-name as passed in a -L option (or the name of the ancestor file by default) on that line. git rerere will not have trouble parsing this output, since instead of looking for a newline, it looks for whitespace after the ||||||| marker. Since rerere includes its own code for recreating conflict hunks, conflict identifiers are unaffected. No other code in git tries to parse conflict hunks. Requested-by: Stefan Monnier Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/merge-file.c | 1 + 1 file changed, 1 insertion(+) (limited to 'builtin') diff --git a/builtin/merge-file.c b/builtin/merge-file.c index 65eb790fd..610849a65 100644 --- a/builtin/merge-file.c +++ b/builtin/merge-file.c @@ -77,6 +77,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix) argv[i]); } + xmp.ancestor = names[1]; xmp.file1 = names[0]; xmp.file2 = names[2]; ret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result); -- cgit v1.2.1 From f01de62e4591a6acc061eb994d44f7eeef7a0cfd Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 20 Mar 2010 19:38:58 -0500 Subject: ll_merge(): add ancestor label parameter for diff3-style output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commands using the ll_merge() function will present conflict hunks imitating ‘diff3 -m’ output if the merge.conflictstyle configuration option is set appropriately. Unlike ‘diff3 -m’, the output does not include a label for the merge base on the ||||||| line of the output, and some tools misparse the conflict hunks without that. Add a new ancestor_label parameter to ll_merge() to give callers the power to rectify this situation. If ancestor_label is NULL, the output format is unchanged. All callers pass NULL for now. Requested-by: Stefan Monnier Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/checkout.c b/builtin/checkout.c index acefaaf41..d67f809a9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -149,7 +149,7 @@ static int checkout_merged(int pos, struct checkout *state) read_mmblob(&ours, active_cache[pos+1]->sha1); read_mmblob(&theirs, active_cache[pos+2]->sha1); - status = ll_merge(&result_buf, path, &ancestor, + status = ll_merge(&result_buf, path, &ancestor, NULL, &ours, "ours", &theirs, "theirs", 0); free(ancestor.ptr); free(ours.ptr); -- cgit v1.2.1 From f0531a2937e919e763ab05e9800aef51658104fc Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 20 Mar 2010 19:40:19 -0500 Subject: checkout --conflict=diff3: add a label for ancestor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git checkout --conflict=diff3 can be used to present conflicts hunks including text from the common ancestor: <<<<<<< ours ourside ||||||| original ======= theirside >>>>>>> theirs The added information is helpful for resolving a merge by hand, and merge tools can usually understand it without trouble because it looks like output from ‘diff3 -m’. diff3 includes a label for the merge base on the ||||||| line, and it seems some tools (for example, Emacs 22’s smerge-mode) cannot parse conflict hunks without such a label. Humans could use help in interpreting the output, too. So change the marker for the start of the text from the common ancestor to include the label “base”. git rerere’s conflict identifiers are not affected: to parse conflict hunks, rerere looks for whitespace after the ||||||| marker rather than a newline, and to compute preimage ids, rerere has its own code for creating conflict hunks. No other code in git tries to parse conflict hunks. Requested-by: Stefan Monnier Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/checkout.c b/builtin/checkout.c index d67f809a9..d652b4c95 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -149,7 +149,7 @@ static int checkout_merged(int pos, struct checkout *state) read_mmblob(&ours, active_cache[pos+1]->sha1); read_mmblob(&theirs, active_cache[pos+2]->sha1); - status = ll_merge(&result_buf, path, &ancestor, NULL, + status = ll_merge(&result_buf, path, &ancestor, "base", &ours, "ours", &theirs, "theirs", 0); free(ancestor.ptr); free(ours.ptr); -- cgit v1.2.1 From c4151629e7019d072cefdac992e164820ce9ed1c Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 20 Mar 2010 19:42:51 -0500 Subject: checkout -m --conflict=diff3: add a label for ancestor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git checkout --merge --conflict=diff3 can be used to present conflict hunks including text from the common ancestor. The added information is helpful for resolving a merge by hand, and merge tools tend to understand it because it is very similar to what ‘diff3 -m’ produces. Unlike current git, diff3 -m includes a label for the merge base on the ||||||| line, and unfortunately, some tools cannot parse the conflict hunks without it. Humans can benefit from a cue when learning to interpreting the format, too. Mark the start of the text from the old branch with a label based on the branch’s name. git rerere does not have trouble parsing this output and its preimage ids are unchanged since it includes its own code for recreating conflict hunks. No other code in git tries to parse conflict hunks. Requested-by: Stefan Monnier Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/checkout.c | 1 + 1 file changed, 1 insertion(+) (limited to 'builtin') diff --git a/builtin/checkout.c b/builtin/checkout.c index d652b4c95..88b1f43e0 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -439,6 +439,7 @@ static int merge_working_tree(struct checkout_opts *opts, ret = reset_tree(new->commit->tree, opts, 1); if (ret) return ret; + o.ancestor = old->name; o.branch1 = new->name; o.branch2 = "local"; merge_trees(&o, new->commit->tree, work, -- cgit v1.2.1 From d68565402a69fac3fe0e0653718feca7c80c178b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 20 Mar 2010 19:45:21 -0500 Subject: revert: clarify label on conflict hunks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When reverting a commit, the commit being merged is not the commit to revert itself but its parent. Add “parent of” to the conflict hunk label to make this more clear. The conflict hunk labels are all pieces of a single string written in the new get_message() function. Avoid some complication by using mempcpy to advance a pointer as the result is written. Also free the corresponding temporary buffer (it was leaked before). This is not important because it is a small one-time allocation. It would become a memory leak if unnoticed when libifying revert. This patch uses calls to strlen() instead of integer constants in some places. GCC will compute the length at compile time; I am not sure about other compilers, but this is not performance-critical anyway. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 101 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 39 deletions(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index eff52687a..5a5b72112 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -45,6 +45,8 @@ static const char *me; #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" +static char *get_encoding(const char *message); + static void parse_args(int argc, const char **argv) { const char * const * usage_str = @@ -73,33 +75,64 @@ static void parse_args(int argc, const char **argv) exit(1); } -static char *get_oneline(const char *message) +struct commit_message { + char *parent_label; + const char *label; + const char *subject; + char *reencoded_message; + const char *message; +}; + +static int get_message(const char *raw_message, struct commit_message *out) { - char *result; - const char *p = message, *abbrev, *eol; + const char *encoding; + const char *p, *abbrev, *eol; + char *q; int abbrev_len, oneline_len; - if (!p) - die ("Could not read commit message of %s", - sha1_to_hex(commit->object.sha1)); + if (!raw_message) + return -1; + encoding = get_encoding(raw_message); + if (!encoding) + encoding = "UTF-8"; + if (!git_commit_encoding) + git_commit_encoding = "UTF-8"; + if ((out->reencoded_message = reencode_string(raw_message, + git_commit_encoding, encoding))) + out->message = out->reencoded_message; + + abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV); + abbrev_len = strlen(abbrev); + + /* Find beginning and end of commit subject. */ + p = out->message; while (*p && (*p != '\n' || p[1] != '\n')) p++; - if (*p) { p += 2; for (eol = p + 1; *eol && *eol != '\n'; eol++) ; /* do nothing */ } else eol = p; - abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV); - abbrev_len = strlen(abbrev); oneline_len = eol - p; - result = xmalloc(abbrev_len + 5 + oneline_len); - memcpy(result, abbrev, abbrev_len); - memcpy(result + abbrev_len, "... ", 4); - memcpy(result + abbrev_len + 4, p, oneline_len); - result[abbrev_len + 4 + oneline_len] = '\0'; - return result; + + out->parent_label = xmalloc(strlen("parent of ") + abbrev_len + + strlen("... ") + oneline_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, p, oneline_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) @@ -248,9 +281,10 @@ static int revert_or_cherry_pick(int argc, const char **argv) { unsigned char head[20]; struct commit *base, *next, *parent; + const char *next_label; int i, index_fd, clean; - char *oneline, *reencoded_message = NULL; - const char *message, *encoding; + 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; @@ -314,14 +348,14 @@ static int revert_or_cherry_pick(int argc, const char **argv) else parent = commit->parents->item; - if (!(message = commit->buffer)) - die ("Cannot get commit message for %s", - sha1_to_hex(commit->object.sha1)); - if (parent && parse_commit(parent) < 0) die("%s: cannot parse parent commit %s", me, sha1_to_hex(parent->object.sha1)); + if (get_message(commit->buffer, &msg) != 0) + die("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" @@ -332,24 +366,12 @@ static int revert_or_cherry_pick(int argc, const char **argv) msg_fd = hold_lock_file_for_update(&msg_file, defmsg, LOCK_DIE_ON_ERROR); - encoding = get_encoding(message); - if (!encoding) - encoding = "UTF-8"; - if (!git_commit_encoding) - git_commit_encoding = "UTF-8"; - if ((reencoded_message = reencode_string(message, - git_commit_encoding, encoding))) - message = reencoded_message; - - oneline = get_oneline(message); - if (action == REVERT) { - char *oneline_body = strchr(oneline, ' '); - base = commit; next = parent; + next_label = msg.parent_label; add_to_msg("Revert \""); - add_to_msg(oneline_body + 1); + add_to_msg(msg.subject); add_to_msg("\"\n\nThis reverts commit "); add_to_msg(sha1_to_hex(commit->object.sha1)); @@ -361,8 +383,9 @@ static int revert_or_cherry_pick(int argc, const char **argv) } else { base = parent; next = commit; - set_author_ident_env(message); - add_message_to_msg(message); + next_label = msg.label; + set_author_ident_env(msg.message); + add_message_to_msg(msg.message); if (no_replay) { add_to_msg("(cherry picked from commit "); add_to_msg(sha1_to_hex(commit->object.sha1)); @@ -373,7 +396,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) read_cache(); init_merge_options(&o); o.branch1 = "HEAD"; - o.branch2 = oneline; + o.branch2 = next ? next_label : "(empty tree)"; head_tree = parse_tree_indirect(head); next_tree = next ? next->tree : empty_tree(); @@ -437,7 +460,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) args[i] = NULL; return execv_git_cmd(args); } - free(reencoded_message); + free_message(&msg); free(defmsg); return 0; -- cgit v1.2.1 From bf975d379d79ab0eb9e8835af4abab4712e71dad Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 20 Mar 2010 19:46:07 -0500 Subject: cherry-pick, revert: add a label for ancestor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When writing conflict hunks in ‘diff3 -m’ format, also add a label to the common ancestor. Especially in a cherry-pick, it is not immediately obvious without such a label what the common ancestor represents. git rerere does not have trouble parsing the new output and its preimage ids are unchanged since it includes its own code for recreating conflict hunks. No other code in git parses conflict hunks. Requested-by: Stefan Monnier Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/revert.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/revert.c b/builtin/revert.c index 5a5b72112..1ddfac15b 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -281,7 +281,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) { unsigned char head[20]; struct commit *base, *next, *parent; - const char *next_label; + const char *base_label, *next_label; int i, index_fd, clean; struct commit_message msg = { NULL, NULL, NULL, NULL, NULL }; @@ -368,6 +368,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) if (action == REVERT) { base = commit; + base_label = msg.label; next = parent; next_label = msg.parent_label; add_to_msg("Revert \""); @@ -382,6 +383,7 @@ static int revert_or_cherry_pick(int argc, const char **argv) add_to_msg(".\n"); } else { base = parent; + base_label = msg.parent_label; next = commit; next_label = msg.label; set_author_ident_env(msg.message); @@ -395,6 +397,7 @@ 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)"; -- cgit v1.2.1