{"thread":{"id":"40146","subject":"[PATCH 2/3] am: do not corrupt the index stat state","startedAt":"2015-08-21T08:02:29Z","lastAt":"2015-08-21T16:51:41Z","messageCount":5,"participants":["David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"268403","messageId":"1440144151-24026-1-git-send-email-davvid@gmail.com","threadId":"40146","inReplyTo":null,"subject":"[PATCH 1/3] t4151: demonstrate that builtin am corrupts index' stat data","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-08-21T08:02:29Z","receivedAt":"2015-08-21T08:02:29Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"From: Johannes Schindelin \n\nReported by Linus Torvalds.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t4151-am-abort.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex 05bdc3e..bf2e6f4 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -168,4 +168,16 @@ test_expect_success 'am --abort on unborn branch will keep local commits intact'\n \ttest_cmp expect actual\n '\n \n+test_expect_failure 'am --abort leaves index stat info alone' '\n+\tgit checkout -f --orphan stat-info &&\n+\tgit reset &&\n+\ttest_commit should-be-untouched &&\n+\ttest-chmtime =0 should-be-untouched.t &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --exit-code --quiet &&\n+\ttest_must_fail git am 0001-*.patch &&\n+\tgit am --abort &&\n+\tgit diff-files --exit-code --quiet\n+'\n+\n test_done\n-- \n2.5.0.403.gd17121e\n"},{"id":"268402","messageId":"1440144151-24026-2-git-send-email-davvid@gmail.com","threadId":"40146","inReplyTo":"1440144151-24026-1-git-send-email-davvid@gmail.com","subject":"[PATCH 2/3] am: do not corrupt the index stat state","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-08-21T08:02:30Z","receivedAt":"2015-08-21T08:02:30Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Reported-by: Linus Torvalds\nSigned-off-by: David Aguilar \n---\n builtin/am.c | 14 +++++++++++++-\n t/t4151-am-abort.sh | 2 +-\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 1399c8d..9db1b34 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1949,6 +1949,8 @@ static int clean_index(const unsigned char *head, const unsigned char *remote)\n \tstruct tree *head_tree, *remote_tree, *index_tree;\n \tunsigned char index[GIT_SHA1_RAWSZ];\n \tstruct pathspec pathspec;\n+\tstruct tree_desc desc;\n+\tstruct unpack_trees_options opts;\n \n \thead_tree = parse_tree_indirect(head);\n \tif (!head_tree)\n@@ -1975,10 +1977,20 @@ static int clean_index(const unsigned char *head, const unsigned char *remote)\n \n \tmemset(&pathspec, 0, sizeof(pathspec));\n \n+\tmemset(&opts, 0, sizeof(opts));\n+\topts.fn = oneway_merge;\n+\topts.pathspec = &pathspec;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n+\topts.head_idx = 1;\n+\topts.merge = 1;\n+\topts.reset = 1;\n+\tinit_tree_desc(&desc, remote_tree->buffer, remote_tree->size);\n+\n \tlock_file = xcalloc(1, sizeof(struct lock_file));\n \thold_locked_index(lock_file, 1);\n \n-\tif (read_tree(remote_tree, 0, &pathspec)) {\n+\tif (unpack_trees(1, &desc, &opts)) {\n \t\trollback_lock_file(lock_file);\n \t\treturn -1;\n \t}\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex bf2e6f4..9c3bbd1 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -168,7 +168,7 @@ test_expect_success 'am --abort on unborn branch will keep local commits intact'\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'am --abort leaves index stat info alone' '\n+test_expect_success 'am --abort leaves index stat info alone' '\n \tgit checkout -f --orphan stat-info &&\n \tgit reset &&\n \ttest_commit should-be-untouched &&\n-- \n2.5.0.403.gd17121e\n"},{"id":"268404","messageId":"1440144151-24026-3-git-send-email-davvid@gmail.com","threadId":"40146","inReplyTo":"1440144151-24026-1-git-send-email-davvid@gmail.com","subject":"[PATCH 3/3] am: rename \"struct tree_desc t\" to \"desc\" for readability","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-08-21T08:02:31Z","receivedAt":"2015-08-21T08:02:31Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Signed-off-by: David Aguilar \n---\n builtin/am.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 9db1b34..7e6860d 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1907,7 +1907,7 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)\n {\n \tstruct lock_file *lock_file;\n \tstruct unpack_trees_options opts;\n-\tstruct tree_desc t[2];\n+\tstruct tree_desc desc[2];\n \n \tif (parse_tree(head) || parse_tree(remote))\n \t\treturn -1;\n@@ -1925,10 +1925,10 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)\n \topts.merge = 1;\n \topts.reset = reset;\n \topts.fn = twoway_merge;\n-\tinit_tree_desc(&t[0], head->buffer, head->size);\n-\tinit_tree_desc(&t[1], remote->buffer, remote->size);\n+\tinit_tree_desc(&desc[0], head->buffer, head->size);\n+\tinit_tree_desc(&desc[1], remote->buffer, remote->size);\n \n-\tif (unpack_trees(2, t, &opts)) {\n+\tif (unpack_trees(2, desc, &opts)) {\n \t\trollback_lock_file(lock_file);\n \t\treturn -1;\n \t}\n-- \n2.5.0.403.gd17121e\n"},{"id":"268432","messageId":"xmqqk2so7g3h.fsf@gitster.dls.corp.google.com","threadId":"40146","inReplyTo":"1440144151-24026-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 2/3] am: do not corrupt the index stat state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-21T16:37:22Z","receivedAt":"2015-08-21T16:37:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> Reported-by: Linus Torvalds\n> Signed-off-by: David Aguilar \n> @@ -1975,10 +1977,20 @@ static int clean_index(const unsigned char *head, const unsigned char *remote)\n> \n> \tmemset(&pathspec, 0, sizeof(pathspec));\n> \n> +\tmemset(&opts, 0, sizeof(opts));\n> +\topts.fn = oneway_merge;\n> +\topts.pathspec = &pathspec;\n> +\topts.src_index = &the_index;\n> +\topts.dst_index = &the_index;\n> +\topts.head_idx = 1;\n> +\topts.merge = 1;\n> +\topts.reset = 1;\n> +\tinit_tree_desc(&desc, remote_tree->buffer, remote_tree->size);\n> +\n> \tlock_file = xcalloc(1, sizeof(struct lock_file));\n> \thold_locked_index(lock_file, 1);\n> \n> -\tif (read_tree(remote_tree, 0, &pathspec)) {\n> +\tif (unpack_trees(1, &desc, &opts)) {\n\nI think the same fix was done in 3ecc7040 (am --skip/--abort: merge\nHEAD/ORIG_HEAD tree into index, 2015-08-19).\n"},{"id":"268433","messageId":"xmqqfv3c7ffm.fsf@gitster.dls.corp.google.com","threadId":"40146","inReplyTo":"1440144151-24026-3-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 3/3] am: rename \"struct tree_desc t\" to \"desc\" for readability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-21T16:51:41Z","receivedAt":"2015-08-21T16:51:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> Signed-off-by: David Aguilar \n> ---\n> builtin/am.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n\nNaming the tree_desc parameter given to unpack_trees() as \"t\" is\n(unfortunately) an established convention, just like we often use\n\"i\" and \"j\" in for() loop control.\n\nDescriptive names are nicer, especially on the calling sites, but we\nhave quite a many hits from\n\n $ git grep unpack_trees\\( \\*.c\n\nso perhaps do this as a clean-up patch after dust settles for all\nthe existing callers in archive, clone, commit, merge, read-tree,\nreset, diff-lib, and merge-recursive?\n"}]}