Re: [PATCH v8] read-cache: call verify_hdr() in a background thread

2017-04-26 Thread Junio C Hamano
Jeff Hostetler  writes:

> Either version is fine.  I'd say use my perl version as I have tested it and
> it is simple enough and already in the tree.  I don't think it's worth the
> bother to switch it to the dd version.

Thanks, I agree what you said is very sensible.


Re: [PATCH v8] read-cache: call verify_hdr() in a background thread

2017-04-26 Thread Jeff Hostetler



On 4/26/2017 12:34 AM, Junio C Hamano wrote:

Junio C Hamano  writes:


g...@jeffhostetler.com writes:


From: Jeff Hostetler 

Version 8 of this patch converts the unit test to use
perl to corrupt the index checksum (rather than altering
a filename) and also verifies the fsck error message as
suggested in response to v7 on the mailing list.

If there are no other suggestions, I think this version
should be considered final.

Oops.  The earlier one has already been in 'master' for a few days.
Let's make this an incremental update.

Is the description in the following something you are OK with (so
that I can add your sign-off)?

Thanks.

By the way, I said I am fine with the two-liner version in Dscho's
message, but I am also OK with this version that does not use a
separate dd and instead does everything in a single invocation of
Perl.  As long as you've tested this version, there is no point
replacing it with yet another one.


Either version is fine.  I'd say use my perl version as I have tested it and
it is simple enough and already in the tree.  I don't think it's worth the
bother to switch it to the dd version.

Thanks
Jeff


Thanks.


-- >8 --
From: Jeff Hostetler 
Date: Tue, 25 Apr 2017 18:41:09 +
Subject: t1450: avoid use of "sed" on the index, which is a binary file

The previous step added a path  to the index, and then used
"sed" to replace this string to  to create a test case where
the checksum at the end of the file does not match the contents.

Unfortunately, use of "sed" on a non-text file is not portable.
Instead, use a Perl script that seeks to the end and modifies the
last byte of the file (where we _know_ stores the trailing
checksum).


---
  t/t1450-fsck.sh | 33 ++---
  1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 677e15a7a4..eff1cd68e9 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -689,16 +689,35 @@ test_expect_success 'bogus head does not fallback to all 
heads' '
! grep $blob out
  '
  
+# Corrupt the checksum on the index.

+# Add 1 to the last byte in the SHA.
+corrupt_index_checksum () {
+perl -w -e '
+   use Fcntl ":seek";
+   open my $fh, "+<", ".git/index" or die "open: $!";
+   binmode $fh;
+   seek $fh, -1, SEEK_END or die "seek: $!";
+   read $fh, my $in_byte, 1 or die "read: $!";
+
+   $in_value = unpack("C", $in_byte);
+   $out_value = ($in_value + 1) & 255;
+
+   $out_byte = pack("C", $out_value);
+
+   seek $fh, -1, SEEK_END or die "seek: $!";
+   print $fh $out_byte;
+   close $fh or die "close: $!";
+'
+}
+
+# Corrupt the checksum on the index and then
+# verify that only fsck notices.
  test_expect_success 'detect corrupt index file in fsck' '
cp .git/index .git/index.backup &&
test_when_finished "mv .git/index.backup .git/index" &&
-   echo  > &&
-   git add  &&
-   sed -e "s///" .git/index >.git/index.yyy &&
-   mv .git/index.yyy .git/index &&
-   # Confirm that fsck detects invalid checksum
-   test_must_fail git fsck --cache &&
-   # Confirm that status no longer complains about invalid checksum
+   corrupt_index_checksum &&
+   test_must_fail git fsck --cache 2>expect &&
+   grep "bad index file" expect &&
git status
  '
  




Re: [PATCH v8] read-cache: call verify_hdr() in a background thread

2017-04-26 Thread Jeff Hostetler


On 4/26/2017 12:11 AM, Junio C Hamano wrote:

g...@jeffhostetler.com writes:


From: Jeff Hostetler 

Version 8 of this patch converts the unit test to use
perl to corrupt the index checksum (rather than altering
a filename) and also verifies the fsck error message as
suggested in response to v7 on the mailing list.

If there are no other suggestions, I think this version
should be considered final.

