{"thread":{"id":"46030","subject":"Passing revs to git-bundle-create via stdin","startedAt":"2017-05-22T23:45:05Z","lastAt":"2017-05-25T02:55:11Z","messageCount":4,"participants":["ch","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"320490","messageId":"76491764-cd43-ef7f-fbfc-939a15f2fb77@onlinehome.de","threadId":"46030","inReplyTo":null,"subject":"Passing revs to git-bundle-create via stdin","fromName":"ch","fromEmail":"cr@onlinehome.de","sentAt":"2017-05-22T23:44:55Z","receivedAt":"2017-05-22T23:45:05Z","isPatch":false,"sender":{"key":"cr@onlinehome.de","avatar":null},"body":"Hi,\n\nI'm using git bundles to create (incremental) backups of my local repositories.\nThis works quite well but for certain repositories I'm getting unexpectedly big\nincremental bundles. I did some testing and from what I can tell it seems\ngit-bundle-create has issues processing revs passed via stdin. To illustrate\nthe problem I have included a small bash script below.\n\nI'm using Git for Windows 2.13.0.windows.1 (64-bit). Unfortunately I don't have\naccess to a non-Windows box to check whether it's a problem specific to the\nWindows port.\n\n----\nadd_file()\n{\n     echo \"$1\" > \"$1\"\n     git add \"$1\"\n     git commit -m \"$1\"\n}\n\ngit init .\n\nadd_file \"test-1\"\nadd_file \"test-2\"\nadd_file \"test-3\"\n\ngit checkout -b feature\nadd_file \"test-4\"\nadd_file \"test-5\"\nadd_file \"test-6\"\n\ngit checkout master\nadd_file \"test-7\"\nadd_file \"test-8\"\nadd_file \"test-9\"\n\necho -e \"\\nCreating test.git...\"\ngit bundle create test.git --all ^feature ^master^\n\necho -e \"\\nCreating test-stdin.git...\"\necho -e \"^feature\\n^master^\\n\" | git bundle create test-stdin.git --all --stdin\n\necho -e \"\\nCreating test-2.git...\"\ngit bundle create test-2.git --all ^feature^ ^master^\n\necho -e \"\\nCreating test-2-stdin.git...\"\necho -e \"^feature^\\n^master^\\n\" | git bundle create test-2-stdin.git --all --stdin\n\necho -e \"\\nCreating test-3-stdin.git...\"\necho -e \"feature\\nmaster\\n\" | git bundle create test-3-stdin.git --stdin\n\necho\ngit branch -D feature\ngit tag -am \"Annotated tag\" annotated-tag master~2\n\necho -e \"\\nCreating annotated.git...\"\ngit bundle create annotated.git --all ^annotated-tag\n\necho -e \"\\nCreating annotated-stdin.git...\"\necho -e \"^annotated-tag\\n\" | git bundle create annotated-stdin.git --all --stdin\n\necho\ngit tag -d annotated-tag\ngit tag lightweight-tag master~2\necho -e \"\\nCreating lightweight-stdin.git...\"\necho -e \"^lightweight-tag\\n\" | git bundle create lightweight-stdin.git --all --stdin\n----\n\nI'd expect test.git and test-stdin.git to be identical. In fact the contained-\nand required-refs lists of both bundles are equal but the pack in\ntest-stdin.git is notably larger compared to the one in test.git. Interestingly\ntest-2.git and test-2-stdin.git are identical.\n\ngit-bundle-create does not appear to handle includes properly either. In this\nspecific case it won't create test-3-stdin.git and dies with\n'error: Refusing to create empty bundle.'.\n\nLast but not least git-bundle-create includes annotated-tag in\nannotated-stdin.git even though the tag is excluded via stdin. It works alright\nif the tag is excluded via commandline like in case of annotated.git. The issue\nalso seems to be specific to annotated tags as lightweight-tag is properly\nexcluded from lightweight-stdin.git.\n\nAny help would be appreciated.\n\nThanks in advance.\n"},{"id":"320561","messageId":"20170523164641.u3rd7bgc7ifvu2su@sigill.intra.peff.net","threadId":"46030","inReplyTo":"76491764-cd43-ef7f-fbfc-939a15f2fb77@onlinehome.de","subject":"Re: Passing revs to git-bundle-create via stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-23T16:46:41Z","receivedAt":"2017-05-23T16:46:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 23, 2017 at 01:44:55AM +0200, ch wrote:\n\n> I'm using git bundles to create (incremental) backups of my local repositories.\n> This works quite well but for certain repositories I'm getting unexpectedly big\n> incremental bundles. I did some testing and from what I can tell it seems\n> git-bundle-create has issues processing revs passed via stdin. To illustrate\n> the problem I have included a small bash script below.\n> \n> I'm using Git for Windows 2.13.0.windows.1 (64-bit). Unfortunately I don't have\n> access to a non-Windows box to check whether it's a problem specific to the\n> Windows port.\n\nThanks for an easy reproduction recipe. I see the problem on Linux, too.\n\nI think what's happening is that git-bundle actually runs _two_\ntraversals using the command-line arguments. It kicks off an external\nrev-list via compute_and_write_prerequisites(), and then feeds the\narguments again to setup_revisions(). The first one eats all of stdin,\nand the second just sees an empty input.\n\nYou can see it working if you do:\n\n  $ git bundle create from-terminal.git --all --stdin\n  ^feature\n  ^master^\n  [press ^D, i.e., ctrl-d]\n  ^feature\n  ^master^\n  [press ^D again]\n\nHitting ^D tells the terminal driver to send an EOF; the first one goes\nto the child rev-list, and then we repeat the input to get read by the\nsecond traversal. The result is identical to your command-line-only\noutput. I have no idea if the ^D thing works at all on Windows, but I\ndon't mean it even as a workaround. It was just a way of confirming my\nguess about the double-read.\n\nThe real solutions I can think of are:\n\n  1. Teach git-bundle not to require the double-read. I'm not sure why\n     it's written the way it is, but presumably it would be tricky to\n     undo it (or we would have just written it the other way in the\n     first place!)\n\n  2. Git-bundle could buffer stdin and feed it to the two traversals. I\n     think this actually ends up a little tricky, because the second\n     traversal is done in-process (so we'd have to actually re-feed the\n     buffer to our stdin via a \"struct async\", which feels pretty\n     hacky).\n\n  3. git-bundle could natively support --stdin, reading each line and\n     convert it into traversal arguments. This is the quickest way to\n     make your example work, but I suspect there will be funky corner\n     cases (because we'd have to replicate the same rules that\n     revision.c uses to read its input).\n\nNone of those are incredibly appealing.\n\n-Peff\n"},{"id":"320608","messageId":"xmqqwp97hvvi.fsf@gitster.mtv.corp.google.com","threadId":"46030","inReplyTo":"20170523164641.u3rd7bgc7ifvu2su@sigill.intra.peff.net","subject":"Re: Passing revs to git-bundle-create via stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-24T01:44:01Z","receivedAt":"2017-05-24T01:44:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think what's happening is that git-bundle actually runs _two_\n> traversals using the command-line arguments. ...\n> ... It was just a way of confirming my\n> guess about the double-read.\n>\n> The real solutions I can think of are:\n>\n>   1. Teach git-bundle not to require the double-read. I'm not sure why\n>      it's written the way it is, but presumably it would be tricky to\n>      undo it (or we would have just written it the other way in the\n>      first place!)\n\nIf I remember correctly, the reason why it does the double-read is\nbecause it wants to cope with things like \"--since\".  There is no\nexplicit bottom of the DAG specified on the command line, and the\nfirst one (without \"--objects\") is done to find \"prerequisites\" that\nare written in the header.\n\nThen the packdata is generated, which does another traversal (this\ntime with \"--objects\" option).\n\nSo perhaps the right way to fix it is to keep the first traversal\nas-is, but update the second one (I think write-bundle-refs is the\nculprit) so that it does not use the user-supplied command line\nas-is; instead it should use the positive end of the history from\nthe command line with the negative end set to these \"prerequisites\"\ncommits.\n\nI said \"command line\" in the above, but read that as \"end user\ninput\"; the list of rev-list command line arguments given from the\nstandard input is exactly the same deal.\n"},{"id":"320688","messageId":"xmqqefvdfxx5.fsf@gitster.mtv.corp.google.com","threadId":"46030","inReplyTo":"xmqqwp97hvvi.fsf@gitster.mtv.corp.google.com","subject":"Re: Passing revs to git-bundle-create via stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-25T02:55:02Z","receivedAt":"2017-05-25T02:55:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> I think what's happening is that git-bundle actually runs _two_\n>> traversals using the command-line arguments. ...\n>> ... It was just a way of confirming my\n>> guess about the double-read.\n>>\n>> The real solutions I can think of are:\n>>\n>>   1. Teach git-bundle not to require the double-read. I'm not sure why\n>>      it's written the way it is, but presumably it would be tricky to\n>>      undo it (or we would have just written it the other way in the\n>>      first place!)\n>\n> If I remember correctly, the reason why it does the double-read is\n> because it wants to cope with things like \"--since\".  There is no\n> explicit bottom of the DAG specified on the command line, and the\n> first one (without \"--objects\") is done to find \"prerequisites\" that\n> are written in the header.\n>\n> Then the packdata is generated, which does another traversal (this\n> time with \"--objects\" option).\n>\n> So perhaps the right way to fix it is to keep the first traversal\n> as-is, but update the second one (I think write-bundle-refs is the\n> culprit) so that it does not use the user-supplied command line\n> as-is; instead it should use the positive end of the history from\n> the command line with the negative end set to these \"prerequisites\"\n> commits.\n>\n> I said \"command line\" in the above, but read that as \"end user\n> input\"; the list of rev-list command line arguments given from the\n> standard input is exactly the same deal.\n\nActually, after thinking a bit more about this, I think the bundle\nwe currently generate may be a bit less efficient than ideal when\noptions like --since or --max-count are used.\n\nImagine a history of this shape (child grows on the right hand side):\n\n A---D-----E-----G---H\n      \\         /\n       B---C---F\n\nThe labels on commits also denote their timestamps in relative\nterms, i.e. A has the oldest timestamp, D, even though it is a\nparent of B, has newer timestamp than B has, etc.\n\nNow, imagine running \"git log --since=$time H\" with time set to the\ntimestamp commit D has.  We traversal from H, following parent chain,\nand stop when we see a commit with timestamp older than $time.  So,\nwe'd enumerate H G F E D; C and A are \"boundaries\"---we looked at,\nbut we decided not to include in the result.\n\nA bundle file format records \"By using this bundle, you can advance\nyour history up to this commit\", which can be seen by running \"git\nls-remote\" on the bundle file.  It also records \"However, this\nbundle does not record the entire history; you need to have the\ncomplete history behind these commits\".  These are called\n\"prerequisites\", and can be checked with \"git bundle verify\".\nAnd then of course it has an actual packfile (which is thin).\n\nSo putting all together,\n\n\tgit bundle create mybundle --since=$time H\n\nwould record H as its head, and also C and A as prerequistes.\n\nThe \"double reading of --stdin\" we have been discussing is there\nbecause we run two traversals; the first one is to find the\nprerequisites (i.e. C and A in the above example).  The second one\nuses the same rev-list arguments (i.e. \"--since=$time H\") to\ngenerate pack, so it will include D.\n\nAs the recipient of a bundle is required to have complete history\nbehind both A and C, however, the packfile generated with the\ncurrent proceess is inefficient--it includes D but it does not need\nto.\n\nIf we change the argument to rev-list used in the actual packfile\ngeneration, and instead use the boundary we learned during the first\ntraversal (i.e. A and C in the above example) and the tip of the\nhistory being recorded in the resulting bundle (i.e. H), then we'd\nrun \"git log ^A ^C H\", which would only walk \"H G F E\".\n\nWhich would be smaller (it no longer includes D), and the recipient\nwho has A and C can still apply.\n"}]}