Re: Split commit graphs and commit-graph read

From: Derrick Stolee <stolee@gmail.com>
Date: Nov 11, 2019, 01:19 UTC
Message-ID: <87f16645-6af4-9703-1d0d-eb64728d2849@gmail.com>
In-Reply-To: <CAGyf7-G3NDp--2nUbri_0EqvSLF21M0gsFCOKDCWMY+e68Htog@mail.gmail.com>

On 11/8/2019 6:41 PM, Bryan Turner wrote:

Hi Bryan,

> Just a quick question about a behavior I've noticed with the commit
> graph. (Amazing feature, by the way!)

Show 18 quoted lines

> If the _very first_ write done is split:
> git commit-graph write --reachable --split
> 
> You end up with something like this:
> .../objects$ ls -R info
> info:
> commit-graphs  packs
> 
> info/commit-graphs:
> commit-graph-chain  graph-6612fcc8fd04d3af2cc268a6bd9161ae40f5fcbf.graph
> 
> info/commit-graph doesn't exist, but I have a 1-graph "chain" in
> place. (And subsequent write --split calls write additional ones; I've
> got a few now in this repository, but still no info/commit-graph.)
> 
> git commit-graph verify seems happy:
> .../objects$ git commit-graph verify
> Verifying commits in commit graph: 100% (98768/98768), done.

This workflow seems expected.

Show 11 quoted lines

> But git commit-graph read isn't:
> .../objects$ git commit-graph read
> fatal: Could not open commit-graph
> '/path/to/repository/objects/info/commit-graph': No such file or
> directory
> 
> Running some tests with commands like git for-each-ref and git
> rev-list shows that the "split" commit graph is being used (setting
> core.commitGraph=false makes commands noticeably slower), so
> functionally all seems well. But should git commit-graph read be
> handling this better?

Unfortunately, you're running into an issue because I designed the "read" subcommand poorly (and also forgot to update it for incremental commit-graph files). The biggest issue is that "read" is not really meant for end-users. It really should have been built as a test-tool. This point was corrected when I got around to writing the multi-pack-index since it uses "test-tool read-midx" instead of add.

To fix this issue, I would probably go about it by removing the "read" subcommand and creating a "test-tool read-commit-graph" for the tests that need that output.

If others on-list think that the better thing to do is to update the "read" subcommand to provide the same output, but iterate over each layer of an incremental commit-graph, then I can do that work instead.

Thanks, -Stolee