{"thread":{"id":"22278","subject":"bug: git-bundle create foo --stdin -> segfault","startedAt":"2010-01-19T00:26:42Z","lastAt":"2010-06-30T20:37:59Z","messageCount":19,"participants":["Joey Hess","Johannes Schindelin","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"132060","messageId":"20100119002641.GA31434@gnu.kitenet.net","threadId":"22278","inReplyTo":null,"subject":"bug: git-bundle create foo --stdin -> segfault","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2010-01-19T00:26:42Z","receivedAt":"2010-01-19T00:26:42Z","isPatch":false,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"joey@gnu:~/tmp/new>echo master | git bundle create ../my.bundle --stdin\nzsh: segmentation fault git bundle create ../my.bundle --stdin\n\nI noticed that git bundle --stdin actually attempts to read from stdin\npast EOF. You can see this if you manually type into its stdin.\n\n% git-bundle create ../bundle --stdin\nmaster\n^D\nmaster\n^D\nfatal: Refusing to create empty bundle.\n\nThe first stdin read is done by the internal call to rev-list --stdin.\nThe second stdin read is done by the call to setup_revisions(),\nwhich has its own handler for --stdin.\n\nBug seen with git version 1.6.5.7 / 1.6.6.243.gff6d2\n\nI also tried going back to 22568f0a336ac37ae7329c917857b455839d1d09, but\nstill see a bug with Adam Brewster's initial code to add --stdin to\ngit-bundle. That code still tries to read stdin twice. If it sees\n\"master\" both times, it does create a bundle.\n\n-- \nsee shy jo\n"},{"id":"132148","messageId":"alpine.DEB.1.00.1001192307090.3164@intel-tinevez-2-302","threadId":"22278","inReplyTo":"20100119002641.GA31434@gnu.kitenet.net","subject":"Re: bug: git-bundle create foo --stdin -> segfault","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-19T23:52:51Z","receivedAt":"2010-01-19T23:52:51Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 18 Jan 2010, Joey Hess wrote:\n\n> joey@gnu:~/tmp/new>echo master | git bundle create ../my.bundle --stdin\n> zsh: segmentation fault git bundle create ../my.bundle --stdin\n\nCurrent 'next' fails, too.\n\nSome previous Git versions failed with a message like this:\n\nerror: unrecognized argument: --stdin'\n\nOther previous Git versions failed at least with a message such as this:\n\nfatal: exec rev-list failed.\nerror: rev-list died\n\nSomething similar happens to me when I run the initial official revision \nof git-bundle. The reason is that \"git rev-list --boundary \n--pretty=oneline --stdin\" refuses to run.\n\nThe bad versions either segfault, or \"refuse to create empty bundles\".\n\nAnd while 8b3dce5 purports to clean things up (even acknowledging that \nsupport code for --stdin was removed from bundle.c!), at that \ntime git-bundle was obviously not tested/fixed.\n\nNow, I invested a lot of time into the new Git wiki, and into trying to \nbisect this (it took many, many more steps than the suggested 13, and \nsomewhere in between, the number of commits to be tested even increased!).\n\nIf you want to fix it, I suggest requiring --stdin to be the only \nparameter after the bundle file name, and adding a function using \nstrbuf_getline() to parse the stdin into a string_list. Once that is \ndone, you can substitute the argv for the rev-list call with that list \n(for that, you need to prepopulate with \"rev-list\", \"--boundary\" and \n\"--pretty=oneline\"). You can reuse that list for the call to \nsetup_revisions().\n\nAlternatively, you can try to implement the rev-list --boundary by hand \n(the --pretty=oneline is only needed to get a boundary marker IIRC), \ntaking care to reset the commit flags that were set in the process. (We \nneed to know the boundary commits before actually starting to write the \npack, because the bundle file format dictates that the boundary commits \nare listed as prerequsites in the bundle header.)\n\nIf you want to go that route (which is arguably more elegant anyway), I \nsuggest having a look at the merge_bases() and get_merge_bases() functions \nin commit.c, which do something similar (i.e. a revwalk without using \nrevision.c's functions -- because you cannot tell what flags they will use \nin the future, and they have to be reset after the walk).\n\nCiao,\nDscho\n"},{"id":"139869","messageId":"20100419071449.GA23166@progeny.tock","threadId":"22278","inReplyTo":"20100119002641.GA31434@gnu.kitenet.net","subject":"[PATCH 0/2] test bundle --stdin, fix objects_array_remove_duplicates()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-19T07:14:49Z","receivedAt":"2010-04-19T07:14:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This series does not make ‘git bundle --stdin’ work. In fact,\nthe bug that it does fix would not be visible if bundle --stdin\nworked as it was supposed to.\n\nInstead, this fixes a segfault that the bug triggers. The\npatches have been sitting in my git tree for a while. They fix\na real problem regardless, and maybe they would help someone\nto get bundle --stdin working properly.\n\nThoughts welcome, as always.\nJonathan Nieder (2):\n t5704 (bundle): add tests for bundle --stdin\n fix \"bundle --stdin\" segfault\n\n object.c | 4 ++--\n t/t5704-bundle.sh | 16 ++++++++++++++++\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\nAs for how to fix bundle --stdin, Joey Hess wrote:\n\n> I also tried going back to 22568f0a336ac37ae7329c917857b455839d1d09, but\n> still see a bug with Adam Brewster's initial code to add --stdin to\n> git-bundle. That code still tries to read stdin twice. If it sees\n> \"master\" both times, it does create a bundle.\n\nand Johannes Schindelin suggested slurping up the input and explicitly\nusing it twice, or:\n\n> Alternatively, you can try to implement the rev-list --boundary by\n> hand (the --pretty=oneline is only needed to get a boundary marker\n> IIRC), taking care to reset the commit flags that were set in the\n> process.\n[...]\n> If you want to go that route (which is arguably more elegant anyway),\n> I suggest having a look at the merge_bases() and get_merge_bases()\n> functions in commit.c\n\nDscho’s full message is pretty helpful:\nhttp://thread.gmane.org/gmane.comp.version-control.git/137414/focus=137503\n"},{"id":"139875","messageId":"20100419080303.GA23609@progeny.tock","threadId":"22278","inReplyTo":"20100419071449.GA23166@progeny.tock","subject":"[PATCH 1/2] t5704 (bundle): add tests for bundle --stdin","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-19T08:03:03Z","receivedAt":"2010-04-19T08:03:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As long as no rev-list arguments are supplied on the command line,\ngit bundle create --stdin currently segfaults. With added rev-list\narguments, it does not segfault, but the revisions from stdin are\nignored.\n\nThanks to Joey Hess for the report.\n\nSigned-off-by: Jonathan Nieder \n---\n t/t5704-bundle.sh | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex a8f4419..1154a4e 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -30,4 +30,20 @@ test_expect_success 'tags can be excluded by rev-list options' '\n \n '\n \n+test_expect_failure 'bundle --stdin' '\n+\n+\techo master | git bundle create stdin-bundle.bdl --stdin &&\n+\tgit ls-remote stdin-bundle.bdl > output &&\n+\tgrep master output\n+\n+'\n+\n+test_expect_failure 'bundle --stdin ' '\n+\n+\techo master | git bundle create hybrid-bundle.bdl --stdin tag &&\n+\tgit ls-remote hybrid-bundle.bdl > output &&\n+\tgrep master output\n+\n+'\n+\n test_done\n-- \n1.7.1.rc1\n"},{"id":"139876","messageId":"20100419080340.GB23609@progeny.tock","threadId":"22278","inReplyTo":"20100419071449.GA23166@progeny.tock","subject":"[PATCH 2/2] fix \"bundle --stdin\" segfault","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-19T08:03:40Z","receivedAt":"2010-04-19T08:03:40Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When passed an empty list, objects_array_remove_duplicates() corrupts\nit by changing the number of entries from 0 to 1.\n\nThe problem lies in the condition of its main loop:\n\n\tfor (ref = 0; ref < array->nr - 1; ref++) {\n\nThe loop body manipulates the supplied object array. In the case of\nan empty array, it should not be doing anything at all. But array->nr\nis an unsigned quantity, so the code enters the loop, in particular\nincreasing array->nr. Fix this by comparing (ref + 1 < array->nr)\ninstead.\n\nThis bug can be triggered by git bundle --stdin:\n\n\t$ echo HEAD | git bundle create some.bundle --stdin’\n\tSegmentation fault (core dumped)\n\nThe list of commits to bundle appears to be empty because of another\nbug: by the time the revision-walking machinery gets to look at it,\nstandard input has already been consumed by rev-list, so\n...remove_duplicates() gets an empty list of revisions.\n\nAfter this patch, git bundle --stdin still does not work; it just\ndoesn’t segfault any more.\n\nReported-by: Joey Hess \nSigned-off-by: Jonathan Nieder \n---\n object.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/object.c b/object.c\nindex 3ca92c4..277b3dd 100644\n--- a/object.c\n+++ b/object.c\n@@ -252,10 +252,10 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj\n \n void object_array_remove_duplicates(struct object_array *array)\n {\n-\tint ref, src, dst;\n+\tunsigned int ref, src, dst;\n \tstruct object_array_entry *objects = array->objects;\n \n-\tfor (ref = 0; ref < array->nr - 1; ref++) {\n+\tfor (ref = 0; ref + 1 < array->nr; ref++) {\n \t\tfor (src = ref + 1, dst = src;\n \t\t src < array->nr;\n \t\t src++) {\n-- \n1.7.1.rc1\n"},{"id":"139950","messageId":"7vd3xuln3s.fsf@alter.siamese.dyndns.org","threadId":"22278","inReplyTo":"20100419071449.GA23166@progeny.tock","subject":"Re: [PATCH 0/2] test bundle --stdin, fix objects_array_remove_duplicates()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-20T05:16:55Z","receivedAt":"2010-04-20T05:16:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue.\n"},{"id":"144286","messageId":"20100626061735.GA15881@burratino","threadId":"22278","inReplyTo":"20100119002641.GA31434@gnu.kitenet.net","subject":"[RFC/PATCH 0/8] Re: bug: git-bundle create foo --stdin -> segfault","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:17:35Z","receivedAt":"2010-06-26T06:17:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Joey Hess wrote:\n\n> I noticed that git bundle --stdin actually attempts to read from stdin\n> past EOF.\n\nPatch 5 below fixes that. Sadly it is not the whole story and I had\nto introduce a memory leak for all users of --stdin to make\n‘bundle --stdin’ actually work (patch 6).\n\nIn the case of bundle --stdin, I think the leak is unavoidable, while\nin general, I suspect a parameter is needed to allow the caller to\nindicate whether the leak is needed (analogous to save_commit_buffer).\n\nThe rest of the patches should be comparatively pleasant.\n\nPatches 1, 3, and 4 give various parts of bundle creation their own\nfunctions, to make the code easier to digest;\n\nPatch 2 teaches the basis discovery code (which currently forks a\nrev-list --boundary process) to use the revision walker directly, to\nprepare for patch 5.\n\nPatch 5 looked like it would be the main point of the series: it saves\nthe list of revs including those read from stdin and resets the\nrevision walker after the boundary is found. This way, stdin is only\nread once and the underlying logic of the revision walk does not need\nto be changed significantly.\n\nEven with patch 5, bundle --stdin still finds no revisions to read.\nThe problem: to write the table of contents, the name passed on the\ncommand line for each ref is needed. Unfortunately, names passed\nthrough stdin are kept in a temporary buffer and then freed; the\nrandom gibberish read instead does not look like a meaningful ref\nname, so no valid toc entries are found.\n\nPatch 6 fixes this, at the cost of a memory leak. Suggestions for\navoiding the leak would of course be welcome.\n\nPatch 7 adopts a similar “fix” in a context where it is not needed.\nThis is just for illustration.\n\nPatch 8 is an unrelated enhancement that came along the way.\n\nThanks to Joey for the report and Johannes for suggestions for fixing\nit.\n\nThoughts? Tests to add?\nJonathan Nieder (8):\n bundle: split basis discovery into its own function\n bundle: use libified rev-list --boundary\n bundle: split body of list_prerequisites() loop into its own function\n bundle: split table of contents output into its own function\n bundle: reuse setup_revisions result\n revision: Keep ref names after reading them from stdin\n bundle: Keep names of basis refs after discovery\n bundle_create: Do not exit when given no revs to bundle\n\n bundle.c | 172 ++++++++++++++++++++++++++++++++++-------------------\n revision.c | 2 +-\n t/t5704-bundle.sh | 4 +-\n 3 files changed, 113 insertions(+), 65 deletions(-)\n"},{"id":"144287","messageId":"20100626061904.GB15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[PATCH 1/8] bundle: split basis discovery into its own function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:19:04Z","receivedAt":"2010-06-26T06:19:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"create_bundle() is getting a little long. Isolate a small\npiece to work on without distractions.\n\nSigned-off-by: Jonathan Nieder \n---\n bundle.c | 58 ++++++++++++++++++++++++++++++++++------------------------\n 1 files changed, 34 insertions(+), 24 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex ff97adc..66948f4 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -193,35 +193,14 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n \t\t(revs->min_age == -1 || revs->min_age > date);\n }\n \n-int create_bundle(struct bundle_header *header, const char *path,\n-\t\tint argc, const char **argv)\n+static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n+\t\tint argc, const char * const *argv)\n {\n-\tstatic struct lock_file lock;\n-\tint bundle_fd = -1;\n-\tint bundle_to_stdout;\n \tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n-\tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n-\tint i, ref_count = 0;\n \tchar buffer[1024];\n-\tstruct rev_info revs;\n \tstruct child_process rls;\n \tFILE *rls_fout;\n \n-\tbundle_to_stdout = !strcmp(path, \"-\");\n-\tif (bundle_to_stdout)\n-\t\tbundle_fd = 1;\n-\telse\n-\t\tbundle_fd = hold_lock_file_for_update(&lock, path,\n-\t\t\t\t\t\t LOCK_DIE_ON_ERROR);\n-\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 \targv_boundary[1] = \"--boundary\";\n@@ -241,7 +220,7 @@ int create_bundle(struct bundle_header *header, const char *path,\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\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@@ -251,6 +230,37 @@ int create_bundle(struct bundle_header *header, const char *path,\n \tfclose(rls_fout);\n \tif (finish_command(&rls))\n \t\treturn error(\"rev-list died\");\n+\treturn 0;\n+}\n+\n+int create_bundle(struct bundle_header *header, const char *path,\n+\t\tint argc, const char **argv)\n+{\n+\tstatic struct lock_file lock;\n+\tint bundle_fd = -1;\n+\tint bundle_to_stdout;\n+\tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n+\tint i, ref_count = 0;\n+\tstruct rev_info revs;\n+\tstruct child_process rls;\n+\n+\tbundle_to_stdout = !strcmp(path, \"-\");\n+\tif (bundle_to_stdout)\n+\t\tbundle_fd = 1;\n+\telse\n+\t\tbundle_fd = hold_lock_file_for_update(&lock, path,\n+\t\t\t\t\t\t LOCK_DIE_ON_ERROR);\n+\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+\tif (list_prerequisites(bundle_fd, &revs, argc, argv))\n+\t\treturn -1;\n \n \t/* write references */\n \targc = setup_revisions(argc, argv, &revs, NULL);\n-- \n1.7.1.198.g8d802\n"},{"id":"144288","messageId":"20100626062005.GC15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[PATCH 2/8] bundle: use libified rev-list --boundary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:20:05Z","receivedAt":"2010-06-26T06:20:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The revision walker produces structured output, which should be a\nlittle easier to work with than the text from rev-list.\n\nSigned-off-by: Jonathan Nieder \n---\n bundle.c | 69 ++++++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 38 insertions(+), 31 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 66948f4..0dd2acb 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -196,40 +196,47 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n \t\tint argc, const char * const *argv)\n {\n-\tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n-\tchar buffer[1024];\n-\tstruct child_process rls;\n-\tFILE *rls_fout;\n+\tconst char **argv_boundary = xmalloc((argc + 1) * sizeof(const char *));\n+\tstruct rev_info boundary_revs;\n+\tstruct commit *rev;\n \n-\tmemcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));\n-\targv_boundary[0] = \"rev-list\";\n-\targv_boundary[1] = \"--boundary\";\n-\targv_boundary[2] = \"--pretty=oneline\";\n-\targv_boundary[argc + 2] = NULL;\n-\tmemset(&rls, 0, sizeof(rls));\n-\trls.argv = argv_boundary;\n-\trls.out = -1;\n-\trls.git_cmd = 1;\n-\tif (start_command(&rls))\n-\t\treturn -1;\n-\trls_fout = xfdopen(rls.out, \"r\");\n-\twhile (fgets(buffer, sizeof(buffer), rls_fout)) {\n-\t\tunsigned char sha1[20];\n-\t\tif (buffer[0] == '-') {\n-\t\t\twrite_or_die(bundle_fd, buffer, strlen(buffer));\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+\tmemcpy(argv_boundary, argv, (argc + 1) * sizeof(const char *));\n+\n+\tinit_revisions(&boundary_revs, NULL);\n+\tboundary_revs.boundary = 1;\n+\tif (setup_revisions(argc, argv_boundary, &boundary_revs, NULL) > 1)\n+\t\treturn error(\"unrecognized argument: %s\", argv_boundary[1]);\n+\tif (prepare_revision_walk(&boundary_revs))\n+\t\treturn error(\"revision walk setup failed\");\n+\n+\twhile ((rev = get_revision(&boundary_revs))) {\n+\t\tif (rev->object.flags & BOUNDARY) {\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\tstruct pretty_print_context ctx = {0};\n+\t\t\tenum object_type type;\n+\t\t\tunsigned long size;\n+\n+\t\t\t/*\n+\t\t\t * The commit buffer is needed\n+\t\t\t * to pretty-print boundary commits.\n+\t\t\t */\n+\t\t\trev->buffer = read_sha1_file(rev->object.sha1,\n+\t\t\t\t\t\t\t&type, &size);\n+\n+\t\t\tstrbuf_addch(&buf, '-');\n+\t\t\tstrbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);\n+\t\t\tstrbuf_addch(&buf, ' ');\n+\t\t\tpretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);\n+\t\t\tstrbuf_addch(&buf, '\\n');\n+\t\t\twrite_or_die(bundle_fd, buf.buf, buf.len);\n+\n+\t\t\trev->object.flags |= UNINTERESTING;\n+\t\t\tadd_pending_object(revs, &rev->object, buf.buf);\n+\t\t\tstrbuf_release(&buf);\n+\t\t} else {\n+\t\t\trev->object.flags |= SHOWN;\n \t\t}\n \t}\n-\tfclose(rls_fout);\n-\tif (finish_command(&rls))\n-\t\treturn error(\"rev-list died\");\n \treturn 0;\n }\n \n-- \n1.7.1.198.g8d802\n"},{"id":"144289","messageId":"20100626062047.GD15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[PATCH 3/8] bundle: give list_prerequisites() loop body its own function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:20:47Z","receivedAt":"2010-06-26T06:20:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"No functional change intended.\n\nSigned-off-by: Jonathan Nieder \n---\n bundle.c | 57 +++++++++++++++++++++++++++++++--------------------------\n 1 files changed, 31 insertions(+), 26 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 0dd2acb..e90b5c5 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -193,6 +193,33 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n \t\t(revs->min_age == -1 || revs->min_age > date);\n }\n \n+static void list_prerequisite(int bundle_fd, struct rev_info *revs,\n+\t\tstruct commit *rev)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct pretty_print_context ctx = {0};\n+\tenum object_type type;\n+\tunsigned long size;\n+\n+\t/*\n+\t * The commit buffer is needed\n+\t * to pretty-print boundary commits.\n+\t */\n+\trev->buffer = read_sha1_file(rev->object.sha1, &type, &size);\n+\n+\tstrbuf_addch(&buf, '-');\n+\tstrbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);\n+\tstrbuf_addch(&buf, ' ');\n+\tpretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);\n+\tstrbuf_addch(&buf, '\\n');\n+\n+\twrite_or_die(bundle_fd, buf.buf, buf.len);\n+\n+\trev->object.flags |= UNINTERESTING;\n+\tadd_pending_object(revs, &rev->object, buf.buf);\n+\tstrbuf_release(&buf);\n+}\n+\n static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n \t\tint argc, const char * const *argv)\n {\n@@ -209,33 +236,11 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n \tif (prepare_revision_walk(&boundary_revs))\n \t\treturn error(\"revision walk setup failed\");\n \n-\twhile ((rev = get_revision(&boundary_revs))) {\n-\t\tif (rev->object.flags & BOUNDARY) {\n-\t\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\t\tstruct pretty_print_context ctx = {0};\n-\t\t\tenum object_type type;\n-\t\t\tunsigned long size;\n-\n-\t\t\t/*\n-\t\t\t * The commit buffer is needed\n-\t\t\t * to pretty-print boundary commits.\n-\t\t\t */\n-\t\t\trev->buffer = read_sha1_file(rev->object.sha1,\n-\t\t\t\t\t\t\t&type, &size);\n-\n-\t\t\tstrbuf_addch(&buf, '-');\n-\t\t\tstrbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);\n-\t\t\tstrbuf_addch(&buf, ' ');\n-\t\t\tpretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);\n-\t\t\tstrbuf_addch(&buf, '\\n');\n-\t\t\twrite_or_die(bundle_fd, buf.buf, buf.len);\n-\n-\t\t\trev->object.flags |= UNINTERESTING;\n-\t\t\tadd_pending_object(revs, &rev->object, buf.buf);\n-\t\t\tstrbuf_release(&buf);\n-\t\t} else {\n+\twhile ((rev = get_revision(revs))) {\n+\t\tif (rev->object.flags & BOUNDARY)\n+\t\t\tlist_prerequisite(bundle_fd, revs, rev);\n+\t\telse\n \t\t\trev->object.flags |= SHOWN;\n-\t\t}\n \t}\n \treturn 0;\n }\n-- \n1.7.1.198.g8d802\n"},{"id":"144290","messageId":"20100626062116.GE15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[PATCH 4/8] bundle: split table of contents output into its own function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:21:16Z","receivedAt":"2010-06-26T06:21:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Isolate another piece of create_bundle() to work on.\n\nSigned-off-by: Jonathan Nieder \n---\n bundle.c | 78 +++++++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 44 insertions(+), 34 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex e90b5c5..8ba6479 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -245,45 +245,20 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n \treturn 0;\n }\n \n-int create_bundle(struct bundle_header *header, const char *path,\n-\t\tint argc, const char **argv)\n+static int bundle_list_refs(int bundle_fd,\n+\t\tint argc, const char **argv, struct rev_info *revs)\n {\n-\tstatic struct lock_file lock;\n-\tint bundle_fd = -1;\n-\tint bundle_to_stdout;\n-\tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n \tint i, ref_count = 0;\n-\tstruct rev_info revs;\n-\tstruct child_process rls;\n \n-\tbundle_to_stdout = !strcmp(path, \"-\");\n-\tif (bundle_to_stdout)\n-\t\tbundle_fd = 1;\n-\telse\n-\t\tbundle_fd = hold_lock_file_for_update(&lock, path,\n-\t\t\t\t\t\t LOCK_DIE_ON_ERROR);\n-\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-\tif (list_prerequisites(bundle_fd, &revs, argc, argv))\n-\t\treturn -1;\n-\n-\t/* write references */\n-\targc = setup_revisions(argc, argv, &revs, NULL);\n+\targc = setup_revisions(argc, argv, revs, NULL);\n \n \tif (argc > 1)\n \t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n \n-\tobject_array_remove_duplicates(&revs.pending);\n+\tobject_array_remove_duplicates(&revs->pending);\n \n-\tfor (i = 0; i < revs.pending.nr; i++) {\n-\t\tstruct object_array_entry *e = revs.pending.objects + i;\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 \t\tchar *ref;\n \t\tconst char *display_ref;\n@@ -298,7 +273,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\tdisplay_ref = (flag & REF_ISSYMREF) ? e->name : ref;\n \n \t\tif (e->item->type == OBJ_TAG &&\n-\t\t\t\t!is_tag_in_date_range(e->item, &revs)) {\n+\t\t\t\t!is_tag_in_date_range(e->item, revs)) {\n \t\t\te->item->flags |= UNINTERESTING;\n \t\t\tcontinue;\n \t\t}\n@@ -344,7 +319,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\t\t\t */\n \t\t\t\tobj = parse_object(sha1);\n \t\t\t\tobj->flags |= SHOWN;\n-\t\t\t\tadd_pending_object(&revs, obj, e->name);\n+\t\t\t\tadd_pending_object(revs, obj, e->name);\n \t\t\t}\n \t\t\tfree(ref);\n \t\t\tcontinue;\n@@ -358,7 +333,42 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\tfree(ref);\n \t}\n \tif (!ref_count)\n-\t\tdie (\"Refusing to create empty bundle.\");\n+\t\tdie(\"Refusing to create empty bundle.\");\n+\treturn 0;\n+}\n+\n+int create_bundle(struct bundle_header *header, const char *path,\n+\t\tint argc, const char **argv)\n+{\n+\tstatic struct lock_file lock;\n+\tint bundle_fd = -1;\n+\tint bundle_to_stdout;\n+\tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n+\tstruct rev_info revs;\n+\tstruct child_process rls;\n+\tint i;\n+\n+\tbundle_to_stdout = !strcmp(path, \"-\");\n+\tif (bundle_to_stdout)\n+\t\tbundle_fd = 1;\n+\telse\n+\t\tbundle_fd = hold_lock_file_for_update(&lock, path,\n+\t\t\t\t\t\t LOCK_DIE_ON_ERROR);\n+\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+\tif (list_prerequisites(bundle_fd, &revs, argc, argv))\n+\t\treturn -1;\n+\n+\t/* write references */\n+\tif (bundle_list_refs(bundle_fd, argc, argv, &revs))\n+\t\treturn -1;\n \n \t/* end header */\n \twrite_or_die(bundle_fd, \"\\n\", 1);\n-- \n1.7.1.198.g8d802\n"},{"id":"144291","messageId":"20100626062212.GF15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[PATCH 5/8] bundle: reuse setup_revisions result","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:22:12Z","receivedAt":"2010-06-26T06:22:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Avoid reading stdin twice for bundle --stdin.\n\nReported-by: Joey Hess \nSigned-off-by: Jonathan Nieder \n---\n bundle.c | 57 +++++++++++++++++++++++++++++++++++++--------------------\n 1 files changed, 37 insertions(+), 20 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 8ba6479..311c554 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -193,7 +193,17 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n \t\t(revs->min_age == -1 || revs->min_age > date);\n }\n \n-static void list_prerequisite(int bundle_fd, struct rev_info *revs,\n+static void object_array_copy(struct object_array *dest, const struct object_array *src)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < src->nr; i++) {\n+\t\tstruct object_array_entry *e = src->objects + i;\n+\t\tadd_object_array_with_mode(e->item, e->name, dest, e->mode);\n+\t}\n+}\n+\n+static void list_prerequisite(int bundle_fd, struct object_array *pending,\n \t\tstruct commit *rev)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -216,45 +226,52 @@ static void list_prerequisite(int bundle_fd, struct rev_info *revs,\n \twrite_or_die(bundle_fd, buf.buf, buf.len);\n \n \trev->object.flags |= UNINTERESTING;\n-\tadd_pending_object(revs, &rev->object, buf.buf);\n+\tadd_object_array(&rev->object, buf.buf, pending);\n \tstrbuf_release(&buf);\n }\n \n static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n-\t\tint argc, const char * const *argv)\n+\t\tint argc, const char **argv)\n {\n-\tconst char **argv_boundary = xmalloc((argc + 1) * sizeof(const char *));\n-\tstruct rev_info boundary_revs;\n \tstruct commit *rev;\n+\tstruct object_array pending_copy = { 0, 0, NULL };\n \n-\tmemcpy(argv_boundary, argv, (argc + 1) * sizeof(const char *));\n+\trevs->simplify_history = 0;\n+\targc = setup_revisions(argc, argv, revs, NULL);\n+\tif (argc > 1)\n+\t\treturn error(\"unrecognized argument: %s\", argv[1]);\n \n-\tinit_revisions(&boundary_revs, NULL);\n-\tboundary_revs.boundary = 1;\n-\tif (setup_revisions(argc, argv_boundary, &boundary_revs, NULL) > 1)\n-\t\treturn error(\"unrecognized argument: %s\", argv_boundary[1]);\n-\tif (prepare_revision_walk(&boundary_revs))\n+\tif (revs->reflog_info)\n+\t\treturn error(\"bundle does not support --walk-reflogs\");\n+\tif (revs->no_walk)\n+\t\treturn error(\"bundle does not support --no-walk\");\n+\tif (revs->simplify_history)\n+\t\treturn error(\"bundle and history simplification do not mix\");\n+\n+\tobject_array_copy(&pending_copy, &revs->pending);\n+\n+\trevs->boundary = 1;\n+\tif (prepare_revision_walk(revs))\n \t\treturn error(\"revision walk setup failed\");\n \n \twhile ((rev = get_revision(revs))) {\n \t\tif (rev->object.flags & BOUNDARY)\n-\t\t\tlist_prerequisite(bundle_fd, revs, rev);\n+\t\t\tlist_prerequisite(bundle_fd, &pending_copy, rev);\n \t\telse\n \t\t\trev->object.flags |= SHOWN;\n \t}\n+\n+\tif (revs->pending.objects != NULL)\n+\t\treturn error(\"%u revisions pending after boundary walk\",\n+\t\t\t\t\t\trevs->pending.nr);\n+\trevs->pending = pending_copy;\n \treturn 0;\n }\n \n-static int bundle_list_refs(int bundle_fd,\n-\t\tint argc, const char **argv, struct rev_info *revs)\n+static int bundle_list_refs(int bundle_fd, struct rev_info *revs)\n {\n \tint i, ref_count = 0;\n \n-\targc = setup_revisions(argc, argv, revs, NULL);\n-\n-\tif (argc > 1)\n-\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n-\n \tobject_array_remove_duplicates(&revs->pending);\n \n \tfor (i = 0; i < revs->pending.nr; i++) {\n@@ -367,7 +384,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\treturn -1;\n \n \t/* write references */\n-\tif (bundle_list_refs(bundle_fd, argc, argv, &revs))\n+\tif (bundle_list_refs(bundle_fd, &revs))\n \t\treturn -1;\n \n \t/* end header */\n-- \n1.7.1.198.g8d802\n"},{"id":"144292","messageId":"20100626062811.GG15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[PATCH 6/8] Fix bundle --stdin","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:28:11Z","receivedAt":"2010-06-26T06:28:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"To write a bundle’s table of contents, the name passed on the command\nline for each ref is needed. Unfortunately, names passed through\nstdin are put in a temporary buffer and then freed; the random\ngibberish that tends to replace them does not look like a meaningful\nref name to bundle_list_refs(), so no valid toc entries are found and\n‘git bundle --stdin’ thinks it has been asked to create an empty\nbundle.\n\nSo teach the revision walker to keep rev names after reading them from\nstdin. This fixes ‘git bundle --stdin’ at the cost of a memory leak.\n\nReported-by: Joey Hess \nSigned-off-by: Jonathan Nieder \n---\n revision.c | 2 +-\n t/t5704-bundle.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex f4b8b38..cf86af3 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1022,7 +1022,7 @@ static void read_revisions_from_stdin(struct rev_info *revs, const char ***prune\n \t\t\t}\n \t\t\tdie(\"options not supported in --stdin mode\");\n \t\t}\n-\t\tif (handle_revision_arg(sb.buf, revs, 0, 1))\n+\t\tif (handle_revision_arg(xstrdup(sb.buf), revs, 0, 1))\n \t\t\tdie(\"bad revision '%s'\", sb.buf);\n \t}\n \tif (seen_dashdash)\ndiff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh\nindex ddc3dc5..cc463f3 100755\n--- a/t/t5704-bundle.sh\n+++ b/t/t5704-bundle.sh\n@@ -30,7 +30,7 @@ test_expect_success 'tags can be excluded by rev-list options' '\n \n '\n \n-test_expect_failure 'bundle --stdin' '\n+test_expect_success 'bundle --stdin' '\n \n \techo master | git bundle create stdin-bundle.bdl --stdin &&\n \tgit ls-remote stdin-bundle.bdl >output &&\n@@ -38,7 +38,7 @@ test_expect_failure 'bundle --stdin' '\n \n '\n \n-test_expect_failure 'bundle --stdin ' '\n+test_expect_success 'bundle --stdin ' '\n \n \techo master | git bundle create hybrid-bundle.bdl --stdin tag &&\n \tgit ls-remote hybrid-bundle.bdl >output &&\n-- \n1.7.1.198.g8d802\n"},{"id":"144293","messageId":"20100626062918.GH15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[RFC/PATCH 7/8] bundle: Keep names of basis refs after discovery","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:29:18Z","receivedAt":"2010-06-26T06:29:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Without this change, attempts to examine revs->pending.objects[i].name\nwhen debugging produce random giberish. On the other hand, it\nintroduces a small per-basis-ref memory leak.\n\nSigned-off-by: Jonathan Nieder \n---\nFor illustration.\n\n bundle.c | 3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 311c554..7aff369 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -226,8 +226,7 @@ static void list_prerequisite(int bundle_fd, struct object_array *pending,\n \twrite_or_die(bundle_fd, buf.buf, buf.len);\n \n \trev->object.flags |= UNINTERESTING;\n-\tadd_object_array(&rev->object, buf.buf, pending);\n-\tstrbuf_release(&buf);\n+\tadd_object_array(&rev->object, strbuf_detach(&buf, NULL), pending);\n }\n \n static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n-- \n1.7.1.198.g8d802\n"},{"id":"144294","messageId":"20100626063158.GI15881@burratino","threadId":"22278","inReplyTo":"20100626061735.GA15881@burratino","subject":"[PATCH 8/8] bundle_create: Do not exit when given no revs to bundle","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T06:31:58Z","receivedAt":"2010-06-26T06:31:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Return an error instead to help with the libification effort.\n\nSigned-off-by: Jonathan Nieder \n---\nThat’s the end of the series. Thanks for reading.\n\nI was a bit torn about whether to present this as a request for\ncomment or a patch series ready for application. On one hand it fixes\na bug; on the other hand, I have very little confidence that it works\nwell in the presence of arbitrary rev-list options. Thoughts and\ntesting would be very welcome.\n\nGood night,\nJonathan\n\n bundle.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 7aff369..7dd3f65 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -349,7 +349,7 @@ static int bundle_list_refs(int bundle_fd, struct rev_info *revs)\n \t\tfree(ref);\n \t}\n \tif (!ref_count)\n-\t\tdie(\"Refusing to create empty bundle.\");\n+\t\treturn error(\"Refusing to create empty bundle.\");\n \treturn 0;\n }\n \n-- \n1.7.1.198.g8d802\n"},{"id":"144538","messageId":"7vtyok5rpl.fsf@alter.siamese.dyndns.org","threadId":"22278","inReplyTo":"20100626062005.GC15881@burratino","subject":"Re: [PATCH 2/8] bundle: use libified rev-list --boundary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-30T17:57:10Z","receivedAt":"2010-06-30T17:57:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> The revision walker produces structured output, which should be a\n> little easier to work with than the text from rev-list.\n\nHmm, doesn't it negatively affect later traversal you would need to do if\nyou smudged the flag bits by running revision traversal like this?\n"},{"id":"144539","messageId":"7vmxuc5rcv.fsf@alter.siamese.dyndns.org","threadId":"22278","inReplyTo":"20100626062047.GD15881@burratino","subject":"Re: [PATCH 3/8] bundle: give list_prerequisites() loop body its own function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-30T18:04:48Z","receivedAt":"2010-06-30T18:04:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> No functional change intended.\n>\n> Signed-off-by: Jonathan Nieder \n> ---\n> bundle.c | 57 +++++++++++++++++++++++++++++++--------------------------\n> 1 files changed, 31 insertions(+), 26 deletions(-)\n>\n> diff --git a/bundle.c b/bundle.c\n> index 0dd2acb..e90b5c5 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -193,6 +193,33 @@ static int is_tag_in_date_range(struct object *tag, struct rev_info *revs)\n> \t\t(revs->min_age == -1 || revs->min_age > date);\n> }\n> \n> +static void list_prerequisite(int bundle_fd, struct rev_info *revs,\n> +\t\tstruct commit *rev)\n> +{\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tstruct pretty_print_context ctx = {0};\n> +\tenum object_type type;\n> +\tunsigned long size;\n> +\n> +\t/*\n> +\t * The commit buffer is needed\n> +\t * to pretty-print boundary commits.\n> +\t */\n> +\trev->buffer = read_sha1_file(rev->object.sha1, &type, &size);\n> +\n> +\tstrbuf_addch(&buf, '-');\n> +\tstrbuf_add(&buf, sha1_to_hex(rev->object.sha1), 40);\n> +\tstrbuf_addch(&buf, ' ');\n> +\tpretty_print_commit(CMIT_FMT_ONELINE, rev, &buf, &ctx);\n> +\tstrbuf_addch(&buf, '\\n');\n> +\n> +\twrite_or_die(bundle_fd, buf.buf, buf.len);\n> +\n> +\trev->object.flags |= UNINTERESTING;\n> +\tadd_pending_object(revs, &rev->object, buf.buf);\n> +\tstrbuf_release(&buf);\n> +}\n> +\n> static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n> \t\tint argc, const char * const *argv)\n> {\n> @@ -209,33 +236,11 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n> \tif (prepare_revision_walk(&boundary_revs))\n> \t\treturn error(\"revision walk setup failed\");\n> \n> -\twhile ((rev = get_revision(&boundary_revs))) {\n> -\t\tif (rev->object.flags & BOUNDARY) {\n> -...\n> -\t\t} else {\n> +\twhile ((rev = get_revision(revs))) {\n> +\t\tif (rev->object.flags & BOUNDARY)\n> +\t\t\tlist_prerequisite(bundle_fd, revs, rev);\n> +\t\telse\n> \t\t\trev->object.flags |= SHOWN;\n> -\t\t}\n\nYou used to walk boundary_revs but now you walk revs that is given by the\ncaller, exhausting the revs.pending the caller wanted to use later to feed\npack_objects with?\n\nConfused...\n"},{"id":"144548","messageId":"20100630203451.GC23400@burratino","threadId":"22278","inReplyTo":"7vtyok5rpl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/8] bundle: use libified rev-list --boundary","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-30T20:34:51Z","receivedAt":"2010-06-30T20:34:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> The revision walker produces structured output, which should be a\n>> little easier to work with than the text from rev-list.\n>\n> Hmm, doesn't it negatively affect later traversal you would need to do if\n> you smudged the flag bits by running revision traversal like this?\n\nI imagine so. I fear this would be the first git command to use the\nrevision walker twice, and I am not sure whether we can really make that\nwork.\n\nThe revision walker uses object flags for the following purposes:\n\n - marking objects uninteresting/SYMMETRIC_LEFT. Luckily for us, if\n an object is uninteresting or SYMMETRIC_LEFT for the first\n --boundary walk, it will be likewise for pack-objects, too.\n\n - history simplification (TREESAME), --cherry-pick (SHOWN),\n --merge (SYMMETRIC_LEFT). There are already other reasons to\n disallow these features for bundle.\n\n - add_parents_to_list (ADDED, SEEN). This one is really worrisome;\n should we walk through again to throw away the added parents?\n Should there be a pass through all revisions to clear the ADDED\n bit?\n\nI’ll figure out a one-pass solution. :(\n\nJonathan\n"},{"id":"144551","messageId":"20100630203759.GD23400@burratino","threadId":"22278","inReplyTo":"7vmxuc5rcv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/8] bundle: give list_prerequisites() loop body its own function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-30T20:37:59Z","receivedAt":"2010-06-30T20:37:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>\n>> @@ -209,33 +236,11 @@ static int list_prerequisites(int bundle_fd, struct rev_info *revs,\n>> \tif (prepare_revision_walk(&boundary_revs))\n>> \t\treturn error(\"revision walk setup failed\");\n>> \n>> -\twhile ((rev = get_revision(&boundary_revs))) {\n>> -\t\tif (rev->object.flags & BOUNDARY) {\n>> -...\n>> -\t\t} else {\n>> +\twhile ((rev = get_revision(revs))) {\n>> +\t\tif (rev->object.flags & BOUNDARY)\n>> +\t\t\tlist_prerequisite(bundle_fd, revs, rev);\n>> +\t\telse\n>> \t\t\trev->object.flags |= SHOWN;\n>> -\t\t}\n>\n> You used to walk boundary_revs but now you walk revs that is given by the\n> caller\n\nAgh! Typo. Thanks for catching it.\n"}]}