Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch

Subsystems: the rest

2 messages, 2 authors, 2018-05-11 · open the first message on its own page

Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch

From: Martin Ågren <hidden>
Date: 2018-05-10 20:46:18

On 10 May 2018 at 21:22, Stefan Beller [off-list ref] wrote:
On Thu, May 10, 2018 at 12:05 PM, Martin Ågren [off-list ref] wrote:
quoted
I hope to find time to do some more hands-on testing of this to see that
errors actually do get caught.
Packfiles and loose objects are primary data, which means that those
need a more advanced way to diagnose and repair them, so I would imagine
the commit graph fsck is closer to bitmaps fsck, which I would have suspected
to be found in t5310, but a quick read doesn't reveal many tests that are
checking for integrity. So I guess the test coverage here is ok, (although we
should always ask for more)
Since I'm wrapping up for today, I'm posting some simple tests that I
assembled. The last of these showcases one or two problems with the
current error-reporting. Depending on the error, there can be *lots* of
errors reported and there are no new-lines, so the result on stdout can
be a wall of not-very-legible text.

Some of these might not make sense. I just started going through the
documentation on the format, causing some sort of corruption in each
field. Maybe this can be helpful somehow.

Martin
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 82f95eb11f..a7e48db2de 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -255,4 +255,49 @@ test_expect_success 'git fsck (checks commit-graph)' '
 	git fsck
 '
 
+# usage: corrupt_data <file> <pos> [<data>]
+corrupt_data() {
+	file=$1
+	pos=$2
+	data="${3:-\0}"
+	printf "$data" | dd of="$file" bs=1 seek="$pos" conv=notrunc
+}
+
+test_expect_success 'detect bad signature' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 0 "\0" &&
+	test_must_fail git commit-graph verify 2>err &&
+	grep "graph signature" err
+'
+
+test_expect_success 'detect bad version number' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 4 "\02" &&
+	test_must_fail git commit-graph verify 2>err &&
+	grep "graph version" err
+'
+
+test_expect_success 'detect bad hash version' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 5 "\02" &&
+	test_must_fail git commit-graph verify 2>err &&
+	grep "hash version" err
+'
+
+test_expect_success 'detect too small chunk-count' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 6 "\01" &&
+	test_must_fail git commit-graph verify 2>err &&
+	cat err
+'
+
+
 test_done
-- 
2.17.0

Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch

From: Derrick Stolee <hidden>
Date: 2018-05-11 12:30:51

On 5/10/2018 4:45 PM, Martin Ågren wrote:
quoted hunk
On 10 May 2018 at 21:22, Stefan Beller [off-list ref] wrote:
quoted
On Thu, May 10, 2018 at 12:05 PM, Martin Ågren [off-list ref] wrote:
quoted
I hope to find time to do some more hands-on testing of this to see that
errors actually do get caught.
Packfiles and loose objects are primary data, which means that those
need a more advanced way to diagnose and repair them, so I would imagine
the commit graph fsck is closer to bitmaps fsck, which I would have suspected
to be found in t5310, but a quick read doesn't reveal many tests that are
checking for integrity. So I guess the test coverage here is ok, (although we
should always ask for more)
Since I'm wrapping up for today, I'm posting some simple tests that I
assembled. The last of these showcases one or two problems with the
current error-reporting. Depending on the error, there can be *lots* of
errors reported and there are no new-lines, so the result on stdout can
be a wall of not-very-legible text.

Some of these might not make sense. I just started going through the
documentation on the format, causing some sort of corruption in each
field. Maybe this can be helpful somehow.

Martin
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 82f95eb11f..a7e48db2de 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -255,4 +255,49 @@ test_expect_success 'git fsck (checks commit-graph)' '
  	git fsck
  '
  
+# usage: corrupt_data <file> <pos> [<data>]
+corrupt_data() {
+	file=$1
+	pos=$2
+	data="${3:-\0}"
+	printf "$data" | dd of="$file" bs=1 seek="$pos" conv=notrunc
+}
+
+test_expect_success 'detect bad signature' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 0 "\0" &&
+	test_must_fail git commit-graph verify 2>err &&
+	grep "graph signature" err
+'
+
+test_expect_success 'detect bad version number' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 4 "\02" &&
+	test_must_fail git commit-graph verify 2>err &&
+	grep "graph version" err
+'
+
+test_expect_success 'detect bad hash version' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 5 "\02" &&
+	test_must_fail git commit-graph verify 2>err &&
+	grep "hash version" err
+'
+
+test_expect_success 'detect too small chunk-count' '
+	cd "$TRASH_DIRECTORY/full" &&
+	cp $objdir/info/commit-graph commit-graph-backup &&
+	test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
+	corrupt_data $objdir/info/commit-graph 6 "\01" &&
+	test_must_fail git commit-graph verify 2>err &&
+	cat err
+'
+
+
  test_done

Martin: thank you so much for these test examples, and for running them 
to find out about the newline issue. This is really helpful.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help