Oops.  The earlier one has already been in 'master' for a few days.
Let's make this an incremental update.

Is the description in the following something you are OK with (so
that I can add your sign-off)?


Yes, this is fine.  Thanks!
Jeff



Thanks.

-- >8 --
From: Jeff Hostetler 
Date: Tue, 25 Apr 2017 18:41:09 +
Subject: t1450: avoid use of "sed" on the index, which is a binary file

The previous step added a path  to the index, and then used
"sed" to replace this string to  to create a test case where
the checksum at the end of the file does not match the contents.

Unfortunately, use of "sed" on a non-text file is not portable.
Instead, use a Perl script that seeks to the end and modifies the
last byte of the file (where we _know_ stores the trailing
checksum).


---
  t/t1450-fsck.sh | 33 ++---
  1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 677e15a7a4..eff1cd68e9 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -689,16 +689,35 @@ test_expect_success 'bogus head does not fallback to all 
heads' '
! grep $blob out
  '
  
+# Corrupt the checksum on the index.

+# Add 1 to the last byte in the SHA.
+corrupt_index_checksum () {
+perl -w -e '
+   use Fcntl ":seek";
+   open my $fh, "+<", ".git/index" or die "open: $!";
+   binmode $fh;
+   seek $fh, -1, SEEK_END or die "seek: $!";
+   read $fh, my $in_byte, 1 or die "read: $!";
+
+   $in_value = unpack("C", $in_byte);
+   $out_value = ($in_value + 1) & 255;
+
+   $out_byte = pack("C", $out_value);
+
+   seek $fh, -1, SEEK_END or die "seek: $!";
+   print $fh $out_byte;
+   close $fh or die "close: $!";
+'
+}
+
+# Corrupt the checksum on the index and then
+# verify that only fsck notices.
  test_expect_success 'detect corrupt index file in fsck' '
cp .git/index .git/index.backup &&
test_when_finished "mv .git/index.backup .git/index" &&
-   echo  > &&
-   git add  &&
-   sed -e "s///" .git/index >.git/index.yyy &&
-   mv .git/index.yyy .git/index &&
-   # Confirm that fsck detects invalid checksum
-   test_must_fail git fsck --cache &&
-   # Confirm that status no longer complains about invalid checksum
+   corrupt_index_checksum &&
+   test_must_fail git fsck --cache 2>expect &&
+   grep "bad index file" expect &&
git status
  '
  




Re: [PATCH v8] read-cache: call verify_hdr() in a background thread

2017-04-25 Thread Junio C Hamano
Junio C Hamano  writes:

> g...@jeffhostetler.com writes:
>
>> From: Jeff Hostetler 
>>
>> Version 8 of this patch converts the unit test to use
>> perl to corrupt the index checksum (rather than altering
>> a filename) and also verifies the fsck error message as
>> suggested in response to v7 on the mailing list.
>>
>> If there are no other suggestions, I think this version
>> should be considered final.
>
> Oops.  The earlier one has already been in 'master' for a few days.
> Let's make this an incremental update.  
>
> Is the description in the following something you are OK with (so
> that I can add your sign-off)?
>
> Thanks.

By the way, I said I am fine with the two-liner version in Dscho's
message, but I am also OK with this version that does not use a
separate dd and instead does everything in a single invocation of
Perl.  As long as you've tested this version, there is no point
replacing it with yet another one.

Thanks.

