You are viewing a plain text version of this content. The canonical link for it is here.
Posted to test-dev@httpd.apache.org by "Philippe M. Chiasson" <go...@cpan.org> on 2003/11/25 00:54:17 UTC

[Patch] Handle RedHat style Include conf.d/*.conf default configuration

Apache-Test currently doesn't inherit RedHat style configuration layout.

Include conf.d/*.conf doesn't get expanded proprely, and most modules
are not found/activated.

Following patch glob()es Include directive just in case

Index: Apache-Test/lib/Apache/TestConfigParse.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigParse.pm,v
retrieving revision 1.37
diff -u -I$Id: -r1.37 TestConfigParse.pm
--- Apache-Test/lib/Apache/TestConfigParse.pm	10 Nov 2003 17:23:52 -0000	1.37
+++ Apache-Test/lib/Apache/TestConfigParse.pm	24 Nov 2003 23:52:13 -0000
@@ -243,8 +243,10 @@
         (my $directive, $_) = split /\s+/, $_, 2;
 
         if ($directive eq "Include") {
-            my $include = $self->server_file_rel2abs($_);
-            $self->inherit_config_file_or_directory($include);
+            foreach (glob($self->server_file_rel2abs($_))) {
+               $self->inherit_config_file_or_directory($_);
+	       }
         }
 
         #parse what we want

-- 
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

Re: [Patch] Handle RedHat style Include conf.d/*.conf default configuration

Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Tue, 2003-11-25 at 18:05, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> 
> >>And it's still off by one in your new patch... have you considered getting a 
> >>decent editor? ;)
> > 
> > 
> > I don't want to even start thinking about Emacs dude ;-) I prefer to put
> > the blame on me, not my editor, sorry ;-)
> 
> where did you see me mention emacs, dude? ;)

I just know ;-)

> Both emacs and vi settings are given here:
> http://perl.apache.org/docs/2.0/devel/core/coding_style.html#Coding_Style_Guide
> 
> > Changed and comitted then ;-)
> 
> cool, also please add an entry to Changes, as this is an important "bug" fix 
> for users to know about.

Done!

> __________________________________________________________________
> Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
> http://stason.org/     mod_perl Guide ---> http://perl.apache.org
> mailto:stas@stason.org http://use.perl.org http://apacheweek.com
> http://modperlbook.org http://apache.org   http://ticketmaster.com
-- 
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

Re: [Patch] Handle RedHat style Include conf.d/*.conf default configuration

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:

>>And it's still off by one in your new patch... have you considered getting a 
>>decent editor? ;)
> 
> 
> I don't want to even start thinking about Emacs dude ;-) I prefer to put
> the blame on me, not my editor, sorry ;-)

where did you see me mention emacs, dude? ;)

Both emacs and vi settings are given here:
http://perl.apache.org/docs/2.0/devel/core/coding_style.html#Coding_Style_Guide

> Changed and comitted then ;-)

cool, also please add an entry to Changes, as this is an important "bug" fix 
for users to know about.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


Re: [Patch] Handle RedHat style Include conf.d/*.conf default configuration

Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Tue, 2003-11-25 at 16:08, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> [...]
> >>+1 after doing a proper indentation ;) please get rid of those tabs! and see 
> >>below:
> > 
> > 
> > Sorry, millions of apologies, my editor popped on me ;-(
> 
> And it's still off by one in your new patch... have you considered getting a 
> decent editor? ;)

I don't want to even start thinking about Emacs dude ;-) I prefer to put
the blame on me, not my editor, sorry ;-)

> >          if ($directive eq "Include") {
> > -            my $include = $self->server_file_rel2abs($_);
> > -            $self->inherit_config_file_or_directory($include);
> > +            foreach my $include (glob($self->server_file_rel2abs($_))) {
> > +               $self->inherit_config_file_or_directory($include);
>                 ^^^
> > +            }
> >          }
> 
> other than that, +1. ;)

Changed and comitted then ;-)

> 
> __________________________________________________________________
> Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
> http://stason.org/     mod_perl Guide ---> http://perl.apache.org
> mailto:stas@stason.org http://use.perl.org http://apacheweek.com
> http://modperlbook.org http://apache.org   http://ticketmaster.com
-- 
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

Re: [Patch] Handle RedHat style Include conf.d/*.conf default configuration

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
[...]
>>+1 after doing a proper indentation ;) please get rid of those tabs! and see 
>>below:
> 
> 
> Sorry, millions of apologies, my editor popped on me ;-(

And it's still off by one in your new patch... have you considered getting a 
decent editor? ;)

>          if ($directive eq "Include") {
> -            my $include = $self->server_file_rel2abs($_);
> -            $self->inherit_config_file_or_directory($include);
> +            foreach my $include (glob($self->server_file_rel2abs($_))) {
> +               $self->inherit_config_file_or_directory($include);
                ^^^
> +            }
>          }

other than that, +1. ;)


__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


Re: [Patch] Handle RedHat style Include conf.d/*.conf default configuration

Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Mon, 2003-11-24 at 16:20, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> > Apache-Test currently doesn't inherit RedHat style configuration layout.
> > 
> > Include conf.d/*.conf doesn't get expanded proprely, and most modules
> > are not found/activated.
> > 
> > Following patch glob()es Include directive just in case
> 
> Nice, Philippe.
> 
> +1 after doing a proper indentation ;) please get rid of those tabs! and see 
> below:

Sorry, millions of apologies, my editor popped on me ;-(

> Also please don't rely on $_ in the for loop that calls other functions. You 
> never know what they will do to $_ without localizing it and suddenly weird 
> things will start to happen. It's much safer to do:
> 
>           if ($directive eq "Include") {
>               my $include = $self->server_file_rel2abs($_);
> -            $self->inherit_config_file_or_directory($include);
> +            foreach my $file ($include)) {
> +               $self->inherit_config_file_or_directory($file);
> +	       }
>           }

Yes, you are correct, here is the new (cleaned) patch:

Index: Apache-Test/Changes
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/Changes,v
retrieving revision 1.67
diff -u -I$Id: -r1.67 Changes
--- Apache-Test/Changes	24 Nov 2003 23:31:31 -0000	1.67
+++ Apache-Test/Changes	25 Nov 2003 20:12:35 -0000
@@ -8,6 +8,9 @@
 
 =item 1.07-dev
 
+Fix Apache::TestConfigParse to handle glob includes, like Include
+conf/*.conf on RedHat [Philippe M. Chiasson]
+
 Fix Apache::TestConfig::add_config not to append " " at the end of the
 config lines. This was a problem with config sections imported from
 .pm files, the appended "" made it impossible to have multi-line
Index: Apache-Test/lib/Apache/TestConfigParse.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigParse.pm,v
retrieving revision 1.37
diff -u -I$Id: -r1.37 TestConfigParse.pm
--- Apache-Test/lib/Apache/TestConfigParse.pm	10 Nov 2003 17:23:52 -0000	1.37
+++ Apache-Test/lib/Apache/TestConfigParse.pm	25 Nov 2003 20:12:35 -0000
@@ -243,8 +243,9 @@
         (my $directive, $_) = split /\s+/, $_, 2;
 
         if ($directive eq "Include") {
-            my $include = $self->server_file_rel2abs($_);
-            $self->inherit_config_file_or_directory($include);
+            foreach my $include (glob($self->server_file_rel2abs($_))) {
+               $self->inherit_config_file_or_directory($include);
+            }
         }
 
         #parse what we want


-- 
--------------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

Re: [Patch] Handle RedHat style Include conf.d/*.conf default configuration

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Apache-Test currently doesn't inherit RedHat style configuration layout.
> 
> Include conf.d/*.conf doesn't get expanded proprely, and most modules
> are not found/activated.
> 
> Following patch glob()es Include directive just in case

Nice, Philippe.

+1 after doing a proper indentation ;) please get rid of those tabs! and see 
below:

> Index: Apache-Test/lib/Apache/TestConfigParse.pm
> ===================================================================
> RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigParse.pm,v
> retrieving revision 1.37
> diff -u -I$Id: -r1.37 TestConfigParse.pm
> --- Apache-Test/lib/Apache/TestConfigParse.pm	10 Nov 2003 17:23:52 -0000	1.37
> +++ Apache-Test/lib/Apache/TestConfigParse.pm	24 Nov 2003 23:52:13 -0000
> @@ -243,8 +243,10 @@
>          (my $directive, $_) = split /\s+/, $_, 2;
>  
>          if ($directive eq "Include") {
> -            my $include = $self->server_file_rel2abs($_);
> -            $self->inherit_config_file_or_directory($include);
> +            foreach (glob($self->server_file_rel2abs($_))) {
> +               $self->inherit_config_file_or_directory($_);
> +	       }

Also please don't rely on $_ in the for loop that calls other functions. You 
never know what they will do to $_ without localizing it and suddenly weird 
things will start to happen. It's much safer to do:

          if ($directive eq "Include") {
              my $include = $self->server_file_rel2abs($_);
-            $self->inherit_config_file_or_directory($include);
+            foreach my $file ($include)) {
+               $self->inherit_config_file_or_directory($file);
+	       }
          }


__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


RE: [Patch] Handle RedHat style Include conf.d/*.conf defaultconfiguration

Posted by ru...@sdwim.org.
-----Original Message-----
From: Philippe M. Chiasson [mailto:gozer@cpan.org] 
Sent: Monday, November 24, 2003 3:54 PM
To: test-dev@httpd.apache.org
Subject: [Patch] Handle RedHat style Include conf.d/*.conf
defaultconfiguration

Apache-Test currently doesn't inherit RedHat style configuration layout.

Include conf.d/*.conf doesn't get expanded proprely, and most modules
are not found/activated.

Following patch glob()es Include directive just in case

Index: Apache-Test/lib/Apache/TestConfigParse.pm
===================================================================
RCS file:
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigPar
se.pm,v
retrieving revision 1.37
diff -u -I$Id: -r1.37 TestConfigParse.pm
--- Apache-Test/lib/Apache/TestConfigParse.pm	10 Nov 2003 17:23:52
-0000	1.37
+++ Apache-Test/lib/Apache/TestConfigParse.pm	24 Nov 2003 23:52:13
-0000
@@ -243,8 +243,10 @@
         (my $directive, $_) = split /\s+/, $_, 2;
 
         if ($directive eq "Include") {
-            my $include = $self->server_file_rel2abs($_);
-            $self->inherit_config_file_or_directory($include);
+            foreach (glob($self->server_file_rel2abs($_))) {
+               $self->inherit_config_file_or_directory($_);
+	       }
         }
 
         #parse what we want

-- 
------------------------------------------------------------------------
--------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5
(122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107
88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so
ingenious.
perl
-e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&red
o}'