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

[Ramsay Jones <ramsay@ramsay1.demon.co.uk>]

Junio C Hamano wrote:
> 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