threads / patch / 21679

patch, 2 partst/lib-http.sh: Restructure finding of default httpd location

Subject: [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location

## tl;dr

10 messages between Nov 20, 2009 and Jan 2, 2010. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Tarmigan Casebolt· Nov 20, 2009, 01:22 UTC · lore

On my machine with CentOS, httpd is located at /usr/sbin/httpd, and the modules are located at /usr/lib64/httpd/modules. To enable easy testing of httpd, we would like those locations to be detected automatically.

uname might not be the best way to determine the default location for httpd since different Linux distributions apparently put httpd in different places, so we test a couple different locations for httpd, and use the first one that we come across. We do the same for the modules directory.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---

Would any machines have httpd or the modules/ directory in several of these locations?

Also I don't really know shell scripting, so while this Works For Me, it may be completely wrong.

 t/lib-httpd.sh |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)
Show changes to t/lib-httpd.sh +13 −6
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 6765b08..6b86353 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -12,16 +12,23 @@ fi
 
 HTTPD_PARA=""
 
+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
+do
+	test -x "$DEFAULT_HTTPD_PATH" && break
+done
+
+for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
+                                 '/usr/lib/apache2/modules' \
+                                 '/usr/lib64/httpd/modules' \
+                                 '/usr/lib/httpd/modules'
+do
+	test -d "$DEFAULT_HTTPD_MODULE_PATH" && break
+done
+
 case $(uname) in
 	Darwin)
-		DEFAULT_HTTPD_PATH='/usr/sbin/httpd'
-		DEFAULT_HTTPD_MODULE_PATH='/usr/libexec/apache2'
 		HTTPD_PARA="$HTTPD_PARA -DDarwin"
 	;;
-	*)
-		DEFAULT_HTTPD_PATH='/usr/sbin/apache2'
-		DEFAULT_HTTPD_MODULE_PATH='/usr/lib/apache2/modules'
-	;;
 esac
 
 LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}

-- 
1.6.5.52.g35487
Tarmigan Casebolt· Nov 20, 2009, 01:22 UTC · re: Tarmigan Casebolt · lore

[PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.

With smart http, git over http is likely to become much more common. To increase testing of smart http, enable the http tests by default.

If we cannot detect httpd, we still skip these tests, so it should not cause problems on platforms where we cannot run the tests.

Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
 t/lib-httpd.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Show changes to t/lib-httpd.sh +4 −3
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 6b86353..db537b4 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -3,11 +3,12 @@
 # Copyright (c) 2008 Clemens Buchacher <drizzd@aon.at>
 #
 
-if test -z "$GIT_TEST_HTTPD"
+if test -n "$NO_GIT_TEST_HTTPD"
 then
-	say "skipping test, network testing disabled by default"
-	say "(define GIT_TEST_HTTPD to enable)"
+	say "Skipping http tests because NO_GIT_TEST_HTTPD is defined"
 	test_done
+else
+	say "Define NO_GIT_TEST_HTTPD to disable http testing"
 fi
 
 HTTPD_PARA=""

-- 
1.6.5.52.g35487
Junio C Hamano· Nov 20, 2009, 08:03 UTC · re: Tarmigan Casebolt · lore

Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.

Tarmigan Casebolt <tarmigan+git@gmail.com> writes:
> With smart http, git over http is likely to become much more common.
> To increase testing of smart http, enable the http tests by default.

Sorry, but no test that listens to network ports should be enabled by default; otherwise it will break automated, unattended tests people have already set up randomly, depending on when the port happens to be available for use by the tests.

Tarmigan· Nov 20, 2009, 19:03 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.

On Fri, Nov 20, 2009 at 12:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Tarmigan <tarmigan+git@gmail.com> writes:
>
>> With smart http, git over http is likely to become much more common.
>> To increase testing of smart http, enable the http tests by default.
>
> Sorry, but no test that listens to network ports should be enabled by
> default; otherwise it will break automated, unattended tests people have
> already set up randomly, depending on when the port happens to be
> available for use by the tests.
Is this the only concern or are there security or other issues as well?
If that is the only concern, we could have the tests automatically
fall back to listening on a different port.  Even if we didn't, if
httpd cannot startup because it can't bind to the port, the http tests
say
* skipping test, web server setup failed
and exit with test_done before any of the tests actually fail.

Here's a patch (cut-n-paste so it will probably be munged) for discussion of the port-fallback idea. If httpd cannot bind to 5541, it tries 15541 etc. You can test this by running "nc -l 5541 &" before the test. If this approach might be acceptable, I can send a properly formatted patch.

Comments?

Thanks, Tarmigan

Show changes to t/lib-httpd.sh +25 −12
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 797a2d6..a8eb6fa 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -77,7 +77,7 @@ prepare_httpd() {

        if test -n "$LIB_HTTPD_SSL"
        then
-               HTTPD_URL=https://127.0.0.1:$LIB_HTTPD_PORT
+               HTTPD_URL=https://127.0.0.1

                RANDFILE_PATH="$HTTPD_ROOT_PATH"/.rnd openssl req \
                        -config "$TEST_PATH/ssl.cnf" \
@@ -88,7 +88,7 @@ prepare_httpd() {
                export GIT_SSL_NO_VERIFY
                HTTPD_PARA="$HTTPD_PARA -DSSL"
        else
-               HTTPD_URL=http://127.0.0.1:$LIB_HTTPD_PORT
+               HTTPD_URL=http://127.0.0.1
        fi

        if test -n "$LIB_HTTPD_DAV" -o -n "$LIB_HTTPD_SVN"
@@ -109,16 +109,29 @@ start_httpd() {

        trap 'code=$?; stop_httpd; (exit $code); die' EXIT

-       "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \
-               -f "$TEST_PATH/apache.conf" $HTTPD_PARA \
-               -c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start \
-               >&3 2>&4
-       if test $? -ne 0
-       then
-               say "skipping test, web server setup failed"
-               trap 'die' EXIT
-               test_done
-       fi
+       while true
+       do
+               "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \
+                       -f "$TEST_PATH/apache.conf" $HTTPD_PARA \
+                       -c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start \
+                       >&3 2>&4
+               if test $? -ne 0
+               then
+                       if test $LIB_HTTPD_PORT -gt 40000
+                       then
+                               say "skipping test, web server setup failed"
+                               trap 'die' EXIT
+                               test_done
+                       fi
+                       LIB_HTTPD_PORT=$(($LIB_HTTPD_PORT + 10000))
+                       say "trying port $LIB_HTTPD_PORT"
+                       continue
+               else
+                       break
+               fi
+       done
+
+       HTTPD_URL="$HTTPD_URL:$LIB_HTTPD_PORT"
 }

 stop_httpd() {
Clemens Buchacher· Nov 20, 2009, 20:11 UTC · re: Tarmigan · lore

Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.

On Fri, Nov 20, 2009 at 11:03:13AM -0800, Tarmigan wrote:
> Here's a patch (cut-n-paste so it will probably be munged) for
> discussion of the port-fallback idea.  If httpd cannot bind to 5541,
> it tries 15541 etc.

I would prefer if we skip the test right away. If we really want to try different ports, we should first check that the port really is the problem. Otherwise, the test will uselessly retry several times. Apache 2 writes

 (98)Address already in use: make_sock: could not bind to address
 127.0.0.1:5541

to stderr, which we could use to detect that error condition. But other web servers are bound to behave differently.

Clemens
Junio C Hamano· Nov 20, 2009, 20:56 UTC · re: Clemens Buchacher · lore

Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.

Clemens Buchacher <drizzd@aon.at> writes:
Show 7 quoted lines
> On Fri, Nov 20, 2009 at 11:03:13AM -0800, Tarmigan wrote:
>
>> Here's a patch (cut-n-paste so it will probably be munged) for
>> discussion of the port-fallback idea.  If httpd cannot bind to 5541,
>> it tries 15541 etc.
>
> I would prefer if we skip the test right away.

Retrying is a different issue, and when tests are enabled I think it is Ok to retry all you want.

But I don't want to see them enabled unless the user explicitly told us to.

Junio C Hamano· Nov 20, 2009, 20:54 UTC · re: Tarmigan · lore

Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.

Tarmigan <tarmigan+git@gmail.com> writes:
Show 12 quoted lines
> On Fri, Nov 20, 2009 at 12:03 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Tarmigan <tarmigan+git@gmail.com> writes:
>>
>>> With smart http, git over http is likely to become much more common.
>>> To increase testing of smart http, enable the http tests by default.
>>
>> Sorry, but no test that listens to network ports should be enabled by
>> default; otherwise it will break automated, unattended tests people have
>> already set up randomly, depending on when the port happens to be
>> available for use by the tests.
>
> Is this the only concern or are there security or other issues as well?
I thought security was too obvious to mention.
Jay Soffian· Nov 20, 2009, 03:14 UTC · re: Tarmigan Casebolt · lore

Re: [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location

On Thu, Nov 19, 2009 at 8:22 PM, Tarmigan Casebolt <tarmigan+git@gmail.com> wrote:

Show 5 quoted lines
> uname might not be the best way to determine the default location for
> httpd since different Linux distributions apparently put httpd in
> different places, so we test a couple different locations for httpd,
> and use the first one that we come across.  We do the same for the
> modules directory.

Perhaps testing the distribution and looking in the known location for that distribution then? That said, going through a list of well known locations should work too.

> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
> +do
> +       test -x "$DEFAULT_HTTPD_PATH" && break
> +done

Unfortunately this leaves DEFAULT_HTTPD_PATH as the last item in the list even if the test does not pass. You can add an empty item to the end of the list if you want to do this way.

Show 7 quoted lines
> +for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
> +                                 '/usr/lib/apache2/modules' \
> +                                 '/usr/lib64/httpd/modules' \
> +                                 '/usr/lib/httpd/modules'
> +do
> +       test -d "$DEFAULT_HTTPD_MODULE_PATH" && break
> +done
Ditto.
j.
Tarmigan· Nov 20, 2009, 03:30 UTC · re: Jay Soffian · lore

Re: [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd location

On Thu, Nov 19, 2009 at 7:14 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
Show 11 quoted lines
> On Thu, Nov 19, 2009 at 8:22 PM, Tarmigan Casebolt
> <tarmigan+git@gmail.com> wrote:
>> uname might not be the best way to determine the default location for
>> httpd since different Linux distributions apparently put httpd in
>> different places, so we test a couple different locations for httpd,
>> and use the first one that we come across.  We do the same for the
>> modules directory.
>
> Perhaps testing the distribution and looking in the known location for
> that distribution then? That said, going through a list of well known
> locations should work too.

Is there a nice way to test the distribution? Seems to me like doing that might be more complicated and also more fragile.

Show 8 quoted lines
>> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
>> +do
>> +       test -x "$DEFAULT_HTTPD_PATH" && break
>> +done
>
> Unfortunately this leaves DEFAULT_HTTPD_PATH as the last item in the
> list even if the test does not pass. You can add an empty item to the
> end of the list if you want to do this way.

Yes. I think this is how it was before though too, and it is caught later in the script with the LIB_HTTPD_PATH setting and testing.

Show 9 quoted lines
>> +for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
>> +                                 '/usr/lib/apache2/modules' \
>> +                                 '/usr/lib64/httpd/modules' \
>> +                                 '/usr/lib/httpd/modules'
>> +do
>> +       test -d "$DEFAULT_HTTPD_MODULE_PATH" && break
>> +done
>
> Ditto.

Yes. Again, this is still more thorough than before, but in this case the script does not check later. Perhaps the script should test this value and test_done if it's not a directory?

Thanks, Tarmigan

Tarmigan Casebolt· Jan 2, 2010, 22:04 UTC · re: Tarmigan · lore

[PATCH v2] t/lib-http.sh: Restructure finding of default httpd location

On CentOS 5, httpd is located at /usr/sbin/httpd, and the modules are located at /usr/lib64/httpd/modules. To enable easy testing of httpd, we would like those locations to be detected automatically.

uname might not be the best way to determine the default location for httpd since different Linux distributions apparently put httpd in different places, so we test a couple different locations for httpd, and use the first one that we come across. We do the same for the modules directory.

cc: Jay Soffian <jaysoffian@gmail.com>
Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com>
---
Jay was concerned about the final fallthrough cases for testing these
lists.  I have added a test for the modules directory and the existing
tests later in the script already test that the apache executable
exists.  If either cannot be found, we do test_done.
Would any machines have httpd or the modules/ directory in several of
these locations?
---
 t/lib-httpd.sh |   30 ++++++++++++++++++++++++------
 1 files changed, 24 insertions(+), 6 deletions(-)
Show changes to t/lib-httpd.sh +24 −6
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 6765b08..27b466b 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -12,16 +12,29 @@ fi
 
 HTTPD_PARA=""
 
+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
+do
+	if test -x "$DEFAULT_HTTPD_PATH"
+	then
+	        break
+	fi
+done
+
+for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
+                                 '/usr/lib/apache2/modules' \
+                                 '/usr/lib64/httpd/modules' \
+                                 '/usr/lib/httpd/modules'
+do
+        if test -d "$DEFAULT_HTTPD_MODULE_PATH"
+	then
+	        break
+	fi
+done
+
 case $(uname) in
 	Darwin)
-		DEFAULT_HTTPD_PATH='/usr/sbin/httpd'
-		DEFAULT_HTTPD_MODULE_PATH='/usr/libexec/apache2'
 		HTTPD_PARA="$HTTPD_PARA -DDarwin"
 	;;
-	*)
-		DEFAULT_HTTPD_PATH='/usr/sbin/apache2'
-		DEFAULT_HTTPD_MODULE_PATH='/usr/lib/apache2/modules'
-	;;
 esac
 
 LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}
@@ -49,6 +62,11 @@ then
 			say "skipping test, at least Apache version 2 is required"
 			test_done
 		fi
+		if ! test -d "$DEFAULT_HTTPD_MODULE_PATH"
+		then
+			say "Apache module directory not found.  Skipping tests."
+			test_done
+		fi
 
 		LIB_HTTPD_MODULE_PATH="$DEFAULT_HTTPD_MODULE_PATH"
 	fi

-- 
1.6.6.236.gc56f3

← back to recent threads