Re: [PATCH 2/2] commit: use a priority queue in merge base functions

[Jeff King <peff@peff.net>]

On Thu, Aug 30, 2012 at 09:03:27AM -0400, Jeff King wrote:

> So I was able to have my queue behave just like commit_list by fixing
> the stability issue. But I still have no clue what is going on in t6024.
> It does this for each commit it makes:
> 
>   [...]
>   GIT_AUTHOR_DATE="2006-12-12 23:00:00" git commit -m 1 a1 &&
>   [...]
>   GIT_AUTHOR_DATE="2006-12-12 23:00:01" git commit -m A a1 &&
>   [...]
> 
> which is just bizarre. At first I thought it was buggy, and that it
> really wanted to be setting COMMITTER_DATE (in which case it should
> really just be using test_tick, anyway). But if you do that, the test
> fails (even using a regular commit_list)!
> 
> So is the test buggy? Or are the identical commit timestamps part of the
> intended effect? I can't see how that would be, since:
> 
>   1. You would need to set COMMITTER_DATE for that anyway, as you are
>      otherwise creating a race condition.
> 
>   2. Why would you set AUTHOR_DATE? It's not used by the merge code at
>      all.
> 
> The script originally comes from here:
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/33566/focus=33852
> 
> and the discussion implies that the AUTHOR_DATEs were added to avoid a
> race condition with the timestamps. But why would that ever have worked?

That thread mentions that this is to fix "Shawn's bug", which I think is
this:

  http://article.gmane.org/gmane.comp.version-control.git/33559

IOW, the real thing that t6024 is trying to test is that we handle the
fake empty tree properly. And the AUTHOR_DATEs probably were just there
to try to fix the race condition, and they should really just be
test_ticks (and I can't see how they ever would have helped anything; I
suspect they were a placebo inserted at the same time as another change,
and got credited with fixing the race).

That still leaves the question of why the test fails when the commits
get distinct timestamps.

-Peff