Strange behavior of git rev-list

Hi,
I have an interesting case. In my repository, there are two commits,
'one' and 'two'. 'one' is reachable from 'two' (as evidenced by 'git
rev-list two | grep $(giv rev-parse one)'). However, the output of
'git rev-list two..one' is not empty, as is 'git rev-list ^two one'.

Here is the repository: https://github.com/pwmarcz/git-wtf/

It seems that the commit dates influence this behavior, because when I
edit all the dates to be the same, the output of 'git rev-list
two..one' is empty. Pruning seemingly irrelevant parents also makes it
empty.

I verified the behavior on git versions 2.14.1, 2.11.0, and on the
'next' branch (2.14.1.586.g1a2e63c10).

Paweł Marczewski

On Thu, Sep 07, 2017 at 11:20:15AM +0200, Paweł Marczewski wrote:

I have an interesting case. In my repository, there are two commits,
'one' and 'two'. 'one' is reachable from 'two' (as evidenced by 'git
rev-list two | grep $(giv rev-parse one)'). However, the output of
'git rev-list two..one' is not empty, as is 'git rev-list ^two one'.

Here is the repository: https://github.com/pwmarcz/git-wtf/

It seems that the commit dates influence this behavior, because when I
edit all the dates to be the same, the output of 'git rev-list
two..one' is empty. Pruning seemingly irrelevant parents also makes it
empty.

I verified the behavior on git versions 2.14.1, 2.11.0, and on the
'next' branch (2.14.1.586.g1a2e63c10).

Yes, this is known. The commit dates are used as a proxy for graph
height (or generation numbers, if you prefer) so that we avoid having to
walk all the way down to a merge base before producing any output. But
it can give the wrong answer in the face of clock skew.

We walk back five extra commits more than we need to in order to avoid
small runs of skewed commits, but obviously you can have arbitrary-sized
runs.

-Peff

Thanks. Any plans to fix that? Or is there a way to turn off this heuristic?

On 7 September 2017 at 11:47, Jeff King [off-list ref] wrote:

On Thu, Sep 07, 2017 at 11:20:15AM +0200, Paweł Marczewski wrote:

quoted

I have an interesting case. In my repository, there are two commits,
'one' and 'two'. 'one' is reachable from 'two' (as evidenced by 'git
rev-list two | grep $(giv rev-parse one)'). However, the output of
'git rev-list two..one' is not empty, as is 'git rev-list ^two one'.

Here is the repository: https://github.com/pwmarcz/git-wtf/

It seems that the commit dates influence this behavior, because when I
edit all the dates to be the same, the output of 'git rev-list
two..one' is empty. Pruning seemingly irrelevant parents also makes it
empty.

I verified the behavior on git versions 2.14.1, 2.11.0, and on the
'next' branch (2.14.1.586.g1a2e63c10).

Yes, this is known. The commit dates are used as a proxy for graph
height (or generation numbers, if you prefer) so that we avoid having to
walk all the way down to a merge base before producing any output. But
it can give the wrong answer in the face of clock skew.

We walk back five extra commits more than we need to in order to avoid
small runs of skewed commits, but obviously you can have arbitrary-sized
runs.

-Peff

On Thu, Sep 07, 2017 at 11:50:25AM +0200, Paweł Marczewski wrote:

Thanks. Any plans to fix that? Or is there a way to turn off this heuristic?

I don't think there's a way to turn it off for `rev-list`. Merge-base
computations are more careful, so you could determine the correct
endpoints that way. But when you fed them to `rev-list`, it would hit
the same run of skewed commits.

We've discussed storing true generation numbers in the past, which would
make this optimization more robust, as well as allow us to speed up many
other traversals (e.g., the "tag --contains"). It's something I'd like
to revisit, but it's not at the top of the pile.

In the meantime, it would probably be possible to write a patch that
conditionally disables the optimization. But it would mean all of your
rev-lists run a _lot_ slower.

-Peff

On Thu, Sep 7, 2017 at 3:11 AM, Jeff King [off-list ref] wrote:

On Thu, Sep 07, 2017 at 11:50:25AM +0200, Paweł Marczewski wrote:

quoted

Thanks. Any plans to fix that? Or is there a way to turn off this heuristic?

I don't think there's a way to turn it off for `rev-list`. Merge-base
computations are more careful, so you could determine the correct
endpoints that way. But when you fed them to `rev-list`, it would hit
the same run of skewed commits.

We've discussed storing true generation numbers in the past, which would
make this optimization more robust, as well as allow us to speed up many
other traversals (e.g., the "tag --contains"). It's something I'd like
to revisit, but it's not at the top of the pile.

(We just had an office discussion if this is part of the hash transition plan,
i.e. have a field in the commit object to contain its generation number.
and as I think the generation numbers would lead to fast and correct
behavior unlike the current heuristic which is only fast, I would try
to make a strong point actual generation numbers inside commit objects)

In the meantime, it would probably be possible to write a patch that
conditionally disables the optimization. But it would mean all of your
rev-lists run a _lot_ slower.

-Peff

On Thu, Sep 07, 2017 at 12:24:02PM -0700, Stefan Beller wrote:

quoted

We've discussed storing true generation numbers in the past, which would
make this optimization more robust, as well as allow us to speed up many
other traversals (e.g., the "tag --contains"). It's something I'd like
to revisit, but it's not at the top of the pile.

(We just had an office discussion if this is part of the hash transition plan,
i.e. have a field in the commit object to contain its generation number.
and as I think the generation numbers would lead to fast and correct
behavior unlike the current heuristic which is only fast, I would try
to make a strong point actual generation numbers inside commit objects)

I'm still moderately against storing generation numbers inside the
objects. They're redundant with the existing parent pointers, which
means it's an opportunity for the two sets of data to disagree. And as
we've seen, once errors are cemented in history it's very hard to fix
them, because you break any history built on top.

I'm much more in favor of building a local cache of generation numbers
(either on the fly or during repacks, where we can piggy-back on the
existing pack .idx for indexing).

-Peff