{"thread":{"id":"52224","subject":"Split commit graphs and commit-graph read","startedAt":"2019-11-08T23:41:49Z","lastAt":"2019-11-12T00:28:36Z","messageCount":5,"participants":["Bryan Turner","Derrick Stolee","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"385821","messageId":"CAGyf7-G3NDp--2nUbri_0EqvSLF21M0gsFCOKDCWMY+e68Htog@mail.gmail.com","threadId":"52224","inReplyTo":null,"subject":"Split commit graphs and commit-graph read","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-11-08T23:41:36Z","receivedAt":"2019-11-08T23:41:49Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"Just a quick question about a behavior I've noticed with the commit\ngraph. (Amazing feature, by the way!)\n\nIf the _very first_ write done is split:\ngit commit-graph write --reachable --split\n\nYou end up with something like this:\n.../objects$ ls -R info\ninfo:\ncommit-graphs  packs\n\ninfo/commit-graphs:\ncommit-graph-chain  graph-6612fcc8fd04d3af2cc268a6bd9161ae40f5fcbf.graph\n\ninfo/commit-graph doesn't exist, but I have a 1-graph \"chain\" in\nplace. (And subsequent write --split calls write additional ones; I've\ngot a few now in this repository, but still no info/commit-graph.)\n\ngit commit-graph verify seems happy:\n.../objects$ git commit-graph verify\nVerifying commits in commit graph: 100% (98768/98768), done.\n\nBut git commit-graph read isn't:\n.../objects$ git commit-graph read\nfatal: Could not open commit-graph\n'/path/to/repository/objects/info/commit-graph': No such file or\ndirectory\n\nRunning some tests with commands like git for-each-ref and git\nrev-list shows that the \"split\" commit graph is being used (setting\ncore.commitGraph=false makes commands noticeably slower), so\nfunctionally all seems well. But should git commit-graph read be\nhandling this better?\n\nBest regards,\nBryan Turner\n"},{"id":"385855","messageId":"87f16645-6af4-9703-1d0d-eb64728d2849@gmail.com","threadId":"52224","inReplyTo":"CAGyf7-G3NDp--2nUbri_0EqvSLF21M0gsFCOKDCWMY+e68Htog@mail.gmail.com","subject":"Re: Split commit graphs and commit-graph read","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-11-11T01:19:20Z","receivedAt":"2019-11-11T01:19:25Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/8/2019 6:41 PM, Bryan Turner wrote:\n\nHi Bryan,\n\n> Just a quick question about a behavior I've noticed with the commit\n> graph. (Amazing feature, by the way!)\n \n> If the _very first_ write done is split:\n> git commit-graph write --reachable --split\n> \n> You end up with something like this:\n> .../objects$ ls -R info\n> info:\n> commit-graphs  packs\n> \n> info/commit-graphs:\n> commit-graph-chain  graph-6612fcc8fd04d3af2cc268a6bd9161ae40f5fcbf.graph\n> \n> info/commit-graph doesn't exist, but I have a 1-graph \"chain\" in\n> place. (And subsequent write --split calls write additional ones; I've\n> got a few now in this repository, but still no info/commit-graph.)\n> \n> git commit-graph verify seems happy:\n> .../objects$ git commit-graph verify\n> Verifying commits in commit graph: 100% (98768/98768), done.\n\nThis workflow seems expected.\n\n> But git commit-graph read isn't:\n> .../objects$ git commit-graph read\n> fatal: Could not open commit-graph\n> '/path/to/repository/objects/info/commit-graph': No such file or\n> directory\n> \n> Running some tests with commands like git for-each-ref and git\n> rev-list shows that the \"split\" commit graph is being used (setting\n> core.commitGraph=false makes commands noticeably slower), so\n> functionally all seems well. But should git commit-graph read be\n> handling this better?\n\nUnfortunately, you're running into an issue because I designed the\n\"read\" subcommand poorly (and also forgot to update it for\nincremental commit-graph files). The biggest issue is that \"read\"\nis not really meant for end-users. It really should have been built\nas a test-tool. This point was corrected when I got around to writing\nthe multi-pack-index since it uses \"test-tool read-midx\" instead of\nadd.\n\nTo fix this issue, I would probably go about it by removing the \"read\"\nsubcommand and creating a \"test-tool read-commit-graph\" for the tests\nthat need that output.\n\nIf others on-list think that the better thing to do is to update the\n\"read\" subcommand to provide the same output, but iterate over each\nlayer of an incremental commit-graph, then I can do that work instead.\n\nThanks,\n-Stolee\n"},{"id":"385860","messageId":"xmqq1rufjh9n.fsf@gitster-ct.c.googlers.com","threadId":"52224","inReplyTo":"87f16645-6af4-9703-1d0d-eb64728d2849@gmail.com","subject":"Re: Split commit graphs and commit-graph read","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-11T02:09:24Z","receivedAt":"2019-11-11T02:09:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> To fix this issue, I would probably go about it by removing the \"read\"\n> subcommand and creating a \"test-tool read-commit-graph\" for the tests\n> that need that output.\n>\n> If others on-list think that the better thing to do is to update the\n> \"read\" subcommand to provide the same output, but iterate over each\n> layer of an incremental commit-graph, then I can do that work instead.\n\nI think moving it to test-tool is a good idea, whether the output\npresents a single result that consolidates all layers together, or\nallows to name just a single layer and shows its contents.  I do not\nhave a strong opinion between the two (both sounds usable debugging\naid).\n\nThanks.\n\n"},{"id":"385865","messageId":"20191111032946.GA5912@sigill.intra.peff.net","threadId":"52224","inReplyTo":"87f16645-6af4-9703-1d0d-eb64728d2849@gmail.com","subject":"Re: Split commit graphs and commit-graph read","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-11T03:29:46Z","receivedAt":"2019-11-11T03:29:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 10, 2019 at 08:19:20PM -0500, Derrick Stolee wrote:\n\n> > Running some tests with commands like git for-each-ref and git\n> > rev-list shows that the \"split\" commit graph is being used (setting\n> > core.commitGraph=false makes commands noticeably slower), so\n> > functionally all seems well. But should git commit-graph read be\n> > handling this better?\n> \n> Unfortunately, you're running into an issue because I designed the\n> \"read\" subcommand poorly (and also forgot to update it for\n> incremental commit-graph files). The biggest issue is that \"read\"\n> is not really meant for end-users. It really should have been built\n> as a test-tool. This point was corrected when I got around to writing\n> the multi-pack-index since it uses \"test-tool read-midx\" instead of\n> add.\n> \n> To fix this issue, I would probably go about it by removing the \"read\"\n> subcommand and creating a \"test-tool read-commit-graph\" for the tests\n> that need that output.\n> \n> If others on-list think that the better thing to do is to update the\n> \"read\" subcommand to provide the same output, but iterate over each\n> layer of an incremental commit-graph, then I can do that work instead.\n\nIn theory I suppose one could use it to debug a commit-graph file \"in\nthe field\" as it were, where somebody does not necessarily have the\ntest-tool programs. But in practice, I have not ever done that (I didn't\neven know \"commit-graph read\" was there), and it's not that big a deal\nto just have a build of git.git handy.\n\nI'd be much more likely to use \"commit-graph verify\". And perhaps it\ncould grow a \"--verbose\" flag if somebody really wants that (but I think\nit would be fine to punt until somebody cares enough to do so).\n\nI guess dropping the sub-command is technically a backwards incompatible\nchange, but since it didn't do anything that normal users would find\nuseful in the first place, I wouldn't be sorry to see it go.\n\n-Peff\n"},{"id":"385971","messageId":"CAGyf7-EqQdvg4fQcZJv6BpqZ9hPDx72KBiLp1Xe3Ur7NrSMBeA@mail.gmail.com","threadId":"52224","inReplyTo":"87f16645-6af4-9703-1d0d-eb64728d2849@gmail.com","subject":"Re: Split commit graphs and commit-graph read","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-11-12T00:28:23Z","receivedAt":"2019-11-12T00:28:36Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Sun, Nov 10, 2019 at 5:19 PM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> > But git commit-graph read isn't:\n> > .../objects$ git commit-graph read\n> > fatal: Could not open commit-graph\n> > '/path/to/repository/objects/info/commit-graph': No such file or\n> > directory\n> >\n> > Running some tests with commands like git for-each-ref and git\n> > rev-list shows that the \"split\" commit graph is being used (setting\n> > core.commitGraph=false makes commands noticeably slower), so\n> > functionally all seems well. But should git commit-graph read be\n> > handling this better?\n>\n> Unfortunately, you're running into an issue because I designed the\n> \"read\" subcommand poorly (and also forgot to update it for\n> incremental commit-graph files). The biggest issue is that \"read\"\n> is not really meant for end-users. It really should have been built\n> as a test-tool. This point was corrected when I got around to writing\n> the multi-pack-index since it uses \"test-tool read-midx\" instead of\n> add.\n>\n> To fix this issue, I would probably go about it by removing the \"read\"\n> subcommand and creating a \"test-tool read-commit-graph\" for the tests\n> that need that output.\n>\n> If others on-list think that the better thing to do is to update the\n> \"read\" subcommand to provide the same output, but iterate over each\n> layer of an incremental commit-graph, then I can do that work instead.\n\nI was only running \"git commit-graph read\" as another way of verifying\nthat everything looked good (to Git, that is). I'd already run \"git\ncommit-graph verify\" and just found the discrepancy in their behavior\nsurprising/interesting. For what it's worth, I personally don't see\nanything amiss with removing the \"read\" subcommand if that's simpler\nthan updating it to handle incremental graphs. (Put differently, it's\nnot a command I'd ever have Bitbucket Server run, or that I'm likely\nto try and run going forward. In this case I just completed some\ncommit graph-related changes to Bitbucket Server and was looking for\nevery possible avenue I could pursue to try and ensure what I did was\nproducing the \"correct\" output after my changes got deployed to our\ninternal dogfooding server.)\n\nThanks for taking the time to look into it and clarify the behavior! I\nreally appreciate it.\n\nBest regards,\nBryan Turner\n\n>\n> Thanks,\n> -Stolee\n"}]}