threads / patch / 18259

patchInclude log_config module in apache.conf

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

## tl;dr

8 messages between Mar 11, 2009 and Mar 11, 2009. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Daniel Barkalow· Mar 11, 2009, 03:23 UTC · lore

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(-)
Show changes to t/lib-httpd/apache.conf +1 −0
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
Johannes Schindelin· Mar 11, 2009, 11:46 UTC · re: Daniel Barkalow · lore

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

Hi,
On Tue, 10 Mar 2009, Daniel Barkalow wrote:
Show 8 quoted lines
> 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

Johannes Schindelin· Mar 11, 2009, 11:47 UTC · re: Johannes Schindelin · lore

[PATCH v2] Include log_config module in apache.conf

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(-)
Show changes to t/lib-httpd/apache.conf +3 −0
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
Junio C Hamano· Mar 11, 2009, 18:58 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 7 quoted lines
> 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(-)
Show changes to t/lib-httpd/apache.conf +3 −1
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
Brian Gernhardt· Mar 11, 2009, 19:33 UTC · re: Junio C Hamano · lore

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

On Mar 11, 2009, at 2:58 PM, Junio C Hamano wrote:
Show 15 quoted lines
> 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?
Junio C Hamano· Mar 11, 2009, 20:13 UTC · re: Brian Gernhardt · lore

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

Brian Gernhardt <benji@silverinsanity.com> writes:
Show 19 quoted lines
> 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(-)
Show changes to t/lib-httpd/apache.conf +1 −6
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
Jay Soffian· Mar 11, 2009, 22:43 UTC · re: Junio C Hamano · lore

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

On Wed, Mar 11, 2009 at 4:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 5 quoted lines
> 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.
Johannes Schindelin· Mar 11, 2009, 19:45 UTC · re: Junio C Hamano · lore

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

Hi,
On Wed, 11 Mar 2009, Junio C Hamano wrote:
Show 11 quoted lines
> 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

← back to recent threads