Make httpd tests run

This series grew from an attempt at enlarging my personal test run coverage on a standard Fedora 21 64bit box. Aka chain-lint fall-out.

With 1/3, I get all httpd tests to run (when port is set, of course).

2/3 and 3/3 are an attempt at getting git-svn over http tests to run. 2/3 is certainly correct but not sufficient.
3/3 gets httpd to run but svn does not connect. This is WIP and RFH, and maybe requires rewriting lib-git-svn to use a config which depends on the apache version (like lib-hhtpd does), or to leverage lib-httpd.

Michael J Gruber (3):
t/lib-httpd: load mod_unixd
t/lib-git-svn: check same httpd module dirs as lib-httpd t/lib-git-svn: adjust config to apache 2.4

 t/lib-git-svn.sh        | 7 +++++--
 t/lib-httpd/apache.conf | 3 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

2.4.0.rc1.221.gf7021fb

With the current config, apache 2.4.10 will not be started and the httpd tests will not run.

Enable mod_unixd to make the httpd tests run.

---

Notes:
Basically, this is required since the switch from 2.2 to 2.4. I'm wondering whether noone is running these tests, or other distros (than Fedora) use different default configs.

 t/lib-httpd/apache.conf | 3 +++
 1 file changed, 3 insertions(+)

index 0b81a00..7b8e21c 100644
@@ -64,6 +64,9 @@ LockFile accept.lock
 <IfModule !mod_mpm_prefork.c>
 	LoadModule mpm_prefork_module modules/mod_mpm_prefork.so
 </IfModule>
+<IfModule !mod_unixd.c>
+	LoadModule unixd_module modules/mod_unixd.so
+</IfModule>
 </IfVersion>
 
 PassEnv GIT_VALGRIND

2.4.0.rc1.221.gf7021fb

Torsten Bögershausen venit, vidit, dixit 08.04.2015 23:47:

Is this with apache 2.4?

In my case (apache 2.4, fedora 21), t/lib-http.sh failed to start the web server, so the tests above would (succeed trivially but) not run.

Maybe debian is using an apache default config that has mod_unixd built in, to ease the transition from apache 2.2? Many were bitten by these new requirements after the switch to 2.4, and some distros may have worked around that.

Do your tests run with this patch applied?

Michael

Yes, this is exactly it. On my Debian system:

  $ apache2 -l
  Compiled in modules:
    core.c
    mod_so.c
    mod_watchdog.c
    http_core.c
    mod_log_config.c
    mod_logio.c
    mod_version.c
    mod_unixd.c

Yes. I think your patch is fine, as the IfModule means we handle both the built-in and dynamic cases.

-Peff

The changes in this series appear to break compatibility with Apache 2.2.

Does that mean that Apache 2.2 is no longer supported for running these tests?

Or am I missing something here...

-Kyle

Never mind. I see the <IfVersion...> they're wrapped in now. Would have been nice if they were indented so there was a hint about that in the diff, or the diff included enough context to see that. But that file formatting has nothing to do with your change.

Sorry for the noise.

-Kyle

The current config is tailored to apache 2.2. Apache 2.4 fails to start with it.

Adjust the config to apache 2.4. [still incomplete]

---

Notes:
This is WIP and RFH. Apache does not start without mpm, and the lock mechanism has changed. It will run with these changes, but svn fails to connect.

Again, I'm wondering who is running these tests at all, but also:
Why do we leverage lib-httpd so little from lib-git-svn?

 t/lib-git-svn.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

index 2a0ef07..24ad8b4 100644
@@ -117,10 +117,21 @@ ServerName "git svn test"
 ServerRoot "$GIT_DIR"
 DocumentRoot "$GIT_DIR"
 PidFile "$GIT_DIR/httpd.pid"
-LockFile logs/accept.lock
 Listen 127.0.0.1:$SVN_HTTPD_PORT
 LoadModule dav_module $SVN_HTTPD_MODULE_PATH/mod_dav.so
 LoadModule dav_svn_module $SVN_HTTPD_MODULE_PATH/mod_dav_svn.so
+<IfModule !mod_version.c>
+        LoadModule version_module $SVN_HTTPD_MODULE_PATH/mod_version.so
+</IfModule>
+<IfVersion < 2.4>
+LockFile logs/accept.lock
+</IfVersion>
+<IfVersion >= 2.4>
+Mutex default
+<IfModule !mod_prefork_module.c>
+	LoadModule mpm_prefork_module $SVN_HTTPD_MODULE_PATH/mod_mpm_prefork.so
+</IfModule>
+</IfVersion>
 <Location /$repo_base_path>
 	DAV svn
 	SVNPath "$rawsvnrepo"

2.4.0.rc3.332.g886447c

In contrast to apache 2.2, apache 2.4 does not load mod_unixd in its default configuration (because there are choices). Thus, with the current config, apache 2.4.10 will not be started and the httpd tests will not run on distros with default apache config (RedHat type).

