{"thread":{"id":"30243","subject":"[PATCH] git-daemon wrapper to wait until daemon is ready","startedAt":"2012-04-14T18:29:07Z","lastAt":"2012-04-19T15:00:04Z","messageCount":18,"participants":["Clemens Buchacher","Ben Walton","Johannes Sixt","Junio C Hamano","Zbigniew Jędrzejewski-Szmek"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"189271","messageId":"20120414182907.GA3915@ecki","threadId":"30243","inReplyTo":null,"subject":"[PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-14T18:29:07Z","receivedAt":"2012-04-14T18:29:07Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"The shell script which is currently used to parse git daemon output does\nnot seem to work unreliably. In order to work around such issues,\nre-implement the same procedure in C and write the daemon pid to a file.\n\nThis means that we can no longer wait on the daemon process, since it is\nno longer a direct child of the shell process.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nDoes this patch improve your situation?\n\nNote that t5570 fails on current pu, because of the push.default\nwarnings. I am sending an independent patch for that.\n\nClemens\n\n .gitignore          |    1 +\n Makefile            |    1 +\n cache.h             |    1 +\n t/lib-git-daemon.sh |   30 +++-------------------\n test-git-daemon.c   |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++\n wrapper.c           |   21 +++++++++++++++\n 6 files changed, 99 insertions(+), 26 deletions(-)\n create mode 100644 test-git-daemon.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 87fcc5f..18a484c 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -177,6 +177,7 @@\n /test-dump-cache-tree\n /test-scrap-cache-tree\n /test-genrandom\n+/test-git-daemon\n /test-index-version\n /test-line-buffer\n /test-match-trees\ndiff --git a/Makefile b/Makefile\nindex be1957a..7317daa 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -477,6 +477,7 @@ TEST_PROGRAMS_NEED_X += test-delta\n TEST_PROGRAMS_NEED_X += test-dump-cache-tree\n TEST_PROGRAMS_NEED_X += test-scrap-cache-tree\n TEST_PROGRAMS_NEED_X += test-genrandom\n+TEST_PROGRAMS_NEED_X += test-git-daemon\n TEST_PROGRAMS_NEED_X += test-index-version\n TEST_PROGRAMS_NEED_X += test-line-buffer\n TEST_PROGRAMS_NEED_X += test-match-trees\ndiff --git a/cache.h b/cache.h\nindex e5e1aa4..6351c15 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1176,6 +1176,7 @@ extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg\n extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg);\n extern void fsync_or_die(int fd, const char *);\n \n+extern ssize_t read_line(int fd, void *buf, size_t count);\n extern ssize_t read_in_full(int fd, void *buf, size_t count);\n extern ssize_t write_in_full(int fd, const void *buf, size_t count);\n static inline ssize_t write_str_in_full(int fd, const char *str)\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex ef2d01f..9fefae1 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -23,27 +23,13 @@ start_git_daemon() {\n \ttrap 'code=$?; stop_git_daemon; (exit $code); die' EXIT\n \n \tsay >&3 \"Starting git daemon ...\"\n-\tmkfifo git_daemon_output\n-\tgit daemon --listen=127.0.0.1 --port=\"$LIB_GIT_DAEMON_PORT\" \\\n+\ttest-git-daemon --listen=127.0.0.1 --port=\"$LIB_GIT_DAEMON_PORT\" \\\n \t\t--reuseaddr --verbose \\\n \t\t--base-path=\"$GIT_DAEMON_DOCUMENT_ROOT_PATH\" \\\n \t\t\"$@\" \"$GIT_DAEMON_DOCUMENT_ROOT_PATH\" \\\n-\t\t>&3 2>git_daemon_output &\n-\tGIT_DAEMON_PID=$!\n-\t{\n-\t\tread line\n-\t\techo >&4 \"$line\"\n-\t\tcat >&4 &\n-\n-\t\t# Check expected output\n-\t\tif test x\"$(expr \"$line\" : \"\\[[0-9]*\\] \\(.*\\)\")\" != x\"Ready to rumble\"\n-\t\tthen\n-\t\t\tkill \"$GIT_DAEMON_PID\"\n-\t\t\twait \"$GIT_DAEMON_PID\"\n-\t\t\ttrap 'die' EXIT\n-\t\t\terror \"git daemon failed to start\"\n-\t\tfi\n-\t} <git_daemon_output\n+\t\t>&3 2>&4 ||\n+\t\terror \"git daemon failed to start\"\n+\tGIT_DAEMON_PID=$(cat git-daemon.pid)\n }\n \n stop_git_daemon() {\n@@ -57,13 +43,5 @@ stop_git_daemon() {\n \t# kill git-daemon child of git\n \tsay >&3 \"Stopping git daemon ...\"\n \tkill \"$GIT_DAEMON_PID\"\n-\twait \"$GIT_DAEMON_PID\" >&3 2>&4\n-\tret=$?\n-\t# expect exit with status 143 = 128+15 for signal TERM=15\n-\tif test $ret -ne 143\n-\tthen\n-\t\terror \"git daemon exited with status: $ret\"\n-\tfi\n \tGIT_DAEMON_PID=\n-\trm -f git_daemon_output\n }\ndiff --git a/test-git-daemon.c b/test-git-daemon.c\nnew file mode 100644\nindex 0000000..323bb65\n--- /dev/null\n+++ b/test-git-daemon.c\n@@ -0,0 +1,71 @@\n+#include \"git-compat-util.h\"\n+#include \"run-command.h\"\n+#include \"exec_cmd.h\"\n+#include \"strbuf.h\"\n+#include \"cache.h\"\n+#include <string.h>\n+#include <errno.h>\n+\n+static int parse_daemon_output(char *s)\n+{\n+\tif (*s++ != '[')\n+\t\treturn 1;\n+\ts = strchr(s, ']');\n+\tif (!s)\n+\t\treturn 1;\n+\tif (strcmp(s, \"] Ready to rumble\\n\"))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+int main(int argc, char **argv)\n+{\n+\tFILE *fp;\n+\tstruct child_process proc, cat;\n+\tchar *cat_argv[] = { \"cat\", NULL };\n+\tint parse_error;\n+\tchar buf[PATH_MAX];\n+\tint r;\n+\n+\tsetup_path();\n+\n+\tmemset(&proc, 0, sizeof(proc));\n+\targv[0] = \"git-daemon\";\n+\tproc.argv = (const char **)argv;\n+\tproc.no_stdin = 1;\n+\tproc.err = -1;\n+\n+\tif (start_command(&proc) < 0)\n+\t\treturn 1;\n+\n+\tr = read_line(proc.err, buf, sizeof(buf));\n+\tif (r < 0) {\n+\t\tfinish_command(&proc);\n+\t\treturn 1;\n+\t}\n+\tfprintf(stderr, \"%s\", buf);\n+\n+\tparse_error = parse_daemon_output(buf);\n+\n+\tmemset(&cat, 0, sizeof(cat));\n+\tcat.argv = (const char **)cat_argv;\n+\tcat.in = proc.err;\n+\tcat.out = 2;\n+\n+\tif (start_command(&cat) < 0)\n+\t\treturn 1;\n+\n+\tif (parse_error) {\n+\t\tkill(proc.pid, SIGTERM);\n+\t\tfinish_command(&proc);\n+\t\tfinish_command(&cat);\n+\t\treturn 1;\n+\t}\n+\n+\tfp = fopen(\"git-daemon.pid\", \"w\");\n+\tfprintf(fp, \"%\"PRIuMAX\"\\n\", (uintmax_t)proc.pid);\n+\tfclose(fp);\n+\n+\treturn 0;\n+}\ndiff --git a/wrapper.c b/wrapper.c\nindex 85f09df..7bf6dda 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -141,6 +141,27 @@ ssize_t xwrite(int fd, const void *buf, size_t len)\n \t}\n }\n \n+ssize_t read_line(int fd, void *buf, size_t count)\n+{\n+\tchar *p = buf;\n+\tssize_t total = 0;\n+\n+\twhile (count > 0) {\n+\t\tssize_t loaded = xread(fd, p, 1);\n+\t\tif (loaded < 0)\n+\t\t\treturn -1;\n+\t\tif (loaded == 0)\n+\t\t\treturn total;\n+\t\tcount -= loaded;\n+\t\ttotal += loaded;\n+\t\tif (*p == '\\n')\n+\t\t\tbreak;\n+\t\tp += loaded;\n+\t}\n+\n+\treturn total;\n+}\n+\n ssize_t read_in_full(int fd, void *buf, size_t count)\n {\n \tchar *p = buf;\n-- \n1.7.9.6\n"},{"id":"189272","messageId":"20120414183225.GB3915@ecki","threadId":"30243","inReplyTo":"20120414182907.GA3915@ecki","subject":"[PATCH] t5570: use explicit push refspec","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-14T18:32:25Z","receivedAt":"2012-04-14T18:32:25Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"The default mode for push without arguments will change. Some warnings\nare about to be enabled for such use, which causes some t5570 tests to\nfail because they do not expect this output. Fix this by passing an\nexplicit refspec to git push.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nOn Sat, Apr 14, 2012 at 08:29:07PM +0200, Clemens Buchacher wrote:\n> \n> Note that t5570 fails on current pu, because of the push.default\n> warnings. I am sending an independent patch for that.\n\nHere we go.\n\n t/t5570-git-daemon.sh |   30 ++++++++++++++----------------\n 1 file changed, 14 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh\nindex 7cbc999..a3a4e47 100755\n--- a/t/t5570-git-daemon.sh\n+++ b/t/t5570-git-daemon.sh\n@@ -103,14 +103,12 @@ test_remote_error()\n \t\tesac\n \tdone\n \n-\tif test $# -ne 3\n-\tthen\n-\t\terror \"invalid number of arguments\"\n-\tfi\n-\n+\tmsg=$1\n+\tshift\n \tcmd=$1\n-\trepo=$2\n-\tmsg=$3\n+\tshift\n+\trepo=$1\n+\tshift || error \"invalid number of arguments\"\n \n \tif test -x \"$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo\"\n \tthen\n@@ -122,7 +120,7 @@ test_remote_error()\n \t\tfi\n \tfi\n \n-\ttest_must_fail git \"$cmd\" \"$GIT_DAEMON_URL/$repo\" 2>output &&\n+\ttest_must_fail git \"$cmd\" \"$GIT_DAEMON_URL/$repo\" \"$@\" 2>output &&\n \techo \"fatal: remote error: $msg: /$repo\" >expect &&\n \ttest_cmp expect output\n \tret=$?\n@@ -131,18 +129,18 @@ test_remote_error()\n }\n \n msg=\"access denied or repository not exported\"\n-test_expect_success 'clone non-existent' \"test_remote_error    clone nowhere.git '$msg'\"\n-test_expect_success 'push disabled'      \"test_remote_error    push  repo.git    '$msg'\"\n-test_expect_success 'read access denied' \"test_remote_error -x fetch repo.git    '$msg'\"\n-test_expect_success 'not exported'       \"test_remote_error -n fetch repo.git    '$msg'\"\n+test_expect_success 'clone non-existent' \"test_remote_error    '$msg' clone nowhere.git    \"\n+test_expect_success 'push disabled'      \"test_remote_error    '$msg' push  repo.git master\"\n+test_expect_success 'read access denied' \"test_remote_error -x '$msg' fetch repo.git       \"\n+test_expect_success 'not exported'       \"test_remote_error -n '$msg' fetch repo.git       \"\n \n stop_git_daemon\n start_git_daemon --informative-errors\n \n-test_expect_success 'clone non-existent' \"test_remote_error    clone nowhere.git 'no such repository'\"\n-test_expect_success 'push disabled'      \"test_remote_error    push  repo.git    'service not enabled'\"\n-test_expect_success 'read access denied' \"test_remote_error -x fetch repo.git    'no such repository'\"\n-test_expect_success 'not exported'       \"test_remote_error -n fetch repo.git    'repository not exported'\"\n+test_expect_success 'clone non-existent' \"test_remote_error    'no such repository'      clone nowhere.git    \"\n+test_expect_success 'push disabled'      \"test_remote_error    'service not enabled'     push  repo.git master\"\n+test_expect_success 'read access denied' \"test_remote_error -x 'no such repository'      fetch repo.git       \"\n+test_expect_success 'not exported'       \"test_remote_error -n 'repository not exported' fetch repo.git       \"\n \n stop_git_daemon\n test_done\n-- \n1.7.9.6\n"},{"id":"189273","messageId":"1334428952-sup-5241@pinkfloyd.chass.utoronto.ca","threadId":"30243","inReplyTo":"20120414182907.GA3915@ecki","subject":"Re: [PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-04-14T18:43:41Z","receivedAt":"2012-04-14T18:43:41Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Clemens Buchacher's message of Sat Apr 14 14:29:07 -0400 2012:\n\nHi Clemens,\n\n> The shell script which is currently used to parse git daemon output does\n> not seem to work unreliably. In order to work around such issues,\n\nPresumably you mean \"work reliably\" here?\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"189274","messageId":"20120414190056.GC3915@ecki","threadId":"30243","inReplyTo":"1334428952-sup-5241@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-14T19:00:56Z","receivedAt":"2012-04-14T19:00:56Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Apr 14, 2012 at 02:43:41PM -0400, Ben Walton wrote:\n> \n> > The shell script which is currently used to parse git daemon output does\n> > not seem to work unreliably. In order to work around such issues,\n> \n> Presumably you mean \"work reliably\" here?\n\nYes, that's what I meant the commit message to say. Thanks for noticing.\n\nBut I would like to wait for more info from the OP before we conclude\nthat this is indeed the case.\n"},{"id":"189276","messageId":"20120414191633.GA28670@ecki","threadId":"30243","inReplyTo":"20120414190056.GC3915@ecki","subject":"[PATCH v2] git-daemon wrapper to wait until daemon is ready","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-14T19:16:34Z","receivedAt":"2012-04-14T19:16:34Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"The shell script which is currently used to parse git daemon output does\nnot seem to work reliably. In order to work around such issues,\nre-implement the same procedure in C and write the daemon pid to a file.\n\nThis means that we can no longer wait on the daemon process, since it is\nnot a direct child of the shell process.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\n> On Sat, Apr 14, 2012 at 02:43:41PM -0400, Ben Walton wrote:\n> > \n> > Presumably you mean \"work reliably\" here?\n\nI fixed the commit message. I also noticed that the buffer returned by\nread_line had no zero terminator. And the parse_error variable was\nunnecessary.\n\n .gitignore          |    1 +\n Makefile            |    1 +\n cache.h             |    1 +\n t/lib-git-daemon.sh |   30 +++-------------------\n test-git-daemon.c   |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++\n wrapper.c           |   21 ++++++++++++++++\n 6 files changed, 97 insertions(+), 26 deletions(-)\n create mode 100644 test-git-daemon.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 87fcc5f..18a484c 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -177,6 +177,7 @@\n /test-dump-cache-tree\n /test-scrap-cache-tree\n /test-genrandom\n+/test-git-daemon\n /test-index-version\n /test-line-buffer\n /test-match-trees\ndiff --git a/Makefile b/Makefile\nindex be1957a..7317daa 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -477,6 +477,7 @@ TEST_PROGRAMS_NEED_X += test-delta\n TEST_PROGRAMS_NEED_X += test-dump-cache-tree\n TEST_PROGRAMS_NEED_X += test-scrap-cache-tree\n TEST_PROGRAMS_NEED_X += test-genrandom\n+TEST_PROGRAMS_NEED_X += test-git-daemon\n TEST_PROGRAMS_NEED_X += test-index-version\n TEST_PROGRAMS_NEED_X += test-line-buffer\n TEST_PROGRAMS_NEED_X += test-match-trees\ndiff --git a/cache.h b/cache.h\nindex e5e1aa4..6351c15 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1176,6 +1176,7 @@ extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg\n extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg);\n extern void fsync_or_die(int fd, const char *);\n \n+extern ssize_t read_line(int fd, void *buf, size_t count);\n extern ssize_t read_in_full(int fd, void *buf, size_t count);\n extern ssize_t write_in_full(int fd, const void *buf, size_t count);\n static inline ssize_t write_str_in_full(int fd, const char *str)\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex ef2d01f..9fefae1 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -23,27 +23,13 @@ start_git_daemon() {\n \ttrap 'code=$?; stop_git_daemon; (exit $code); die' EXIT\n \n \tsay >&3 \"Starting git daemon ...\"\n-\tmkfifo git_daemon_output\n-\tgit daemon --listen=127.0.0.1 --port=\"$LIB_GIT_DAEMON_PORT\" \\\n+\ttest-git-daemon --listen=127.0.0.1 --port=\"$LIB_GIT_DAEMON_PORT\" \\\n \t\t--reuseaddr --verbose \\\n \t\t--base-path=\"$GIT_DAEMON_DOCUMENT_ROOT_PATH\" \\\n \t\t\"$@\" \"$GIT_DAEMON_DOCUMENT_ROOT_PATH\" \\\n-\t\t>&3 2>git_daemon_output &\n-\tGIT_DAEMON_PID=$!\n-\t{\n-\t\tread line\n-\t\techo >&4 \"$line\"\n-\t\tcat >&4 &\n-\n-\t\t# Check expected output\n-\t\tif test x\"$(expr \"$line\" : \"\\[[0-9]*\\] \\(.*\\)\")\" != x\"Ready to rumble\"\n-\t\tthen\n-\t\t\tkill \"$GIT_DAEMON_PID\"\n-\t\t\twait \"$GIT_DAEMON_PID\"\n-\t\t\ttrap 'die' EXIT\n-\t\t\terror \"git daemon failed to start\"\n-\t\tfi\n-\t} <git_daemon_output\n+\t\t>&3 2>&4 ||\n+\t\terror \"git daemon failed to start\"\n+\tGIT_DAEMON_PID=$(cat git-daemon.pid)\n }\n \n stop_git_daemon() {\n@@ -57,13 +43,5 @@ stop_git_daemon() {\n \t# kill git-daemon child of git\n \tsay >&3 \"Stopping git daemon ...\"\n \tkill \"$GIT_DAEMON_PID\"\n-\twait \"$GIT_DAEMON_PID\" >&3 2>&4\n-\tret=$?\n-\t# expect exit with status 143 = 128+15 for signal TERM=15\n-\tif test $ret -ne 143\n-\tthen\n-\t\terror \"git daemon exited with status: $ret\"\n-\tfi\n \tGIT_DAEMON_PID=\n-\trm -f git_daemon_output\n }\ndiff --git a/test-git-daemon.c b/test-git-daemon.c\nnew file mode 100644\nindex 0000000..82a9a8f\n--- /dev/null\n+++ b/test-git-daemon.c\n@@ -0,0 +1,69 @@\n+#include \"git-compat-util.h\"\n+#include \"run-command.h\"\n+#include \"exec_cmd.h\"\n+#include \"strbuf.h\"\n+#include \"cache.h\"\n+#include <string.h>\n+#include <errno.h>\n+\n+static int parse_daemon_output(char *s)\n+{\n+\tif (*s++ != '[')\n+\t\treturn 1;\n+\ts = strchr(s, ']');\n+\tif (!s)\n+\t\treturn 1;\n+\tif (strcmp(s, \"] Ready to rumble\\n\"))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+int main(int argc, char **argv)\n+{\n+\tFILE *fp;\n+\tstruct child_process proc, cat;\n+\tchar *cat_argv[] = { \"cat\", NULL };\n+\tchar buf[PATH_MAX];\n+\tint r;\n+\n+\tsetup_path();\n+\n+\tmemset(&proc, 0, sizeof(proc));\n+\targv[0] = \"git-daemon\";\n+\tproc.argv = (const char **)argv;\n+\tproc.no_stdin = 1;\n+\tproc.err = -1;\n+\n+\tif (start_command(&proc) < 0)\n+\t\treturn 1;\n+\n+\tr = read_line(proc.err, buf, sizeof(buf)-1);\n+\tif (r < 0) {\n+\t\tfinish_command(&proc);\n+\t\treturn 1;\n+\t}\n+\tbuf[r] = '\\0';\n+\tfprintf(stderr, \"%s\", buf);\n+\n+\tmemset(&cat, 0, sizeof(cat));\n+\tcat.argv = (const char **)cat_argv;\n+\tcat.in = proc.err;\n+\tcat.out = 2;\n+\n+\tif (start_command(&cat) < 0)\n+\t\treturn 1;\n+\n+\tif (parse_daemon_output(buf)) {\n+\t\tkill(proc.pid, SIGTERM);\n+\t\tfinish_command(&proc);\n+\t\tfinish_command(&cat);\n+\t\treturn 1;\n+\t}\n+\n+\tfp = fopen(\"git-daemon.pid\", \"w\");\n+\tfprintf(fp, \"%\"PRIuMAX\"\\n\", (uintmax_t)proc.pid);\n+\tfclose(fp);\n+\n+\treturn 0;\n+}\ndiff --git a/wrapper.c b/wrapper.c\nindex 85f09df..7bf6dda 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -141,6 +141,27 @@ ssize_t xwrite(int fd, const void *buf, size_t len)\n \t}\n }\n \n+ssize_t read_line(int fd, void *buf, size_t count)\n+{\n+\tchar *p = buf;\n+\tssize_t total = 0;\n+\n+\twhile (count > 0) {\n+\t\tssize_t loaded = xread(fd, p, 1);\n+\t\tif (loaded < 0)\n+\t\t\treturn -1;\n+\t\tif (loaded == 0)\n+\t\t\treturn total;\n+\t\tcount -= loaded;\n+\t\ttotal += loaded;\n+\t\tif (*p == '\\n')\n+\t\t\tbreak;\n+\t\tp += loaded;\n+\t}\n+\n+\treturn total;\n+}\n+\n ssize_t read_in_full(int fd, void *buf, size_t count)\n {\n \tchar *p = buf;\n-- \n1.7.9.6\n"},{"id":"189280","messageId":"4F89D1C6.8090705@kdbg.org","threadId":"30243","inReplyTo":"20120414182907.GA3915@ecki","subject":"Re: [PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-04-14T19:36:38Z","receivedAt":"2012-04-14T19:36:38Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.04.2012 20:29, schrieb Clemens Buchacher:\n> +\tr = read_line(proc.err, buf, sizeof(buf));\n\nWe have strbuf_getwholeline_fd().\n\n> +\tmemset(&cat, 0, sizeof(cat));\n> +\tcat.argv = (const char **)cat_argv;\n> +\tcat.in = proc.err;\n> +\tcat.out = 2;\n\nUseless use of cat?\n\n-- Hannes\n"},{"id":"189295","messageId":"20120414220606.GA18137@ecki","threadId":"30243","inReplyTo":"4F89D1C6.8090705@kdbg.org","subject":"Re: [PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-14T22:06:06Z","receivedAt":"2012-04-14T22:06:06Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Apr 14, 2012 at 09:36:38PM +0200, Johannes Sixt wrote:\n> Am 14.04.2012 20:29, schrieb Clemens Buchacher:\n> > +\tr = read_line(proc.err, buf, sizeof(buf));\n> \n> We have strbuf_getwholeline_fd().\n\nThanks. Will fix.\n\n> > +\tmemset(&cat, 0, sizeof(cat));\n> > +\tcat.argv = (const char **)cat_argv;\n> > +\tcat.in = proc.err;\n> > +\tcat.out = 2;\n> \n> Useless use of cat?\n\nI don't see how I could avoid cat here. I have to create a pipe first so\nthat I can read the first line. And then I have to terminate\ntest-git-daemon in order to start the tests. So I cannot continue\nreading synchronously.\n"},{"id":"189305","messageId":"7viph1288e.fsf@alter.siamese.dyndns.org","threadId":"30243","inReplyTo":"20120414183225.GB3915@ecki","subject":"Re: [PATCH] t5570: use explicit push refspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-14T23:40:01Z","receivedAt":"2012-04-14T23:40:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> The default mode for push without arguments will change. Some warnings\n> are about to be enabled for such use, which causes some t5570 tests to\n> fail because they do not expect this output. Fix this by passing an\n> explicit refspec to git push.\n\nI wonder if a better fix is to configure \"push.default = matching\" in the\ntest repository.  Otherwise wouldn't the result of the push change once\nthe default changes?\n\n> Signed-off-by: Clemens Buchacher <drizzd@aon.at>\n> ---\n>\n> On Sat, Apr 14, 2012 at 08:29:07PM +0200, Clemens Buchacher wrote:\n>> \n>> Note that t5570 fails on current pu, because of the push.default\n>> warnings. I am sending an independent patch for that.\n>\n> Here we go.\n>\n>  t/t5570-git-daemon.sh |   30 ++++++++++++++----------------\n>  1 file changed, 14 insertions(+), 16 deletions(-)\n>\n> diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh\n> index 7cbc999..a3a4e47 100755\n> --- a/t/t5570-git-daemon.sh\n> +++ b/t/t5570-git-daemon.sh\n> @@ -103,14 +103,12 @@ test_remote_error()\n>  \t\tesac\n>  \tdone\n>  \n> -\tif test $# -ne 3\n> -\tthen\n> -\t\terror \"invalid number of arguments\"\n> -\tfi\n> -\n> +\tmsg=$1\n> +\tshift\n>  \tcmd=$1\n> -\trepo=$2\n> -\tmsg=$3\n> +\tshift\n> +\trepo=$1\n> +\tshift || error \"invalid number of arguments\"\n>  \n>  \tif test -x \"$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo\"\n>  \tthen\n> @@ -122,7 +120,7 @@ test_remote_error()\n>  \t\tfi\n>  \tfi\n>  \n> -\ttest_must_fail git \"$cmd\" \"$GIT_DAEMON_URL/$repo\" 2>output &&\n> +\ttest_must_fail git \"$cmd\" \"$GIT_DAEMON_URL/$repo\" \"$@\" 2>output &&\n>  \techo \"fatal: remote error: $msg: /$repo\" >expect &&\n>  \ttest_cmp expect output\n>  \tret=$?\n> @@ -131,18 +129,18 @@ test_remote_error()\n>  }\n>  \n>  msg=\"access denied or repository not exported\"\n> -test_expect_success 'clone non-existent' \"test_remote_error    clone nowhere.git '$msg'\"\n> -test_expect_success 'push disabled'      \"test_remote_error    push  repo.git    '$msg'\"\n> -test_expect_success 'read access denied' \"test_remote_error -x fetch repo.git    '$msg'\"\n> -test_expect_success 'not exported'       \"test_remote_error -n fetch repo.git    '$msg'\"\n> +test_expect_success 'clone non-existent' \"test_remote_error    '$msg' clone nowhere.git    \"\n> +test_expect_success 'push disabled'      \"test_remote_error    '$msg' push  repo.git master\"\n> +test_expect_success 'read access denied' \"test_remote_error -x '$msg' fetch repo.git       \"\n> +test_expect_success 'not exported'       \"test_remote_error -n '$msg' fetch repo.git       \"\n>  \n>  stop_git_daemon\n>  start_git_daemon --informative-errors\n>  \n> -test_expect_success 'clone non-existent' \"test_remote_error    clone nowhere.git 'no such repository'\"\n> -test_expect_success 'push disabled'      \"test_remote_error    push  repo.git    'service not enabled'\"\n> -test_expect_success 'read access denied' \"test_remote_error -x fetch repo.git    'no such repository'\"\n> -test_expect_success 'not exported'       \"test_remote_error -n fetch repo.git    'repository not exported'\"\n> +test_expect_success 'clone non-existent' \"test_remote_error    'no such repository'      clone nowhere.git    \"\n> +test_expect_success 'push disabled'      \"test_remote_error    'service not enabled'     push  repo.git master\"\n> +test_expect_success 'read access denied' \"test_remote_error -x 'no such repository'      fetch repo.git       \"\n> +test_expect_success 'not exported'       \"test_remote_error -n 'repository not exported' fetch repo.git       \"\n>  \n>  stop_git_daemon\n>  test_done\n"},{"id":"189309","messageId":"20120415001133.GB32140@ecki","threadId":"30243","inReplyTo":"7viph1288e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t5570: use explicit push refspec","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-15T00:11:33Z","receivedAt":"2012-04-15T00:11:33Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Apr 14, 2012 at 04:40:01PM -0700, Junio C Hamano wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > The default mode for push without arguments will change. Some warnings\n> > are about to be enabled for such use, which causes some t5570 tests to\n> > fail because they do not expect this output. Fix this by passing an\n> > explicit refspec to git push.\n> \n> I wonder if a better fix is to configure \"push.default = matching\" in the\n> test repository.  Otherwise wouldn't the result of the push change once\n> the default changes?\n\nThe push.default option matters only if a refspec is not specified. By\nadding a refspec, push.default should not matter any more. Unless that\nis going to change as well?\n"},{"id":"189332","messageId":"20120415115322.GA11786@ecki","threadId":"30243","inReplyTo":"20120414220606.GA18137@ecki","subject":"[PATCH v3] git-daemon wrapper to wait until daemon is ready","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-15T11:53:22Z","receivedAt":"2012-04-15T11:53:22Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"The shell script which is currently used to parse git daemon output does\nnot seem to work reliably. In order to work around such issues,\nre-implement the same procedure in C and write the daemon pid to a file.\n\nThis means that we can no longer wait on the daemon process, since it is\nno longer a direct child of the shell process.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n\nOn Sun, Apr 15, 2012 at 12:06:06AM +0200, Clemens Buchacher wrote:\n> On Sat, Apr 14, 2012 at 09:36:38PM +0200, Johannes Sixt wrote:\n> > Am 14.04.2012 20:29, schrieb Clemens Buchacher:\n> > > +\tr = read_line(proc.err, buf, sizeof(buf));\n> > \n> > We have strbuf_getwholeline_fd().\n> \n> Thanks. Will fix.\n\nHere's the re-roll for completeness. Still waiting on feedback from the\nOP if this actually solves the problem, though.\n \n .gitignore          |    1 +\n Makefile            |    1 +\n t/lib-git-daemon.sh |   30 ++++---------------------\n test-git-daemon.c   |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 68 insertions(+), 26 deletions(-)\n create mode 100644 test-git-daemon.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 87fcc5f..18a484c 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -177,6 +177,7 @@\n /test-dump-cache-tree\n /test-scrap-cache-tree\n /test-genrandom\n+/test-git-daemon\n /test-index-version\n /test-line-buffer\n /test-match-trees\ndiff --git a/Makefile b/Makefile\nindex be1957a..7317daa 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -477,6 +477,7 @@ TEST_PROGRAMS_NEED_X += test-delta\n TEST_PROGRAMS_NEED_X += test-dump-cache-tree\n TEST_PROGRAMS_NEED_X += test-scrap-cache-tree\n TEST_PROGRAMS_NEED_X += test-genrandom\n+TEST_PROGRAMS_NEED_X += test-git-daemon\n TEST_PROGRAMS_NEED_X += test-index-version\n TEST_PROGRAMS_NEED_X += test-line-buffer\n TEST_PROGRAMS_NEED_X += test-match-trees\ndiff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh\nindex ef2d01f..9fefae1 100644\n--- a/t/lib-git-daemon.sh\n+++ b/t/lib-git-daemon.sh\n@@ -23,27 +23,13 @@ start_git_daemon() {\n \ttrap 'code=$?; stop_git_daemon; (exit $code); die' EXIT\n \n \tsay >&3 \"Starting git daemon ...\"\n-\tmkfifo git_daemon_output\n-\tgit daemon --listen=127.0.0.1 --port=\"$LIB_GIT_DAEMON_PORT\" \\\n+\ttest-git-daemon --listen=127.0.0.1 --port=\"$LIB_GIT_DAEMON_PORT\" \\\n \t\t--reuseaddr --verbose \\\n \t\t--base-path=\"$GIT_DAEMON_DOCUMENT_ROOT_PATH\" \\\n \t\t\"$@\" \"$GIT_DAEMON_DOCUMENT_ROOT_PATH\" \\\n-\t\t>&3 2>git_daemon_output &\n-\tGIT_DAEMON_PID=$!\n-\t{\n-\t\tread line\n-\t\techo >&4 \"$line\"\n-\t\tcat >&4 &\n-\n-\t\t# Check expected output\n-\t\tif test x\"$(expr \"$line\" : \"\\[[0-9]*\\] \\(.*\\)\")\" != x\"Ready to rumble\"\n-\t\tthen\n-\t\t\tkill \"$GIT_DAEMON_PID\"\n-\t\t\twait \"$GIT_DAEMON_PID\"\n-\t\t\ttrap 'die' EXIT\n-\t\t\terror \"git daemon failed to start\"\n-\t\tfi\n-\t} <git_daemon_output\n+\t\t>&3 2>&4 ||\n+\t\terror \"git daemon failed to start\"\n+\tGIT_DAEMON_PID=$(cat git-daemon.pid)\n }\n \n stop_git_daemon() {\n@@ -57,13 +43,5 @@ stop_git_daemon() {\n \t# kill git-daemon child of git\n \tsay >&3 \"Stopping git daemon ...\"\n \tkill \"$GIT_DAEMON_PID\"\n-\twait \"$GIT_DAEMON_PID\" >&3 2>&4\n-\tret=$?\n-\t# expect exit with status 143 = 128+15 for signal TERM=15\n-\tif test $ret -ne 143\n-\tthen\n-\t\terror \"git daemon exited with status: $ret\"\n-\tfi\n \tGIT_DAEMON_PID=\n-\trm -f git_daemon_output\n }\ndiff --git a/test-git-daemon.c b/test-git-daemon.c\nnew file mode 100644\nindex 0000000..f44fa6a\n--- /dev/null\n+++ b/test-git-daemon.c\n@@ -0,0 +1,62 @@\n+#include \"git-compat-util.h\"\n+#include \"run-command.h\"\n+#include \"exec_cmd.h\"\n+#include \"strbuf.h\"\n+#include <string.h>\n+#include <errno.h>\n+\n+static int parse_daemon_output(char *s)\n+{\n+\tif (*s++ != '[')\n+\t\treturn 1;\n+\ts = strchr(s, ']');\n+\tif (!s)\n+\t\treturn 1;\n+\tif (strcmp(s, \"] Ready to rumble\\n\"))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct strbuf line = STRBUF_INIT;\n+\tFILE *fp;\n+\tstruct child_process proc, cat;\n+\tchar *cat_argv[] = { \"cat\", NULL };\n+\n+\tsetup_path();\n+\n+\tmemset(&proc, 0, sizeof(proc));\n+\targv[0] = \"git-daemon\";\n+\tproc.argv = (const char **)argv;\n+\tproc.no_stdin = 1;\n+\tproc.err = -1;\n+\n+\tif (start_command(&proc) < 0)\n+\t\treturn 1;\n+\n+\tstrbuf_getwholeline_fd(&line, proc.err, '\\n');\n+\tfprintf(stderr, line.buf);\n+\n+\tmemset(&cat, 0, sizeof(cat));\n+\tcat.argv = (const char **)cat_argv;\n+\tcat.in = proc.err;\n+\tcat.out = 2;\n+\n+\tif (start_command(&cat) < 0)\n+\t\treturn 1;\n+\n+\tif (parse_daemon_output(line.buf)) {\n+\t\tkill(proc.pid, SIGTERM);\n+\t\tfinish_command(&proc);\n+\t\tfinish_command(&cat);\n+\t\treturn 1;\n+\t}\n+\n+\tfp = fopen(\"git-daemon.pid\", \"w\");\n+\tfprintf(fp, \"%\"PRIuMAX\"\\n\", (uintmax_t)proc.pid);\n+\tfclose(fp);\n+\n+\treturn 0;\n+}\n-- \n1.7.9.6\n"},{"id":"189341","messageId":"4F8B0158.4040407@kdbg.org","threadId":"30243","inReplyTo":"20120414220606.GA18137@ecki","subject":"Re: [PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-04-15T17:11:52Z","receivedAt":"2012-04-15T17:11:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 15.04.2012 00:06, schrieb Clemens Buchacher:\n> On Sat, Apr 14, 2012 at 09:36:38PM +0200, Johannes Sixt wrote:\n>> Am 14.04.2012 20:29, schrieb Clemens Buchacher:\n>>> +\tmemset(&cat, 0, sizeof(cat));\n>>> +\tcat.argv = (const char **)cat_argv;\n>>> +\tcat.in = proc.err;\n>>> +\tcat.out = 2;\n>>\n>> Useless use of cat?\n> \n> I don't see how I could avoid cat here. I have to create a pipe first so\n> that I can read the first line. And then I have to terminate\n> test-git-daemon in order to start the tests. So I cannot continue\n> reading synchronously.\n\nOK, I got it.\n\nBut reading the first line in this way needs a few assumptions to be true:\n\n- git-daemon does not write an incomplete line and then waits.\n\n- git-daemon does not write more than one line, because xread() happily\nreads everything it can get. Your implementation differs from the old\nversion because the shell's 'read' is required to read no more than one\nline, i.e., to read byte-wise from the pipe until it sees the LF.\n\n-- Hannes\n"},{"id":"189344","messageId":"7v7gxg2461.fsf@alter.siamese.dyndns.org","threadId":"30243","inReplyTo":"20120415001133.GB32140@ecki","subject":"Re: [PATCH] t5570: use explicit push refspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-15T19:20:06Z","receivedAt":"2012-04-15T19:20:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> On Sat, Apr 14, 2012 at 04:40:01PM -0700, Junio C Hamano wrote:\n>> Clemens Buchacher <drizzd@aon.at> writes:\n>> \n>> > The default mode for push without arguments will change. Some warnings\n>> > are about to be enabled for such use, which causes some t5570 tests to\n>> > fail because they do not expect this output. Fix this by passing an\n>> > explicit refspec to git push.\n>> \n>> I wonder if a better fix is to configure \"push.default = matching\" in the\n>> test repository.  Otherwise wouldn't the result of the push change once\n>> the default changes?\n>\n> The push.default option matters only if a refspec is not specified. By\n> adding a refspec, push.default should not matter any more. Unless that\n> is going to change as well?\n\nNo, I was thinking more about testing cases where there is no refspec on\nthe command line, which we used to test, but with your patch we no longer\ndo.  In other words, your fix not just squelches the advice message and\nmake them pass, but it changes the way the command behaves, no?\n\nBesides, that way you do not have to swap the parameters to test_remote_error\nso we do not have scratch our heads wondering why we have changes to test\nvectors that run clones and fetches.\n"},{"id":"189353","messageId":"20120415193242.GA1960@ecki","threadId":"30243","inReplyTo":"4F8B0158.4040407@kdbg.org","subject":"Re: [PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-15T19:32:42Z","receivedAt":"2012-04-15T19:32:42Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Apr 15, 2012 at 07:11:52PM +0200, Johannes Sixt wrote:\n> \n> But reading the first line in this way needs a few assumptions to be true:\n> \n> - git-daemon does not write an incomplete line and then waits.\n\nYes. One way to avoid that assumption would be a timeout.\n\n> - git-daemon does not write more than one line, because xread() happily\n> reads everything it can get. Your implementation differs from the old\n> version because the shell's 'read' is required to read no more than one\n> line, i.e., to read byte-wise from the pipe until it sees the LF.\n\nThe strbuf_getwholeline_fd implementation calls xread(fd, buf, 1),\nreading only one byte at a time. It does not try to read beyond the\nnewline.\n"},{"id":"189355","messageId":"20120415195231.GB1960@ecki","threadId":"30243","inReplyTo":"7v7gxg2461.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t5570: use explicit push refspec","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-15T19:52:35Z","receivedAt":"2012-04-15T19:52:35Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, Apr 15, 2012 at 12:20:06PM -0700, Junio C Hamano wrote:\n> Clemens Buchacher <drizzd@aon.at> writes:\n> \n> > On Sat, Apr 14, 2012 at 04:40:01PM -0700, Junio C Hamano wrote:\n> >> Clemens Buchacher <drizzd@aon.at> writes:\n> >> \n> >> > The default mode for push without arguments will change. Some warnings\n> >> > are about to be enabled for such use, which causes some t5570 tests to\n> >> > fail because they do not expect this output. Fix this by passing an\n> >> > explicit refspec to git push.\n> >> \n> >> I wonder if a better fix is to configure \"push.default = matching\" in the\n> >> test repository.  Otherwise wouldn't the result of the push change once\n> >> the default changes?\n> >\n> > The push.default option matters only if a refspec is not specified. By\n> > adding a refspec, push.default should not matter any more. Unless that\n> > is going to change as well?\n> \n> No, I was thinking more about testing cases where there is no refspec on\n> the command line, which we used to test, but with your patch we no longer\n> do.  In other words, your fix not just squelches the advice message and\n> make them pass, but it changes the way the command behaves, no?\n\nIt does exactly the same thing it did before, since there is only one\nlocal branch. The goal of the test is not to check behavior of git push\nwithout arguments. Since that is subject to change, and is causing\ncausing the test to fail already for no good reason, I think it better\nto use the more explicit version of the command.\n\n> Besides, that way you do not have to swap the parameters to test_remote_error\n> so we do not have scratch our heads wondering why we have changes to test\n> vectors that run clones and fetches.\n\nIn my opinion, this change is also an improvement in itself, since now\nwe can more easily pass extra arguments to test_remote_error. Maybe the\nscratching of heads can be alleviated by amending the commit message\nlike so?\n\n-->o--\nSubject: [PATCH] t5570: use explicit push refspec\n\nThe default mode for push without arguments will change. Some warnings\nare about to be enabled for such use, which causes some t5570 tests to\nfail because they do not expect this output.\n\nFix this by passing an explicit refspec to git push. To that end, change\nthe calling conventions of test_remote_error in order to accomodate\nextra command arguments.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n t/t5570-git-daemon.sh |   30 ++++++++++++++----------------\n 1 file changed, 14 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh\nindex 7cbc999..a3a4e47 100755\n--- a/t/t5570-git-daemon.sh\n+++ b/t/t5570-git-daemon.sh\n@@ -103,14 +103,12 @@ test_remote_error()\n \t\tesac\n \tdone\n \n-\tif test $# -ne 3\n-\tthen\n-\t\terror \"invalid number of arguments\"\n-\tfi\n-\n+\tmsg=$1\n+\tshift\n \tcmd=$1\n-\trepo=$2\n-\tmsg=$3\n+\tshift\n+\trepo=$1\n+\tshift || error \"invalid number of arguments\"\n \n \tif test -x \"$GIT_DAEMON_DOCUMENT_ROOT_PATH/$repo\"\n \tthen\n@@ -122,7 +120,7 @@ test_remote_error()\n \t\tfi\n \tfi\n \n-\ttest_must_fail git \"$cmd\" \"$GIT_DAEMON_URL/$repo\" 2>output &&\n+\ttest_must_fail git \"$cmd\" \"$GIT_DAEMON_URL/$repo\" \"$@\" 2>output &&\n \techo \"fatal: remote error: $msg: /$repo\" >expect &&\n \ttest_cmp expect output\n \tret=$?\n@@ -131,18 +129,18 @@ test_remote_error()\n }\n \n msg=\"access denied or repository not exported\"\n-test_expect_success 'clone non-existent' \"test_remote_error    clone nowhere.git '$msg'\"\n-test_expect_success 'push disabled'      \"test_remote_error    push  repo.git    '$msg'\"\n-test_expect_success 'read access denied' \"test_remote_error -x fetch repo.git    '$msg'\"\n-test_expect_success 'not exported'       \"test_remote_error -n fetch repo.git    '$msg'\"\n+test_expect_success 'clone non-existent' \"test_remote_error    '$msg' clone nowhere.git    \"\n+test_expect_success 'push disabled'      \"test_remote_error    '$msg' push  repo.git master\"\n+test_expect_success 'read access denied' \"test_remote_error -x '$msg' fetch repo.git       \"\n+test_expect_success 'not exported'       \"test_remote_error -n '$msg' fetch repo.git       \"\n \n stop_git_daemon\n start_git_daemon --informative-errors\n \n-test_expect_success 'clone non-existent' \"test_remote_error    clone nowhere.git 'no such repository'\"\n-test_expect_success 'push disabled'      \"test_remote_error    push  repo.git    'service not enabled'\"\n-test_expect_success 'read access denied' \"test_remote_error -x fetch repo.git    'no such repository'\"\n-test_expect_success 'not exported'       \"test_remote_error -n fetch repo.git    'repository not exported'\"\n+test_expect_success 'clone non-existent' \"test_remote_error    'no such repository'      clone nowhere.git    \"\n+test_expect_success 'push disabled'      \"test_remote_error    'service not enabled'     push  repo.git master\"\n+test_expect_success 'read access denied' \"test_remote_error -x 'no such repository'      fetch repo.git       \"\n+test_expect_success 'not exported'       \"test_remote_error -n 'repository not exported' fetch repo.git       \"\n \n stop_git_daemon\n test_done\n-- \n1.7.9.6\n"},{"id":"189354","messageId":"4F8B281D.2000305@kdbg.org","threadId":"30243","inReplyTo":"20120415193242.GA1960@ecki","subject":"Re: [PATCH] git-daemon wrapper to wait until daemon is ready","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-04-15T19:57:17Z","receivedAt":"2012-04-15T19:57:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 15.04.2012 21:32, schrieb Clemens Buchacher:\n> The strbuf_getwholeline_fd implementation calls xread(fd, buf, 1),\n> reading only one byte at a time. It does not try to read beyond the\n> newline.\n\nPoint taken. I only saw \"xread\" and didn't look further. Sorry for the\nnoise. I now crawl back under my rock.\n\n-- Hannes\n"},{"id":"189356","messageId":"7vpqb8zr44.fsf@alter.siamese.dyndns.org","threadId":"30243","inReplyTo":"20120415195231.GB1960@ecki","subject":"Re: [PATCH] t5570: use explicit push refspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-15T20:18:03Z","receivedAt":"2012-04-15T20:18:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> In my opinion, this change is also an improvement in itself, since now\n> we can more easily pass extra arguments to test_remote_error. Maybe the\n> scratching of heads can be alleviated by amending the commit message\n> like so?\n>\n> -->o--\n> Subject: [PATCH] t5570: use explicit push refspec\n>\n> The default mode for push without arguments will change. Some warnings\n> are about to be enabled for such use, which causes some t5570 tests to\n> fail because they do not expect this output.\n>\n> Fix this by passing an explicit refspec to git push. To that end, change\n> the calling conventions of test_remote_error in order to accomodate\n> extra command arguments.\n>\n> Signed-off-by: Clemens Buchacher <drizzd@aon.at>\n\nSounds good.  Thanks.\n"},{"id":"189418","messageId":"4F8C3ED6.5050802@in.waw.pl","threadId":"30243","inReplyTo":"20120415115322.GA11786@ecki","subject":"Re: [PATCH v3] git-daemon wrapper to wait until daemon is ready","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-04-16T15:46:30Z","receivedAt":"2012-04-16T15:46:30Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 04/15/2012 01:53 PM, Clemens Buchacher wrote:\n> The shell script which is currently used to parse git daemon output does\n> not seem to work reliably. In order to work around such issues,\n> re-implement the same procedure in C and write the daemon pid to a file.\n>\n> This means that we can no longer wait on the daemon process, since it is\n> no longer a direct child of the shell process.\n>\n> Signed-off-by: Clemens Buchacher<drizzd@aon.at>\n> ---\n\n> Here's the re-roll for completeness. Still waiting on feedback from the\n> OP if this actually solves the problem, though.\nI posted a reply in the other thread, but I'm replying here too for \ncompleteness: yes, the problem is solved.\n\nThanks,\nZbyszek\n"},{"id":"189706","messageId":"CAPc5daX7aQhuez+Q28s5jMcZzofxDvA-g1nnnEA_9tZFCJm-gw@mail.gmail.com","threadId":"30243","inReplyTo":"20120415115322.GA11786@ecki","subject":"Re: [PATCH v3] git-daemon wrapper to wait until daemon is ready","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-19T15:00:04Z","receivedAt":"2012-04-19T15:00:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"2012/4/15 Clemens Buchacher <drizzd@aon.at>\n>\n> The shell script which is currently used to parse git daemon output does\n> not seem to work reliably. In order to work around such issues,\n> re-implement the same procedure in C and write the daemon pid to a file.\n> ...\n> +       strbuf_getwholeline_fd(&line, proc.err, '\\n');\n> +       fprintf(stderr, line.buf);\n\nJust a note. I'll update this part with \"fputs(line.buf, stderr)\".\n"}]}