{"thread":{"id":"45696","subject":"[PATCH v7] read-cache: call verify_hdr() in a background thread","startedAt":"2017-04-14T20:32:41Z","lastAt":"2017-05-09T05:28:00Z","messageCount":13,"participants":["git@jeffhostetler.com","Johannes Sixt","Jeff Hostetler","Junio C Hamano","Johannes Schindelin","Christian Couder"],"isPatch":true,"patchVersion":7,"patchTotal":null},"messages":[{"id":"316856","messageId":"20170414203221.43015-1-git@jeffhostetler.com","threadId":"45696","inReplyTo":null,"subject":"[PATCH v7] read-cache: call verify_hdr() in a background thread","fromName":"","fromEmail":"git@jeffhostetler.com","sentAt":"2017-04-14T20:32:20Z","receivedAt":"2017-04-14T20:32:41Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nVersion 7 of this patch cleans up the fsck test using\ntest_when_finished and eliminates unnecessary mv/rm's.\n\nJeff Hostetler (1):\n read-cache: force_verify_index_checksum\n\n builtin/fsck.c | 1 +\n cache.h | 2 ++\n read-cache.c | 7 +++++++\n t/t1450-fsck.sh | 13 +++++++++++++\n 4 files changed, 23 insertions(+)\n\n-- \n2.9.3\n\n"},{"id":"316857","messageId":"20170414203221.43015-2-git@jeffhostetler.com","threadId":"45696","inReplyTo":"20170414203221.43015-1-git@jeffhostetler.com","subject":"[PATCH v7] read-cache: force_verify_index_checksum","fromName":"","fromEmail":"git@jeffhostetler.com","sentAt":"2017-04-14T20:32:21Z","receivedAt":"2017-04-14T20:32:45Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler \n\nTeach git to skip verification of the SHA1-1 checksum at the end of\nthe index file in verify_hdr() which is called from read_index()\nunless the \"force_verify_index_checksum\" global variable is set.\n\nTeach fsck to force this verification.\n\nThe checksum verification is for detecting disk corruption, and for\nsmall projects, the time it takes to compute SHA-1 is not that\nsignificant, but for gigantic repositories this calculation adds\nsignificant time to every command.\n\nThese effect can be seen using t/perf/p0002-read-cache.sh:\n\nTest HEAD~1 HEAD\n--------------------------------------------------------------------------------------\n0002.1: read_cache/discard_cache 1000 times 0.66(0.44+0.20) 0.30(0.27+0.02) -54.5%\n\nSigned-off-by: Jeff Hostetler \n---\n builtin/fsck.c | 1 +\n cache.h | 2 ++\n read-cache.c | 7 +++++++\n t/t1450-fsck.sh | 13 +++++++++++++\n 4 files changed, 23 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 1a5cacc..5512d06 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -771,6 +771,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (keep_cache_objects) {\n+\t\tverify_index_checksum = 1;\n \t\tread_cache();\n \t\tfor (i = 0; i < active_nr; i++) {\n \t\t\tunsigned int mode;\ndiff --git a/cache.h b/cache.h\nindex 80b6372..87f13bf 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -685,6 +685,8 @@ extern void update_index_if_able(struct index_state *, struct lock_file *);\n extern int hold_locked_index(struct lock_file *, int);\n extern void set_alternate_index_output(const char *);\n \n+extern int verify_index_checksum;\n+\n /* Environment bits from configuration mechanism */\n extern int trust_executable_bit;\n extern int trust_ctime;\ndiff --git a/read-cache.c b/read-cache.c\nindex 9054369..c4205aa 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1371,6 +1371,9 @@ struct ondisk_cache_entry_extended {\n \t\t\t ondisk_cache_entry_extended_size(ce_namelen(ce)) : \\\n \t\t\t ondisk_cache_entry_size(ce_namelen(ce)))\n \n+/* Allow fsck to force verification of the index checksum. */\n+int verify_index_checksum;\n+\n static int verify_hdr(struct cache_header *hdr, unsigned long size)\n {\n \tgit_SHA_CTX c;\n@@ -1382,6 +1385,10 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size)\n \thdr_version = ntohl(hdr->hdr_version);\n \tif (hdr_version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < hdr_version)\n \t\treturn error(\"bad index version %d\", hdr_version);\n+\n+\tif (!verify_index_checksum)\n+\t\treturn 0;\n+\n \tgit_SHA1_Init(&c);\n \tgit_SHA1_Update(&c, hdr, size - 20);\n \tgit_SHA1_Final(sha1, &c);\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 33a51c9..677e15a 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -689,4 +689,17 @@ test_expect_success 'bogus head does not fallback to all heads' '\n \t! grep $blob out\n '\n \n+test_expect_success 'detect corrupt index file in fsck' '\n+\tcp .git/index .git/index.backup &&\n+\ttest_when_finished \"mv .git/index.backup .git/index\" &&\n+\techo zzzzzzzz >zzzzzzzz &&\n+\tgit add zzzzzzzz &&\n+\tsed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n+\tmv .git/index.yyy .git/index &&\n+\t# Confirm that fsck detects invalid checksum\n+\ttest_must_fail git fsck --cache &&\n+\t# Confirm that status no longer complains about invalid checksum\n+\tgit status\n+'\n+\n test_done\n-- \n2.9.3\n\n"},{"id":"317703","messageId":"870a8a36-fc6c-6b07-d09e-eec8a9f46a5c@kdbg.org","threadId":"45696","inReplyTo":"20170414203221.43015-2-git@jeffhostetler.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-04-24T17:26:23Z","receivedAt":"2017-04-24T17:26:32Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.04.2017 um 22:32 schrieb git@jeffhostetler.com:\n> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\n> index 33a51c9..677e15a 100755\n> --- a/t/t1450-fsck.sh\n> +++ b/t/t1450-fsck.sh\n> @@ -689,4 +689,17 @@ test_expect_success 'bogus head does not fallback to all heads' '\n> \t! grep $blob out\n> '\n>\n> +test_expect_success 'detect corrupt index file in fsck' '\n> +\tcp .git/index .git/index.backup &&\n> +\ttest_when_finished \"mv .git/index.backup .git/index\" &&\n> +\techo zzzzzzzz >zzzzzzzz &&\n> +\tgit add zzzzzzzz &&\n> +\tsed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n\nsed on a binary file? Sooner or later we are going to run into \nportability issues.\n\n> +\tmv .git/index.yyy .git/index &&\n> +\t# Confirm that fsck detects invalid checksum\n> +\ttest_must_fail git fsck --cache &&\n\nYou should ensure that this failure is really because of an invalid \nchecksum. The failure could also be due to an extra LF at the end that \nsed inserted, no?\n\n> +\t# Confirm that status no longer complains about invalid checksum\n> +\tgit status\n> +'\n> +\n> test_done\n\n-- Hannes\n\n"},{"id":"317712","messageId":"8bde0b54-c0f6-364d-1f08-ce5207eb6c86@jeffhostetler.com","threadId":"45696","inReplyTo":"870a8a36-fc6c-6b07-d09e-eec8a9f46a5c@kdbg.org","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-04-24T18:39:27Z","receivedAt":"2017-04-24T18:39:34Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 4/24/2017 1:26 PM, Johannes Sixt wrote:\n> Am 14.04.2017 um 22:32 schrieb git@jeffhostetler.com:\n>> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\n>> index 33a51c9..677e15a 100755\n>> --- a/t/t1450-fsck.sh\n>> +++ b/t/t1450-fsck.sh\n>> @@ -689,4 +689,17 @@ test_expect_success 'bogus head does not \n>> fallback to all heads' '\n>> ! grep $blob out\n>> '\n>>\n>> +test_expect_success 'detect corrupt index file in fsck' '\n>> + cp .git/index .git/index.backup &&\n>> + test_when_finished \"mv .git/index.backup .git/index\" &&\n>> + echo zzzzzzzz >zzzzzzzz &&\n>> + git add zzzzzzzz &&\n>> + sed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n>\n> sed on a binary file? Sooner or later we are going to run into \n> portability issues.\n\nIn v5 of this patch series I used \"perl\" and it was suggested that I use \n\"sed\" instead.\nIt doesn't matter to me which we use. My testing showed that it was \nsafe, but that\nwas only Linux.\n\nDoes the mailing list have a preference for this ?\n\n>\n>\n>> + mv .git/index.yyy .git/index &&\n>> + # Confirm that fsck detects invalid checksum\n>> + test_must_fail git fsck --cache &&\n>\n> You should ensure that this failure is really because of an invalid \n> checksum. The failure could also be due to an extra LF at the end that \n> sed inserted, no?\n\nI suppose we could, but I'm tempted to wait on that for now.\n\nJeff\n\n"},{"id":"317753","messageId":"xmqqk269xmmq.fsf@gitster.mtv.corp.google.com","threadId":"45696","inReplyTo":"8bde0b54-c0f6-364d-1f08-ce5207eb6c86@jeffhostetler.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-25T02:10:53Z","receivedAt":"2017-04-25T02:11:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n>>> +test_expect_success 'detect corrupt index file in fsck' '\n>>> + cp .git/index .git/index.backup &&\n>>> + test_when_finished \"mv .git/index.backup .git/index\" &&\n>>> + echo zzzzzzzz >zzzzzzzz &&\n>>> + git add zzzzzzzz &&\n>>> + sed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n>>\n>> sed on a binary file? Sooner or later we are going to run into\n>> portability issues.\n>\n> In v5 of this patch series I used \"perl\" and it was suggested that I\n> use \"sed\" instead.\n> It doesn't matter to me which we use. My testing showed that it was\n> safe, but that\n> was only Linux.\n>\n> Does the mailing list have a preference for this ?\n\nInstead of munging pathnames z* to y*, I'd prefer to see the actual\nchecksum bytes at the end replaced in the index file. After all\nthat is what this test really cares about, and it ensures that the\nfailure detected is due to checksum mismatch.\n\n>>> + mv .git/index.yyy .git/index &&\n>>> + # Confirm that fsck detects invalid checksum\n>>> + test_must_fail git fsck --cache &&\n>>\n>> You should ensure that this failure is really because of an invalid\n>> checksum. The failure could also be due to an extra LF at the end\n>> that sed inserted, no?\n>\n> I suppose we could, but I'm tempted to wait on that for now.\n>\n> Jeff\n"},{"id":"317838","messageId":"alpine.DEB.2.20.1704252225450.3480@virtualbox","threadId":"45696","inReplyTo":"xmqqk269xmmq.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-04-25T21:04:33Z","receivedAt":"2017-04-25T21:05:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 24 Apr 2017, Junio C Hamano wrote:\n\n> Jeff Hostetler writes:\n> \n> >>> +test_expect_success 'detect corrupt index file in fsck' '\n> >>> + cp .git/index .git/index.backup &&\n> >>> + test_when_finished \"mv .git/index.backup .git/index\" &&\n> >>> + echo zzzzzzzz >zzzzzzzz &&\n> >>> + git add zzzzzzzz &&\n> >>> + sed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n> >>\n> >> sed on a binary file? Sooner or later we are going to run into\n> >> portability issues.\n> >\n> > In v5 of this patch series I used \"perl\" and it was suggested that I\n> > use \"sed\" instead. It doesn't matter to me which we use. My testing\n> > showed that it was safe, but that was only Linux.\n\nI am sorry to hear that the Git mailing list's review gives you whiplash.\n\nThe problem with sed is that BSD sed behaves a bit differently than GNU\nsed, and we quietly expect every contributor to be an expert in the\nportability aspects of sed.\n\nTBH I am quite surprised that anybody would have suggested to use sed\nrather than Perl to edit binary files in the first place. In my opinion,\nthat was bad advice.\n\n> > Does the mailing list have a preference for this ?\n> \n> Instead of munging pathnames z* to y*, I'd prefer to see the actual\n> checksum bytes at the end replaced in the index file. After all\n> that is what this test really cares about, and it ensures that the\n> failure detected is due to checksum mismatch.\n\nI see that v8 uses a Perl script again, and it is well written and\nobvious.\n\nJust in case that certain reviewers favor length over readability, let me\noffer this snippet:\n\n\tsize=$(perl -e \"print -s \\\".git/index\\\"\") &&\n\tdd if=/dev/zero of=.git/index bs=1 seek=$(($size-20) count=20\n\nSince whatever hash will be used in the future is most likely larger than\n20 bytes, this should still work fine (and even if somebody sane replaces\nthe SHA-1 of the index with a CRC-32 for the same benefit we have now, the\ntest will fail quickly and it is easy to replace the 20 by 4).\n\nCiao,\nDscho\n"},{"id":"317894","messageId":"xmqq60hru9xg.fsf@gitster.mtv.corp.google.com","threadId":"45696","inReplyTo":"alpine.DEB.2.20.1704252225450.3480@virtualbox","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-26T03:25:47Z","receivedAt":"2017-04-26T03:25:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Just in case that certain reviewers favor length over readability, let me\n> offer this snippet:\n>\n> \tsize=$(perl -e \"print -s \\\".git/index\\\"\") &&\n> \tdd if=/dev/zero of=.git/index bs=1 seek=$(($size-20) count=20\n\nYup, this does sound like a good direction to go (I think you have\nsome unbalanced parens but it is sufficient to convey the idea).\n\n> Since whatever hash will be used in the future is most likely larger than\n> 20 bytes, this should still work fine (and even if somebody sane replaces\n> the SHA-1 of the index with a CRC-32 for the same benefit we have now, the\n> test will fail quickly and it is easy to replace the 20 by 4).\n\nTrue that, too.\n"},{"id":"319073","messageId":"CAP8UFD2v8R8bSjUi8+1271fJ=jR5JbbGfgO_fdeYpzE=EGx_Pw@mail.gmail.com","threadId":"45696","inReplyTo":"20170414203221.43015-2-git@jeffhostetler.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-05-08T09:45:02Z","receivedAt":"2017-05-08T09:45:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Apr 14, 2017 at 10:32 PM, wrote:\n> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\n> index 33a51c9..677e15a 100755\n> --- a/t/t1450-fsck.sh\n> +++ b/t/t1450-fsck.sh\n> @@ -689,4 +689,17 @@ test_expect_success 'bogus head does not fallback to all heads' '\n> ! grep $blob out\n> '\n>\n> +test_expect_success 'detect corrupt index file in fsck' '\n> + cp .git/index .git/index.backup &&\n> + test_when_finished \"mv .git/index.backup .git/index\" &&\n> + echo zzzzzzzz >zzzzzzzz &&\n> + git add zzzzzzzz &&\n> + sed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n> + mv .git/index.yyy .git/index &&\n> + # Confirm that fsck detects invalid checksum\n> + test_must_fail git fsck --cache &&\n> + # Confirm that status no longer complains about invalid checksum\n> + git status\n> +'\n\nThis test does not pass when the GIT_TEST_SPLIT_INDEX env variable is\nset on my Linux machine.\n\nAlso it looks like you sent a v8 of this patch series with a different\ntest, but what is in master looks like the above test instead of the\ntest in your v8.\n"},{"id":"319096","messageId":"962eefec-2ea6-6b43-7bd5-51da4150bb6a@jeffhostetler.com","threadId":"45696","inReplyTo":"CAP8UFD2v8R8bSjUi8+1271fJ=jR5JbbGfgO_fdeYpzE=EGx_Pw@mail.gmail.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-05-08T16:50:51Z","receivedAt":"2017-05-08T16:50:57Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 5/8/2017 5:45 AM, Christian Couder wrote:\n> On Fri, Apr 14, 2017 at 10:32 PM, wrote:\n>> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\n>> index 33a51c9..677e15a 100755\n>> --- a/t/t1450-fsck.sh\n>> +++ b/t/t1450-fsck.sh\n>> @@ -689,4 +689,17 @@ test_expect_success 'bogus head does not fallback to all heads' '\n>> ! grep $blob out\n>> '\n>>\n>> +test_expect_success 'detect corrupt index file in fsck' '\n>> + cp .git/index .git/index.backup &&\n>> + test_when_finished \"mv .git/index.backup .git/index\" &&\n>> + echo zzzzzzzz >zzzzzzzz &&\n>> + git add zzzzzzzz &&\n>> + sed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n>> + mv .git/index.yyy .git/index &&\n>> + # Confirm that fsck detects invalid checksum\n>> + test_must_fail git fsck --cache &&\n>> + # Confirm that status no longer complains about invalid checksum\n>> + git status\n>> +'\n>\n> This test does not pass when the GIT_TEST_SPLIT_INDEX env variable is\n> set on my Linux machine.\n>\n> Also it looks like you sent a v8 of this patch series with a different\n> test, but what is in master looks like the above test instead of the\n> test in your v8.\n>\n\nThere was concern about using sed on a binary file not being portable\nand a request to change the test to just corrupt the checksum rather\nthan an index-entry, so I changed it in v8.\n\nDoes the v8 version of the test also fail on your machine ?\n\nThanks\nJeff\n"},{"id":"319111","messageId":"CAP8UFD1dR1QY81tCAcbB6RP61vBxXy-u8zFAhLpk9uP71Jna-Q@mail.gmail.com","threadId":"45696","inReplyTo":"962eefec-2ea6-6b43-7bd5-51da4150bb6a@jeffhostetler.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-05-08T20:03:00Z","receivedAt":"2017-05-08T20:03:06Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, May 8, 2017 at 6:50 PM, Jeff Hostetler wrote:\n>\n>\n> On 5/8/2017 5:45 AM, Christian Couder wrote:\n>>\n>> This test does not pass when the GIT_TEST_SPLIT_INDEX env variable is\n>> set on my Linux machine.\n>>\n>> Also it looks like you sent a v8 of this patch series with a different\n>> test, but what is in master looks like the above test instead of the\n>> test in your v8.\n>\n> There was concern about using sed on a binary file not being portable\n> and a request to change the test to just corrupt the checksum rather\n> than an index-entry, so I changed it in v8.\n>\n> Does the v8 version of the test also fail on your machine ?\n\nThe v8 version of the test succeeds on my machine.\n"},{"id":"319113","messageId":"c9f77c02-c498-1b0d-26c2-27d6f576ee71@jeffhostetler.com","threadId":"45696","inReplyTo":"CAP8UFD1dR1QY81tCAcbB6RP61vBxXy-u8zFAhLpk9uP71Jna-Q@mail.gmail.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-05-08T20:18:31Z","receivedAt":"2017-05-08T20:18:53Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 5/8/2017 4:03 PM, Christian Couder wrote:\n> On Mon, May 8, 2017 at 6:50 PM, Jeff Hostetler wrote:\n>>\n>>\n>> On 5/8/2017 5:45 AM, Christian Couder wrote:\n>>>\n>>> This test does not pass when the GIT_TEST_SPLIT_INDEX env variable is\n>>> set on my Linux machine.\n>>>\n>>> Also it looks like you sent a v8 of this patch series with a different\n>>> test, but what is in master looks like the above test instead of the\n>>> test in your v8.\n>>\n>> There was concern about using sed on a binary file not being portable\n>> and a request to change the test to just corrupt the checksum rather\n>> than an index-entry, so I changed it in v8.\n>>\n>> Does the v8 version of the test also fail on your machine ?\n>\n> The v8 version of the test succeeds on my machine.\n\nOK, thanks. It worked on mine too. Since the v8 version is a\nbetter test, I'm content to stick with it and not try to address\nthe problem with the previous version.\n\nJeff\n"},{"id":"319139","messageId":"xmqqk25q6aph.fsf@gitster.mtv.corp.google.com","threadId":"45696","inReplyTo":"c9f77c02-c498-1b0d-26c2-27d6f576ee71@jeffhostetler.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-09T02:11:22Z","receivedAt":"2017-05-09T02:11:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> On 5/8/2017 4:03 PM, Christian Couder wrote:\n>> On Mon, May 8, 2017 at 6:50 PM, Jeff Hostetler wrote:\n>>>\n>>>\n>>> On 5/8/2017 5:45 AM, Christian Couder wrote:\n>>>>\n>>>> This test does not pass when the GIT_TEST_SPLIT_INDEX env variable is\n>>>> set on my Linux machine.\n>>>>\n>>>> Also it looks like you sent a v8 of this patch series with a different\n>>>> test, but what is in master looks like the above test instead of the\n>>>> test in your v8.\n>>>\n>>> There was concern about using sed on a binary file not being portable\n>>> and a request to change the test to just corrupt the checksum rather\n>>> than an index-entry, so I changed it in v8.\n>>>\n>>> Does the v8 version of the test also fail on your machine ?\n>>\n>> The v8 version of the test succeeds on my machine.\n>\n> OK, thanks. It worked on mine too. Since the v8 version is a\n> better test, I'm content to stick with it and not try to address\n> the problem with the previous version.\n\nThanks. Let's merge that in.\n"},{"id":"319168","messageId":"xmqqtw4u38h2.fsf@gitster.mtv.corp.google.com","threadId":"45696","inReplyTo":"CAP8UFD2v8R8bSjUi8+1271fJ=jR5JbbGfgO_fdeYpzE=EGx_Pw@mail.gmail.com","subject":"Re: [PATCH v7] read-cache: force_verify_index_checksum","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-09T05:27:53Z","receivedAt":"2017-05-09T05:28:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> On Fri, Apr 14, 2017 at 10:32 PM, wrote:\n>> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\n>> index 33a51c9..677e15a 100755\n>> --- a/t/t1450-fsck.sh\n>> +++ b/t/t1450-fsck.sh\n>> @@ -689,4 +689,17 @@ test_expect_success 'bogus head does not fallback to all heads' '\n>> ! grep $blob out\n>> '\n>>\n>> +test_expect_success 'detect corrupt index file in fsck' '\n>> + cp .git/index .git/index.backup &&\n>> + test_when_finished \"mv .git/index.backup .git/index\" &&\n>> + echo zzzzzzzz >zzzzzzzz &&\n>> + git add zzzzzzzz &&\n>> + sed -e \"s/zzzzzzzz/yyyyyyyy/\" .git/index >.git/index.yyy &&\n>> + mv .git/index.yyy .git/index &&\n>> + # Confirm that fsck detects invalid checksum\n>> + test_must_fail git fsck --cache &&\n>> + # Confirm that status no longer complains about invalid checksum\n>> + git status\n>> +'\n>\n> This test does not pass when the GIT_TEST_SPLIT_INDEX env variable is\n> set on my Linux machine.\n>\n> Also it looks like you sent a v8 of this patch series with a different\n> test, but what is in master looks like the above test instead of the\n> test in your v8.\n\nThanks for raising this. I almost forgot that the final finishing\ntouch to update the test was needed in 'master', and this exchange\ncaught it before it is too late.\n\n"}]}