diff mbox series

[2/4] t/lib-http.sh: add functions related to serve-git.py

Message ID 20240612115028.1169183-3-cmn@dwim.me (mailing list archive)
State New, archived
Headers show
Series Report rejections over HTTP when the remote rejects during the transfer | expand

Commit Message

Carlos Martín Nieto June 12, 2024, 11:50 a.m. UTC
These functions manage the custom git serving script for use in tests.

Signed-off-by: Carlos Martín Nieto <cmn@dwim.me>
---
 t/lib-httpd.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Jeff King June 13, 2024, 9:19 a.m. UTC | #1
On Wed, Jun 12, 2024 at 01:50:26PM +0200, Carlos Martín Nieto wrote:

> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index d83bafeab32..6454300a041 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> [...]
> +start_serve_git() {

I can see why you'd stick this in lib-httpd.sh. But note that we'll bail
from that script's setup phase if we don't find Apache. That's not the
end of the world, but it does mean we'd fail to run this test on
platforms that otherwise could.

> +	test_atexit stop_serve_git

OK, you do the auto-kill on exit, which is good.

> +	"$TEST_DIRECTORY"/lib-httpd/serve-git.py \
> +		--document-root "$HTTPD_ROOT_PATH"/www \
> +		--port "$LIB_GIT_SERVE_PORT" &
> +
> +	mkdir -p "$HTTPD_ROOT_PATH"
> +	echo $! >"$HTTPD_ROOT_PATH"/git-serve.pid
> +
> +	GIT_SERVE_DEST=127.0.0.1:$LIB_GIT_SERVE_PORT
> +	GIT_SERVE_URL=http://$GIT_SERVE_DEST
> +}

But I suspect this part is racy. We started serve-git.py in the
background, but we have no guarantee that it finished starting up, or
even started listening on the port.

We've run into those kinds of races with git-daemon; you can find the
gross fifo-based solution in lib-git-daemon.sh. We don't do anything
special for apache, but I think that's because we depend on "apache -k
start" to handle this (we don't background it ourselves).

> +stop_serve_git() {
> +	kill -9 $(cat "$HTTPD_ROOT_PATH"/git-serve.pid)
> +}

This looks reasonable. You probably want to redirect stderr to
/dev/null, since any script which calls stop_serve_git() itself will
double-kill and see an error on the second one.

-Peff
diff mbox series

Patch

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index d83bafeab32..6454300a041 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -94,6 +94,8 @@  esac
 LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}
 test_set_port LIB_HTTPD_PORT
 
+test_set_port LIB_GIT_SERVE_PORT
+
 TEST_PATH="$TEST_DIRECTORY"/lib-httpd
 HTTPD_ROOT_PATH="$PWD"/httpd
 HTTPD_DOCUMENT_ROOT_PATH=$HTTPD_ROOT_PATH/www
@@ -250,6 +252,24 @@  stop_httpd() {
 		-f "$TEST_PATH/apache.conf" $HTTPD_PARA -k stop
 }
 
+start_serve_git() {
+	test_atexit stop_serve_git
+
+	"$TEST_DIRECTORY"/lib-httpd/serve-git.py \
+		--document-root "$HTTPD_ROOT_PATH"/www \
+		--port "$LIB_GIT_SERVE_PORT" &
+
+	mkdir -p "$HTTPD_ROOT_PATH"
+	echo $! >"$HTTPD_ROOT_PATH"/git-serve.pid
+
+	GIT_SERVE_DEST=127.0.0.1:$LIB_GIT_SERVE_PORT
+	GIT_SERVE_URL=http://$GIT_SERVE_DEST
+}
+
+stop_serve_git() {
+	kill -9 $(cat "$HTTPD_ROOT_PATH"/git-serve.pid)
+}
+
 test_http_push_nonff () {
 	REMOTE_REPO=$1
 	LOCAL_REPO=$2