git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] experimental: default to fetch.writeCommitGraph=false

From
Derrick Stolee <stolee@gmail.com>
Date
Jul 7, 2020, 13:24 UTC
Message-ID
<0b7435e3-95fc-8085-9e26-6d451809faa8@gmail.com>
In-Reply-To
<20200707062039.GC784740@google.com>
On 7/7/2020 2:20 AM, Jonathan Nieder wrote:
Show 47 quoted lines
> The fetch.writeCommitGraph feature makes fetches write out a commit
> graph file for the newly downloaded pack on fetch.  This improves the
> performance of various commands that would perform a revision walk and
> eventually ought to be the default for everyone.  To prepare for that
> future, it's enabled by default for users that set
> feature.experimental=true to experience such future defaults.
> 
> Alas, for --unshallow fetches from a shallow clone it runs into a
> snag: by the time Git has fetched the new objects and is writing a
> commit graph, it has performed a revision walk and r->parsed_objects
> contains information about the shallow boundary from *before* the
> fetch.  The commit graph writing code is careful to avoid writing a
> commit graph file in shallow repositories, but the new state is not
> shallow, and the result is that from that point on, commands like "git
> log" make use of a newly written commit graph file representing a
> fictional history with the old shallow boundary.
> 
> We could fix this by making the commit graph writing code more careful
> to avoid writing a commit graph that could have used any grafts or
> shallow state, but it is possible that there are other pieces of
> mutated state that fetch's commit graph writing code may be relying
> on.  So disable it in the feature.experimental configuration.
> 
> Google developers have been running in this configuration (by setting
> fetch.writeCommitGraph=false in the system config) to work around this
> bug since it was discovered in April.  Once the fix lands, we'll
> enable fetch.writeCommitGraph=true again to give it some early testing
> before rolling out to a wider audience.
> 
> In other words:
> 
> - this patch only affects behavior with feature.experimental=true
> 
> - it makes feature.experimental match the configuration Google has
>   been using for the last few months, meaning it would leave users in
>   a better tested state than without it
> 
> - this should improve testing for other features guarded by
>   feature.experimental, by making feature.experimental safer to use
> 
> Reported-by: Jay Conrod <jayconrod@google.com>
> Helped-by: Taylor Blau <me@ttaylorr.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> I realize this is late to send.  That said, as described above, I
> think it's a good way to buy time by minimizing user exposure to
> fetch.writeCommitGraph=true until a fix for it is well cooked.

While it would certainly be better to fix the commit_graph_compatible() method, I understand that that is a more complicated problem.

> In other words, I'd like to see this patch in Git 2.28-rc0.
This patch is 100% correct.

Normally, I would say "this is experimental, and a user can always disable fetch.writeCommitGraph manually if they are running into this." This is especially true because unshallowing a repo is (probably) a rare operation. At least, I expect that very few users actually do it, and those who do are expert users.

But, it is best to reduce user pain, even in rare cases like this.

Further, I hope to submit new maintenance tasks soon which can replace fetch.writeCommitGraph.

Thanks, -Stolee

Previous: Jonathan NiederNext: Junio C Hamano
Message 2 of 7 in “experimental: default to fetch.writeCommitGraph=false”
  1. experimental: default to fetch.writeCommitGraph=falseJonathan Nieder, Jul 7, 2020
  2. Derrick StoleeJul 7, 2020
  3. Junio C HamanoJul 7, 2020
  4. Taylor BlauJul 7, 2020
  5. Junio C HamanoJul 7, 2020
  6. Taylor BlauJul 7, 2020
  7. Jonathan NiederJul 8, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.