Subject: Re: [PATCH v4 10/10] commit-graph.txt: update design document

From: Jakub Narebski <[email-masked]>
Date: Wed, 02 May 2018 09:57:45 +0200

Derrick Stolee <[email-masked]> writes:

> On 4/30/2018 7:32 PM, Jakub Narebski wrote:
>> Derrick Stolee <[email-masked]> writes:
[...]
>>>   - After computing and storing generation numbers, we must make graph
>>>     walks aware of generation numbers to gain the performance benefits they
>>>     enable. This will mostly be accomplished by swapping a commit-date-ordered
>>>     priority queue with one ordered by generation number. The following
>>> -  operations are important candidates:
>>> +  operation is an important candidate:
>>>   -    - paint_down_to_common()
>>>       - 'log --topo-order'
>>
>> Another possible candidates:
>>
>>         - remove_redundant() - see comment in previous patch
>>         - still_interesting() - where Git uses date slop to stop walking
>>           too far
>
> remove_redundant() will be included in v5, thanks.

Oh.  Nice.

I'll try to review the new patch in detail soon.

> Instead of "still_interesting()" I'll add "git tag --merged" as the
> candidate to consider, as discussed in [1].
>
> [1] https://public-inbox.org/git/[email-masked]/t/#u
>     "branch --contains / tag --merged inconsistency"

All right.  I have mentioned still_interesting() as a hint where
possible additional generation numbers based optimization may lurk
(because that's where heuristic based on dates is used - similarly to
how it was done in this series with paint_down_to_common()).

[...]
>> One important issue left is handling features that change view of
>> project history, and their interaction with commit-graph feature.
>>
>> What would happen, if we turn on commit-graph feature, generate commit
>> graph file, and then:
>>
>>    * use graft file or remove graft entries to cut history, or remove cut
>>      or join two [independent] histories.
>>    * use git-replace mechanims to do the same
>>    * in shallow clone, deepen or shorten the clone
>>
>> What would happen if without re-generating commit-graph file (assuming
>> tha Git wouldn't do it for us), we run some feature that makes use of
>> commit-graph data:
>>
>>    - git branch --contains
>>    - git tag --contains
>>    - git rev-list A..B
>>
>
> The commit-graph is not supported in these scenarios (yet). grafts are
> specifically mentioned in the future work section.
>
> I'm not particularly interested in supporting these features, so they
> are good venues for other contributors to get involved in the
> commit-graph feature. Eventually, they will be blockers to making the
> commit-graph feature a "default" feature. That is when I will pay
> attention to these situations. For now, a user must opt-in to having a
> commit-graph file (and that same user has possibly opted in to these
> history modifying features).

Well, that is sensible approach.  Get commit-graph features in working
condition, and worry about beng able to make it on by default later.

Nice to have it clarified.  I'll stop nagging about that, then ;-P

One issue: 'grafts' are mentioned in the future work section of the
technical documentation, but we don't have *any* warning about
commit-graph limitations in user-facing documentation, that is
git-commit-graph(1) manpage.

Best,
-- 
Jakub Narębski