{"thread":{"id":"30126","subject":"[PATCH v3 0/4] Clone fails on a repo with too many heads/tags","startedAt":"2012-04-02T15:11:17Z","lastAt":"2012-04-03T00:23:32Z","messageCount":9,"participants":["Ivan Todoroski","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"188308","messageId":"4F79C195.6090905@gmx.net","threadId":"30126","inReplyTo":null,"subject":"[PATCH v3 0/4] Clone fails on a repo with too many heads/tags","fromName":"Ivan Todoroski","fromEmail":"grnch@gmx.net","sentAt":"2012-04-02T15:11:17Z","receivedAt":"2012-04-02T15:11:17Z","isPatch":true,"sender":{"key":"grnch@gmx.net","avatar":"https://avatars.githubusercontent.com/u/90292?v=4"},"body":"The full description of the problem can be found in the first patch.\n\n\nChanges since v2:\n\n* fix a bunch of code style and documentation issues spotted by Junio\n* tighten ref format checking on stdin to not allow extra whitespace\n* make fetch-pack --stdin tests independent of the order of refs\n* add test for duplicate refs on stdin\n* drop the two ugly and redundant --stateless-rpc tests\n* drop the test that tolerated extra whitespace\n\n\nChanges since the original patch:\n\n* add test cases\n* add full commit messages\n* fix formatting problem in --stdin doc\n* split overly long fetch_pack_usage line\n* use strbuf_getline() instead of fgets() for reading refs from stdin\n* minor optimization of the pkt-line reading loop, it was using xstrdup()\n even though the string length was already known, use xmemdupz() instead\n* rework the remote-curl.c patch to not add new parameters to rpc_service(),\n instead add a new strbuf member to rpc_state to pass the info around\n\nIvan Todoroski (4):\n fetch-pack: new --stdin option to read refs from stdin\n remote-curl: send the refs to fetch-pack on stdin\n fetch-pack: test cases for the new --stdin option\n remote-curl: main test case for the OS command line overflow\n\n Documentation/git-fetch-pack.txt | 10 ++++++\n builtin/fetch-pack.c | 42 +++++++++++++++++++++++-\n fetch-pack.h | 1 +\n remote-curl.c | 14 ++++++--\n t/t5500-fetch-pack.sh | 66 ++++++++++++++++++++++++++++++++++++++\n t/t5551-http-fetch.sh | 31 ++++++++++++++++++\n 6 files changed, 161 insertions(+), 3 deletions(-)\n\n-- \n1.7.9.5.4.g4f508\n"},{"id":"188310","messageId":"4F79C22C.9080702@gmx.net","threadId":"30126","inReplyTo":"4F79C195.6090905@gmx.net","subject":"[PATCH v3 1/4] fetch-pack: new --stdin option to read refs from stdin","fromName":"Ivan Todoroski","fromEmail":"grnch@gmx.net","sentAt":"2012-04-02T15:13:48Z","receivedAt":"2012-04-02T15:13:48Z","isPatch":true,"sender":{"key":"grnch@gmx.net","avatar":"https://avatars.githubusercontent.com/u/90292?v=4"},"body":"If a remote repo has too many tags (or branches), cloning it over the\nsmart HTTP transport can fail because remote-curl.c puts all the refs\nfrom the remote repo on the fetch-pack command line. This can make the\ncommand line longer than the global OS command line limit, causing\nfetch-pack to fail.\n\nThis is especially a problem on Windows where the command line limit is\norders of magnitude shorter than Linux. There are already real repos out\nthere that msysGit cannot clone over smart HTTP due to this problem.\n\nHere is an easy way to trigger this problem:\n\n\tgit init too-many-refs\n\tcd too-many-refs\n\techo bla > bla.txt\n\tgit add .\n\tgit commit -m test\n\tsha=$(git rev-parse HEAD)\n\ttag=$(perl -e 'print \"bla\" x 30')\n\tfor i in `seq 50000`; do\n\t\techo $sha refs/tags/$tag-$i >> .git/packed-refs\n\tdone\n\nThen share this repo over the smart HTTP protocol and try cloning it:\n\n\t$ git clone http://localhost/.../too-many-refs/.git\n\tCloning into 'too-many-refs'...\n\tfatal: cannot exec 'fetch-pack': Argument list too long\n\n50k tags is obviously an absurd number, but it is required to\ndemonstrate the problem on Linux because it has a much more generous\ncommand line limit. On Windows the clone fails with as little as 500\ntags in the above loop, which is getting uncomfortably close to the\nnumber of tags you might see in real long lived repos.\n\nThis is not just theoretical, msysGit is already failing to clone our\ncompany repo due to this. It's a large repo converted from CVS, nearly\n10 years of history.\n\nFour possible solutions were discussed on the Git mailing list (in no\nparticular order):\n\n1) Call fetch-pack multiple times with smaller batches of refs.\n\nThis was dismissed as inefficient and inelegant.\n\n2) Add option --refs-fd=$n to pass a an fd from where to read the refs.\n\nThis was rejected because inheriting descriptors other than\nstdin/stdout/stderr through exec() is apparently problematic on Windows,\nplus it would require changes to the run-command API to open extra\npipes.\n\n3) Add option --refs-from=$tmpfile to pass the refs using a temp file.\n\nThis was not favored because of the temp file requirement.\n\n4) Add option --stdin to pass the refs on stdin, one per line.\n\nIn the end this option was chosen as the most efficient and most\ndesirable from scripting perspective.\n\nThere was however a small complication when using stdin to pass refs to\nfetch-pack. The --stateless-rpc option to fetch-pack also uses stdin for\ncommunication with the remote server.\n\nIf we are going to sneak refs on stdin line by line, it would have to be\ndone very carefully in the presence of --stateless-rpc, because when\nreading refs line by line we might read ahead too much data into our\nbuffer and eat some of the remote protocol data which is also coming on\nstdin.\n\nOne way to solve this would be to refactor get_remote_heads() in\nfetch-pack.c to accept a residual buffer from our stdin line parsing\nabove, but this function is used in several places so other callers\nwould be burdened by this residual buffer interface even when most of\nthem don't need it.\n\nIn the end we settled on the following solution:\n\nIf --stdin is specified without --stateless-rpc, fetch-pack would read\nthe refs from stdin one per line, in a script friendly format.\n\nHowever if --stdin is specified together with --stateless-rpc,\nfetch-pack would read the refs from stdin in packetized format\n(pkt-line) with a flush packet terminating the list of refs. This way we\ncan read the exact number of bytes that we need from stdin, and then\nget_remote_heads() can continue reading from the same fd without losing\na single byte of remote protocol data.\n\nThis way the --stdin option only loses generality and scriptability when\nused together with --stateless-rpc, which is not easily scriptable\nanyway because it also uses pkt-line when talking to the remote server.\n\nSigned-off-by: Ivan Todoroski \n---\n Documentation/git-fetch-pack.txt | 10 +++++++++\n builtin/fetch-pack.c | 42 +++++++++++++++++++++++++++++++++++++-\n fetch-pack.h | 1 +\n 3 files changed, 52 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt\nindex ed1bdaacd1..474fa307a0 100644\n--- a/Documentation/git-fetch-pack.txt\n+++ b/Documentation/git-fetch-pack.txt\n@@ -32,6 +32,16 @@ OPTIONS\n --all::\n \tFetch all remote refs.\n \n+--stdin::\n+\tTake the list of refs from stdin, one per line. If there\n+\tare refs specified on the command line in addition to this\n+\toption, then the refs from stdin are processed after those\n+\ton the command line.\n++\n+If '--stateless-rpc' is specified together with this option then\n+the list of refs must be in packet format (pkt-line). Each ref must\n+be in a separate packet, and the list must end with a flush packet.\n+\n -q::\n --quiet::\n \tPass '-q' flag to 'git unpack-objects'; this makes the\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex a4d3e90a86..36bc83975a 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -23,7 +23,9 @@ static struct fetch_pack_args args = {\n };\n \n static const char fetch_pack_usage[] =\n-\"git fetch-pack [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag] [--upload-pack=] [--depth=] [--no-progress] [-v] [:] [...]\";\n+\"git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] \"\n+\"[--include-tag] [--upload-pack=] [--depth=] \"\n+\"[--no-progress] [-v] [:] [...]\";\n \n #define COMPLETE\t(1U << 0)\n #define COMMON\t\t(1U << 1)\n@@ -941,6 +943,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\targs.fetch_all = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(\"--stdin\", arg)) {\n+\t\t\t\targs.stdin_refs = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(\"-v\", arg)) {\n \t\t\t\targs.verbose = 1;\n \t\t\t\tcontinue;\n@@ -972,6 +978,40 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tif (!dest)\n \t\tusage(fetch_pack_usage);\n \n+\tif (args.stdin_refs) {\n+\t\t/* copy refs from cmdline to new growable list,\n+\t\t * then append the refs from stdin\n+\t\t */\n+\t\tint alloc_heads = nr_heads;\n+\t\tint size = nr_heads * sizeof(*heads);\n+\t\theads = memcpy(xmalloc(size), heads, size);\n+\t\tif (args.stateless_rpc) {\n+\t\t\t/* in stateless RPC mode we use pkt-line to read\n+\t\t\t * from stdin, until we get a flush packet\n+\t\t\t */\n+\t\t\tstatic char line[1000];\n+\t\t\tfor (;;) {\n+\t\t\t\tint n = packet_read_line(0, line, sizeof(line));\n+\t\t\t\tif (!n)\n+\t\t\t\t\tbreak;\n+\t\t\t\tif (line[n-1] == '\\n')\n+\t\t\t\t\tn--;\n+\t\t\t\tALLOC_GROW(heads, nr_heads + 1, alloc_heads);\n+\t\t\t\theads[nr_heads++] = xmemdupz(line, n);\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\t/* read from stdin one ref per line, until EOF */\n+\t\t\tstruct strbuf line;\n+\t\t\tstrbuf_init(&line, 0);\n+\t\t\twhile (strbuf_getline(&line, stdin, '\\n') != EOF) {\n+\t\t\t\tALLOC_GROW(heads, nr_heads + 1, alloc_heads);\n+\t\t\t\theads[nr_heads++] = strbuf_detach(&line, NULL);\n+\t\t\t}\n+\t\t\tstrbuf_release(&line);\n+\t\t}\n+\t}\n+\n \tif (args.stateless_rpc) {\n \t\tconn = NULL;\n \t\tfd[0] = 0;\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex 0608edae3f..7c2069c972 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -10,6 +10,7 @@ struct fetch_pack_args {\n \t\tlock_pack:1,\n \t\tuse_thin_pack:1,\n \t\tfetch_all:1,\n+\t\tstdin_refs:1,\n \t\tverbose:1,\n \t\tno_progress:1,\n \t\tinclude_tag:1,\n-- \n1.7.9.5.4.g4f508\n"},{"id":"188311","messageId":"4F79C264.1060508@gmx.net","threadId":"30126","inReplyTo":"4F79C195.6090905@gmx.net","subject":"[PATCH v3 2/4] remote-curl: send the refs to fetch-pack on stdin","fromName":"Ivan Todoroski","fromEmail":"grnch@gmx.net","sentAt":"2012-04-02T15:14:44Z","receivedAt":"2012-04-02T15:14:44Z","isPatch":true,"sender":{"key":"grnch@gmx.net","avatar":"https://avatars.githubusercontent.com/u/90292?v=4"},"body":"Now that we can throw an arbitrary number of refs at fetch-pack using\nits --stdin option, we use it in the remote-curl helper to bypass the\nOS command line length limit.\n\nSigned-off-by: Ivan Todoroski \n---\n remote-curl.c | 14 ++++++++++++--\n 1 file changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex d159fe7f34..a728edfa7f 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -290,6 +290,7 @@ static void output_refs(struct ref *refs)\n struct rpc_state {\n \tconst char *service_name;\n \tconst char **argv;\n+\tstruct strbuf *stdin_preamble;\n \tchar *service_url;\n \tchar *hdr_content_type;\n \tchar *hdr_accept;\n@@ -535,6 +536,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)\n {\n \tconst char *svc = rpc->service_name;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf *preamble = rpc->stdin_preamble;\n \tstruct child_process client;\n \tint err = 0;\n \n@@ -545,6 +547,8 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)\n \tclient.argv = rpc->argv;\n \tif (start_command(&client))\n \t\texit(1);\n+\tif (preamble)\n+\t\twrite_or_die(client.in, preamble->buf, preamble->len);\n \tif (heads)\n \t\twrite_or_die(client.in, heads->buf, heads->len);\n \n@@ -626,6 +630,7 @@ static int fetch_git(struct discovery *heads,\n \tint nr_heads, struct ref **to_fetch)\n {\n \tstruct rpc_state rpc;\n+\tstruct strbuf preamble = STRBUF_INIT;\n \tchar *depth_arg = NULL;\n \tconst char **argv;\n \tint argc = 0, i, err;\n@@ -633,6 +638,7 @@ static int fetch_git(struct discovery *heads,\n \targv = xmalloc((15 + nr_heads) * sizeof(char*));\n \targv[argc++] = \"fetch-pack\";\n \targv[argc++] = \"--stateless-rpc\";\n+\targv[argc++] = \"--stdin\";\n \targv[argc++] = \"--lock-pack\";\n \tif (options.followtags)\n \t\targv[argc++] = \"--include-tag\";\n@@ -651,23 +657,27 @@ static int fetch_git(struct discovery *heads,\n \t\targv[argc++] = depth_arg;\n \t}\n \targv[argc++] = url;\n+\targv[argc++] = NULL;\n+\n \tfor (i = 0; i < nr_heads; i++) {\n \t\tstruct ref *ref = to_fetch[i];\n \t\tif (!ref->name || !*ref->name)\n \t\t\tdie(\"cannot fetch by sha1 over smart http\");\n-\t\targv[argc++] = ref->name;\n+\t\tpacket_buf_write(&preamble, \"%s\\n\", ref->name);\n \t}\n-\targv[argc++] = NULL;\n+\tpacket_buf_flush(&preamble);\n \n \tmemset(&rpc, 0, sizeof(rpc));\n \trpc.service_name = \"git-upload-pack\",\n \trpc.argv = argv;\n+\trpc.stdin_preamble = &preamble;\n \trpc.gzip_request = 1;\n \n \terr = rpc_service(&rpc, heads);\n \tif (rpc.result.len)\n \t\tsafe_write(1, rpc.result.buf, rpc.result.len);\n \tstrbuf_release(&rpc.result);\n+\tstrbuf_release(&preamble);\n \tfree(argv);\n \tfree(depth_arg);\n \treturn err;\n-- \n1.7.9.5.4.g4f508\n"},{"id":"188312","messageId":"4F79C2C8.3020009@gmx.net","threadId":"30126","inReplyTo":"4F79C195.6090905@gmx.net","subject":"[PATCH v3 3/4] fetch-pack: test cases for the new --stdin option","fromName":"Ivan Todoroski","fromEmail":"grnch@gmx.net","sentAt":"2012-04-02T15:16:24Z","receivedAt":"2012-04-02T15:16:24Z","isPatch":true,"sender":{"key":"grnch@gmx.net","avatar":"https://avatars.githubusercontent.com/u/90292?v=4"},"body":"These test cases focus only on testing the parsing of refs on stdin,\nwithout bothering with the rest of the fetch-pack machinery. We pass in\nthe refs using different combinations of command line and stdin and then\nwe watch fetch-pack's stdout to see whether it prints all the refs we\nspecified (but we ignore their order).\n\nSigned-off-by: Ivan Todoroski \n---\n t/t5500-fetch-pack.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 66 insertions(+)\n\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 9bf69e9a0f..feaa222c78 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -248,4 +248,70 @@ test_expect_success 'clone shallow object count' '\n \tgrep \"^count: 52\" count.shallow\n '\n \n+test_expect_success 'setup tests for the --stdin parameter' '\n+\tfor head in C D E F\n+\tdo\n+\t\tadd $head\n+\tdone &&\n+\tfor head in A B C D E F\n+\tdo\n+\t\tgit tag $head $head\n+\tdone\n+\tcat >input <expect\n+\t(\n+\techo refs/heads/E &&\n+\techo refs/tags/E &&\n+\tcat input\n+\t) >input.dup\n+'\n+\n+test_expect_success 'fetch refs from cmdline, make sure it still works OK' '\n+\t(\n+\tcd client &&\n+\tgit fetch-pack --no-progress .. $(cat ../input)\n+\t) >output &&\n+\tcut -d \" \" -f 2 actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'fetch refs from stdin' '\n+\t(\n+\tcd client &&\n+\tgit fetch-pack --stdin --no-progress .. <../input\n+\t) >output &&\n+\tcut -d \" \" -f 2 actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'fetch mixed refs from cmdline and stdin' '\n+\t(\n+\tcd client &&\n+\ttail -n +5 ../input |\n+\tgit fetch-pack --stdin --no-progress .. $(head -n 4 ../input)\n+\t) >output &&\n+\tcut -d \" \" -f 2 actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'test duplicate refs from stdin' '\n+\t(\n+\tcd client &&\n+\ttest_must_fail git fetch-pack --stdin --no-progress .. <../input.dup\n+\t) >output &&\n+\tcut -d \" \" -f 2 actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.7.9.5.4.g4f508\n"},{"id":"188313","messageId":"4F79C2EF.2050401@gmx.net","threadId":"30126","inReplyTo":"4F79C195.6090905@gmx.net","subject":"[PATCH v3 4/4] remote-curl: main test case for the OS command line overflow","fromName":"Ivan Todoroski","fromEmail":"grnch@gmx.net","sentAt":"2012-04-02T15:17:03Z","receivedAt":"2012-04-02T15:17:03Z","isPatch":true,"sender":{"key":"grnch@gmx.net","avatar":"https://avatars.githubusercontent.com/u/90292?v=4"},"body":"This is main test case for the original problem that triggered this\npatch series. We create a repo with 50k tags and then test whether\ngit-clone over the smart HTTP protocol succeeds.\n\nNote that we construct the repo in a slightly different way than the\noriginal script used to reproduce the problem. This is because the\noriginal script just created 50k tags all pointing to the same commit,\nso if there was a bug where remote-curl.c was not passing all the refs\nto fetch-pack we wouldn't know. The clone would succeed even if only one\ntag was passed, because all the other tags were pointing at the same SHA\nand would be considered present.\n\nInstead we create a repo with 50k independent (dangling) commits and\nthen tag each of those commits with a unique tag. This way if one of the\ntags is not given to fetch-pack, later stages of the clone would\ncomplain about it.\n\nThis allows us to test both that the command line overflow was fixed, as\nwell as that it was fixed in a way that doesn't leave out any of the\nrefs.\n\nSigned-off-by: Ivan Todoroski \n---\n t/t5551-http-fetch.sh | 31 +++++++++++++++++++++++++++++++\n 1 file changed, 31 insertions(+)\n\ndiff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh\nindex 26d355725f..be6094be77 100755\n--- a/t/t5551-http-fetch.sh\n+++ b/t/t5551-http-fetch.sh\n@@ -109,5 +109,36 @@ test_expect_success 'follow redirects (302)' '\n \tgit clone $HTTPD_URL/smart-redir-temp/repo.git --quiet repo-t\n '\n \n+test -n \"$GIT_TEST_LONG\" && test_set_prereq EXPENSIVE\n+\n+test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '\n+\t(\n+\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\tfor i in `seq 50000`\n+\tdo\n+\t\techo \"commit refs/heads/too-many-refs\"\n+\t\techo \"mark :$i\"\n+\t\techo \"committer git $i +0000\"\n+\t\techo \"data 0\"\n+\t\techo \"M 644 inline bla.txt\"\n+\t\techo \"data 4\"\n+\t\techo \"bla\"\n+\t\t# make every commit dangling by always\n+\t\t# rewinding the branch after each commit\n+\t\techo \"reset refs/heads/too-many-refs\"\n+\t\techo \"from :1\"\n+\tdone | git fast-import --export-marks=marks &&\n+\n+\t# now assign tags to all the dangling commits we created above\n+\ttag=$(perl -e \"print \\\"bla\\\" x 30\") &&\n+\tsed -e \"s/^:\\(.\\+\\) \\(.\\+\\)$/\\2 refs\\/tags\\/$tag-\\1/\" >packed-refs\n+\t)\n+'\n+\n+test_expect_success EXPENSIVE 'clone the 50,000 tag repo to check OS command line overflow' '\n+\tgit clone $HTTPD_URL/smart/repo.git too-many-refs 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n stop_httpd\n test_done\n-- \n1.7.9.5.4.g4f508\n"},{"id":"188339","messageId":"7vfwcmezn1.fsf@alter.siamese.dyndns.org","threadId":"30126","inReplyTo":"4F79C264.1060508@gmx.net","subject":"Re: [PATCH v3 2/4] remote-curl: send the refs to fetch-pack on stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-02T18:50:58Z","receivedAt":"2012-04-02T18:50:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ivan Todoroski writes:\n\n> Now that we can throw an arbitrary number of refs at fetch-pack using\n> its --stdin option, we use it in the remote-curl helper to bypass the\n> OS command line length limit.\n>\n> Signed-off-by: Ivan Todoroski \n> ---\n> remote-curl.c | 14 ++++++++++++--\n> 1 file changed, 12 insertions(+), 2 deletions(-)\n>\n> diff --git a/remote-curl.c b/remote-curl.c\n> index d159fe7f34..a728edfa7f 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -633,6 +638,7 @@ static int fetch_git(struct discovery *heads,\n> \targv = xmalloc((15 + nr_heads) * sizeof(char*));\n\nYou no longer need an argv array whose size is proportional to nr_heads.\nI'll queue the patch without \"+ nr_heads\" part, but we should probably\nswitch this to use argv_array API after this series settles.\n\n> \targv[argc++] = \"fetch-pack\";\n> \targv[argc++] = \"--stateless-rpc\";\n> +\targv[argc++] = \"--stdin\";\n> \targv[argc++] = \"--lock-pack\";\n> \tif (options.followtags)\n> \t\targv[argc++] = \"--include-tag\";\n> @@ -651,23 +657,27 @@ static int fetch_git(struct discovery *heads,\n> \t\targv[argc++] = depth_arg;\n> \t}\n> \targv[argc++] = url;\n> +\targv[argc++] = NULL;\n> +\n> \tfor (i = 0; i < nr_heads; i++) {\n> \t\tstruct ref *ref = to_fetch[i];\n> \t\tif (!ref->name || !*ref->name)\n> \t\t\tdie(\"cannot fetch by sha1 over smart http\");\n> -\t\targv[argc++] = ref->name;\n> +\t\tpacket_buf_write(&preamble, \"%s\\n\", ref->name);\n> \t}\n> -\targv[argc++] = NULL;\n> +\tpacket_buf_flush(&preamble);\n"},{"id":"188342","messageId":"7v4nt2exlr.fsf@alter.siamese.dyndns.org","threadId":"30126","inReplyTo":"4F79C2C8.3020009@gmx.net","subject":"Re: [PATCH v3 3/4] fetch-pack: test cases for the new --stdin option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-02T19:34:56Z","receivedAt":"2012-04-02T19:34:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ivan Todoroski writes:\n\n> +test_expect_success 'setup tests for the --stdin parameter' '\n> +\tfor head in C D E F\n> +\tdo\n> +\t\tadd $head\n> +\tdone &&\n> +\tfor head in A B C D E F\n> +\tdo\n> +\t\tgit tag $head $head\n> +\tdone\n> +\tcat >input < +refs/heads/C\n> +refs/heads/A\n> +refs/heads/D\n> +refs/tags/C\n> +refs/heads/B\n> +refs/tags/A\n> +refs/heads/E\n> +refs/tags/B\n> +refs/tags/E\n> +refs/tags/D\n> +EOF\n> +\tsort expect\n> +\t(\n> +\techo refs/heads/E &&\n> +\techo refs/tags/E &&\n> +\tcat input\n> +\t) >input.dup\n> +'\n\n\nThis breaks && chain; also it is easier to read if indentation is used\nproperly.\n\nI'll queue it after fixing the above locally (the rest looked OK to me).\n\nThanks.\n"},{"id":"188348","messageId":"20120402204220.GC26503@sigill.intra.peff.net","threadId":"30126","inReplyTo":"4F79C22C.9080702@gmx.net","subject":"Re: [PATCH v3 1/4] fetch-pack: new --stdin option to read refs from stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-02T20:42:21Z","receivedAt":"2012-04-02T20:42:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 02, 2012 at 05:13:48PM +0200, Ivan Todoroski wrote:\n\n> +\t\telse {\n> +\t\t\t/* read from stdin one ref per line, until EOF */\n> +\t\t\tstruct strbuf line;\n> +\t\t\tstrbuf_init(&line, 0);\n\nA minor style nit, but we usually spell this:\n\n struct strbuf line = STRBUF_INIT;\n\nOther than that, this version looks good to me. Thanks.\n\n-Peff\n"},{"id":"188372","messageId":"4F7A4304.9040800@gmx.net","threadId":"30126","inReplyTo":"7v4nt2exlr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 3/4] fetch-pack: test cases for the new --stdin option","fromName":"Ivan Todoroski","fromEmail":"grnch@gmx.net","sentAt":"2012-04-03T00:23:32Z","receivedAt":"2012-04-03T00:23:32Z","isPatch":true,"sender":{"key":"grnch@gmx.net","avatar":"https://avatars.githubusercontent.com/u/90292?v=4"},"body":"On 02.04.2012 21:34, Junio C Hamano wrote:\n> Ivan Todoroski writes:\n> \n>> +test_expect_success 'setup tests for the --stdin parameter' '\n>> +\tfor head in C D E F\n>> +\tdo\n>> +\t\tadd $head\n>> +\tdone &&\n>> +\tfor head in A B C D E F\n>> +\tdo\n>> +\t\tgit tag $head $head\n>> +\tdone\n>> +\tcat >input <> +refs/heads/C\n>> +refs/heads/A\n>> +refs/heads/D\n>> +refs/tags/C\n>> +refs/heads/B\n>> +refs/tags/A\n>> +refs/heads/E\n>> +refs/tags/B\n>> +refs/tags/E\n>> +refs/tags/D\n>> +EOF\n>> +\tsort expect\n>> +\t(\n>> +\techo refs/heads/E &&\n>> +\techo refs/tags/E &&\n>> +\tcat input\n>> +\t) >input.dup\n>> +'\n> \n> \n> This breaks && chain; also it is easier to read if indentation is used\n> properly.\n> \n> I'll queue it after fixing the above locally (the rest looked OK to me).\n> \n> Thanks.\n\nI just saw the neat <<- trick in your reworked version, nice! Somehow \nI've never stumbled on to that part of the bash man page before. :|\n\nWhen I look back how many shell scripts I've mangled with indentation \nlike above, it breaks my heart. :)\n"}]}