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

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

From
Jeff King <peff@peff.net>
Date
Sep 12, 2019, 14:03 UTC
Message-ID
<20190912140332.GD23031@sigill.intra.peff.net>
In-Reply-To
<20190912020848.GB76228@syl.lan>
On Wed, Sep 11, 2019 at 10:08:48PM -0400, Taylor Blau wrote:
Show 20 quoted lines
> > The test suite passes with my patch both with and without
> > GIT_TEST_COMMIT_GRAPH=1. But to my surprise, it also passes if I delete
> > the close_commit_graph() line added by 829a321569!
> >
> > So it's not clear to me whether this whole thing is truly unnecessary
> > (and Stolee was just being overly cautious because the code is related
> > to shallow-ness, even though it is OK doing a true-parent traversal
> > itself), or if we just don't have good test coverage for the case that
> > requires it.
> >
> > If it _is_ necessary, I'm a little worried there are other problems
> > lurking. The whole issue is that we've seen and parsed some commits
> > before we get to this shallow deepen-since code-path. So just disabling
> > commit-graphs isn't enough. Even without them, we might have parsed some
> > commits the old-fashioned way and filled in their parent pointers. Is
> > that a problem?
> 
> I am, too, but I don't think we should hold this patch up which is
> obviously improving the situation in the meantime while we figure that
> out.

Yeah, I'd agree here, unless we determine quickly that we do need the bigger fix, and somebody with a bit more knowledge of this shallow code offers a fix. I believe my patch is a strict improvement, and puts the commit-graph code path on par with the regular one.

-Peff
Previous: Taylor BlauNext: Taylor Blau
Message 10 of 25 in “upload-pack: disable commit graph more gently for shallow traversal”
  1. upload-pack: disable commit graph more gently for shallow traversalJeff King, Sep 12, 2019
  2. Jeff KingSep 12, 2019
  3. list-objects: don't queue root trees unless revs->tree_objects is setJeff King, Sep 12, 2019
  4. Jeff KingSep 12, 2019
  5. Derrick StoleeSep 12, 2019
  6. Junio C HamanoSep 12, 2019
  7. Jeff KingSep 12, 2019
  8. Junio C HamanoSep 13, 2019
  9. Taylor BlauSep 12, 2019
  10. Jeff KingSep 12, 2019
  11. Taylor BlauSep 12, 2019
  12. SZEDER GáborSep 12, 2019
  13. Jeff KingSep 12, 2019
  14. Derrick StoleeSep 12, 2019
  15. Jeff KingSep 12, 2019
  16. Derrick StoleeSep 12, 2019
  17. Jeff KingSep 12, 2019
  18. Derrick StoleeSep 12, 2019
  19. upload-pack commit graph segfault fixJeff King, Sep 12, 2019
  20. Jeff KingSep 12, 2019
  21. 1/2 commit-graph: bump DIE_ON_LOAD check to actual load-timeJeff King, Sep 12, 2019
  22. Derrick StoleeSep 12, 2019
  23. 2/2 upload-pack: disable commit graph more gently for shallow traversalJeff King, Sep 12, 2019
  24. Derrick StoleeSep 13, 2019
  25. Taylor BlauSep 12, 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.