[PATCH v2] git-daemon wrapper to wait until daemon is ready

[Clemens Buchacher <drizzd@aon.at>]

The shell script which is currently used to parse git daemon output does
not seem to work reliably. In order to work around such issues,
re-implement the same procedure in C and write the daemon pid to a file.

This means that we can no longer wait on the daemon process, since it is
not a direct child of the shell process.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

> On Sat, Apr 14, 2012 at 02:43:41PM -0400, Ben Walton wrote:
> > 
> > Presumably you mean "work reliably" here?

I fixed the commit message. I also noticed that the buffer returned by
read_line had no zero terminator. And the parse_error variable was
unnecessary.

 .gitignore          |    1 +
 Makefile            |    1 +
 cache.h             |    1 +
 t/lib-git-daemon.sh |   30 +++-------------------
 test-git-daemon.c   |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++
 wrapper.c           |   21 ++++++++++++++++
 6 files changed, 97 insertions(+), 26 deletions(-)
 create mode 100644 test-git-daemon.c

diff --git a/.gitignore b/.gitignore
index 87fcc5f..18a484c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
 /test-dump-cache-tree
 /test-scrap-cache-tree
 /test-genrandom
+/test-git-daemon
 /test-index-version
 /test-line-buffer
 /test-match-trees
diff --git a/Makefile b/Makefile
index be1957a..7317daa 100644
--- a/Makefile
+++ b/Makefile
@@ -477,6 +477,7 @@ TEST_PROGRAMS_NEED_X += test-delta
 TEST_PROGRAMS_NEED_X += test-dump-cache-tree
 TEST_PROGRAMS_NEED_X += test-scrap-cache-tree
 TEST_PROGRAMS_NEED_X += test-genrandom
+TEST_PROGRAMS_NEED_X += test-git-daemon
 TEST_PROGRAMS_NEED_X += test-index-version
 TEST_PROGRAMS_NEED_X += test-line-buffer
 TEST_PROGRAMS_NEED_X += test-match-trees
diff --git a/cache.h b/cache.h
index e5e1aa4..6351c15 100644
--- a/cache.h
+++ b/cache.h
@@ -1176,6 +1176,7 @@ extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg
 extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg);
 extern void fsync_or_die(int fd, const char *);
 
+extern ssize_t read_line(int fd, void *buf, size_t count);
 extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 static inline ssize_t write_str_in_full(int fd, const char *str)
diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index ef2d01f..9fefae1 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -23,27 +23,13 @@ start_git_daemon() {
 	trap 'code=$?; stop_git_daemon; (exit $code); die' EXIT
 
 	say >&3 "Starting git daemon ..."
-	mkfifo git_daemon_output
-	git daemon --listen=127.0.0.1 --port="$LIB_GIT_DAEMON_PORT" \
+	test-git-daemon --listen=127.0.0.1 --port="$LIB_GIT_DAEMON_PORT" \
 		--reuseaddr --verbose \
 		--base-path="$GIT_DAEMON_DOCUMENT_ROOT_PATH" \
 		"$@" "$GIT_DAEMON_DOCUMENT_ROOT_PATH" \
-		>&3 2>git_daemon_output &
-	GIT_DAEMON_PID=$!
-	{
-		read line
-		echo >&4 "$line"
-		cat >&4 &
-
-		# Check expected output
-		if test x"$(expr "$line" : "\[[0-9]*\] \(.*\)")" != x"Ready to rumble"
-		then
-			kill "$GIT_DAEMON_PID"
-			wait "$GIT_DAEMON_PID"
-			trap 'die' EXIT
-			error "git daemon failed to start"
-		fi
-	} <git_daemon_output
+		>&3 2>&4 ||
+		error "git daemon failed to start"
+	GIT_DAEMON_PID=$(cat git-daemon.pid)
 }
 
 stop_git_daemon() {
@@ -57,13 +43,5 @@ stop_git_daemon() {
 	# kill git-daemon child of git
 	say >&3 "Stopping git daemon ..."
 	kill "$GIT_DAEMON_PID"
-	wait "$GIT_DAEMON_PID" >&3 2>&4
-	ret=$?
-	# expect exit with status 143 = 128+15 for signal TERM=15
-	if test $ret -ne 143
-	then
-		error "git daemon exited with status: $ret"
-	fi
 	GIT_DAEMON_PID=
-	rm -f git_daemon_output
 }
diff --git a/test-git-daemon.c b/test-git-daemon.c
new file mode 100644
index 0000000..82a9a8f
--- /dev/null
+++ b/test-git-daemon.c
@@ -0,0 +1,69 @@
+#include "git-compat-util.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+#include "strbuf.h"
+#include "cache.h"
+#include <string.h>
+#include <errno.h>
+
+static int parse_daemon_output(char *s)
+{
+	if (*s++ != '[')
+		return 1;
+	s = strchr(s, ']');
+	if (!s)
+		return 1;
+	if (strcmp(s, "] Ready to rumble\n"))
+		return 1;
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	FILE *fp;
+	struct child_process proc, cat;
+	char *cat_argv[] = { "cat", NULL };
+	char buf[PATH_MAX];
+	int r;
+
+	setup_path();
+
+	memset(&proc, 0, sizeof(proc));
+	argv[0] = "git-daemon";
+	proc.argv = (const char **)argv;
+	proc.no_stdin = 1;
+	proc.err = -1;
+
+	if (start_command(&proc) < 0)
+		return 1;
+
+	r = read_line(proc.err, buf, sizeof(buf)-1);
+	if (r < 0) {
+		finish_command(&proc);
+		return 1;
+	}
+	buf[r] = '\0';
+	fprintf(stderr, "%s", buf);
+
+	memset(&cat, 0, sizeof(cat));
+	cat.argv = (const char **)cat_argv;
+	cat.in = proc.err;
+	cat.out = 2;
+
+	if (start_command(&cat) < 0)
+		return 1;
+
+	if (parse_daemon_output(buf)) {
+		kill(proc.pid, SIGTERM);
+		finish_command(&proc);
+		finish_command(&cat);
+		return 1;
+	}
+
+	fp = fopen("git-daemon.pid", "w");
+	fprintf(fp, "%"PRIuMAX"\n", (uintmax_t)proc.pid);
+	fclose(fp);
+
+	return 0;
+}
diff --git a/wrapper.c b/wrapper.c
index 85f09df..7bf6dda 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -141,6 +141,27 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
 	}
 }
 
+ssize_t read_line(int fd, void *buf, size_t count)
+{
+	char *p = buf;
+	ssize_t total = 0;
+
+	while (count > 0) {
+		ssize_t loaded = xread(fd, p, 1);
+		if (loaded < 0)
+			return -1;
+		if (loaded == 0)
+			return total;
+		count -= loaded;
+		total += loaded;
+		if (*p == '\n')
+			break;
+		p += loaded;
+	}
+
+	return total;
+}
+
 ssize_t read_in_full(int fd, void *buf, size_t count)
 {
 	char *p = buf;
-- 
1.7.9.6