{"thread":{"id":"15481","subject":"[PATCH 3/3] t7603: add new testcases to ensure builtin-commit uses reduce_heads()","startedAt":"2008-09-10T20:10:30Z","lastAt":"2008-09-10T20:10:33Z","messageCount":4,"participants":["Miklos Vajna"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"90384","messageId":"cover.1221077214.git.vmiklos@frugalware.org","threadId":"15481","inReplyTo":null,"subject":"[PATCH 0/3] make builtin-commit use commit_tree() and reduce_heads()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-09-10T20:10:30Z","receivedAt":"2008-09-10T20:10:30Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Hi,\n\nOn Tue, Sep 09, 2008 at 05:52:37PM +0100, Russell King <rmk@arm.linux.org.uk> wrote:\n> Is this intentional, or is it a bug?\n\nIt is because builtin-commit does not use reduce_heads(), but\nbuiltin-merge does.\n\nHere are 3 patches to make builtin-commit use commit_tree() and\nreduce_heads() as well.\n\nThe first one just improves the commit_tree() interface.\n\nThe second one is a cleanup, but it also adds reduce_heads() usage, to\nfix the issue, pointed out by Russell King.\n\nThe third one adds new testcases, based on the ones provided by him.\n\nMiklos Vajna (3):\n  commit_tree(): add a new author parameter\n  builtin-commit: use commit_tree()\n  t7603: add new testcases to ensure builtin-commit uses reduce_heads()\n\n builtin-commit-tree.c         |    9 ++++--\n builtin-commit.c              |   63 +++++++++-------------------------------\n builtin-merge.c               |    4 +-\n builtin.h                     |    3 +-\n t/t7603-merge-reduce-heads.sh |   53 ++++++++++++++++++++++++++++++++++\n 5 files changed, 77 insertions(+), 55 deletions(-)\n"},{"id":"90382","messageId":"11d30964e7de652a790f5362f0b49e2a19de17ee.1221077214.git.vmiklos@frugalware.org","threadId":"15481","inReplyTo":"cover.1221077214.git.vmiklos@frugalware.org","subject":"[PATCH 1/3] commit_tree(): add a new author parameter","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-09-10T20:10:31Z","receivedAt":"2008-09-10T20:10:31Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"In case it's NULL, it is still determined automatically, but now you\nhave the ability to specify one yourself.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n builtin-commit-tree.c |    9 ++++++---\n builtin-merge.c       |    4 ++--\n builtin.h             |    3 ++-\n 3 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-commit-tree.c b/builtin-commit-tree.c\nindex 8a5ba4c..33c0e82 100644\n--- a/builtin-commit-tree.c\n+++ b/builtin-commit-tree.c\n@@ -46,7 +46,8 @@ static const char commit_utf8_warn[] =\n \"variable i18n.commitencoding to the encoding your project uses.\\n\";\n \n int commit_tree(const char *msg, unsigned char *tree,\n-\t\tstruct commit_list *parents, unsigned char *ret)\n+\t\tstruct commit_list *parents, unsigned char *ret,\n+\t\tconst char *author)\n {\n \tint result;\n \tint encoding_is_utf8;\n@@ -74,7 +75,9 @@ int commit_tree(const char *msg, unsigned char *tree,\n \t}\n \n \t/* Person/date information */\n-\tstrbuf_addf(&buffer, \"author %s\\n\", git_author_info(IDENT_ERROR_ON_NO_NAME));\n+\tif (!author)\n+\t\tauthor = git_author_info(IDENT_ERROR_ON_NO_NAME);\n+\tstrbuf_addf(&buffer, \"author %s\\n\", author);\n \tstrbuf_addf(&buffer, \"committer %s\\n\", git_committer_info(IDENT_ERROR_ON_NO_NAME));\n \tif (!encoding_is_utf8)\n \t\tstrbuf_addf(&buffer, \"encoding %s\\n\", git_commit_encoding);\n@@ -123,7 +126,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)\n \tif (strbuf_read(&buffer, 0, 0) < 0)\n \t\tdie(\"git commit-tree: read returned %s\", strerror(errno));\n \n-\tif (!commit_tree(buffer.buf, tree_sha1, parents, commit_sha1)) {\n+\tif (!commit_tree(buffer.buf, tree_sha1, parents, commit_sha1, NULL)) {\n \t\tprintf(\"%s\\n\", sha1_to_hex(commit_sha1));\n \t\treturn 0;\n \t}\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex 9ad9791..4a8ec60 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -691,7 +691,7 @@ static int merge_trivial(void)\n \tparent->next = xmalloc(sizeof(struct commit_list *));\n \tparent->next->item = remoteheads->item;\n \tparent->next->next = NULL;\n-\tcommit_tree(merge_msg.buf, result_tree, parent, result_commit);\n+\tcommit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);\n \tfinish(result_commit, \"In-index merge\");\n \tdrop_save();\n \treturn 0;\n@@ -720,7 +720,7 @@ static int finish_automerge(struct commit_list *common,\n \t}\n \tfree_commit_list(remoteheads);\n \tstrbuf_addch(&merge_msg, '\\n');\n-\tcommit_tree(merge_msg.buf, result_tree, parents, result_commit);\n+\tcommit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);\n \tstrbuf_addf(&buf, \"Merge made by %s.\", wt_strategy);\n \tfinish(result_commit, buf.buf);\n \tstrbuf_release(&buf);\ndiff --git a/builtin.h b/builtin.h\nindex e67cb20..8893b3c 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -17,7 +17,8 @@ extern int read_line_with_nul(char *buf, int size, FILE *file);\n extern int fmt_merge_msg(int merge_summary, struct strbuf *in,\n \tstruct strbuf *out);\n extern int commit_tree(const char *msg, unsigned char *tree,\n-\t\tstruct commit_list *parents, unsigned char *ret);\n+\t\tstruct commit_list *parents, unsigned char *ret,\n+\t\tconst char *author);\n extern int check_pager_config(const char *cmd);\n \n extern int cmd_add(int argc, const char **argv, const char *prefix);\n-- \n1.6.0.1\n"},{"id":"90383","messageId":"2984711bae2f0777b210ed04210693ac02164519.1221077214.git.vmiklos@frugalware.org","threadId":"15481","inReplyTo":"11d30964e7de652a790f5362f0b49e2a19de17ee.1221077214.git.vmiklos@frugalware.org","subject":"[PATCH 2/3] builtin-commit: use commit_tree()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-09-10T20:10:32Z","receivedAt":"2008-09-10T20:10:32Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"First, it adds less code than removes, second this allows us to use\nrecuce_heads() for parents, so that the parents of a merge will be\nalways the same with or without a conflict.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n builtin-commit.c |   63 ++++++++++++------------------------------------------\n 1 files changed, 14 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 8165bb3..ad055ea 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -670,11 +670,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n  * Find out if the message starting at position 'start' in the strbuf\n  * contains only whitespace and Signed-off-by lines.\n  */\n-static int message_is_empty(struct strbuf *sb, int start)\n+static int message_is_empty(struct strbuf *sb)\n {\n \tstruct strbuf tmpl;\n \tconst char *nl;\n-\tint eol, i;\n+\tint eol, i, start = 0;\n \n \tif (cleanup_mode == CLEANUP_NONE && sb->len)\n \t\treturn 0;\n@@ -929,34 +929,14 @@ static const char commit_utf8_warn[] =\n \"You may want to amend it after fixing the message, or set the config\\n\"\n \"variable i18n.commitencoding to the encoding your project uses.\\n\";\n \n-static void add_parent(struct strbuf *sb, const unsigned char *sha1)\n-{\n-\tstruct object *obj = parse_object(sha1);\n-\tconst char *parent = sha1_to_hex(sha1);\n-\tconst char *cp;\n-\n-\tif (!obj)\n-\t\tdie(\"Unable to find commit parent %s\", parent);\n-\tif (obj->type != OBJ_COMMIT)\n-\t\tdie(\"Parent %s isn't a proper commit\", parent);\n-\n-\tfor (cp = sb->buf; cp && (cp = strstr(cp, \"\\nparent \")); cp += 8) {\n-\t\tif (!memcmp(cp + 8, parent, 40) && cp[48] == '\\n') {\n-\t\t\terror(\"duplicate parent %s ignored\", parent);\n-\t\t\treturn;\n-\t\t}\n-\t}\n-\tstrbuf_addf(sb, \"parent %s\\n\", parent);\n-}\n-\n int cmd_commit(int argc, const char **argv, const char *prefix)\n {\n-\tint header_len;\n \tstruct strbuf sb;\n \tconst char *index_file, *reflog_msg;\n \tchar *nl, *p;\n \tunsigned char commit_sha1[20];\n \tstruct ref_lock *ref_lock;\n+\tstruct commit_list *parents = NULL, **pptr = &parents;\n \n \tgit_config(git_commit_config, NULL);\n \n@@ -971,13 +951,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\treturn 1;\n \t}\n \n-\t/*\n-\t * The commit object\n-\t */\n-\tstrbuf_init(&sb, 0);\n-\tstrbuf_addf(&sb, \"tree %s\\n\",\n-\t\t    sha1_to_hex(active_cache_tree->sha1));\n-\n \t/* Determine parents */\n \tif (initial_commit) {\n \t\treflog_msg = \"commit (initial)\";\n@@ -991,13 +964,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"could not parse HEAD commit\");\n \n \t\tfor (c = commit->parents; c; c = c->next)\n-\t\t\tadd_parent(&sb, c->item->object.sha1);\n+\t\t\tpptr = &commit_list_insert(c->item, pptr)->next;\n \t} else if (in_merge) {\n \t\tstruct strbuf m;\n \t\tFILE *fp;\n \n \t\treflog_msg = \"commit (merge)\";\n-\t\tadd_parent(&sb, head_sha1);\n+\t\tpptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;\n \t\tstrbuf_init(&m, 0);\n \t\tfp = fopen(git_path(\"MERGE_HEAD\"), \"r\");\n \t\tif (fp == NULL)\n@@ -1007,24 +980,18 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t\tunsigned char sha1[20];\n \t\t\tif (get_sha1_hex(m.buf, sha1) < 0)\n \t\t\t\tdie(\"Corrupt MERGE_HEAD file (%s)\", m.buf);\n-\t\t\tadd_parent(&sb, sha1);\n+\t\t\tpptr = &commit_list_insert(lookup_commit(sha1), pptr)->next;\n \t\t}\n \t\tfclose(fp);\n \t\tstrbuf_release(&m);\n \t} else {\n \t\treflog_msg = \"commit\";\n-\t\tstrbuf_addf(&sb, \"parent %s\\n\", sha1_to_hex(head_sha1));\n+\t\tpptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;\n \t}\n-\n-\tstrbuf_addf(&sb, \"author %s\\n\",\n-\t\t    fmt_ident(author_name, author_email, author_date, IDENT_ERROR_ON_NO_NAME));\n-\tstrbuf_addf(&sb, \"committer %s\\n\", git_committer_info(IDENT_ERROR_ON_NO_NAME));\n-\tif (!is_encoding_utf8(git_commit_encoding))\n-\t\tstrbuf_addf(&sb, \"encoding %s\\n\", git_commit_encoding);\n-\tstrbuf_addch(&sb, '\\n');\n+\tparents = reduce_heads(parents);\n \n \t/* Finally, get the commit message */\n-\theader_len = sb.len;\n+\tstrbuf_init(&sb, 0);\n \tif (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {\n \t\trollback_index_files();\n \t\tdie(\"could not read commit message\");\n@@ -1037,16 +1004,15 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \n \tif (cleanup_mode != CLEANUP_NONE)\n \t\tstripspace(&sb, cleanup_mode == CLEANUP_ALL);\n-\tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n+\tif (message_is_empty(&sb)) {\n \t\trollback_index_files();\n \t\tfprintf(stderr, \"Aborting commit due to empty commit message.\\n\");\n \t\texit(1);\n \t}\n-\tstrbuf_addch(&sb, '\\0');\n-\tif (is_encoding_utf8(git_commit_encoding) && !is_utf8(sb.buf))\n-\t\tfprintf(stderr, commit_utf8_warn);\n \n-\tif (write_sha1_file(sb.buf, sb.len - 1, commit_type, commit_sha1)) {\n+\tif (commit_tree(sb.buf, active_cache_tree->sha1, parents, commit_sha1,\n+\t\t\tfmt_ident(author_name, author_email, author_date,\n+\t\t\t\tIDENT_ERROR_ON_NO_NAME))) {\n \t\trollback_index_files();\n \t\tdie(\"failed to write commit object\");\n \t}\n@@ -1055,12 +1021,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t   initial_commit ? NULL : head_sha1,\n \t\t\t\t\t   0);\n \n-\tnl = strchr(sb.buf + header_len, '\\n');\n+\tnl = strchr(sb.buf, '\\n');\n \tif (nl)\n \t\tstrbuf_setlen(&sb, nl + 1 - sb.buf);\n \telse\n \t\tstrbuf_addch(&sb, '\\n');\n-\tstrbuf_remove(&sb, 0, header_len);\n \tstrbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));\n \tstrbuf_insert(&sb, strlen(reflog_msg), \": \", 2);\n \n-- \n1.6.0.1\n"},{"id":"90381","messageId":"e2b601d3a89785ec9da5de061adc37693a3bd323.1221077214.git.vmiklos@frugalware.org","threadId":"15481","inReplyTo":"2984711bae2f0777b210ed04210693ac02164519.1221077214.git.vmiklos@frugalware.org","subject":"[PATCH 3/3] t7603: add new testcases to ensure builtin-commit uses reduce_heads()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-09-10T20:10:33Z","receivedAt":"2008-09-10T20:10:33Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n t/t7603-merge-reduce-heads.sh |   53 +++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 53 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh\nindex b47b7b9..7e17eb4 100755\n--- a/t/t7603-merge-reduce-heads.sh\n+++ b/t/t7603-merge-reduce-heads.sh\n@@ -60,4 +60,57 @@ test_expect_success 'merge c1 with c2, c3, c4, c5' '\n \ttest -f c5.c\n '\n \n+test_expect_success 'setup' '\n+\tfor i in A B C D E\n+\tdo\n+\t\techo $i > $i.c &&\n+\t\tgit add $i.c &&\n+\t\tgit commit -m $i &&\n+\t\tgit tag $i\n+\tdone &&\n+\tgit reset --hard A &&\n+\tfor i in F G H I\n+\tdo\n+\t\techo $i > $i.c &&\n+\t\tgit add $i.c &&\n+\t\tgit commit -m $i &&\n+\t\tgit tag $i\n+\tdone\n+'\n+\n+test_expect_success 'merge E and I' '\n+\tgit reset --hard A &&\n+\tgit merge E I\n+'\n+\n+test_expect_success 'verify merge result' '\n+\ttest $(git rev-parse HEAD^1) = $(git rev-parse E) &&\n+\ttest $(git rev-parse HEAD^2) = $(git rev-parse I)\n+'\n+\n+test_expect_success 'add conflicts' '\n+\tgit reset --hard E &&\n+\techo foo > file.c &&\n+\tgit add file.c &&\n+\tgit commit -m E2 &&\n+\tgit tag E2 &&\n+\tgit reset --hard I &&\n+\techo bar >file.c &&\n+\tgit add file.c &&\n+\tgit commit -m I2 &&\n+\tgit tag I2\n+'\n+\n+test_expect_success 'merge E2 and I2, causing a conflict and resolve it' '\n+\tgit reset --hard A &&\n+\ttest_must_fail git merge E2 I2 &&\n+\techo baz > file.c &&\n+\tgit add file.c &&\n+\tgit commit -m \"resolve conflict\"\n+'\n+\n+test_expect_success 'verify merge result' '\n+\ttest $(git rev-parse HEAD^1) = $(git rev-parse E2) &&\n+\ttest $(git rev-parse HEAD^2) = $(git rev-parse I2)\n+'\n test_done\n-- \n1.6.0.1\n"}]}