{"thread":{"id":"48463","subject":"Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch","startedAt":"2018-05-10T20:46:18Z","lastAt":"2018-05-11T12:30:51Z","messageCount":2,"participants":["Martin Ågren","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":12},"messages":[{"id":"347310","messageId":"20180510204557.16361-1-martin.agren@gmail.com","threadId":"48463","inReplyTo":null,"subject":"Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-05-10T20:45:57Z","receivedAt":"2018-05-10T20:46:18Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 10 May 2018 at 21:22, Stefan Beller <sbeller@google.com> wrote:\n> On Thu, May 10, 2018 at 12:05 PM, Martin Ågren <martin.agren@gmail.com> wrote:\n\n>> I hope to find time to do some more hands-on testing of this to see that\n>> errors actually do get caught.\n\n> Packfiles and loose objects are primary data, which means that those\n> need a more advanced way to diagnose and repair them, so I would imagine\n> the commit graph fsck is closer to bitmaps fsck, which I would have suspected\n> to be found in t5310, but a quick read doesn't reveal many tests that are\n> checking for integrity. So I guess the test coverage here is ok, (although we\n> should always ask for more)\n\nSince I'm wrapping up for today, I'm posting some simple tests that I\nassembled. The last of these showcases one or two problems with the\ncurrent error-reporting. Depending on the error, there can be *lots* of\nerrors reported and there are no new-lines, so the result on stdout can\nbe a wall of not-very-legible text.\n\nSome of these might not make sense. I just started going through the\ndocumentation on the format, causing some sort of corruption in each\nfield. Maybe this can be helpful somehow.\n\nMartin\n\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 82f95eb11f..a7e48db2de 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -255,4 +255,49 @@ test_expect_success 'git fsck (checks commit-graph)' '\n \tgit fsck\n '\n \n+# usage: corrupt_data <file> <pos> [<data>]\n+corrupt_data() {\n+\tfile=$1\n+\tpos=$2\n+\tdata=\"${3:-\\0}\"\n+\tprintf \"$data\" | dd of=\"$file\" bs=1 seek=\"$pos\" conv=notrunc\n+}\n+\n+test_expect_success 'detect bad signature' '\n+\tcd \"$TRASH_DIRECTORY/full\" &&\n+\tcp $objdir/info/commit-graph commit-graph-backup &&\n+\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n+\tcorrupt_data $objdir/info/commit-graph 0 \"\\0\" &&\n+\ttest_must_fail git commit-graph verify 2>err &&\n+\tgrep \"graph signature\" err\n+'\n+\n+test_expect_success 'detect bad version number' '\n+\tcd \"$TRASH_DIRECTORY/full\" &&\n+\tcp $objdir/info/commit-graph commit-graph-backup &&\n+\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n+\tcorrupt_data $objdir/info/commit-graph 4 \"\\02\" &&\n+\ttest_must_fail git commit-graph verify 2>err &&\n+\tgrep \"graph version\" err\n+'\n+\n+test_expect_success 'detect bad hash version' '\n+\tcd \"$TRASH_DIRECTORY/full\" &&\n+\tcp $objdir/info/commit-graph commit-graph-backup &&\n+\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n+\tcorrupt_data $objdir/info/commit-graph 5 \"\\02\" &&\n+\ttest_must_fail git commit-graph verify 2>err &&\n+\tgrep \"hash version\" err\n+'\n+\n+test_expect_success 'detect too small chunk-count' '\n+\tcd \"$TRASH_DIRECTORY/full\" &&\n+\tcp $objdir/info/commit-graph commit-graph-backup &&\n+\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n+\tcorrupt_data $objdir/info/commit-graph 6 \"\\01\" &&\n+\ttest_must_fail git commit-graph verify 2>err &&\n+\tcat err\n+'\n+\n+\n test_done\n-- \n2.17.0\n\n"},{"id":"347352","messageId":"cf443352-7840-9ff9-77af-cb4c52932fcb@gmail.com","threadId":"48463","inReplyTo":"20180510204557.16361-1-martin.agren@gmail.com","subject":"Re: [PATCH 00/12] Integrate commit-graph into fsck, gc, and fetch","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-05-11T12:30:45Z","receivedAt":"2018-05-11T12:30:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/10/2018 4:45 PM, Martin Ågren wrote:\n> On 10 May 2018 at 21:22, Stefan Beller <sbeller@google.com> wrote:\n>> On Thu, May 10, 2018 at 12:05 PM, Martin Ågren <martin.agren@gmail.com> wrote:\n>>> I hope to find time to do some more hands-on testing of this to see that\n>>> errors actually do get caught.\n>> Packfiles and loose objects are primary data, which means that those\n>> need a more advanced way to diagnose and repair them, so I would imagine\n>> the commit graph fsck is closer to bitmaps fsck, which I would have suspected\n>> to be found in t5310, but a quick read doesn't reveal many tests that are\n>> checking for integrity. So I guess the test coverage here is ok, (although we\n>> should always ask for more)\n> Since I'm wrapping up for today, I'm posting some simple tests that I\n> assembled. The last of these showcases one or two problems with the\n> current error-reporting. Depending on the error, there can be *lots* of\n> errors reported and there are no new-lines, so the result on stdout can\n> be a wall of not-very-legible text.\n>\n> Some of these might not make sense. I just started going through the\n> documentation on the format, causing some sort of corruption in each\n> field. Maybe this can be helpful somehow.\n>\n> Martin\n>\n> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> index 82f95eb11f..a7e48db2de 100755\n> --- a/t/t5318-commit-graph.sh\n> +++ b/t/t5318-commit-graph.sh\n> @@ -255,4 +255,49 @@ test_expect_success 'git fsck (checks commit-graph)' '\n>   \tgit fsck\n>   '\n>   \n> +# usage: corrupt_data <file> <pos> [<data>]\n> +corrupt_data() {\n> +\tfile=$1\n> +\tpos=$2\n> +\tdata=\"${3:-\\0}\"\n> +\tprintf \"$data\" | dd of=\"$file\" bs=1 seek=\"$pos\" conv=notrunc\n> +}\n> +\n> +test_expect_success 'detect bad signature' '\n> +\tcd \"$TRASH_DIRECTORY/full\" &&\n> +\tcp $objdir/info/commit-graph commit-graph-backup &&\n> +\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n> +\tcorrupt_data $objdir/info/commit-graph 0 \"\\0\" &&\n> +\ttest_must_fail git commit-graph verify 2>err &&\n> +\tgrep \"graph signature\" err\n> +'\n> +\n> +test_expect_success 'detect bad version number' '\n> +\tcd \"$TRASH_DIRECTORY/full\" &&\n> +\tcp $objdir/info/commit-graph commit-graph-backup &&\n> +\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n> +\tcorrupt_data $objdir/info/commit-graph 4 \"\\02\" &&\n> +\ttest_must_fail git commit-graph verify 2>err &&\n> +\tgrep \"graph version\" err\n> +'\n> +\n> +test_expect_success 'detect bad hash version' '\n> +\tcd \"$TRASH_DIRECTORY/full\" &&\n> +\tcp $objdir/info/commit-graph commit-graph-backup &&\n> +\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n> +\tcorrupt_data $objdir/info/commit-graph 5 \"\\02\" &&\n> +\ttest_must_fail git commit-graph verify 2>err &&\n> +\tgrep \"hash version\" err\n> +'\n> +\n> +test_expect_success 'detect too small chunk-count' '\n> +\tcd \"$TRASH_DIRECTORY/full\" &&\n> +\tcp $objdir/info/commit-graph commit-graph-backup &&\n> +\ttest_when_finished mv commit-graph-backup $objdir/info/commit-graph &&\n> +\tcorrupt_data $objdir/info/commit-graph 6 \"\\01\" &&\n> +\ttest_must_fail git commit-graph verify 2>err &&\n> +\tcat err\n> +'\n> +\n> +\n>   test_done\n\n\nMartin: thank you so much for these test examples, and for running them \nto find out about the newline issue. This is really helpful.\n"}]}