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

[Patrick Steinhardt <ps@pks.im>]

On Fri, Oct 30, 2026 at 11:55:03PM -0700, Linus Torvalds 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.

Out of curiosity, how do you calculate the localversion...? *goes
checking* Ah, you seem to use `git rev-list --count --left-right $tag`,
so you basically reimplement parts of git-describe(1).

Is there a reason why you use your own script, e.g. because of a missing
feature? Besides the fact of course that git-describe(1) is buggy in
this particular case.

> 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.

Hmm. The big downside of this particular change is that you may now get
different results depending on whether or not you have a recent commit
graph. So you may get different results based on when git-maintenance(1)
last decided to update it, and that would in turn result in highly
puzzling behaviour for the end user.

So...

> diff --git a/builtin/describe.c b/builtin/describe.c
> index c0abc931a5..2cd597d919 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -328,7 +328,7 @@ static void append_suffix(int depth, const struct object_id *oid, struct strbuf
>  static void describe_commit(struct commit *cmit, struct strbuf *dst)
>  {
>  	struct commit *c, *gave_up_on = NULL;
> -	struct prio_queue queue = { compare_commits_by_commit_date };
> +	struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
>  	struct commit_name *n;
>  	struct possible_tag all_matches[MAX_TAGS];
>  	unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;

... I wonder whether we should also call `ensure_generations_valid()`
here. That function is very expensive in cases where we don't have a
commit graph as it backfills generation numbers, and it may have to walk
the complete commit graph to do so. But isn't that better compared to
silently yielding a potentially-wrong result?

Thanks!

Patrick