{"thread":{"id":"7102","subject":"builtin git-bundle - pack contains many more objects than required","startedAt":"2007-03-06T04:17:04Z","lastAt":"2007-03-08T20:36:41Z","messageCount":22,"participants":["Mark Levedahl","Junio C Hamano","Johannes Schindelin","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"36415","messageId":"45ECEB40.4000907@gmail.com","threadId":"7102","inReplyTo":null,"subject":"builtin git-bundle - pack contains many more objects than required","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-06T04:17:04Z","receivedAt":"2007-03-06T04:17:04Z","isPatch":false,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"builtin-bundle in version v1.5.0.3-271-g5ced057 (commit \n5ced0572217f82f20c4a3460) (and probably before) includes far more \nobjects in the pack than are actually necessary. Comparison to the \noriginal shell scripts I wrote shows the prerequisites and refs always \nagree between those (and that is what I tested before), so the problem \nis apparently in the arguments as passed to pack-objects. I confess to \nnot understanding the revision walking code called from \nbuiltin-bundle.c, so I do not have a patch.\n\nHowever, a simple example:\n\ngit>git-rev-list --objects HEAD~1..HEAD\n5ced0572217f82f20c4a3460492768e07c08aeea\n1d30eaabe4b6f7218e4e4cfff5670a493aa7358e\n2a1d6a2be1511f65b601897f05962e5f673257d8 contrib\ncbd77b2f57114d4fa2f119f1b1ee17968d6d67d4 contrib/emacs\n8554e3967cc692c6916e5aee35952074d07e8bf0 contrib/emacs/Makefile\n\nshows that the bundle for this case should have five objects as shown. \nIn this case, 916 objects are actually included in the pack when executing\n\ngit bundle create test.bdl HEAD~1..HEAD\n\nMark\n"},{"id":"36419","messageId":"7vmz2quc7s.fsf@assigned-by-dhcp.cox.net","threadId":"7102","inReplyTo":"45ECEB40.4000907@gmail.com","subject":"Re: builtin git-bundle - pack contains many more objects than required","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-06T06:18:15Z","receivedAt":"2007-03-06T06:18:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> However, a simple example:\n>\n> git>git-rev-list --objects HEAD~1..HEAD\n> 5ced0572217f82f20c4a3460492768e07c08aeea\n> 1d30eaabe4b6f7218e4e4cfff5670a493aa7358e\n> 2a1d6a2be1511f65b601897f05962e5f673257d8 contrib\n> cbd77b2f57114d4fa2f119f1b1ee17968d6d67d4 contrib/emacs\n> 8554e3967cc692c6916e5aee35952074d07e8bf0 contrib/emacs/Makefile\n>\n> shows that the bundle for this case should have five objects as\n> shown. In this case, 916 objects are actually included in the pack\n> when executing\n>\n> git bundle create test.bdl HEAD~1..HEAD\n\nIndeed. The internal rev-list reimplemented in builtin-bundle.c\nlacks the code to mark trees and blobs in commits that are on\nthe other side of the edge uninteresting.\n\nThis is on top of the patches I send to the list and Dscho\nearlier.\n\n--\n\n builtin-bundle.c | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 4fe74a7..199a30d 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -278,6 +278,11 @@ static void show_object(struct object_array_entry *p)\n \twrite_or_die(1, \"\\n\", 1);\n }\n \n+static void show_edge(struct commit *commit)\n+{\n+\t; /* nothing to do */\n+}\n+\n static int create_bundle(struct bundle_header *header, const char *path,\n \t\tint argc, const char **argv)\n {\n@@ -361,6 +366,7 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \tdup2(in, 1);\n \tclose(in);\n \tprepare_revision_walk(&revs);\n+\tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n \ttraverse_commit_list(&revs, show_commit, show_object);\n \tclose(1);\n \twhile (waitpid(pid, &status, 0) < 0)\n"},{"id":"36433","messageId":"7vejo2stlw.fsf@assigned-by-dhcp.cox.net","threadId":"7102","inReplyTo":"45ECEB40.4000907@gmail.com","subject":"[PATCH] git-bundle: fix pack generation.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-06T07:45:31Z","receivedAt":"2007-03-06T07:45:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The handcrafted built-in rev-list lookalike forgot to mark the trees\nand blobs contained in the boundary commits uninteresting, resulting\nin unnecessary objects in the pack.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I'll have this in 'master' tonight. Also I am thinking about\n changing 'git bundle verify' to spit out heads and prereqs\n unconditionally, in addition to 'foo.bdl is ok'. Comments?\n\n builtin-bundle.c | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex d41a413..279b8f8 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -263,6 +263,11 @@ static void show_object(struct object_array_entry *p)\n \twrite_or_die(1, \"\\n\", 1);\n }\n \n+static void show_edge(struct commit *commit)\n+{\n+\t; /* nothing to do */\n+}\n+\n static int create_bundle(struct bundle_header *header, const char *path,\n \t\tint argc, const char **argv)\n {\n@@ -341,6 +346,7 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \tdup2(in, 1);\n \tclose(in);\n \tprepare_revision_walk(&revs);\n+\tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n \ttraverse_commit_list(&revs, show_commit, show_object);\n \tclose(1);\n \twhile (waitpid(pid, &status, 0) < 0)\n-- \n1.5.0.3.862.g71037\n"},{"id":"36510","messageId":"45EE1242.8080405@gmail.com","threadId":"7102","inReplyTo":"7vejo2stlw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-07T01:15:46Z","receivedAt":"2007-03-07T01:15:46Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> The handcrafted built-in rev-list lookalike forgot to mark the trees\n> and blobs contained in the boundary commits uninteresting, resulting\n> in unnecessary objects in the pack.\n>\n> Signed-off-by: Junio C Hamano \n> \nThis works for things like master~1..master, but fails on git bundle \ncreate t.bdl master --since=1.day.ago.\nApparently the boundary commits marked with a date are not being honored \nin creating a pack.\n\nMark\n"},{"id":"36511","messageId":"45EE1281.5040506@gmail.com","threadId":"7102","inReplyTo":"7vejo2stlw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-07T01:16:49Z","receivedAt":"2007-03-07T01:16:49Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 3/6/07, *Junio C Hamano* > wrote:\n\n\n * I'll have this in 'master' tonight. Also I am thinking about\n changing 'git bundle verify' to spit out heads and prereqs\n unconditionally, in addition to 'foo.bdl is ok'. Comments?\n\n \nThat is a good feature, much better than trying to list the bundle, and \nI have no issue with this suggestion.\n \nWe could also define \"git bundle [depends|show-refs] bundle-file\" to be \nmore suggestive of the actual result. As the prerequisites have a common \nconceptual definition with those of a shallow clone, perhaps there \nshould be a common way to get those. In this vein, git-ls-remote bundle \nalready shows the references in git-like fashion.\n \nMark\n"},{"id":"36524","messageId":"7vhcsxkb8n.fsf@assigned-by-dhcp.cox.net","threadId":"7102","inReplyTo":"45EE1242.8080405@gmail.com","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-07T03:01:44Z","receivedAt":"2007-03-07T03:01:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> Junio C Hamano wrote:\n>> The handcrafted built-in rev-list lookalike forgot to mark the trees\n>> and blobs contained in the boundary commits uninteresting, resulting\n>> in unnecessary objects in the pack.\n>>\n>> Signed-off-by: Junio C Hamano \n>>\n> This works for things like master~1..master, but fails on git bundle\n> create t.bdl master --since=1.day.ago.\n> Apparently the boundary commits marked with a date are not being\n> honored in creating a pack.\n\nThat one is caused by the broken revision traversal in 'master'\nand being worked on in 'next'. Care to try the one from 'next'\ninstead?\n"},{"id":"36525","messageId":"45EE2ECA.60403@gmail.com","threadId":"7102","inReplyTo":"7vhcsxkb8n.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-07T03:17:30Z","receivedAt":"2007-03-07T03:17:30Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> That one is caused by the broken revision traversal in 'master'\n> and being worked on in 'next'. Care to try the one from 'next'\n> instead?\nusing next as just pulled from kernel.org \n(09890a9bce0bc27182bc1f74a34b53) ...\n\ngit>git bundle create test.bdl HEAD~1..HEAD\nerror: rev-list died 255\n\nI have not found any rev-args set that avoids that error.\n\nMark\n"},{"id":"36526","messageId":"Pine.LNX.4.63.0703070419410.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7102","inReplyTo":"45EE2ECA.60403@gmail.com","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-07T03:22:12Z","receivedAt":"2007-03-07T03:22:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Mar 2007, Mark Levedahl wrote:\n\n> Junio C Hamano wrote:\n> > That one is caused by the broken revision traversal in 'master'\n> > and being worked on in 'next'. Care to try the one from 'next'\n> > instead?\n> using next as just pulled from kernel.org (09890a9bce0bc27182bc1f74a34b53)\n> ...\n> \n> git>git bundle create test.bdl HEAD~1..HEAD\n> error: rev-list died 255\n> \n> I have not found any rev-args set that avoids that error.\n\nHave you tried \"make test\"? In particular, t5510-fetch.sh?\n\nIf it passes, git-bundle works at least for one particular case, and I'd \nsuspect then that the inbuilt GIT_EXEC_PATH bites you. To avoid that \nparticular peculiarity, just \"export GIT_EXEC_PATH=/path/to/next/\", and \ntry again.\n\nHiw,\nDscho\n"},{"id":"36527","messageId":"45EE36A1.30001@gmail.com","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703070419410.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-07T03:50:57Z","receivedAt":"2007-03-07T03:50:57Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> Have you tried \"make test\"? In particular, t5510-fetch.sh?\n>\n> If it passes, git-bundle works at least for one particular case, and I'd \n> suspect then that the inbuilt GIT_EXEC_PATH bites you. To avoid that \n> particular peculiarity, just \"export GIT_EXEC_PATH=/path/to/next/\", and \n> try again.\n>\n> Hiw,\n> Dscho\nYes, setting GIT_EXEC_PATH fixed the problem, thanks.\n\nI just tried git-bundle in a repository where I just committed 1 file, \nthe previous commit is several weeks old.\n\ngit-bundle create test.bdl HEAD --since=1.day.ago ==>> pack with 1531 \nobjects\ngit-bundle create test.bdl HEAD ^HEAD~1 ==>> pack with 3 objects\n\nBut, both should only have 3 objects. So, I think the boundary marking \nwith date limiting still has a problem. Apparently, every blob \nsupporting the included commits are included in the pack, even if those \nblobs are also part of the pack's prerequisites.\n\nMark\n"},{"id":"36528","messageId":"Pine.LNX.4.63.0703070504140.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7102","inReplyTo":"45EE36A1.30001@gmail.com","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-07T04:05:40Z","receivedAt":"2007-03-07T04:05:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Mar 2007, Mark Levedahl wrote:\n\n> Yes, setting GIT_EXEC_PATH fixed the problem, thanks.\n\nWonderful!\n\n> I just tried git-bundle in a repository where I just committed 1 file, \n> the previous commit is several weeks old.\n> \n> git-bundle create test.bdl HEAD --since=1.day.ago ==>> pack with 1531\n> objects\n\nDid you test with \"--since=1.day.ago HEAD\", i.e. with the correct order? I \nknow you'd like the options to be interminglable, but \"HEAD\" really is not \nan option, but an argument.\n\nHth,\nDscho\n"},{"id":"36531","messageId":"7vy7m9iojy.fsf@assigned-by-dhcp.cox.net","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703070504140.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-07T05:57:05Z","receivedAt":"2007-03-07T05:57:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> git-bundle create test.bdl HEAD --since=1.day.ago ==>> pack with 1531\n>> objects\n>\n> Did you test with \"--since=1.day.ago HEAD\", i.e. with the correct order? I \n> know you'd like the options to be interminglable, but \"HEAD\" really is not \n> an option, but an argument.\n\nThat is probably true but I do not think we have ever had date\nlimiter and --objects interact correctly. It is understandable\nas traditionally packs were to generate needed objects between\nrevisions, and unlimited but counted (--max-count) or timed\n(--max-age) are unexpected uses for \"rev-list --objects\" (in\nother words, nobody had done so and I am not surprised it does\nnot work as you expect -- rather I would be surprised if anybody\nhas even expectd the current code to work that way without\nmodification).\n\nI am a bit too tired tonight so anybody is welcome to beat me to\nfix this one, preferrably on top of 'next'.\n"},{"id":"36553","messageId":"30e4a070703070834s3dd5bdd7x2e1639aa2979d1cf@mail.gmail.com","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703070504140.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-07T16:34:15Z","receivedAt":"2007-03-07T16:34:15Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"On 3/6/07, Johannes Schindelin wrote:\n> Hi,\n>\n> >\n> > git-bundle create test.bdl HEAD --since=1.day.ago ==>> pack with 1531\n> > objects\n>\n> Did you test with \"--since=1.day.ago HEAD\", i.e. with the correct order? I\n> know you'd like the options to be interminglable, but \"HEAD\" really is not\n> an option, but an argument.\n>\n\nChanging the order of arguments makes no difference, same result either way.\n\nMark\n"},{"id":"36577","messageId":"Pine.LNX.4.63.0703072330200.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7102","inReplyTo":"30e4a070703070834s3dd5bdd7x2e1639aa2979d1cf@mail.gmail.com","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-07T22:35:06Z","receivedAt":"2007-03-07T22:35:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Mar 2007, Mark Levedahl wrote:\n\n> On 3/6/07, Johannes Schindelin wrote:\n> > \n> > > git-bundle create test.bdl HEAD --since=1.day.ago ==>> pack with \n> > > 1531 objects\n> > \n> > Did you test with \"--since=1.day.ago HEAD\", i.e. with the correct \n> > order? I know you'd like the options to be interminglable, but \"HEAD\" \n> > really is not an option, but an argument.\n> \n> Changing the order of arguments makes no difference, same result either \n> way.\n\nWe don't do thin packs. Should we? I guess that \n\n\t$ git ls-tree -r HEAD | wc\n\nresults in something close to 1500 in that repo. Which basically means \nthat the 1531 objects (including trees and the commit) sounds correct.\n\nOf course, we _could_ make the packs thin, but the disadvantage would be \nthat we can never decide at a later stage to allow shallow fetches from \nthat bundle.\n\nThe advantage of the thin packs would be that the bundles would be much \nsmaller.\n\nCiao,\nDscho\n"},{"id":"36581","messageId":"Pine.LNX.4.64.0703071516330.5963@woody.linux-foundation.org","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703072330200.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-07T23:17:49Z","receivedAt":"2007-03-07T23:17:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 7 Mar 2007, Johannes Schindelin wrote:\n> \n> We don't do thin packs. Should we?\n\nSince a bundle doesn't make any sense *anyway* unless you have the \nprerequisites at the other end, I think you might as well do thin packs. \nThat will cut down on the bundle size a *lot* for the common cases.\n\n\t\tLinus\n"},{"id":"36583","messageId":"7vfy8geikk.fsf@assigned-by-dhcp.cox.net","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703072330200.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: fix pack generation.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-07T23:32:11Z","receivedAt":"2007-03-07T23:32:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Wed, 7 Mar 2007, Mark Levedahl wrote:\n>\n>> On 3/6/07, Johannes Schindelin wrote:\n>> > \n>> > > git-bundle create test.bdl HEAD --since=1.day.ago ==>> pack with \n>> > > 1531 objects\n>> > \n>> > Did you test with \"--since=1.day.ago HEAD\", i.e. with the correct \n>> > order? I know you'd like the options to be interminglable, but \"HEAD\" \n>> > really is not an option, but an argument.\n>> \n>> Changing the order of arguments makes no difference, same result either \n>> way.\n>\n> We don't do thin packs. Should we? I guess that \n>\n> \t$ git ls-tree -r HEAD | wc\n>\n> results in something close to 1500 in that repo. Which basically means \n> that the 1531 objects (including trees and the commit) sounds correct.\n\nWe should do thin packs, but 1500+ objects is a different story.\n\n\trev-list --objects \n\nmeans \"show me the revs and its objects\".\n\n\trev-list --max-count= \n\nmeans \"show me the revs and its objects, but stop at revs\".\n\nOn the other hand, \n\n\trev-list --objects ^ \n\nmeans something totally different. \"never show me what is\nreachable from but I am interested to see what are\nreachable from \".\n\nSo it is natural that \"rev-list --objects --max-count=1 HEAD\"\nshows the same set of tree and blob objects as \"git-tar HEAD\".\n\"show me the revs and its objects\" does not say \"but exclude\ntrees and blobs reachable from commits beyond the revs\nboundary,\" which is \"rev-list --objects HEAD^..HEAD\".\n\nIn other words, \"rev-list -1 HEAD\" and \"rev-list HEAD^..HEAD\"\nare _different_.\n\nWe fixed (in 'next') the revision traversal so that --max-count\nand --boundary interact correctly. What's not fixed is that we\nstill use \"rev-list --objects --max-*\" given by the user, which\nis not the semantics git-bundle wants. We should rewrite the\nrevspec to drive pack-objects when generating the packfile part\nof the bundle to do \" --not \" internally in\ngit-bundle.\n"},{"id":"36584","messageId":"Pine.LNX.4.63.0703080039420.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7102","inReplyTo":"7vfy8geikk.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] git-bundle: avoid packing objects which are in the prerequisites","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-07T23:43:05Z","receivedAt":"2007-03-07T23:43:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWhen saying something like \"--since=1.day.ago\" or \"--max-count=5\",\ngit-bundle finds the boundary commits which are recorded as\nprerequisites. However, it failed to tell pack-objects _not_ to\npack the objects which are in these.\n\nFix that. And add a test for that.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Wed, 7 Mar 2007, Junio C Hamano wrote:\n\n\t> What's not fixed is that we still use \"rev-list --objects \n\t> --max-*\" given by the user, which is not the semantics \n\t> git-bundle wants. We should rewrite the revspec to drive \n\t> pack-objects when generating the packfile part of the bundle to \n\t> do \" --not \" internally in git-bundle.\n\n\tSince we already calculate the prerequisites, it's easy to pass\n\tthem as uninteresting, too. It does not hurt to keep the max*\n\tlimiters, so they are not unset.\n\n builtin-bundle.c | 15 ++++++++++++---\n t/t5510-fetch.sh | 11 +++++++++++\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 3b3bc25..70d4479 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -305,6 +305,10 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t/* write signature */\n \twrite_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));\n \n+\t/* init revs to list objects for pack-objects later */\n+\tsave_commit_buffer = 0;\n+\tinit_revisions(&revs, NULL);\n+\n \t/* write prerequisites */\n \tmemcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));\n \targv_boundary[0] = \"rev-list\";\n@@ -316,8 +320,15 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \tif (pid < 0)\n \t\treturn -1;\n \twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0)\n-\t\tif (buffer[0] == '-')\n+\t\tif (buffer[0] == '-') {\n+\t\t\tunsigned char sha1[20];\n \t\t\twrite_or_die(bundle_fd, buffer, i);\n+\t\t\tif (!get_sha1_hex(buffer + 1, sha1)) {\n+\t\t\t\tstruct object *object = parse_object(sha1);\n+\t\t\t\tobject->flags |= UNINTERESTING;\n+\t\t\t\tadd_pending_object(&revs, object, buffer);\n+\t\t\t}\n+\t\t}\n \twhile ((i = waitpid(pid, &status, 0)) < 0)\n \t\tif (errno != EINTR)\n \t\t\treturn error(\"rev-list died\");\n@@ -325,8 +336,6 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\treturn error(\"rev-list died %d\", WEXITSTATUS(status));\n \n \t/* write references */\n-\tsave_commit_buffer = 0;\n-\tinit_revisions(&revs, NULL);\n \trevs.tag_objects = 1;\n \trevs.tree_objects = 1;\n \trevs.blob_objects = 1;\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex ce96b4b..7be9793 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -121,4 +121,15 @@ test_expect_success 'unbundle 2' '\n \ttest \"tip\" = \"$(git log -1 --pretty=oneline master | cut -b42-)\"\n '\n \n+test_expect_success 'bundle does not prerequisite objects' '\n+\tcd \"$D\" &&\n+\ttouch file2 &&\n+\tgit add file2 &&\n+\tgit commit -m add.file2 file2 &&\n+\tgit bundle create bundle3 -1 HEAD &&\n+\tsed \"1,4d\" < bundle3 > bundle.pack &&\n+\tgit index-pack bundle.pack &&\n+\ttest 4 = $(git verify-pack -v bundle.pack | wc -l)\n+'\n+\n test_done\n-- \n1.5.0.3.2562.gfcfe0\n"},{"id":"36586","messageId":"Pine.LNX.4.63.0703080121210.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7102","inReplyTo":"Pine.LNX.4.64.0703071516330.5963@woody.linux-foundation.org","subject":"[PATCH] git-bundle: Make thin packs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-08T00:27:44Z","receivedAt":"2007-03-08T00:27:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThin packs are way smaller, but they rely on the receiving end to have the \nbase objects. However, Git's pack protocol also uses thin packs by \ndefault. So make the packs contained in bundles thin, since bundles are \njust another transport.\n\nThe patch looks a bit bigger than intended, mainly because --thin\n_implies_ that pack-objects should run its own rev-list. Therefore, this \npatch removes all the stuff we used to roll rev-list ourselves.\n\nThis commit also changes behaviour slightly: since we now know early \nenough if a specified ref is _not_ contained in the pack, we can avoid \nputting that ref into the pack. So, we don't die() here, but warn() \ninstead, and skip that ref.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tIt came to me as a surprise that --thin implies --revs, and\n\tI have to admit that I still don't understand it.\n\n\tAnyway, this commit deletes more than double the lines as it\n\tadds, so I'm not complaining.\n\n\tOn Wed, 7 Mar 2007, Linus Torvalds wrote:\n\n\t> On Wed, 7 Mar 2007, Johannes Schindelin wrote:\n\t> \n\t> > We don't do thin packs. Should we?\n\t> \n\t> Since a bundle doesn't make any sense *anyway* unless you have \n\t> the prerequisites at the other end, I think you might as well do \n\t> thin packs. That will cut down on the bundle size a *lot* for \n\t> the common cases.\n\n\tWell, I disagree on the blanket \"*anyway*\". Shallow fetches are no \n\tlonger possible from these bundles (at least after this commit \n\t_and_ \":/git-bundle: avoid packing\" which I just sent out).\n\n\tBut then, people who want shallow fetches via bundles are more \n\tlikely to want tar balls _anyway_.\n\n builtin-bundle.c | 85 ++++++++++++++++-------------------------------------\n 1 files changed, 26 insertions(+), 59 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 70d4479..6163358 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -257,45 +257,15 @@ static int list_heads(struct bundle_header *header, int argc, const char **argv)\n \treturn list_refs(&header->references, argc, argv);\n }\n \n-static void show_commit(struct commit *commit)\n-{\n-\twrite_or_die(1, sha1_to_hex(commit->object.sha1), 40);\n-\twrite_or_die(1, \"\\n\", 1);\n-\tif (commit->parents) {\n-\t\tfree_commit_list(commit->parents);\n-\t\tcommit->parents = NULL;\n-\t}\n-}\n-\n-static void show_object(struct object_array_entry *p)\n-{\n-\t/* An object with name \"foo\\n0000000...\" can be used to\n-\t * confuse downstream git-pack-objects very badly.\n-\t */\n-\tconst char *ep = strchr(p->name, '\\n');\n-\tint len = ep ? ep - p->name : strlen(p->name);\n-\twrite_or_die(1, sha1_to_hex(p->item->sha1), 40);\n-\twrite_or_die(1, \" \", 1);\n-\tif (len)\n-\t\twrite_or_die(1, p->name, len);\n-\twrite_or_die(1, \"\\n\", 1);\n-}\n-\n-static void show_edge(struct commit *commit)\n-{\n-\t; /* nothing to do */\n-}\n-\n static int create_bundle(struct bundle_header *header, const char *path,\n \t\tint argc, const char **argv)\n {\n \tint bundle_fd = -1;\n \tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n-\tconst char **argv_pack = xmalloc(4 * sizeof(const char *));\n+\tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n \tint pid, in, out, i, status;\n \tchar buffer[1024];\n \tstruct rev_info revs;\n-\tstruct object_array tips;\n \n \tbundle_fd = (!strcmp(path, \"-\") ? 1 :\n \t\t\topen(path, O_CREAT | O_WRONLY, 0666));\n@@ -319,16 +289,20 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \tpid = fork_with_pipe(argv_boundary, NULL, &out);\n \tif (pid < 0)\n \t\treturn -1;\n-\twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0)\n+\twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0) {\n+\t\tunsigned char sha1[20];\n \t\tif (buffer[0] == '-') {\n-\t\t\tunsigned char sha1[20];\n \t\t\twrite_or_die(bundle_fd, buffer, i);\n \t\t\tif (!get_sha1_hex(buffer + 1, sha1)) {\n \t\t\t\tstruct object *object = parse_object(sha1);\n \t\t\t\tobject->flags |= UNINTERESTING;\n \t\t\t\tadd_pending_object(&revs, object, buffer);\n \t\t\t}\n+\t\t} else if (!get_sha1_hex(buffer, sha1)) {\n+\t\t\tstruct object *object = parse_object(sha1);\n+\t\t\tobject->flags |= SHOWN;\n \t\t}\n+\t}\n \twhile ((i = waitpid(pid, &status, 0)) < 0)\n \t\tif (errno != EINTR)\n \t\t\treturn error(\"rev-list died\");\n@@ -336,14 +310,10 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\treturn error(\"rev-list died %d\", WEXITSTATUS(status));\n \n \t/* write references */\n-\trevs.tag_objects = 1;\n-\trevs.tree_objects = 1;\n-\trevs.blob_objects = 1;\n \targc = setup_revisions(argc, argv, &revs, NULL);\n \tif (argc > 1)\n \t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n \n-\tmemset(&tips, 0, sizeof(tips));\n \tfor (i = 0; i < revs.pending.nr; i++) {\n \t\tstruct object_array_entry *e = revs.pending.objects + i;\n \t\tunsigned char sha1[20];\n@@ -353,11 +323,20 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\t\tcontinue;\n \t\tif (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)\n \t\t\tcontinue;\n+\t\t/*\n+\t\t * Make sure the refs we wrote out is correct; --max-count and\n+\t\t * other limiting options could have prevented all the tips\n+\t\t * from getting output.\n+\t\t */\n+\t\tif (!(e->item->flags & SHOWN)) {\n+\t\t\twarn(\"ref '%s' is excluded by the rev-list options\",\n+\t\t\t\te->name);\n+\t\t\tcontinue;\n+\t\t}\n \t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n \t\twrite_or_die(bundle_fd, \" \", 1);\n \t\twrite_or_die(bundle_fd, ref, strlen(ref));\n \t\twrite_or_die(bundle_fd, \"\\n\", 1);\n-\t\tadd_object_array(e->item, e->name, &tips);\n \t\tfree(ref);\n \t}\n \n@@ -368,39 +347,27 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \targv_pack[0] = \"pack-objects\";\n \targv_pack[1] = \"--all-progress\";\n \targv_pack[2] = \"--stdout\";\n-\targv_pack[3] = NULL;\n+\targv_pack[3] = \"--thin\";\n+\targv_pack[4] = NULL;\n \tin = -1;\n \tout = bundle_fd;\n \tpid = fork_with_pipe(argv_pack, &in, &out);\n \tif (pid < 0)\n \t\treturn error(\"Could not spawn pack-objects\");\n-\tclose(1);\n-\tdup2(in, 1);\n+\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\tstruct object *object = revs.pending.objects[i].item;\n+\t\tif (object->flags & UNINTERESTING)\n+\t\t\twrite(in, \"^\", 1);\n+\t\twrite(in, sha1_to_hex(object->sha1), 40);\n+\t\twrite(in, \"\\n\", 1);\n+\t}\n \tclose(in);\n-\tprepare_revision_walk(&revs);\n-\tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n-\ttraverse_commit_list(&revs, show_commit, show_object);\n-\tclose(1);\n \twhile (waitpid(pid, &status, 0) < 0)\n \t\tif (errno != EINTR)\n \t\t\treturn -1;\n \tif (!WIFEXITED(status) || WEXITSTATUS(status))\n \t\treturn error (\"pack-objects died\");\n \n-\t/*\n-\t * Make sure the refs we wrote out is correct; --max-count and\n-\t * other limiting options could have prevented all the tips\n-\t * from getting output.\n-\t */\n-\tstatus = 0;\n-\tfor (i = 0; i < tips.nr; i++) {\n-\t\tif (!(tips.objects[i].item->flags & SHOWN)) {\n-\t\t\tstatus = 1;\n-\t\t\terror(\"%s: not included in the resulting pack\",\n-\t\t\t tips.objects[i].name);\n-\t\t}\n-\t}\n-\n \treturn status;\n }\n \n-- \n1.5.0.3.2562.gfcfe0\n"},{"id":"36587","messageId":"Pine.LNX.4.64.0703071632140.5963@woody.linux-foundation.org","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703080121210.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: Make thin packs","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-08T00:34:16Z","receivedAt":"2007-03-08T00:34:16Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 8 Mar 2007, Johannes Schindelin wrote:\n>\n> \t> Since a bundle doesn't make any sense *anyway* unless you have \n> \t> the prerequisites at the other end, I think you might as well do \n> \t> thin packs. That will cut down on the bundle size a *lot* for \n> \t> the common cases.\n> \n> \tWell, I disagree on the blanket \"*anyway*\". Shallow fetches are no \n> \tlonger possible from these bundles (at least after this commit \n> \t_and_ \":/git-bundle: avoid packing\" which I just sent out).\n\nDoes anybody actually use shallow clones in real life?\n\nWhen I did the numbers a long time ago, the shallow clone didn't actually \nhelp much, because it meant that there were no deltas. Which meant that \nyou got 1% of the history for 60% of the price of all history, and the \nshallow thing didn't really seem to make much sense.\n\nI guess that for something with a really long history, you'd get 0.001% of \nthe history for 10% of the price, and maybe it makes sense then.\n\n\t\tLinus\n"},{"id":"36588","messageId":"Pine.LNX.4.63.0703080149320.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7102","inReplyTo":"Pine.LNX.4.64.0703071632140.5963@woody.linux-foundation.org","subject":"Re: [PATCH] git-bundle: Make thin packs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-08T00:56:29Z","receivedAt":"2007-03-08T00:56:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Mar 2007, Linus Torvalds wrote:\n\n> Does anybody actually use shallow clones in real life?\n\nI don't. That's why I work on push/fetch from/via/into shallow repos.\n\n> When I did the numbers a long time ago, the shallow clone didn't \n> actually help much, because it meant that there were no deltas. Which \n> meant that you got 1% of the history for 60% of the price of all \n> history, and the shallow thing didn't really seem to make much sense.\n> \n> I guess that for something with a really long history, you'd get 0.001% \n> of the history for 10% of the price, and maybe it makes sense then.\n\nYou -- being blessed by not having to work with anything closed -- miss an \nimportant fact of commercial software development. Most, if not all, \nprojects in a commercial setting contain binary blobs. For example, a DLL, \nor a PNG, or a Firmware blob. These are updated regularly. And they delta \nreally awfully bad.\n\nAlso, for something as OpenOffice or Mozilla, I guess that if you really \nonly want to work on the newest revision, the _initial_ fetch will be way \ncheaper than a full clone. Of course I have no numbers here, but that is \nwhat my gut feeling says.\n\nNaturally, over time, the shallow clone will fetch in more and more \nobjects from the upstream, eventually being almost as large as if having \nthe complete history, but the user's experience is different: the amount \nof time needed to get that amount of data is much more widely spread.\n\nCiao,\nDscho\n"},{"id":"36589","messageId":"7v4powebmr.fsf@assigned-by-dhcp.cox.net","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703080121210.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: Make thin packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-08T02:02:04Z","receivedAt":"2007-03-08T02:02:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This commit also changes behaviour slightly: since we now know early \n> enough if a specified ref is _not_ contained in the pack, we can avoid \n> putting that ref into the pack. So, we don't die() here, but warn() \n> instead, and skip that ref.\n\nI am a bit worried about that part. \n\nIf somebody says \"bundle create foo.bdl --since=1.day maint\" and\nmaint's tip hasn't changed for a month, we would end up having\nno refs and no pack in the bundle with just a warning.\n\n\"bundle create foo.bdl -20 master next\" does _not_ mean \"20 revs\nbut at least have master and next tip\", but it may surprise an\nuninitiated to find out that running \"bundle list foo.bdl\" on\nthe resulting bundle did not talk about master at all.\n\nI have a feeling that it would avoid confusion if we did not\neven start the pack generation and die early when we find the\ncounting caused us not to include all the positive refs\nspecified on the command line.\n"},{"id":"36621","messageId":"Pine.LNX.4.63.0703081406570.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7102","inReplyTo":"7v4powebmr.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-bundle: Make thin packs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-08T13:07:14Z","receivedAt":"2007-03-08T13:07:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Mar 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > This commit also changes behaviour slightly: since we now know early \n> > enough if a specified ref is _not_ contained in the pack, we can avoid \n> > putting that ref into the pack. So, we don't die() here, but warn() \n> > instead, and skip that ref.\n> \n> I am a bit worried about that part. \n> \n> If somebody says \"bundle create foo.bdl --since=1.day maint\" and\n> maint's tip hasn't changed for a month, we would end up having\n> no refs and no pack in the bundle with just a warning.\n> \n> \"bundle create foo.bdl -20 master next\" does _not_ mean \"20 revs\n> but at least have master and next tip\", but it may surprise an\n> uninitiated to find out that running \"bundle list foo.bdl\" on\n> the resulting bundle did not talk about master at all.\n> \n> I have a feeling that it would avoid confusion if we did not\n> even start the pack generation and die early when we find the\n> counting caused us not to include all the positive refs\n> specified on the command line.\n\nFair enough.\n\nCiao,\nDscho\n"},{"id":"36650","messageId":"45F073D9.7060209@gmail.com","threadId":"7102","inReplyTo":"Pine.LNX.4.63.0703080039420.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: avoid packing objects which are in the prerequisites","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2007-03-08T20:36:41Z","receivedAt":"2007-03-08T20:36:41Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n\nUsing next (2432cbc253e8ef5cf09062abb8f7325813476d1d) that has your \npatches for creating a thin-pack and limiting objects, I generally find \nthe bundle file created cannot be used.\n\nSpecific example you can repeat in a current git repo:\n\ngit>git branch foo 2432cbc253e8ef5c\ngit>git bundle create t.bdl foo~1..foo\nGenerating pack...\nDone counting 20 objects.\nResult has 14 objects.\nDeltifying 14 objects.\n 100% (14/14) done\nWriting 14 objects.\n 100% (14/14) done\nTotal 14 (delta 10), reused 10 (delta 6)\ngit>git fetch t.bdl\nfatal: pack has junk at the end\nerror: index-pack exited with status 128\nfatal: Fetch failure: t.bdl\ngit>\n\n\nAlso, we get an interesting failure if the references are not touched in \na given date range:\ngit>git bundle create t.bdl --since=5.minutes.ago foo\nwarning: ref 'foo' is excluded by the rev-list options\nGenerating pack...\nDone counting 38321 objects.\nDeltifying 38321 objects.\n 100% (38321/38321) done\nWriting 38321 objects.\n 100% (38321/38321) done\nTotal 38321 (delta 26678), reused 38180 (delta 26551)\ngit>\n\nNo references requies 38000+ objects?? In this case, the bundle includes \nevery object in the repository, the exact opposite of what should happen \n(zero objects).\n\nMark\n"}]}