{"thread":{"id":"52704","subject":"[PATCH] Make git rm submodule succeed if .gitmodules index stat info is zero","startedAt":"2020-01-27T18:59:46Z","lastAt":"2020-01-28T23:16:08Z","messageCount":2,"participants":["David Turner","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"390574","messageId":"20200127185856.2619317-1-dturner@twosigma.com","threadId":"52704","inReplyTo":null,"subject":"[PATCH] Make git rm submodule succeed if .gitmodules index stat info is zero","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2020-01-27T18:58:56Z","receivedAt":"2020-01-27T18:59:46Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"The bug was that ie_match_stat was used to compare if the stat info\nfor the file was compatible with the stat info in the index, rather\nusing ie_modified to check if the file was in fact different from the\nversion in the index.\n\nA version of this (with deinit instead of rm) was reported here:\nhttps://public-inbox.org/git/CAPOqYV+C-P9M2zcUBBkD2LALPm4K3sxSut+BjAkZ9T1AKLEr+A@mail.gmail.com/\n\nIt seems that in that case, the user's clone command left the index\nwith empty stat info.  The mailing list was unable to reproduce this.\nBut we (Two Sigma) hit the bug while using some plumbing commands, so\nI'm fixing it.  I manually confirmed that the fix also repairs deinit\nin this scenario.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\nReported-by: Thomas Bétous <th.betous@gmail.com>\n---\n submodule.c   | 2 +-\n t/t3600-rm.sh | 7 +++++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 9da7181321..86e46d3dce 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -82,7 +82,7 @@ int is_staging_gitmodules_ok(struct index_state *istate)\n \tif ((pos >= 0) && (pos < istate->cache_nr)) {\n \t\tstruct stat st;\n \t\tif (lstat(GITMODULES_FILE, &st) == 0 &&\n-\t\t    ie_match_stat(istate, istate->cache[pos], &st, 0) & DATA_CHANGED)\n+\t\t    ie_modified(istate, istate->cache[pos], &st, 0) & DATA_CHANGED)\n \t\t\treturn 0;\n \t}\n \ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 0ea858d652..f2c0168941 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -425,6 +425,13 @@ test_expect_success 'rm will error out on a modified .gitmodules file unless sta\n \tgit status -s -uno >actual &&\n \ttest_cmp expect actual\n '\n+test_expect_success 'rm will not error out on .gitmodules file with zero stat data' '\n+\tgit reset --hard &&\n+\tgit submodule update &&\n+\tgit read-tree HEAD &&\n+\tgit rm submod &&\n+\ttest_path_is_missing submod\n+'\n \n test_expect_success 'rm issues a warning when section is not found in .gitmodules' '\n \tgit reset --hard &&\n-- \n2.11.GIT\n\n"},{"id":"390676","messageId":"xmqqftfz6ukt.fsf@gitster-ct.c.googlers.com","threadId":"52704","inReplyTo":"20200127185856.2619317-1-dturner@twosigma.com","subject":"Re: [PATCH] Make git rm submodule succeed if .gitmodules index stat info is zero","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-28T23:16:02Z","receivedAt":"2020-01-28T23:16:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twosigma.com> writes:\n\n> The bug was that ie_match_stat was used to compare if the stat info\n> for the file was compatible with the stat info in the index, rather\n> using ie_modified to check if the file was in fact different from the\n> version in the index.\n\nMakes sense.  ie_match_stat() often ends up comparing the file\ncontents in our tests due to sequence Git commands firing too\nrapidly, leading to a racy-index status, but read-tree is a reliable\nway to clear the cached stat information to force it say \"I dunno---\nsuspect it got modified\".  Will queue.\n\n> +test_expect_success 'rm will not error out on .gitmodules file with zero stat data' '\n> +\tgit reset --hard &&\n> +\tgit submodule update &&\n> +\tgit read-tree HEAD &&\n> +\tgit rm submod &&\n> +\ttest_path_is_missing submod\n> +'\n>  \n>  test_expect_success 'rm issues a warning when section is not found in .gitmodules' '\n>  \tgit reset --hard &&\n"}]}