[PATCH] Include log_config module in apache.conf

[PATCH] Include log_config module in apache.conf

[Daniel Barkalow]

The log_config module is needed for at least some versions of apache to 
support the LogFormat directive.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Johannes, you added the LogFormat line without this; does the test still 
work for you with this added, or do we need something clever to decide 
whether the module needs to be loaded or needs to not be loaded?

 t/lib-httpd/apache.conf |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index af6e5e1..a0d4077 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -4,6 +4,7 @@ DocumentRoot www
 LogFormat "%h %l %u %t \"%r\" %>s %b" common
 CustomLog access.log common
 ErrorLog error.log
+LoadModule log_config_module modules/mod_log_config.so
 
 <IfDefine Darwin>
 	LoadModule log_config_module modules/mod_log_config.so
-- 
1.6.2.77.g8cc3f.dirty

Re: [PATCH] Include log_config module in apache.conf

[Johannes Schindelin]

Hi,

On Tue, 10 Mar 2009, Daniel Barkalow wrote:

> The log_config module is needed for at least some versions of apache to 
> support the LogFormat directive.
> 
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> Johannes, you added the LogFormat line without this; does the test still 
> work for you with this added, or do we need something clever to decide 
> whether the module needs to be loaded or needs to not be loaded?

Sorry, no:

-- snip --
apache2: Syntax error on line 7 of 
/home/schindelin/git/t/lib-httpd/apache.conf: module log_config_module is 
built-in and can't be loaded
-- snap --

Ciao,
Dscho

[PATCH v2] Include log_config module in apache.conf

[Johannes Schindelin]

From: Daniel Barkalow <barkalow@iabervon.org>

The log_config module is needed for at least some versions of apache to
support the LogFormat directive.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This one works.

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

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index af6e5e1..f460e40 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -4,6 +4,9 @@ DocumentRoot www
 LogFormat "%h %l %u %t \"%r\" %>s %b" common
 CustomLog access.log common
 ErrorLog error.log
+<IfModule !mod_log_config.c>
+	LoadModule log_config_module modules/mod_log_config.so
+</IfModule>
 
 <IfDefine Darwin>
 	LoadModule log_config_module modules/mod_log_config.so
-- 
1.6.2.363.g9793e

Re: [PATCH] Include log_config module in apache.conf

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Sorry, no:
>
> -- snip --
> apache2: Syntax error on line 7 of 
> /home/schindelin/git/t/lib-httpd/apache.conf: module log_config_module is 
> built-in and can't be loaded
> -- snap --

Sorry and thanks---I'll apply an interdiff and credit it to you.

-- >8 --
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Wed, 11 Mar 2009 12:47:06 +0100 (CET)
Subject: [PATCH] test: do not LoadModule log_config_module unconditionally

LoadModule directive for log_config_module will not work if the module is
built-in.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/lib-httpd/apache.conf |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index a0d4077..f460e40 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -4,7 +4,9 @@ DocumentRoot www
 LogFormat "%h %l %u %t \"%r\" %>s %b" common
 CustomLog access.log common
 ErrorLog error.log
-LoadModule log_config_module modules/mod_log_config.so
+<IfModule !mod_log_config.c>
+	LoadModule log_config_module modules/mod_log_config.so
+</IfModule>
 
 <IfDefine Darwin>
 	LoadModule log_config_module modules/mod_log_config.so
-- 

Re: [PATCH] Include log_config module in apache.conf

[Brian Gernhardt]

On Mar 11, 2009, at 2:58 PM, Junio C Hamano wrote:

> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index a0d4077..f460e40 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -4,7 +4,9 @@ DocumentRoot www
> LogFormat "%h %l %u %t \"%r\" %>s %b" common
> CustomLog access.log common
> ErrorLog error.log
> -LoadModule log_config_module modules/mod_log_config.so
> +<IfModule !mod_log_config.c>
> +	LoadModule log_config_module modules/mod_log_config.so
> +</IfModule>
>
> <IfDefine Darwin>
> 	LoadModule log_config_module modules/mod_log_config.so

Isn't this last line redundant now?

Re: [PATCH] Include log_config module in apache.conf

[Junio C Hamano]

Brian Gernhardt <benji@silverinsanity.com> writes:

> On Mar 11, 2009, at 2:58 PM, Junio C Hamano wrote:
>
>> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
>> index a0d4077..f460e40 100644
>> --- a/t/lib-httpd/apache.conf
>> +++ b/t/lib-httpd/apache.conf
>> @@ -4,7 +4,9 @@ DocumentRoot www
>> LogFormat "%h %l %u %t \"%r\" %>s %b" common
>> CustomLog access.log common
>> ErrorLog error.log
>> -LoadModule log_config_module modules/mod_log_config.so
>> +<IfModule !mod_log_config.c>
>> +	LoadModule log_config_module modules/mod_log_config.so
>> +</IfModule>
>>
>> <IfDefine Darwin>
>> 	LoadModule log_config_module modules/mod_log_config.so
>
> Isn't this last line redundant now?

<IfDefine Darwin>
	LoadModule log_config_module modules/mod_log_config.so
	LockFile accept.lock
	PidFile httpd.pid
</IfDefine>

I think we can lose the first among these three lines safely, but I wonder 
if we can get rid of the Darwin specific part altogether.

-- >8 --
Subject: http tests: Darwin is not that special

We have PidFile definition in the file already, and we have added
necessary LoadModule for log_config_module recently.

This patch will end up giving LockFile to everybody not just limited to
Darwin, but why not?

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/lib-httpd/apache.conf |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index f460e40..6533c2d 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -1,5 +1,6 @@
 ServerName dummy
+LockFile accept.lock
 PidFile httpd.pid
 DocumentRoot www
 LogFormat "%h %l %u %t \"%r\" %>s %b" common
 CustomLog access.log common
@@ -8,12 +9,6 @@ ErrorLog error.log
 	LoadModule log_config_module modules/mod_log_config.so
 </IfModule>
 
-<IfDefine Darwin>
-	LoadModule log_config_module modules/mod_log_config.so
-	LockFile accept.lock
-	PidFile httpd.pid
-</IfDefine>
-
 <IfDefine SSL>
 LoadModule ssl_module modules/mod_ssl.so
 

Re: [PATCH] Include log_config module in apache.conf

[Jay Soffian]

On Wed, Mar 11, 2009 at 4:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> We have PidFile definition in the file already, and we have added
> necessary LoadModule for log_config_module recently.
>
> This patch will end up giving LockFile to everybody not just limited to
> Darwin, but why not?

In that case, the -DDarwin added to lib-httpd.sh by commit 69707d61 is
no longer needed.

j.

Re: [PATCH] Include log_config module in apache.conf

[Johannes Schindelin]

Hi,

On Wed, 11 Mar 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Sorry, no:
> >
> > -- snip --
> > apache2: Syntax error on line 7 of 
> > /home/schindelin/git/t/lib-httpd/apache.conf: module log_config_module is 
> > built-in and can't be loaded
> > -- snap --
> 
> Sorry and thanks---I'll apply an interdiff and credit it to you.

Thanks...

Ciao,
Dscho