Re: [PATCH] pack-check: fix bogus corruption report with core.bigFileThreshold

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> When verifying packed blobs for consistency we have two different paths:
>
>   - When the blob is larger than or equal to "core.bigFileThreshold"
>     we verify its object ID by streaming it.
>
>   - Otherwise we read the object's data into memory and then hash it.
>
> When a packed blob is exactly at the threshold then we'll also take the
> streaming path. This is at odds with the rest of the codebase though,
> where we consider an object large only when it exceeds the threshold.

> diff --git a/pack-check.c b/pack-check.c
> index c7275f4b92..605811b7b0 100644
> --- a/pack-check.c
> +++ b/pack-check.c
> @@ -134,7 +134,7 @@ static int verify_packfile(struct repository *r,
>  		unuse_pack(w_curs);
>  
>  		if (type == OBJ_BLOB &&
> -		    repo_settings_get_big_file_threshold(r) <= size) {
> +		    repo_settings_get_big_file_threshold(r) < size) {
>  			/*
>  			 * Let stream_object_signature() check it with
>  			 * the streaming interface; no point slurping

OK, this signals a later step with the (data==NULL, data_valid==0)
that the size of the pack entry is sufficiently large that we do not
want to hold it in-core only to verify the object name.  And the later
step calls packfile_read_object_stream() to prepare an istream.

I agree with you that "Use the streaming interface only for objects
larger than what we are willing to hold in core" is a policy thing,
and checking the policy in packfile_read_object_stream() is dubious,
but since the above is checking the same policy, so we should do the
comparison the same way.

> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index 1b4074304c..16cc5d5dcb 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -1152,4 +1152,17 @@ test_expect_success 'fsck reports problems in current worktree index without fil
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'fsck with blob size equal to core.bigFileThreshold' '
> +	test_when_finished rm -rf big-blob-fsck &&
> +	git init big-blob-fsck &&
> +	(
> +		cd big-blob-fsck &&
> +		test-tool genzeros 1024 >blob &&
> +		git add blob &&
> +		git commit -m "blob equal to threshold" &&
> +		git repack -ad &&
> +		git -c core.bigFileThreshold=1k fsck --no-dangling
> +	)
> +'

Looks obviously correct.  Will queue.  Thanks.

> +
>  test_done
>
> ---
> base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
> change-id: 20261014-b4-pks-pack-check-matching-large-file-threshold-fd7be721d6a4