[BUG] git describe returns wrong tag as closest tag

[BUG] git describe returns wrong tag as closest tag

[Mikael Knutsson]

Hi!

I've discovered that if you have two non-rewinding branches that you
merge between you can get the wrong tag as closest tag if the
COMMITTER_DATE of any of the commits after the branch split is before
the date of the branch split. You can end up in this state simply by
having a committer doing a rebase after a branch split and pushing
their code. (This is an assumption of what happened when we saw the
behaviour)

Reproduction case:
mkdir testcase
cd testcase
git init
git commit --allow-empty -m init
git tag -a -m 'Test tag 1' 1
git branch tagerror
git checkout tagerror
git commit --allow-empty -m test01
git commit --allow-empty -m test02
git checkout master
GIT_COMMITTER_DATE=`date --date '-10 min'` git commit --allow-empty -m
test03 # this is what causes the error
git tag -a -m 'Test tag 2' 2
git checkout tagerror
git commit --allow-empty -m test04
git checkout master
git merge --no-ff tagerror
git describe --long # (expected output is '2-4-COMMITHASH', output is
'1-5-COMMITHASH')

If you remove the GIT_COMMITTER_DATE from the repro, you'll notice
that you get the expected output.
You can verify the ANY part by adding a commit on each branch before
the COMMITTER_DATE like this:
mkdir testcase
cd testcase
git init
git commit --allow-empty -m init
git tag -a -m 'Test tag 1' 1
git branch tagerror
git checkout tagerror
git commit --allow-empty -m test01
git commit --allow-empty -m test02
git commit --allow-empty -m test03
git checkout master
git commit --allow-empty -m test04
git tag -a -m 'Test tag 2' 2
GIT_COMMITTER_DATE=`date --date '-10 min'` git commit --allow-empty -m
test05 # this is what causes the error
git checkout tagerror
git commit --allow-empty -m test06
git checkout master
git merge --no-ff tagerror
git describe --long # (expected output is '2-4-COMMITHASH', output is
'1-5-COMMITHASH')

If you run a "git describe --long --first-parent", you'll notice that
it does actually pick up the correct tag in both above cases.

It seems to me that in the source code for describe, it relies on
sorting the commits on a strict date basis, rather than sorting them
in traversal order:
https://github.com/git/git/blob/master/builtin/describe.c#L204

Caveats to the reproduction case:
In our "real" case, the date of the commit causing the issue is before
the last commit before the branch split, but not before the root
commit nor before tag "1" in the repro case.
The distance to the tag that is erroneously picked up is much greater
than the closest tag (around 1200 commits more).

I'm sadly not very well-versed in C nor in the Git source code, so I'm
not sure how the appropriate fix would look like - that's why I'm not
also submitting a patch to fix this. :)

// Mikael K.