Re: [PATCH 1/1] fetch: add fetch.writeCommitGraph config setting

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 6, 2019, 21:46 UTC
Message-ID: <xmqqlfv1m72p.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <d550ac23-cb65-b547-d9dc-1428ee2e9420@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 19 quoted lines

>>> +		write_commit_graph_reachable(get_object_directory(),
>>> +					     commit_graph_flags,
>>> +					     &split_opts);
>>> +	}
>> 
>> As a low-impact change this is good.  
>> 
>> For longer term, it feels a bit unfortunate that this is still a
>> separate phase of the program, though.  We know what new refs we
>> added, we know what new objects we received, and we even scanned
>> each and every one of them while running the index-pack step to
>> store the .pack and compute the .idx file, i.e. it feels that we
>> have most of the information already in-core to extend the commit
>> graph for new parts of the history we just received.
>
> You're right that we could isolate the new write to the refs we
> just received. We could use the more cumbersome write_commit_graph()
> method with a list of commit oids as starting points. I'm happy to
> make that change if we see a lot of value there.

Well, that is not the kind of information reuse I am talking about.

I was wondering if "index-pack" has enough information in-core after it receives and processes the incoming pack data, scanned each and every object in it, in order to write out the commit graph _without_ having to do a lot of duplicate computation and enumeration of the objects done in the current commit-graph.c::write_commit_graph(), so that it can learn a "--write-commit-graph" option that performs much better than running "git fetch && git commit-graph write".

Perhaps that would require too much refactoring of both index-pack and commit-graph code and infeasible in short to medium term and that is why I said "for longer term, it feels a bit unfortunate".

Thanks.