threads / patch / 26056

patch, 14 partslib-git-svn.sh: Add check for missing mod_dav_svn module

Subject: [PATCH 04/14] lib-git-svn.sh: Add check for missing mod_dav_svn module

## tl;dr

3 messages between Dec 14, 2010 and Dec 16, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Ramsay Jones· Dec 14, 2010, 18:23 UTC · lore

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 t/lib-git-svn.sh |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

Show changes to t/lib-git-svn.sh +6 −0

diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 919d45a..154f3d3 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -101,6 +101,11 @@ start_httpd () {
 		echo >&2 'SVN_HTTPD_PORT is not defined!'
 		return
 	fi
+	if test ! -e "$SVN_HTTPD_MODULE_PATH/mod_dav_svn.so"
+	then
+		echo >&2 'Apache module "mod_dav_svn.so" not found'
+		return 1
+	fi
 	if test -z "$repo_base_path"
 	then
 		repo_base_path=svn
@@ -128,6 +133,7 @@ EOF
 
 stop_httpd () {
 	test -z "$SVN_HTTPD_PORT" && return
+	test ! -e "$GIT_DIR/httpd.conf" && return
 	"$SVN_HTTPD_PATH" -f "$GIT_DIR"/httpd.conf -k stop
 }

-- 
1.7.3

Junio C Hamano· Dec 14, 2010, 19:11 UTC · re: Ramsay Jones · lore

Re: [PATCH 04/14] lib-git-svn.sh: Add check for missing mod_dav_svn module

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

Show 18 quoted lines

> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>  t/lib-git-svn.sh |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
> index 919d45a..154f3d3 100644
> --- a/t/lib-git-svn.sh
> +++ b/t/lib-git-svn.sh
> @@ -101,6 +101,11 @@ start_httpd () {
>  		echo >&2 'SVN_HTTPD_PORT is not defined!'
>  		return
>  	fi
> +	if test ! -e "$SVN_HTTPD_MODULE_PATH/mod_dav_svn.so"
> +	then
> +		echo >&2 'Apache module "mod_dav_svn.so" not found'
> +		return 1
> +	fi

Others seem to check with "test -f" for things like this. Also why "return 1" only on this codepath?

Show 13 quoted lines

>  	if test -z "$repo_base_path"
>  	then
>  		repo_base_path=svn
> @@ -128,6 +133,7 @@ EOF
>  
>  stop_httpd () {
>  	test -z "$SVN_HTTPD_PORT" && return
> +	test ! -e "$GIT_DIR/httpd.conf" && return
>  	"$SVN_HTTPD_PATH" -f "$GIT_DIR"/httpd.conf -k stop
>  }
>  
> -- 
> 1.7.3

Ramsay Jones· Dec 16, 2010, 20:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 04/14] lib-git-svn.sh: Add check for missing mod_dav_svn module

Junio C Hamano wrote:

Show 16 quoted lines

> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>> diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
>> index 919d45a..154f3d3 100644
>> --- a/t/lib-git-svn.sh
>> +++ b/t/lib-git-svn.sh
>> @@ -101,6 +101,11 @@ start_httpd () {
>>  		echo >&2 'SVN_HTTPD_PORT is not defined!'
>>  		return
>>  	fi
>> +	if test ! -e "$SVN_HTTPD_MODULE_PATH/mod_dav_svn.so"
>> +	then
>> +		echo >&2 'Apache module "mod_dav_svn.so" not found'
>> +		return 1
>> +	fi
> 
> Others seem to check with "test -f" for things like this.

OK, do you want me to send an updated patch?

> Also why "return 1" only on this codepath?

This test, along with t9115-*.sh, t9118-*.sh and t9120-*.sh, have been written in such a way that it can access the svn repo using either file or http urls. By default (SVN_HTTPD_PORT not set), it uses file urls so that, despite it noisily printing to stderr that "SVN_HTTPD_PORT is not defined!", lack of an Apache installation is not a problem. (In fact, I suspect very few people run it with SVN_HTTPD_PORT defined).

So, the initial test (of SVN_HTTPD_PORT) and return is not particularly noteworthy, let alone an error. (Yes I have a patch to remove the message, see below). However, *if* the user requests the test be run with http urls and the mod_sav_svn.so module is missing, then we want to fail the test noisily; thus we return 1.

Note that we could easily reset SVN_HTTPD_PORT and return, thus silently converting to the "use file urls" case, but I don't think that would be acceptable.

The reason I didn't submit the patch to remove the above message is
because I felt I should add a note about how/why you would use the
SVN_HTTPD_PORT variable to the t/README file. However, that started
me thinking about other similar variables like:
    SVN_HTTPD_{PATH,MODULE_PATH}, LIB_HTTPD_{PATH,PORT,SSL,DAV,SVN},
    GIT_SVN_NO_OPTIMIZE_COMMITS, GIT_TEST_HTTPD, .....
So, that patch never got finished! :-P

ATB, Ramsay Jones