# [RFC] t5500 considered dangerous 5 messages from 2005-12-26 to 2005-12-27. Participants: Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/2933 ## Johannes Schindelin, 2005-12-26 23:35 Subject: [RFC] t5500 considered dangerous Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512270018310.3067%40wbgn013.biozentrum.uni-wuerzburg.de ``` 'corrupt a pack and see if verify catches' is bound to fail sometimes for non-obvious reasons: The test case corrupts a pack, and then also the index of that pack, by writing "0" at certain offsets, and then tests if git-verify-pack fails. However, said pack contains a commit, which is variable by virtue of storing two dates. Therefore, the deflated objects are no longer deterministic, and neither the pack. What happens is that in roughly every 256th run, there *is* a 0 at one of these offsets, and the test fails. This patch makes the test deterministic by skipping the commit. Signed-off-by: Johannes Schindelin --- I just hunted down a bug which showed up very rarely, and it did not make the hunting easier. t/t5300-pack-object.sh | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index df6549c..d6b6697 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -20,12 +20,11 @@ test_expect_success \ done && cat c >d && echo foo >>d && git-update-index --add d && tree=`git-write-tree` && - commit=`git-commit-tree $tree obj-list && { - git-diff-tree --root -p $commit && + git-diff-tree --root -p $tree && while read object do t=`git-cat-file -t $object` && @@ -99,7 +98,7 @@ test_expect_success \ GIT_OBJECT_DIRECTORY=.git2/objects && export GIT_OBJECT_DIRECTORY && cp test-1-${packname_1}.pack test-1-${packname_1}.idx .git2/objects/pack && { - git-diff-tree --root -p $commit && + git-diff-tree --root -p $tree && while read object do t=`git-cat-file -t $object` && @@ -114,7 +113,7 @@ test_expect_success \ export GIT_OBJECT_DIRECTORY && rm -f .git2/objects/pack/test-?.idx && cp test-2-${packname_2}.pack test-2-${packname_2}.idx .git2/objects/pack && { - git-diff-tree --root -p $commit && + git-diff-tree --root -p $tree && while read object do t=`git-cat-file -t $object` && ``` ## Johannes Schindelin, 2005-12-26 23:37 Subject: Re: [RFC] t5500 considered dangerous Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512270037240.4002%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` Of course, it is t5300. Sorry for the noise. ``` ## Junio C Hamano, 2005-12-27 02:10 Subject: Re: [RFC] t5300 considered dangerous Message-ID: <7vd5jjffxu.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vd5jjffxu.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin writes: > The test case corrupts a pack, and then also the index of that pack, by > writing "0" at certain offsets, and then tests if git-verify-pack fails. These certain offsets are: . byte 2 in pack This corrupts PACK_SIGNATURE ("A" in "PACK"). . byte 7 in pack This corrupts PACK_VERSION ("\002" in "\0\0\0\002"). . byte 12 in pack This corrupts the type/size byte of whatever the first object happens to be (the type/size byte has non-zero upper nibble). . byte 1200 in idx that is currently 1208 bytes long. This tries to corrupt the checksum of the index file itself, You are right about the last one; this location has 1/256 chance of validly being 0, and making the set of objects to be packed constant is one way to fix it. More appropriate way would be to measure how long the .idx file is, and corrupt the last 20 bytes; I was too lazy. -- >8 -- diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh index 7dfb1ab..dd719bb 100755 --- a/t/t5300-pack-object.sh +++ b/t/t5300-pack-object.sh @@ -163,8 +163,10 @@ test_expect_success \ else :; fi && + l=`wc -c URL: https://gitlist.dev/e/Pine.LNX.4.63.0512271452470.17086%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vd5jjffxu.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Mon, 26 Dec 2005, Junio C Hamano wrote: > Johannes Schindelin writes: > > > The test case corrupts a pack, and then also the index of that pack, by > > writing "0" at certain offsets, and then tests if git-verify-pack fails. > > These certain offsets are: > > . byte 2 in pack > This corrupts PACK_SIGNATURE ("A" in "PACK"). > > . byte 7 in pack > This corrupts PACK_VERSION ("\002" in "\0\0\0\002"). > > . byte 12 in pack > This corrupts the type/size byte of whatever the first object > happens to be (the type/size byte has non-zero upper nibble). > > . byte 1200 in idx that is currently 1208 bytes long. > This tries to corrupt the checksum of the index file itself, Thank you for clarifying this. > diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh > index 7dfb1ab..dd719bb 100755 > --- a/t/t5300-pack-object.sh > +++ b/t/t5300-pack-object.sh > @@ -163,8 +163,10 @@ test_expect_success \ > else :; > fi && > > + l=`wc -c + l=`expr "$l" - 20` && This needs to be > + l=`expr $l - 20` && since there are textutils out there (notably version 2.1) where "wc -c" prints a leading space. > cp test-1-${packname_1}.pack test-3.pack && > - dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 && > + dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l && > if git-verify-pack test-3.pack > then false > else :; When the commit is not skipped, I fail to see why this should not fail in one out of 256 cases: the input is (partially) pseudo-random. The last 20 bytes are the SHA-1 which should inherit this pseudo-randomness. So, the first byte should be pseudo-random, too. Something I missed? Ciao, Dscho ``` ## Johannes Schindelin, 2005-12-27 14:08 Subject: Re: [RFC] t5300 considered dangerous Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0512271507490.318%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` Hi, On Tue, 27 Dec 2005, Johannes Schindelin wrote: > On Mon, 26 Dec 2005, Junio C Hamano wrote: > > > cp test-1-${packname_1}.pack test-3.pack && > > - dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 && > > + dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l && > > if git-verify-pack test-3.pack > > then false > > else :; > > When the commit is not skipped, I fail to see why this should not fail in > one out of 256 cases: the input is (partially) pseudo-random. The last 20 > bytes are the SHA-1 which should inherit this pseudo-randomness. So, the > first byte should be pseudo-random, too. > > Something I missed? Yes, I missed the "count=20". Sorry, Dscho ```