git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: Johannes Sixt <j6t@kdbg.org>
Cc: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
	git@vger.kernel.org, gitster@pobox.com,
	"Jeff King" <peff@peff.net>
Subject: [PATCH v3] git-daemon wrapper to wait until daemon is ready
Date: Sun, 15 Apr 2012 13:53:22 +0200	[thread overview]
Message-ID: <20120415115322.GA11786@ecki> (raw)
In-Reply-To: <20120414220606.GA18137@ecki>

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
no longer a direct child of the shell process.

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

On Sun, Apr 15, 2012 at 12:06:06AM +0200, Clemens Buchacher wrote:
> On Sat, Apr 14, 2012 at 09:36:38PM +0200, Johannes Sixt wrote:
> > Am 14.04.2012 20:29, schrieb Clemens Buchacher:
> > > +	r = read_line(proc.err, buf, sizeof(buf));
> > 
> > We have strbuf_getwholeline_fd().
> 
> Thanks. Will fix.

Here's the re-roll for completeness. Still waiting on feedback from the
OP if this actually solves the problem, though.
 
 .gitignore          |    1 +
 Makefile            |    1 +
 t/lib-git-daemon.sh |   30 ++++---------------------
 test-git-daemon.c   |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 68 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/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..f44fa6a
--- /dev/null
+++ b/test-git-daemon.c
@@ -0,0 +1,62 @@
+#include "git-compat-util.h"
+#include "run-command.h"
+#include "exec_cmd.h"
+#include "strbuf.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)
+{
+	struct strbuf line = STRBUF_INIT;
+	FILE *fp;
+	struct child_process proc, cat;
+	char *cat_argv[] = { "cat", NULL };
+
+	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;
+
+	strbuf_getwholeline_fd(&line, proc.err, '\n');
+	fprintf(stderr, line.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(line.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;
+}
-- 
1.7.9.6

  reply	other threads:[~2012-04-15 11:54 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-14 18:29 [PATCH] git-daemon wrapper to wait until daemon is ready Clemens Buchacher
2012-04-14 18:32 ` [PATCH] t5570: use explicit push refspec Clemens Buchacher
2012-04-14 23:40   ` Junio C Hamano
2012-04-15  0:11     ` Clemens Buchacher
2012-04-15 19:20       ` Junio C Hamano
2012-04-15 19:52         ` Clemens Buchacher
2012-04-15 20:18           ` Junio C Hamano
2012-04-14 18:43 ` [PATCH] git-daemon wrapper to wait until daemon is ready Ben Walton
2012-04-14 19:00   ` Clemens Buchacher
2012-04-14 19:16     ` [PATCH v2] " Clemens Buchacher
2012-04-14 19:36 ` [PATCH] " Johannes Sixt
2012-04-14 22:06   ` Clemens Buchacher
2012-04-15 11:53     ` Clemens Buchacher [this message]
2012-04-16 15:46       ` [PATCH v3] " Zbigniew Jędrzejewski-Szmek
2012-04-19 15:00       ` Junio C Hamano
2012-04-15 17:11     ` [PATCH] " Johannes Sixt
2012-04-15 19:32       ` Clemens Buchacher
2012-04-15 19:57         ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120415115322.GA11786@ecki \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=peff@peff.net \
    --cc=zbyszek@in.waw.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).