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

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

[Tarmigan Casebolt]

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(-)

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

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

[Tarmigan Casebolt]

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(-)

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

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

[Jay Soffian]

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.

> +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.

> +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.

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

[Tarmigan]

On Thu, Nov 19, 2009 at 7:14 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> 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.

>> +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.

>> +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

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

[Junio C Hamano]

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.

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

[Tarmigan]

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?

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

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() {

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

[Clemens Buchacher]

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

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

[Junio C Hamano]

Tarmigan <tarmigan+git@gmail.com> writes:

> 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.

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

[Junio C Hamano]

Clemens Buchacher <drizzd@aon.at> writes:

> 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.

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

[Tarmigan Casebolt]

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(-)

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