Re: [PATCH 4/7] commit-graph: fix generation number v2 overflow values

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 24, 2022, 22:35 UTC
Message-ID: <xmqqbkyvki54.fsf@gitster.g>
In-Reply-To: <de7ab2f39d90c6b33b21a5adf126ec2ef5ce27e6.1645735117.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -			graph_data->generation = get_be64(g->chunk_generation_data_overflow + 8 * offset_pos);
> +			graph_data->generation = item->date + get_be64(g->chunk_generation_data_overflow + 8 * offset_pos);

Wow, that's embarrassing.

Show 20 quoted lines

> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index 1afee1c2705..5e4b0216fa6 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -815,6 +815,19 @@ test_expect_success 'corrupt commit-graph write (missing tree)' '
>  	)
>  '
>  
> +# The remaining tests check timestamps that flow over
> +# 32-bits. The graph_git_behavior checks can't take a
> +# prereq, so just stop here if we are on a 32-bit machine.
> +
> +if ! test_have_prereq TIME_IS_64BIT
> +then
> +	test_done
> +fi
> +if ! test_have_prereq TIME_T_IS_64BIT
> +then
> +	test_done
> +fi

The above is OK but is there a reason why we cannot do

	if A || B
	then
		test_done
	fi

here (I am assuming not, but in case I am missing the reason why it has to be separate)?

Show 10 quoted lines

> @@ -832,10 +845,10 @@ test_expect_success 'corrupt commit-graph write (missing tree)' '
>  # The largest offset observed is 2 ^ 31, just large enough to overflow.
>  #
>  
> -test_expect_success 'set up and verify repo with generation data overflow chunk' '
> +test_expect_success TIME_IS_64BIT,TIME_T_IS_64BIT 'set up and verify repo with generation data overflow chunk' '
>  	objdir=".git/objects" &&
>  	UNIX_EPOCH_ZERO="@0 +0000" &&
> -	FUTURE_DATE="@2147483646 +0000" &&
> +	FUTURE_DATE="@4000000000 +0000" &&

OK. 16#EE6B2800 too large to fit and will cause wrapping around with signed 32-bit integer.

Show 9 quoted lines

> @@ -867,4 +880,8 @@ test_expect_success 'set up and verify repo with generation data overflow chunk'
>  
>  graph_git_behavior 'generation data overflow chunk repo' repo left right
>  
> +# Do not add tests at the end of this file, unless they require 64-bit
> +# timestamps, since this portion of the script is only executed when
> +# time data types have 64 bits.
> +
>  test_done