{"thread":{"id":"2933","subject":"[RFC] t5500 considered dangerous","startedAt":"2005-12-26T23:35:21Z","lastAt":"2005-12-27T14:08:30Z","messageCount":5,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"14052","messageId":"Pine.LNX.4.63.0512270018310.3067@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2933","inReplyTo":null,"subject":"[RFC] t5500 considered dangerous","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-12-26T23:35:21Z","receivedAt":"2005-12-26T23:35:21Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n'corrupt a pack and see if verify catches' is bound to fail sometimes for \nnon-obvious reasons:\n\nThe test case corrupts a pack, and then also the index of that pack, by \nwriting \"0\" at certain offsets, and then tests if git-verify-pack fails.\n\nHowever, said pack contains a commit, which is variable by virtue of \nstoring two dates. Therefore, the deflated objects are no longer \ndeterministic, and neither the pack. What happens is that in roughly every \n256th run, there *is* a 0 at one of these offsets, and the test fails.\n\nThis patch makes the test deterministic by skipping the commit.\n\nSigned-off-by: Johannes Schindelin \n\n---\n\n\tI just hunted down a bug which showed up very rarely, and it did \n\tnot make the hunting easier.\n\n t/t5300-pack-object.sh | 9 ++++-----\n 1 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex df6549c..d6b6697 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -20,12 +20,11 @@ test_expect_success \\\n done &&\n cat c >d && echo foo >>d && git-update-index --add d &&\n tree=`git-write-tree` &&\n- commit=`git-commit-tree $tree obj-list && {\n-\t git-diff-tree --root -p $commit &&\n+\t git-diff-tree --root -p $tree &&\n \t while read object\n \t do\n \t t=`git-cat-file -t $object` &&\n@@ -99,7 +98,7 @@ test_expect_success \\\n GIT_OBJECT_DIRECTORY=.git2/objects &&\n export GIT_OBJECT_DIRECTORY &&\n cp test-1-${packname_1}.pack test-1-${packname_1}.idx .git2/objects/pack && {\n-\t git-diff-tree --root -p $commit &&\n+\t git-diff-tree --root -p $tree &&\n \t while read object\n \t do\n \t t=`git-cat-file -t $object` &&\n@@ -114,7 +113,7 @@ test_expect_success \\\n export GIT_OBJECT_DIRECTORY &&\n rm -f .git2/objects/pack/test-?.idx &&\n cp test-2-${packname_2}.pack test-2-${packname_2}.idx .git2/objects/pack && {\n-\t git-diff-tree --root -p $commit &&\n+\t git-diff-tree --root -p $tree &&\n \t while read object\n \t do\n \t t=`git-cat-file -t $object` &&\n"},{"id":"14053","messageId":"Pine.LNX.4.63.0512270037240.4002@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2933","inReplyTo":"Pine.LNX.4.63.0512270018310.3067@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [RFC] t5500 considered dangerous","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-12-26T23:37:42Z","receivedAt":"2005-12-26T23:37:42Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Of course, it is t5300. Sorry for the noise.\n"},{"id":"14056","messageId":"7vd5jjffxu.fsf@assigned-by-dhcp.cox.net","threadId":"2933","inReplyTo":"Pine.LNX.4.63.0512270018310.3067@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [RFC] t5300 considered dangerous","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-27T02:10:21Z","receivedAt":"2005-12-27T02:10:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> The test case corrupts a pack, and then also the index of that pack, by \n> writing \"0\" at certain offsets, and then tests if git-verify-pack fails.\n\nThese certain offsets are:\n\n . byte 2 in pack\n This corrupts PACK_SIGNATURE (\"A\" in \"PACK\").\n\n . byte 7 in pack\n This corrupts PACK_VERSION (\"\\002\" in \"\\0\\0\\0\\002\").\n\n . byte 12 in pack\n This corrupts the type/size byte of whatever the first object\n happens to be (the type/size byte has non-zero upper nibble).\n\n . byte 1200 in idx that is currently 1208 bytes long.\n This tries to corrupt the checksum of the index file itself,\n\nYou are right about the last one; this location has 1/256 chance\nof validly being 0, and making the set of objects to be packed\nconstant is one way to fix it. More appropriate way would be to\nmeasure how long the .idx file is, and corrupt the last 20 bytes;\nI was too lazy.\n\n-- >8 --\n\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\nindex 7dfb1ab..dd719bb 100755\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -163,8 +163,10 @@ test_expect_success \\\n else :;\n fi &&\n \n+ l=`wc -c Johannes Schindelin writes:\n> \n> > The test case corrupts a pack, and then also the index of that pack, by \n> > writing \"0\" at certain offsets, and then tests if git-verify-pack fails.\n> \n> These certain offsets are:\n> \n> . byte 2 in pack\n> This corrupts PACK_SIGNATURE (\"A\" in \"PACK\").\n> \n> . byte 7 in pack\n> This corrupts PACK_VERSION (\"\\002\" in \"\\0\\0\\0\\002\").\n> \n> . byte 12 in pack\n> This corrupts the type/size byte of whatever the first object\n> happens to be (the type/size byte has non-zero upper nibble).\n> \n> . byte 1200 in idx that is currently 1208 bytes long.\n> This tries to corrupt the checksum of the index file itself,\n\nThank you for clarifying this.\n\n> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n> index 7dfb1ab..dd719bb 100755\n> --- a/t/t5300-pack-object.sh\n> +++ b/t/t5300-pack-object.sh\n> @@ -163,8 +163,10 @@ test_expect_success \\\n> else :;\n> fi &&\n> \n> + l=`wc -c + l=`expr \"$l\" - 20` &&\n\nThis needs to be\n\n> + l=`expr $l - 20` &&\n\nsince there are textutils out there (notably version 2.1) where \"wc -c\" \nprints a leading space.\n\n> cp test-1-${packname_1}.pack test-3.pack &&\n> - dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 &&\n> + dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l &&\n> if git-verify-pack test-3.pack\n> then false\n> else :;\n\nWhen the commit is not skipped, I fail to see why this should not fail in \none out of 256 cases: the input is (partially) pseudo-random. The last 20 \nbytes are the SHA-1 which should inherit this pseudo-randomness. So, the \nfirst byte should be pseudo-random, too.\n\nSomething I missed?\n\nCiao,\nDscho\n"},{"id":"14064","messageId":"Pine.LNX.4.63.0512271507490.318@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2933","inReplyTo":"Pine.LNX.4.63.0512271452470.17086@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [RFC] t5300 considered dangerous","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-12-27T14:08:30Z","receivedAt":"2005-12-27T14:08:30Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Dec 2005, Johannes Schindelin wrote:\n\n> On Mon, 26 Dec 2005, Junio C Hamano wrote:\n> \n> > cp test-1-${packname_1}.pack test-3.pack &&\n> > - dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 &&\n> > + dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l &&\n> > if git-verify-pack test-3.pack\n> > then false\n> > else :;\n> \n> When the commit is not skipped, I fail to see why this should not fail in \n> one out of 256 cases: the input is (partially) pseudo-random. The last 20 \n> bytes are the SHA-1 which should inherit this pseudo-randomness. So, the \n> first byte should be pseudo-random, too.\n> \n> Something I missed?\n\nYes, I missed the \"count=20\".\n\nSorry,\nDscho\n"}]}