Re: [PATCH v3 6/6] Make sure that index-pack --strict checks tag objects
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:29
Johannes Schindelin [off-list ref] writes:
One of the most important use cases for the strict tag object checking is when transfer.fsckobjects is set to true to catch invalid objects early on. This new regression test essentially tests the same code path by directly calling 'index-pack --strict' on a pack containing an tag object without a 'tagger' line. Technically, this test is not enough: it only exercises a code path that *warns*, not one that *fails*. The reason is that it would be exquisitely convoluted to test that: not only hash-object, but also pack-index actually *parse* tag objects when encountering them. Therefore we would have to actively *break* pack-index in order to test this. Or rewrite both hash-object and pack-index in shell script. Ain't gonna happen.
It is perfectly OK to feel and even say "I am not going to do that in this series" here, but is not very welcome to cast such a negative "Ain't gonna happen." attitude in stone in the log message in our history. When our toolset has become too tight without leaving enough escape hatch to hinder further development, it is very sensible to at least think about adding a new "--for-debug" option to hash-object and pack-objects that allows us to deliberately create broken datastreams to test against. I think peff recently added helpers to t5303 to allow us test corrupt pack streams, which is a way different from what you envisioned above (i.e. "actively break pack-index") to test breakages like the ones you were trying to test here. Having said all that, I do think the series that added new warnings, errors and a test for the new warnings is an improvement without a test for the new errors. So let's queue this, see if somebody comes up a way to check for these error detection codepath on top of this series. Thanks.
quoted hunk
Signed-off-by: Johannes Schindelin <redacted> --- t/t5302-pack-index.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh index 4bbb718..4d033df 100755 --- a/t/t5302-pack-index.sh +++ b/t/t5302-pack-index.sh@@ -243,4 +243,23 @@ test_expect_success 'running index-pack in the object store' ' test -f .git/objects/pack/pack-${pack1}.idx ' +test_expect_success 'index-pack --strict warns upon missing tagger in tag' ' + sha=$(git rev-parse HEAD) && + cat >wrong-tag <<EOF && +object $sha +type commit +tag guten tag + +This is an invalid tag. +EOF + + tag=$(git hash-object -t tag -w --stdin <wrong-tag) && + pack1=$(echo $tag $sha | git pack-objects tag-test) && + echo remove tag object && + thirtyeight=${tag#??} && + rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight && + git index-pack --strict tag-test-${pack1}.pack 2> err &&
s/2> err/2>err/;
+ grep "^error:.* expected .tagger. line" err +' + test_done