Re: [PATCH] describe: count the distance correctly when commit dates are skewed

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

Tuomas Ahola <taahol@utu.fi> writes:

> Linus Torvalds <torvalds@kernel.org> wrote:
>
>> From: Linus Torvalds <torvalds@linux-foundation.org>
>> 
>> The current merge window for Linux v7.4 showed annoying behavior with
>> 'git describe', where it doesn't actually follow the documented intent
>> of the result: the commit distance is miscomputed when the commit dates
>> aren't entirely reliable.
>> 
>> This makes the 'git describe' output not match what the kernel
>> calculates in its localversion:
>> 
>> 	linux$ git describe && uname -r
>> 	v7.3-19342-g2a500a04d6f9
>> 	7.3.0-19059-g2a500a04d6f9
>> 
>> and the kernel calculation is correct, and 'git describe' is wrong. The
>> number that 'git describe' gives (19342) is nonsense.
>> 
>> The reason is that 'git describe' has walked the commit list past the
>> actual tagged release due to walking the commits purely in date order,
>> and is thus counting a few hundred commits that were already included in
>> Linux 7.3.
>> 
>> The fix is to take the commit generation number into account when
>> ordering the commits. That will still fail if the repository has no
>> commit graph, but it at least improves on the situation in the common
>> case. With that one-liner fix, 'git describe' gets the correct commit
>> distance on my kernel tree:
>> 
>> 	linux$ git describe && uname -r
>> 	v7.3-19059-g2a500a04d6f9
>> 	7.3.0-19059-g2a500a04d6f9
>> 
>> See also commit 1b600e659a ("Compute accurate distances in git-describe
>> before output.", 2007-01-27) which fixed a different issue in this same
>> area. Clearly the *intent* is that the distance number is actually the
>> real distance.
>> 
>> Also add a test-case for this. Thanks to Claude.
>> 
>> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>> ---
>> +test_expect_success 'setup: describe a tag dated before its parent' '
>> +	git init skew &&
>> +	(
>> +		cd skew &&
>> +		git config core.commitGraph true &&
>> +		test_commit --date "2020-01-01 18:00" base &&
>> +		test_commit --date "2020-01-01 18:01" o &&
>> +		test_commit --annotate --date "2020-01-01 17:01" T &&
>> +		git checkout -b side o &&
>> +		test_commit --date "2020-01-01 18:02" B &&
>> +		git checkout main &&
>> +		GIT_COMMITTER_DATE="2020-01-01 18:03" git merge --no-ff -m x side &&
>> +		git commit-graph write --reachable
>> +	)
>> +'
>
> Just a clueless rewrite of the above.  I had quite fun writing it, so why not
> to share... it works and might even offer some not-completely-bogus food for
> thought.
>
> 	git init skew &&
> 	(
> 		cd skew &&
> 		test_config core.commitGraph true &&

Doesn't this barf when it tries to register a clean-up action from
inside a subshell?

> 		test_commit base &&
> 		test_commit o &&
> 		test_tick=$(($test_tick - 3600)) &&
> 		test_commit --annotate T &&
> 		test_tick=$(($test_tick + 3600)) &&
> 		git checkout o &&
> 		test_commit B &&

Quite honestly, I do not see much value in this rewrite.  Explicit
dates and times from six years ago will eventually become distant
history, but at least they made it clear, even in the source code,
which event was recorded with what exact timestamp.  In the
original, it was much easier to tell which commit had a newer or
older timestamp than in the rewritten version.  Can you tell if the
annotated commit 'T' has a timestamp older by 3600, 3599, or 3601
seconds than the next commit 'B', just by looking at the sequence
above and without inspecting how 'test_commit --annotate' is
implemented?

Unless the focus of the test in question is "I do not really care
what time they record as long as they are reasonable and
reproducible from one run to the next", relying on $test_tick is
probably counterproductive.  The test for this fix, where the issue
is entirely about the relative ordering of timestamps on the
involved commits, is a prime example of where using explicit --date
options would make things much easier to understand.

> 		git checkout main &&
> 		test_merge x B &&
> 		git commit-graph write --reachable
> 	)
>
>> +
>> +check_describe -C skew "T-2-gHASH" HEAD
>> +
>>  test_expect_success 'setup misleading taggerdates' '
>>  	GIT_COMMITTER_DATE="2006-12-12 12:31" git tag -a -m "another tag" newer-tag-older-commit unique-file~1
>>  '
>> -- 
>> 2.56.0.237.g993442f913
>>