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

Re: [PATCH v3 4/8] commit-graph: don't early exit(1) on e.g. "git status"

From
Jeff King <peff@peff.net>
Date
May 1, 2019, 18:31 UTC
Message-ID
<20190501183108.GE4109@sigill.intra.peff.net>
In-Reply-To
<3518ad3e-bc4a-c2c3-d4bd-c87f9e828b1c@gmail.com>
On Mon, Apr 29, 2019 at 08:48:51AM -0400, Derrick Stolee wrote:
Show 8 quoted lines
> Q: How should we handle a detectably-invalid commit-graph?
> 
> I think most of your patches have done a good job so far of detecting
> an invalid header, and responding by ignoring the commit-graph. This case
> of a detectable error in the chunk data itself is not something we can
> check on the first load without serious performance issues.
> 
> I hope we can decide on a good solution.

I think it's OK to die() unceremoniously in these cases. We already have similar behavior when dealing with packfiles. While we try to just return an error for bit-flips in data (so we can try finding the object elsewhere), if there are things like nonsensical file offsets, we generally just give up.

So I think it makes sense to start with the _safe_ thing, which is just giving up on the operation and informing the user. If somebody wants to then loosen things up on a case by case basis (after seeing that it wouldn't produce incorrect results to do so), that can be future work.

For the commit-graph, though, and given that we're talking about file corruption, I don't know that it's even worth the effort. The solution is basically always going to be "delete the commit-graph file and regenerate it". It's perhaps slightly inconvenient compared to falling back, but this just isn't something we'd expect to happen often.

-Peff
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 24 of 36 in “commit-graph: segfault & other fixes for broken graphs”
  1. 0/8 commit-graph: segfault & other fixes for broken graphsÆvar Arnfjörð Bjarmason, Feb 21, 2019
  2. 1/8 commit-graph tests: split up corrupt_graph_and_verify()Ævar Arnfjörð Bjarmason, Feb 21, 2019
  3. 2/8 commit-graph tests: test a graph that's too smallÆvar Arnfjörð Bjarmason, Feb 21, 2019
  4. 3/8 commit-graph: fix segfault on e.g. "git status"Ævar Arnfjörð Bjarmason, Feb 21, 2019
  5. 4/8 commit-graph: don't early exit(1) on e.g. "git status"Ævar Arnfjörð Bjarmason, Feb 21, 2019
  6. 6/8 commit-graph verify: detect inability to read the graphÆvar Arnfjörð Bjarmason, Feb 21, 2019
  7. SZEDER GáborFeb 21, 2019
  8. 5/8 commit-graph: don't pass filename to load_commit_graph_one_fd_st()Ævar Arnfjörð Bjarmason, Feb 21, 2019
  9. 7/8 commit-graph write: don't die if the existing graph is corruptÆvar Arnfjörð Bjarmason, Feb 21, 2019
  10. Junio C HamanoFeb 22, 2019
  11. Eric SunshineFeb 23, 2019
  12. 8/8 commit-graph: improve & i18n error messagesÆvar Arnfjörð Bjarmason, Feb 21, 2019
  13. 0/8 commit-graph: segfault & other fixes for broken graphsÆvar Arnfjörð Bjarmason, Mar 14, 2019
  14. Eric SunshineMar 15, 2019
  15. Ævar Arnfjörð BjarmasonMar 15, 2019
  16. 0/8 commit-graph: segfault & other fixes for broken graphsÆvar Arnfjörð Bjarmason, Mar 25, 2019
  17. 1/8 commit-graph tests: split up corrupt_graph_and_verify()Ævar Arnfjörð Bjarmason, Mar 25, 2019
  18. 2/8 commit-graph tests: test a graph that's too smallÆvar Arnfjörð Bjarmason, Mar 25, 2019
  19. 3/8 commit-graph: fix segfault on e.g. "git status"Ævar Arnfjörð Bjarmason, Mar 25, 2019
  20. 4/8 commit-graph: don't early exit(1) on e.g. "git status"Ævar Arnfjörð Bjarmason, Mar 25, 2019
  21. Ævar Arnfjörð BjarmasonApr 27, 2019
  22. Derrick StoleeApr 29, 2019
  23. Ævar Arnfjörð BjarmasonApr 29, 2019
  24. Jeff KingMay 1, 2019
  25. 5/8 commit-graph: don't pass filename to load_commit_graph_one_fd_st()Ævar Arnfjörð Bjarmason, Mar 25, 2019
  26. 8/8 commit-graph: improve & i18n error messagesÆvar Arnfjörð Bjarmason, Mar 25, 2019
  27. 7/8 commit-graph write: don't die if the existing graph is corruptÆvar Arnfjörð Bjarmason, Mar 25, 2019
  28. 6/8 commit-graph verify: detect inability to read the graphÆvar Arnfjörð Bjarmason, Mar 25, 2019
  29. 1/8 commit-graph tests: split up corrupt_graph_and_verify()Ævar Arnfjörð Bjarmason, Mar 14, 2019
  30. 2/8 commit-graph tests: test a graph that's too smallÆvar Arnfjörð Bjarmason, Mar 14, 2019
  31. 3/8 commit-graph: fix segfault on e.g. "git status"Ævar Arnfjörð Bjarmason, Mar 14, 2019
  32. 5/8 commit-graph: don't pass filename to load_commit_graph_one_fd_st()Ævar Arnfjörð Bjarmason, Mar 14, 2019
  33. 4/8 commit-graph: don't early exit(1) on e.g. "git status"Ævar Arnfjörð Bjarmason, Mar 14, 2019
  34. 6/8 commit-graph verify: detect inability to read the graphÆvar Arnfjörð Bjarmason, Mar 14, 2019
  35. 7/8 commit-graph write: don't die if the existing graph is corruptÆvar Arnfjörð Bjarmason, Mar 14, 2019
  36. 8/8 commit-graph: improve & i18n error messagesÆvar Arnfjörð Bjarmason, Mar 14, 2019

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.