> -- >8 --
> From: Jeff Hostetler 
> Date: Tue, 25 Apr 2017 18:41:09 +
> Subject: t1450: avoid use of "sed" on the index, which is a binary file
>
> The previous step added a path  to the index, and then used
> "sed" to replace this string to  to create a test case where
> the checksum at the end of the file does not match the contents.
>
> Unfortunately, use of "sed" on a non-text file is not portable.
> Instead, use a Perl script that seeks to the end and modifies the
> last byte of the file (where we _know_ stores the trailing
> checksum).
>
>
> ---
>  t/t1450-fsck.sh | 33 ++---
>  1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index 677e15a7a4..eff1cd68e9 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -689,16 +689,35 @@ test_expect_success 'bogus head does not fallback to 
> all heads' '
>   ! grep $blob out
>  '
>  
> +# Corrupt the checksum on the index.
> +# Add 1 to the last byte in the SHA.
> +corrupt_index_checksum () {
> +perl -w -e '
> + use Fcntl ":seek";
> + open my $fh, "+<", ".git/index" or die "open: $!";
> + binmode $fh;
> + seek $fh, -1, SEEK_END or die "seek: $!";
> + read $fh, my $in_byte, 1 or die "read: $!";
> +
> + $in_value = unpack("C", $in_byte);
> + $out_value = ($in_value + 1) & 255;
> +
> + $out_byte = pack("C", $out_value);
> +
> + seek $fh, -1, SEEK_END or die "seek: $!";
> + print $fh $out_byte;
> + close $fh or die "close: $!";
> +'
> +}
> +
> +# Corrupt the checksum on the index and then
> +# verify that only fsck notices.
>  test_expect_success 'detect corrupt index file in fsck' '
>   cp .git/index .git/index.backup &&
>   test_when_finished "mv .git/index.backup .git/index" &&
> - echo  > &&
> - git add  &&
> - sed -e "s///" .git/index >.git/index.yyy &&
> - mv .git/index.yyy .git/index &&
> - # Confirm that fsck detects invalid checksum
> - test_must_fail git fsck --cache &&
> - # Confirm that status no longer complains about invalid checksum
> + corrupt_index_checksum &&
> + test_must_fail git fsck --cache 2>expect &&
> + grep "bad index file" expect &&
>   git status
>  '
>  


Re: [PATCH v8] read-cache: call verify_hdr() in a background thread

2017-04-25 Thread Junio C Hamano
g...@jeffhostetler.com writes:

> From: Jeff Hostetler 
>
> Version 8 of this patch converts the unit test to use
> perl to corrupt the index checksum (rather than altering
> a filename) and also verifies the fsck error message as
> suggested in response to v7 on the mailing list.
>
> If there are no other suggestions, I think this version
> should be considered final.

Oops.  The earlier one has already been in 'master' for a few days.
Let's make this an incremental update.  

Is the description in the following something you are OK with (so
that I can add your sign-off)?

Thanks.

-- >8 --
From: Jeff Hostetler 
Date: Tue, 25 Apr 2017 18:41:09 +
Subject: t1450: avoid use of "sed" on the index, which is a binary file

The previous step added a path  to the index, and then used
"sed" to replace this string to  to create a test case where
the checksum at the end of the file does not match the contents.

Unfortunately, use of "sed" on a non-text file is not portable.
Instead, use a Perl script that seeks to the end and modifies the
last byte of the file (where we _know_ stores the trailing
checksum).


---
 t/t1450-fsck.sh | 33 ++---
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 677e15a7a4..eff1cd68e9 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -689,16 +689,35 @@ test_expect_success 'bogus head does not fallback to all 
heads' '
! grep $blob out
 '
 
+# Corrupt the checksum on the index.
+# Add 1 to the last byte in the SHA.
+corrupt_index_checksum () {
+perl -w -e '
+   use Fcntl ":seek";
+   open my $fh, "+<", ".git/index" or die "open: $!";
+   binmode $fh;
+   seek $fh, -1, SEEK_END or die "seek: $!";
+   read $fh, my $in_byte, 1 or die "read: $!";
+
+   $in_value = unpack("C", $in_byte);
+   $out_value = ($in_value + 1) & 255;
+
+   $out_byte = pack("C", $out_value);
+
+   seek $fh, -1, SEEK_END or die "seek: $!";
+   print $fh $out_byte;
+   close $fh or die "close: $!";
+'
+}
+
+# Corrupt the checksum on the index and then
+# verify that only fsck notices.
 test_expect_success 'detect corrupt index file in fsck' '
cp .git/index .git/index.backup &&
test_when_finished "mv .git/index.backup .git/index" &&
-   echo  > &&
-   git add  &&
-   sed -e "s///" .git/index >.git/index.yyy &&
-   mv .git/index.yyy .git/index &&
-   # Confirm that fsck detects invalid checksum
-   test_must_fail git fsck --cache &&
-   # Confirm that status no longer complains about invalid checksum
+   corrupt_index_checksum &&
+   test_must_fail git fsck --cache 2>expect &&
+   grep "bad index file" expect &&
git status
 '