Enable mod_unixd to make the httpd tests run. This does not affect distros negatively which have that config already in their default (Debian type). httpd tests will run on these before and after this patch.

---
 t/lib-httpd/apache.conf | 3 +++
 1 file changed, 3 insertions(+)

index 0b81a00..7b8e21c 100644
@@ -64,6 +64,9 @@ LockFile accept.lock
 <IfModule !mod_mpm_prefork.c>
 	LoadModule mpm_prefork_module modules/mod_mpm_prefork.so
 </IfModule>
+<IfModule !mod_unixd.c>
+	LoadModule unixd_module modules/mod_unixd.so
+</IfModule>
 </IfVersion>
 
 PassEnv GIT_VALGRIND

2.4.0.rc3.332.g886447c

Currently, lib-git-svn checks a proper subset of the paths that lib-httpd checks for apache modules.

Make it check the same set so that apache is run by one when it is run by the other (provided ports have been set and there are no other configuration issues).

---
 t/lib-git-svn.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index b0ec12f..2a0ef07 100644
@@ -87,8 +87,10 @@ prepare_httpd () {
 	fi
 	for d in \
 		"$SVN_HTTPD_MODULE_PATH" \
-		/usr/lib/apache2/modules \
 		/usr/libexec/apache2 \
+		/usr/lib/apache2/modules \
+		/usr/lib64/httpd/modules \
+		/usr/lib/httpd/modules \
 	; do
 		if test -d "$d"
 		then

2.4.0.rc3.332.g886447c

v2 updates the commit messages as per the dicsussion after v1 and adds a clean-up.

1/4, 2/4 are ready to go as discussed (commit messages updated). 3/4 is a new independent cleanup noted while being there. 4/4 is the old 3/3 and still WIP. Please hold back.

Michael J Gruber (4):
t/lib-httpd: load mod_unixd
t/lib-git-svn: check same httpd module dirs as lib-httpd t/t91*: donot say how to avoid the tests t/lib-git-svn: adjust config to apache 2.4 WIP

 t/lib-git-svn.sh                       | 17 +++++++++++++++--
 t/lib-httpd/apache.conf                |  3 +++
 t/t9100-git-svn-basic.sh               |  2 --
 t/t9158-git-svn-mergeinfo.sh           |  2 --
 t/t9160-git-svn-preserve-empty-dirs.sh |  1 -
 5 files changed, 18 insertions(+), 7 deletions(-)

2.4.0.rc3.332.g886447c

Some of the tests "say" how to stop the svn tests from running, some do not.

The test suite is directed at people reading t/README where we keep all information about running the test suite (partly, with options etc.).

Remove said "say" occurences.

---

Notes:
Note on the side while messing with svn tests.

 t/t9100-git-svn-basic.sh               | 2 --
 t/t9158-git-svn-mergeinfo.sh           | 2 --
 t/t9160-git-svn-preserve-empty-dirs.sh | 1 -
 3 files changed, 5 deletions(-)

index 4fea8d9..3e34b42 100755
@@ -8,8 +8,6 @@ GIT_SVN_LC_ALL=${LC_ALL:-$LANG}
 
 . ./lib-git-svn.sh
 
-say 'define NO_SVN_TESTS to skip git svn tests'
-
 case "$GIT_SVN_LC_ALL" in
 *.UTF-8)
 	test_set_prereq UTF8

index 13f78f2..a875b45 100755
@@ -7,8 +7,6 @@ test_description='git svn mergeinfo propagation'
 
 . ./lib-git-svn.sh
 
-say 'define NO_SVN_TESTS to skip git svn tests'
-
 test_expect_success 'initialize source svn repo' '
 	svn_cmd mkdir -m x "$svnrepo"/trunk &&
 	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&

index b4a4434..0ede3cf 100755
@@ -11,7 +11,6 @@ local Git repository with placeholder files.'
 
 . ./lib-git-svn.sh
 
-say 'define NO_SVN_TESTS to skip git svn tests'
 GIT_REPO=git-svn-repo
 
 test_expect_success 'initialize source svn repo containing empty dirs' '

2.4.0.rc3.332.g886447c

Currently, lib-git-svn checks a proper subset of the paths that lib-httpd checks for apache modules.

Make it check the same set so that apache is run by one when it is run by the other (provide ports have been set).

---

Notes:
Again, I'm wondering how many are running these tests. They would not run on a 64bit system with redhat type fs layout.

 t/lib-git-svn.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index b0ec12f..2a0ef07 100644
