# [PATCH 2/3] am: do not corrupt the index stat state 5 messages from 2015-08-21 to 2015-08-21. Participants: David Aguilar, Junio C Hamano. Thread: https://gitlist.dev/t/40146 ## David Aguilar, 2015-08-21 08:02 Subject: [PATCH 1/3] t4151: demonstrate that builtin am corrupts index' stat data Message-ID: <1440144151-24026-1-git-send-email-davvid@gmail.com> URL: https://gitlist.dev/e/1440144151-24026-1-git-send-email-davvid%40gmail.com ``` From: Johannes Schindelin Reported by Linus Torvalds. Signed-off-by: Johannes Schindelin --- t/t4151-am-abort.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index 05bdc3e..bf2e6f4 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -168,4 +168,16 @@ test_expect_success 'am --abort on unborn branch will keep local commits intact' test_cmp expect actual ' +test_expect_failure 'am --abort leaves index stat info alone' ' + git checkout -f --orphan stat-info && + git reset && + test_commit should-be-untouched && + test-chmtime =0 should-be-untouched.t && + git update-index --refresh && + git diff-files --exit-code --quiet && + test_must_fail git am 0001-*.patch && + git am --abort && + git diff-files --exit-code --quiet +' + test_done -- 2.5.0.403.gd17121e ``` ## David Aguilar, 2015-08-21 08:02 Subject: [PATCH 2/3] am: do not corrupt the index stat state Message-ID: <1440144151-24026-2-git-send-email-davvid@gmail.com> URL: https://gitlist.dev/e/1440144151-24026-2-git-send-email-davvid%40gmail.com In-Reply-To: <1440144151-24026-1-git-send-email-davvid@gmail.com> ``` Reported-by: Linus Torvalds Signed-off-by: David Aguilar --- builtin/am.c | 14 +++++++++++++- t/t4151-am-abort.sh | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 1399c8d..9db1b34 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1949,6 +1949,8 @@ static int clean_index(const unsigned char *head, const unsigned char *remote) struct tree *head_tree, *remote_tree, *index_tree; unsigned char index[GIT_SHA1_RAWSZ]; struct pathspec pathspec; + struct tree_desc desc; + struct unpack_trees_options opts; head_tree = parse_tree_indirect(head); if (!head_tree) @@ -1975,10 +1977,20 @@ static int clean_index(const unsigned char *head, const unsigned char *remote) memset(&pathspec, 0, sizeof(pathspec)); + memset(&opts, 0, sizeof(opts)); + opts.fn = oneway_merge; + opts.pathspec = &pathspec; + opts.src_index = &the_index; + opts.dst_index = &the_index; + opts.head_idx = 1; + opts.merge = 1; + opts.reset = 1; + init_tree_desc(&desc, remote_tree->buffer, remote_tree->size); + lock_file = xcalloc(1, sizeof(struct lock_file)); hold_locked_index(lock_file, 1); - if (read_tree(remote_tree, 0, &pathspec)) { + if (unpack_trees(1, &desc, &opts)) { rollback_lock_file(lock_file); return -1; } diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index bf2e6f4..9c3bbd1 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -168,7 +168,7 @@ test_expect_success 'am --abort on unborn branch will keep local commits intact' test_cmp expect actual ' -test_expect_failure 'am --abort leaves index stat info alone' ' +test_expect_success 'am --abort leaves index stat info alone' ' git checkout -f --orphan stat-info && git reset && test_commit should-be-untouched && -- 2.5.0.403.gd17121e ``` ## David Aguilar, 2015-08-21 08:02 Subject: [PATCH 3/3] am: rename "struct tree_desc t" to "desc" for readability Message-ID: <1440144151-24026-3-git-send-email-davvid@gmail.com> URL: https://gitlist.dev/e/1440144151-24026-3-git-send-email-davvid%40gmail.com In-Reply-To: <1440144151-24026-1-git-send-email-davvid@gmail.com> ``` Signed-off-by: David Aguilar --- builtin/am.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 9db1b34..7e6860d 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1907,7 +1907,7 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset) { struct lock_file *lock_file; struct unpack_trees_options opts; - struct tree_desc t[2]; + struct tree_desc desc[2]; if (parse_tree(head) || parse_tree(remote)) return -1; @@ -1925,10 +1925,10 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset) opts.merge = 1; opts.reset = reset; opts.fn = twoway_merge; - init_tree_desc(&t[0], head->buffer, head->size); - init_tree_desc(&t[1], remote->buffer, remote->size); + init_tree_desc(&desc[0], head->buffer, head->size); + init_tree_desc(&desc[1], remote->buffer, remote->size); - if (unpack_trees(2, t, &opts)) { + if (unpack_trees(2, desc, &opts)) { rollback_lock_file(lock_file); return -1; } -- 2.5.0.403.gd17121e ``` ## Junio C Hamano, 2015-08-21 16:37 Subject: Re: [PATCH 2/3] am: do not corrupt the index stat state Message-ID: URL: https://gitlist.dev/e/xmqqk2so7g3h.fsf%40gitster.dls.corp.google.com In-Reply-To: <1440144151-24026-2-git-send-email-davvid@gmail.com> ``` David Aguilar writes: > Reported-by: Linus Torvalds > Signed-off-by: David Aguilar > @@ -1975,10 +1977,20 @@ static int clean_index(const unsigned char *head, const unsigned char *remote) > > memset(&pathspec, 0, sizeof(pathspec)); > > + memset(&opts, 0, sizeof(opts)); > + opts.fn = oneway_merge; > + opts.pathspec = &pathspec; > + opts.src_index = &the_index; > + opts.dst_index = &the_index; > + opts.head_idx = 1; > + opts.merge = 1; > + opts.reset = 1; > + init_tree_desc(&desc, remote_tree->buffer, remote_tree->size); > + > lock_file = xcalloc(1, sizeof(struct lock_file)); > hold_locked_index(lock_file, 1); > > - if (read_tree(remote_tree, 0, &pathspec)) { > + if (unpack_trees(1, &desc, &opts)) { I think the same fix was done in 3ecc7040 (am --skip/--abort: merge HEAD/ORIG_HEAD tree into index, 2015-08-19). ``` ## Junio C Hamano, 2015-08-21 16:51 Subject: Re: [PATCH 3/3] am: rename "struct tree_desc t" to "desc" for readability Message-ID: URL: https://gitlist.dev/e/xmqqfv3c7ffm.fsf%40gitster.dls.corp.google.com In-Reply-To: <1440144151-24026-3-git-send-email-davvid@gmail.com> ``` David Aguilar writes: > Signed-off-by: David Aguilar > --- > builtin/am.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) Naming the tree_desc parameter given to unpack_trees() as "t" is (unfortunately) an established convention, just like we often use "i" and "j" in for() loop control. Descriptive names are nicer, especially on the calling sites, but we have quite a many hits from $ git grep unpack_trees\( \*.c so perhaps do this as a clean-up patch after dust settles for all the existing callers in archive, clone, commit, merge, read-tree, reset, diff-lib, and merge-recursive? ```