@@ -87,8 +87,10 @@ prepare_httpd () {
 	fi
 	for d in \
 		"$SVN_HTTPD_MODULE_PATH" \
-		/usr/lib/apache2/modules \
 		/usr/libexec/apache2 \
+		/usr/lib/apache2/modules \
+		/usr/lib64/httpd/modules \
+		/usr/lib/httpd/modules \
 	; do
 		if test -d "$d"
 		then

2.4.0.rc1.221.gf7021fb

The current config is tailored to apache 2.2. Apache 2.4 fails to start with it.

Adjust the config to apache 2.4.

---

Notes:
This is WIP and RFH. Apache does not start without mpm, and the lock mechanism has changed. It will run with these changes, but svn fails to connect.

Again, I'm wondering who is running these tests at all, but also:
Why do we leverage lib-httpd so little from lib-git-svn?

 t/lib-git-svn.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 2a0ef07..b0ca893 100644
@@ -117,10 +117,11 @@ ServerName "git svn test"
 ServerRoot "$GIT_DIR"
 DocumentRoot "$GIT_DIR"
 PidFile "$GIT_DIR/httpd.pid"
-LockFile logs/accept.lock
+Mutex default
 Listen 127.0.0.1:$SVN_HTTPD_PORT
 LoadModule dav_module $SVN_HTTPD_MODULE_PATH/mod_dav.so
 LoadModule dav_svn_module $SVN_HTTPD_MODULE_PATH/mod_dav_svn.so
+LoadModule mpm_prefork_module $SVN_HTTPD_MODULE_PATH/mod_mpm_prefork.so
 <Location /$repo_base_path>
 	DAV svn
 	SVNPath "$rawsvnrepo"

2.4.0.rc1.221.gf7021fb

1/3 and 2/3 tested fine on Debian stable (wheezy). You can add my Tested-by to 1/3 and my S-o-b to 2/3.

...but not this one.

I'm only testing on Apache 2.2 (from Debian stable (wheezy)).

I doubt anybody here, myself included, likes dealing with SVN, and would rather ignore the tests than try to fix them.

lib-git-svn predates lib-httpd, and it isn't very interesting work to port and cleanup.

I think you need an IfModule guard around this?

I have no mod_mpm_prefork.so file with the apache2-mpm-prefork (2.2.22-13+deb7u4) package on Debian wheezy.

Eric Wong venit, vidit, dixit 08.04.2015 22:26:

Thanks, that's good to know.

So debian stable has apache 2.2? That explains why many don't see these problems.

I agree with everything above. But if (and that's a big if) the tests don't run for anyone I'd rather remove them if there's no way to port them.

Sure, the changes in 3/3 are for apache 2.4 only and should be guarded by a version check the same way as the ones in lib-httpd.

I'm just trying to figure out how to run them at all first.

So, before 3/3, t9118 and such do run the web server and test git-svn against that for you?

Michael

Right.

Yes.

I think this needs more conditional config. See the similar fixes I did in:

296f0b3 (t/lib-httpd/apache.conf: configure an MPM module for apache 2.4, 2013-06-09)

and:

0442743 (t/lib-httpd/apache.conf: do not use LockFile in apache >= 2.4, 2013-06-09)

And also beware of the ordering problem fixed by:

5bdc47e (lib-httpd/apache.conf: check version only after mod_version loads, 2013-06-21)

Curiously, though, the git-svn tests seem to run fine for me on Apache 2.4 without your patch. I wonder if the fixes I mentioned above (which I definitely needed to get the regular http tests running on Debian back then) are not necessary on my system anymore (e.g., because the apache2 package now ships with better-compatible config).

I run the http tests regularly (and have for a long time). I don't usually run the svn tests, though.

I don't mind us doing the minimal work here to bring the svn tests to a working state for your platform. As Eric mentions, refactoring test infrastructure is boring and thankless work. :)

But an alternate solution for patches 2 and 3 might be to share more of the startup code and config between the two. I don't know how painful that would be. From a cursory look, I think t/lib-httpd/apache.conf would have to get split into two parts: one to get apache running at all, and one to setup the <Location> and other bits related directly to the http tests.

-Peff

Ah, I see. I am not in fact running apache.

If you do not set SVN_HTTPD_PORT (which I don't), then lib-git-svn.sh's start_httpd silently returns success without bothering to setup the apache server at all. And yet the rest of the tests run to completion just fine.

It looks like setting this variable is a way to say "run the same set of tests, but do it over svn-over-httpd instead of svn's direct filesystem access". Setting SVN_HTTPD_PORT does cause the tests to fail for me.

I don't know how important it is to run these tests over httpd. If so, it would be nice to do something like lib-httpd does: try the apache setup by default, and if it fails, gracefully skip the tests (or fallback to the non-httpd version).

I'm also not sure if there's value in running both the httpd and local-filesystem forms of the test. IOW, should we be factoring out the tests and having two separate scripts that set $svnrepo as appropriate, and then runs the tests?

-Peff

Jeff King venit, vidit, dixit 10.04.2015 05:39:

Yep.

Oh, I didn't even notice that. That's a bit weird.

Hmm, if those tests are run (with file://) I'm inclined to leave things as they are (scratch 3/3)... Though the fact that on my system, lib-git-svn starts the server but fails to connect isn't overly comforting. But git-svn is being used less and less. World domination is almost complete.

Michael