You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl-cvs@perl.apache.org by do...@apache.org on 2001/09/15 22:17:35 UTC

cvs commit: modperl-2.0/src/modules/perl modperl_config.c

dougm       01/09/15 13:17:35

  Modified:    src/modules/perl modperl_config.c
  Log:
  only inherit base PerlSwitches if explicitly told to with "PerlSwitches +inherit"
  
  Revision  Changes    Path
  1.36      +9 -2      modperl-2.0/src/modules/perl/modperl_config.c
  
  Index: modperl_config.c
  ===================================================================
  RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_config.c,v
  retrieving revision 1.35
  retrieving revision 1.36
  diff -u -r1.35 -r1.36
  --- modperl_config.c	2001/08/10 23:37:33	1.35
  +++ modperl_config.c	2001/09/15 20:17:35	1.36
  @@ -168,8 +168,15 @@
       merge_item(perl);
   #endif
   
  -    /* argv always initialized to 1 with ap_server_argv0 */
  -    mrg->argv = add->argv->nelts > 1 ? add->argv : base->argv;
  +    if (add->argv->nelts == 2 &&
  +        strEQ(((char **)add->argv->elts)[1], "+inherit"))
  +    {
  +        /* only inherit base PerlSwitches if explicitly told to */
  +        mrg->argv = base->argv;
  +    }
  +    else {
  +        mrg->argv = add->argv;
  +    }
   
       mrg->flags = modperl_options_merge(p, base->flags, add->flags);
   
  
  
  

Re: [patch] Re: genfile and gendir to emulate mkdir -p

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 17 Sep 2001, Stas Bekman wrote:

patch looks good, +1
 
> If you agree with me that _makepath should be made private as in my patch,
> I also suggest that genwarning() is made private too. I don't see a reason
> why any of these two should be exposed outside.
> 
> Hmm, doesn't it make sense at all for Apache-Test to have a concept of
> private subs, or am I barking on the wrong tree? I'm just trying to make
> it easier to change the functionality later, and it'll be much easier if
> we protect some of our subs making them private.

i'd rather not see any "_private" names.  once there a docs for the apis,
that is what we commit to.  anything not documented is subject to
change.  the underscore prefix is just an eyesore to me.




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Stas Bekman <st...@stason.org>.
On Tue, 18 Sep 2001, Doug MacEachern wrote:

> On Wed, 19 Sep 2001, Stas Bekman wrote:
>
> > what happens if you don't want any warning added? Or shouldn't this be a
> > case?
>
> i think we should always have a warning in generated files.  if we really
> need an option to turn it off, we can revisit later.  of course, your
> warn_style constants patch was still great, because it led an idea that
> is much better than current :)

I'm looking forward to the day when we write code that writes more new
code... not just a preprogrammed one :)


_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Doug MacEachern <do...@covalent.net>.
On Wed, 19 Sep 2001, Stas Bekman wrote:
 
> what happens if you don't want any warning added? Or shouldn't this be a
> case?

i think we should always have a warning in generated files.  if we really
need an option to turn it off, we can revisit later.  of course, your
warn_style constants patch was still great, because it led an idea that
is much better than current :)





---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Stas Bekman <st...@stason.org>.
On Mon, 17 Sep 2001, Doug MacEachern wrote:

> On Tue, 18 Sep 2001, Stas Bekman wrote:
>
> > It's a good idea to let the code decide which comment style to use, but I
> > won't drop the explicit option yet. So we can probably add the code from
> > above and remove the WARN_STYLE_ arg in most calls to genfile, but still
> > keep the explicit option around in case we need it.
>
> i don't see any reason to have the new constants and explict option if we
> figure out the style based on file extension.

what happens if you don't want any warning added? Or shouldn't this be a
case?

_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 18 Sep 2001, Stas Bekman wrote:
 
> It's a good idea to let the code decide which comment style to use, but I
> won't drop the explicit option yet. So we can probably add the code from
> above and remove the WARN_STYLE_ arg in most calls to genfile, but still
> keep the explicit option around in case we need it.

i don't see any reason to have the new constants and explict option if we
figure out the style based on file extension.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Stas Bekman <st...@stason.org>.
On Mon, 17 Sep 2001, Doug MacEachern wrote:

> On Tue, 18 Sep 2001, Stas Bekman wrote:
>
> > +use constant WARN_STYLE_NONE => 0;
> > +use constant WARN_STYLE_PERL => 1;
> > +use constant WARN_STYLE_C    => 2;
>
> one thing to consider though, ModPerl::Code has
> noedit_warning_{c,hash} and this comment:
>
> #this is named hash after the `#' character
> #rather than named perl, since #comments are used
> #non-Perl files, e.g. Makefile, typemap, etc.
>
> many of the genfile() calls are generating files that are not Perl code.
> if you look at your patch, there's genfile called for mime.types, *.conf,
> .gdb*, etc.  how about this, rather than a style argument to genfile(),
> let genfile figure it out for you based on the file extension.
>
> something like:
>
> my %warnings = (
>     html => sub {
>         "<!-- @_ -->"
>     },
>     c   => sub {
>         "/* @_ */"
>     },
>     default => sub {
>         my $string = join '', @_;
>         $string =~ s/^/\#/g;
>         $string;
>     },
> );
> $warnings{h} = $warnings{c};


OK these are two issues:

1. s/WARN_STYLE_PERL/WARNI_STYLE_HASH/
2. DWIM magic

It's a good idea to let the code decide which comment style to use, but I
won't drop the explicit option yet. So we can probably add the code from
above and remove the WARN_STYLE_ arg in most calls to genfile, but still
keep the explicit option around in case we need it.



_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Doug MacEachern <do...@covalent.net>.
On Wed, 19 Sep 2001, Stas Bekman wrote:
 
> my %warn_style =
>     (
>      html    => sub { "<!-- @_ -->"               },
>      c       => sub { "/* @_ */"                  },
>      default => sub { join '', map {s/^/\#/g} @_; },
>     );

one thing i've been meaning to point out style wise, these should be in
the form of:

my %hash = (
    one => 1,
    two => 2,
);

no need to align the closing } for those subs.
 
> sub warn_style_sub {
>     my ($self, $filename) = @_;
>     my $ext = (File::Basename::fileparse($filename, '\..*'))[3] || '';
>     $ext =~ s/^\.(.*)/"\L$1"/e;

maybe put those two lines into a filename_ext function that returns the
$ext, could be useful elsewhere.

>     return $warn_style{ $file_ext{$ext} || 'default' } || sub {};

no need for the '|| sub {}', since we always have a default.

otherwise, looks good!



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Stas Bekman <st...@stason.org>.
On Mon, 17 Sep 2001, Doug MacEachern wrote:

> On Tue, 18 Sep 2001, Stas Bekman wrote:
>
> > +use constant WARN_STYLE_NONE => 0;
> > +use constant WARN_STYLE_PERL => 1;
> > +use constant WARN_STYLE_C    => 2;
>
> one thing to consider though, ModPerl::Code has
> noedit_warning_{c,hash} and this comment:
>
> #this is named hash after the `#' character
> #rather than named perl, since #comments are used
> #non-Perl files, e.g. Makefile, typemap, etc.
>
> many of the genfile() calls are generating files that are not Perl code.
> if you look at your patch, there's genfile called for mime.types, *.conf,
> .gdb*, etc.  how about this, rather than a style argument to genfile(),
> let genfile figure it out for you based on the file extension.
>
> something like:
>
> my %warnings = (
>     html => sub {
>         "<!-- @_ -->"
>     },
>     c   => sub {
>         "/* @_ */"
>     },
>     default => sub {
>         my $string = join '', @_;
>         $string =~ s/^/\#/g;
>         $string;
>     },
> );
> $warnings{h} = $warnings{c};

In any case here is something that I wrote (untested, just the concept):

my %warn_style =
    (
     html    => sub { "<!-- @_ -->"               },
     c       => sub { "/* @_ */"                  },
     default => sub { join '', map {s/^/\#/g} @_; },
    );

my %file_ext =
    (
     map({$_ => 'html'}, qw(htm html)),
     map({$_ => 'c'},    qw(c h)),
    );

sub warn_style_sub {
    my ($self, $filename) = @_;
    my $ext = (File::Basename::fileparse($filename, '\..*'))[3] || '';
    $ext =~ s/^\.(.*)/"\L$1"/e;
    return $warn_style{ $file_ext{$ext} || 'default' } || sub {};
}

sub genwarning {
    my($self, $filename) = @_;
    return unless $type;
    my $warning = "#WARNING: this file is generated, do not edit\n" .
      calls_trace();
    return warn_style_sub($filename)->($warning);
}

hope that warn_style_sub is not too cryptic.


_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 18 Sep 2001, Stas Bekman wrote:
 
> +use constant WARN_STYLE_NONE => 0;
> +use constant WARN_STYLE_PERL => 1;
> +use constant WARN_STYLE_C    => 2;

one thing to consider though, ModPerl::Code has
noedit_warning_{c,hash} and this comment:

#this is named hash after the `#' character
#rather than named perl, since #comments are used
#non-Perl files, e.g. Makefile, typemap, etc.

many of the genfile() calls are generating files that are not Perl code.
if you look at your patch, there's genfile called for mime.types, *.conf,
.gdb*, etc.  how about this, rather than a style argument to genfile(),
let genfile figure it out for you based on the file extension.

something like:

my %warnings = (
    html => sub {
        "<!-- @_ -->"
    },
    c   => sub {
        "/* @_ */"
    },
    default => sub {
        my $string = join '', @_;
        $string =~ s/^/\#/g;
        $string;
    },
);
$warnings{h} = $warnings{c};


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Stas Bekman <st...@stason.org>.
On Mon, 17 Sep 2001, Doug MacEachern wrote:

> On Mon, 17 Sep 2001, Stas Bekman wrote:
>
> > > - use better API naming s/warn/warn_type/. it's not clear what 'warn' is.
> >
> > I still don't like the use of raw numbers 1, 2 for
> > Apache::TestConfig::genwarning. We try to make the code self-documenting,
> > so let's do that.
> >
> > May I add:
> >
> >   use constant WARN_STYLE_NONE => 0;
> >   use constant WARN_STYLE_PERL => 1;
> >   use constant WARN_STYLE_C    => 2;
>
> that would be fine.

here goes:

Index: Apache-Test/lib/Apache/TestConfig.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfig.pm,v
retrieving revision 1.62
diff -u -r1.62 TestConfig.pm
--- Apache-Test/lib/Apache/TestConfig.pm	2001/09/18 01:59:19	1.62
+++ Apache-Test/lib/Apache/TestConfig.pm	2001/09/18 02:22:31
@@ -8,6 +8,10 @@
 use constant NETWARE => $^O eq 'NetWare';
 use constant WINFU   => WIN32 || CYGWIN || NETWARE;

+use constant WARN_STYLE_NONE => 0;
+use constant WARN_STYLE_PERL => 1;
+use constant WARN_STYLE_C    => 2;
+
 use File::Copy ();
 use File::Find qw(finddepth);
 use File::Basename qw(dirname);
@@ -513,13 +517,15 @@

 #generate files and directories

+# warning types:
+# WARN_STYLE_NONE => no warning returned
+# WARN_STYLE_PERL => #  warning
+# WARN_STYLE_C    => /* warning */
 sub genwarning {
     my($self, $type) = @_;
     return unless $type;
     my $warning = "#WARNING: this file is generated, do not edit\n" .
       calls_trace();
-    #1 => #  warning
-    #2 => /* warning */
     return $type > 1 ? "/*\n$warning*/\n" : $warning;
 }

@@ -729,7 +735,7 @@
     unless ($self->{inherit_config}->{TypesConfig}) {
         my $types = catfile $self->{vars}->{t_conf}, 'mime.types';
         unless (-e $types) {
-            my $fh = $self->genfile($types, 1);
+            my $fh = $self->genfile($types, WARN_STYLE_PERL);
             print $fh $self->types_config_template;
             close $fh;
         }
@@ -768,7 +774,7 @@

         open(my $in, $file) or next;

-        my $out = $self->genfile($generated, 1);
+        my $out = $self->genfile($generated, WARN_STYLE_PERL);
         $self->replace_vars($in, $out);

         close $in;
@@ -832,7 +838,7 @@

     my $in = $self->httpd_conf_template($conf_file_in);

-    my $out = $self->genfile($conf_file, 1);
+    my $out = $self->genfile($conf_file, WARN_STYLE_PERL);

     $self->preamble_run($out);

@@ -1000,7 +1006,7 @@

     my $name = 'apache_test_config';
     my $file = catfile $self->{vars}->{t_conf}, "$name.pm";
-    my $fh = $self->genfile($file, 1);
+    my $fh = $self->genfile($file, WARN_STYLE_PERL);

     $self->trace("saving config data to $name.pm");

@@ -1053,12 +1059,16 @@
 useful for navigating through autogenerated files.

 The returned warnings are enclosed into a suitable for Perl or C
-comment style, if the C<$warn_type> variable:
-
-    1 => #  Perl warning
-    2 => /* # C warning */
+comment style. The following constants accessible from other packages
+define the following values for the C<$warn_type> variable:

-If C<$warn_type> is FALSE, no warning string will be returned.
+  WARN_STYLE_NONE => no warning returned
+  WARN_STYLE_PERL => #  Perl warning
+  WARN_STYLE_C    => /* C warning */
+
+If C<$warn_type> is FALSE, no warning string will be returned.  To
+access the constant from a different package, use the fully qualified
+style, e.g. C<Apache::TestConfig::WARN_STYLE_PERL>.

 =item genfile()

Index: Apache-Test/lib/Apache/TestConfigC.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigC.pm,v
retrieving revision 1.13
diff -u -r1.13 TestConfigC.pm
--- Apache-Test/lib/Apache/TestConfigC.pm	2001/09/09 18:27:39	1.13
+++ Apache-Test/lib/Apache/TestConfigC.pm	2001/09/18 02:22:31
@@ -300,7 +300,7 @@
     my $self = shift;

     my $file = "$self->{cmodules_dir}/apache_httpd_test.h";
-    my $fh = $self->genfile($file, 2);
+    my $fh = $self->genfile($file, WARN_STYLE_C);

     while (read Apache::TestConfigC::DATA, my $buf, 1024) {
         print $fh $buf;
Index: Apache-Test/lib/Apache/TestConfigPerl.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigPerl.pm,v
retrieving revision 1.25
diff -u -r1.25 TestConfigPerl.pm
--- Apache-Test/lib/Apache/TestConfigPerl.pm	2001/09/15 19:29:30	1.25
+++ Apache-Test/lib/Apache/TestConfigPerl.pm	2001/09/18 02:22:31
@@ -64,7 +64,7 @@
     return if -e $t;

     $self->gendir($dir);
-    my $fh = $self->genfile($t, 1);
+    my $fh = $self->genfile($t, WARN_STYLE_PERL);

     print $fh <<EOF;
 use Apache::TestConfig ();
@@ -98,7 +98,7 @@
     #but this will work for both 2.0 and 1.xx
     if (my $inc = $self->{inc}) {
         my $include_pl = catfile $self->{vars}->{t_conf}, 'modperl_inc.pl';
-        my $fh = $self->genfile($include_pl, 1);
+        my $fh = $self->genfile($include_pl, WARN_STYLE_PERL);
         for (@$inc) {
             print $fh "use lib '$_';\n";
         }
@@ -112,7 +112,7 @@
     my $startup_pl = catfile $self->{vars}->{t_conf}, 'modperl_startup.pl';

     unless (-e $startup_pl) {
-        my $fh = $self->genfile($startup_pl, 1);
+        my $fh = $self->genfile($startup_pl, WARN_STYLE_PERL);
         print $fh $self->startup_pl_code;
         close $fh;
     }
Index: Apache-Test/lib/Apache/TestServer.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestServer.pm,v
retrieving revision 1.30
diff -u -r1.30 TestServer.pm
--- Apache-Test/lib/Apache/TestServer.pm	2001/09/08 17:40:43	1.30
+++ Apache-Test/lib/Apache/TestServer.pm	2001/09/18 02:22:31
@@ -122,7 +122,8 @@
     my $strace_cmd  = $self->strace_cmd($opts->{debugger}, $file);
     my $httpd       = $config->{vars}->{httpd};

-    $config->genfile($file, 1); #just mark for cleanup
+    # just mark for cleanup
+    $config->genfile($file, Apache::TestConfig::WARN_STYLE_PERL);

     my $command = "$strace_cmd $httpd $one_process $args";

@@ -141,7 +142,7 @@
     my $one_process = $self->version_of(\%one_process);

     my $file = catfile $config->{vars}->{serverroot}, '.gdb-test-start';
-    my $fh   = $config->genfile($file, 1);
+    my $fh   = $config->genfile($file, Apache::TestConfig::WARN_STYLE_PERL);

     print $fh default_gdbinit();

Index: t/response/TestDirective/perlrequire.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/response/TestDirective/perlrequire.pm,v
retrieving revision 1.1
diff -u -r1.1 perlrequire.pm
--- t/response/TestDirective/perlrequire.pm	2001/09/18 02:00:19	1.1
+++ t/response/TestDirective/perlrequire.pm	2001/09/18 02:22:31
@@ -37,7 +37,7 @@
 1;
 EOF
         my $file = catfile $target_dir, $test, 'PerlRequireTest.pm';
-        $self->writefile($file, $content, 1);
+        $self->writefile($file, $content, Apache::TestConfig::WARN_STYLE_PERL);
     }
 }


_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genwarning constants

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 17 Sep 2001, Stas Bekman wrote:

> > - use better API naming s/warn/warn_type/. it's not clear what 'warn' is.
> 
> I still don't like the use of raw numbers 1, 2 for
> Apache::TestConfig::genwarning. We try to make the code self-documenting,
> so let's do that.
> 
> May I add:
> 
>   use constant WARN_STYLE_NONE => 0;
>   use constant WARN_STYLE_PERL => 1;
>   use constant WARN_STYLE_C    => 2;

that would be fine.
 
> or if you don't mind, adding a dependency on CPAN's enum.pm, which we may
> find useful in more places:

i do mind, but i think you already know that :-)
 
> Since we may find useful the context of NONE, PERL, C in other cases, we
> may as well define:
> 
>   use constant NONE => 0;
>   use constant PERL => 1;
>   use constant C    => 2;

just go with the WARN_STYLE_ for now.  if it turns out we need those other
constants, the WARN_STYLE_* can be implemented in terms of those:

use constant WARN_STYLE_PERL => Apache::Test::PERL;




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


genwarning constants

Posted by Stas Bekman <st...@stason.org>.
> - use better API naming s/warn/warn_type/. it's not clear what 'warn' is.

I still don't like the use of raw numbers 1, 2 for
Apache::TestConfig::genwarning. We try to make the code self-documenting,
so let's do that.

May I add:

  use constant WARN_STYLE_NONE => 0;
  use constant WARN_STYLE_PERL => 1;
  use constant WARN_STYLE_C    => 2;

or if you don't mind, adding a dependency on CPAN's enum.pm, which we may
find useful in more places:

  use enum qw(WARN_STYLE_NONE WARN_STYLE_PERL WARN_STYLE_C);

and then in the code write:

genfile($file, WARN_STYLE_PERL);

or outside of Apache::TestConfig:

genfile($file, Apache::TestConfig::WARN_STYLE_PERL);

Since we may find useful the context of NONE, PERL, C in other cases, we
may as well define:

  use constant NONE => 0;
  use constant PERL => 1;
  use constant C    => 2;

in Apache::Test

and then reuse it everywhere, like so:

genfile($file, Apache::Test::PERL);

writefile($file, $content, Apache::Test::NONE);

hmm, this probably makes the code less clear.

_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


[patch] Re: genfile and gendir to emulate mkdir -p

Posted by Stas Bekman <st...@stason.org>.
On Mon, 17 Sep 2001, Stas Bekman wrote:

> Doug MacEachern wrote:
>
> > On Mon, 17 Sep 2001, Stas Bekman wrote:
> >
> >
> >>I was wondering whether it'd be better to use File::Path::mkpath in
> >>genfile and gendir so the test writers won't have to call this sub many
> >>times if they need to create a dir foo/bar/baz or a file
> >>foo/bar/baz/test.conf, when foo doesn't exist. Of course in this case
> >>the cleanup code should use File::Path::rmtree. If you think it's cool,
> >>I'll send a patch.
> >>
> >
> > +1 for s/mkdir/mkpath/ in both gendir and t_mkdir.  however, the cleanup
> > code intentionally does not use rmtree.  it carefully removes generated
> > files first (created via genfile), then only removes the generated
> > directories if they are empty.  i don't think rmtree provides that
> > functionality.
> > also, switching to gendir to use mkpath will require you to add each
> > subdirectory created to $self->{clean}->{dirs}
>
>
> Yes, I'm aware of that. And that's exactly what I've planned to do.
> It'll be somewhat slower than mkpath, but not slower than the current
> code, and will take away from the test writer the mess of doing all this
> work.

The following patch does:

- adds a private _makepath, which takes care of creating paths and
  maintaining the cleanup list of dirs that it has created
- clean()'s dir section is corrected to make sure that the sub-dirs are
  attempted to be deleted first.
- use better API naming s/warn/warn_type/. it's not clear what 'warn' is.
- starts the documentation of the module and documents genwarning,
genfile, gendir, writefile

If you agree with me that _makepath should be made private as in my patch,
I also suggest that genwarning() is made private too. I don't see a reason
why any of these two should be exposed outside.

Hmm, doesn't it make sense at all for Apache-Test to have a concept of
private subs, or am I barking on the wrong tree? I'm just trying to make
it easier to change the functionality later, and it'll be much easier if
we protect some of our subs making them private.

Anyway, here is the patch.

Index: Apache-Test/lib/Apache/TestConfig.pm
===================================================================
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfig.pm,v
retrieving revision 1.60
diff -u -r1.60 TestConfig.pm
--- Apache-Test/lib/Apache/TestConfig.pm	2001/09/15 19:14:16	1.60
+++ Apache-Test/lib/Apache/TestConfig.pm	2001/09/17 05:14:35
@@ -10,6 +10,8 @@

 use File::Copy ();
 use File::Find qw(finddepth);
+use File::Basename qw(dirname);
+use File::Path ();
 use File::Spec::Functions qw(catfile abs2rel splitdir
                              catdir file_name_is_absolute);
 use Cwd qw(fastcwd);
@@ -536,14 +538,18 @@
 }

 sub genfile {
-    my($self, $file, $warn) = @_;
+    my($self, $file, $warn_type) = @_;

+    # create the parent dir if it doesn't exist yet
+    my $dir = dirname $file;
+    $self->_makepath($dir);
+
     my $name = abs2rel $file, $self->{vars}->{t_dir};
     $self->trace("generating $name");

     open my $fh, '>', $file or die "open $file: $!";

-    if (my $msg = $self->genwarning($warn)) {
+    if (my $msg = $self->genwarning($warn_type)) {
         print $fh $msg, "\n";
     }

@@ -554,14 +560,18 @@

 # gen + write file
 sub writefile {
-    my($self, $file, $content, $warn) = @_;
+    my($self, $file, $content, $warn_type) = @_;
+
+    # create the parent dir if it doesn't exist yet
+    my $dir = dirname $file;
+    $self->_makepath($dir);

     my $name = abs2rel $file, $self->{vars}->{t_dir};
     $self->trace("generating $name");

     open my $fh, '>', $file or die "open $file: $!";

-    if (my $msg = $self->genwarning($warn)) {
+    if (my $msg = $self->genwarning($warn_type)) {
         print $fh $msg, "\n";
     }

@@ -588,9 +598,24 @@

 sub gendir {
     my($self, $dir) = @_;
+    $self->_makepath($dir);
+}
+
+# returns a list of dirs successfully created
+sub _makepath {
+    my($self, $path) = @_;
+
+    return if !defined($path) || -e $path;
+    my $full_path = $path;
+
+    # remember which dirs were created and should be cleaned up
+    while (1) {
+        $self->{clean}->{dirs}->{$path} = 1;
+        $path = dirname $path;
+        last if -e $path;
+    }

-    mkdir $dir, 0755 unless -d $dir;
-    $self->{clean}->{dirs}->{$dir} = 1;
+    return File::Path::mkpath($full_path, 0, 0755);
 }

 sub open_cmd {
@@ -617,11 +642,13 @@
             unlink $_;
         }
         else {
-            #print "unlink $_: $!\n";
+            $self->trace("unlink $_: $!");
         }
     }

-    for (keys %{ $self->{clean}->{dirs} }) {
+    # if /foo comes before /foo/bar, /foo will never be removed
+    # hence ensure that sub-dirs are always treated before a parent dir
+    for (reverse sort keys %{ $self->{clean}->{dirs} }) {
         if (-d $_) {
             opendir(my $dh, $_);
             my $notempty = grep { ! /^\.{1,2}$/ } readdir $dh;
@@ -994,6 +1021,105 @@
 }

 1;
+
+=head1 NAME
+
+Apache::TestConfig -- Test Configuration setup module
+
+=head1 SYNOPSIS
+
+  use Apache::TestConfig;
+
+  my $cfg = Apache::TestConfig->new(%args)
+  my $fh = $cfg->genfile($file, $warn_type);
+  $cfg->writefile($file, $content, $warn_type);
+  $cfg->gendir($dir);
+  ...
+
+=head1 DESCRIPTION
+
+C<Apache::TestConfig> is used in creating the C<Apache::Test>
+configuration files.
+
+=head1 FUNCTIONS
+
+=over
+
+=item genwarning()
+
+  my $warn = $cfg->genwarning($warn_type)
+
+genwarning() returns a string warning that the file is autogenerated,
+plus a perl trace of calls to this this function. This function is
+useful for navigating through autogenerated files.
+
+The returned warnings are enclosed into a suitable for Perl or C
+comment style, if the C<$warn_type> variable:
+
+    1 => #  Perl warning
+    2 => /* # C warning */
+
+If C<$warn_type> is FALSE, no warning string will be returned.
+
+=item genfile()
+
+  my $fh = $cfg->genfile($file, $warn_type);
+
+genfile() creates a new file C<$file> for writing and returns a file
+handle.
+
+If C<$warn_type> is true a warning header, which states that the file
+was autogenerated and the trace of calls that generated this file,
+will be automatically added to the top of the created file. For valid
+C<$warn_type> values see genwarning().
+
+If parent directories of C<$file> don't exist they will be
+automagically created.
+
+The file C<$file> and any created parent directories (if found empty)
+will be automatically removed on cleanup.
+
+=item writefile()
+
+  $cfg->writefile($file, $content, $warn_type);
+
+writefile() creates a new file C<$file> with the content of
+C<$content>.
+
+If C<$warn_type> is true a warning header, which states that the file
+was autogenerated and the trace of calls that generated this file,
+will be automatically added to the top of the created file. For valid
+C<$warn_type> values see genwarning().
+
+If parent directories of C<$file> don't exist they will be
+automagically created.
+
+The file C<$file> and any created parent directories (if found empty)
+will be automatically removed on cleanup.
+
+=item gendir()
+
+  $cfg->gendir($dir);
+
+gendir() creates a new directory C<$dir>.
+
+If parent directories of C<$dir> don't exist they will be
+automagically created.
+
+The directory C<$dir> and any created parent directories will be
+automatically removed on cleanup if found empty.
+
+=back
+
+=head1 AUTHOR
+
+=head1 SEE ALSO
+
+perl(1), Apache::Test(3)
+
+=cut
+
+
 __DATA__
 ServerRoot   "@ServerRoot@"
 DocumentRoot "@DocumentRoot@"


_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genfile and gendir to emulate mkdir -p

Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:

> On Mon, 17 Sep 2001, Stas Bekman wrote:
> 
> 
>>I was wondering whether it'd be better to use File::Path::mkpath in 
>>genfile and gendir so the test writers won't have to call this sub many 
>>times if they need to create a dir foo/bar/baz or a file 
>>foo/bar/baz/test.conf, when foo doesn't exist. Of course in this case 
>>the cleanup code should use File::Path::rmtree. If you think it's cool, 
>>I'll send a patch.
>>
> 
> +1 for s/mkdir/mkpath/ in both gendir and t_mkdir.  however, the cleanup
> code intentionally does not use rmtree.  it carefully removes generated
> files first (created via genfile), then only removes the generated
> directories if they are empty.  i don't think rmtree provides that
> functionality.
> also, switching to gendir to use mkpath will require you to add each
> subdirectory created to $self->{clean}->{dirs}


Yes, I'm aware of that. And that's exactly what I've planned to do. 
It'll be somewhat slower than mkpath, but not slower than the current 
code, and will take away from the test writer the mess of doing all this 
work.



_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: genfile and gendir to emulate mkdir -p

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 17 Sep 2001, Stas Bekman wrote:

> 
> I was wondering whether it'd be better to use File::Path::mkpath in 
> genfile and gendir so the test writers won't have to call this sub many 
> times if they need to create a dir foo/bar/baz or a file 
> foo/bar/baz/test.conf, when foo doesn't exist. Of course in this case 
> the cleanup code should use File::Path::rmtree. If you think it's cool, 
> I'll send a patch.

+1 for s/mkdir/mkpath/ in both gendir and t_mkdir.  however, the cleanup
code intentionally does not use rmtree.  it carefully removes generated
files first (created via genfile), then only removes the generated
directories if they are empty.  i don't think rmtree provides that
functionality.
also, switching to gendir to use mkpath will require you to add each
subdirectory created to $self->{clean}->{dirs}


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


genfile and gendir to emulate mkdir -p

Posted by Stas Bekman <st...@stason.org>.
I was wondering whether it'd be better to use File::Path::mkpath in 
genfile and gendir so the test writers won't have to call this sub many 
times if they need to create a dir foo/bar/baz or a file 
foo/bar/baz/test.conf, when foo doesn't exist. Of course in this case 
the cleanup code should use File::Path::rmtree. If you think it's cool, 
I'll send a patch.

_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Parent/Clone?

Posted by Stas Bekman <st...@stason.org>.
On Mon, 17 Sep 2001, Doug MacEachern wrote:

> On Mon, 17 Sep 2001, Stas Bekman wrote:
>
> > What's the exact difference between the two? If parent implies Clone,
> > how  is it different?
>
> +Clone will share the parent interpreter, but give the vhost its own
> interpreter pool
>
> +Parent will create a new parent interpreter for the given vhost and have
> its own interpreter pool

Thanks, I've also reread the modperl-2.0.pod, which covers this in
details.

_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: Parent/Clone?

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 17 Sep 2001, Stas Bekman wrote:

> What's the exact difference between the two? If parent implies Clone, 
> how  is it different?

+Clone will share the parent interpreter, but give the vhost its own
interpreter pool

+Parent will create a new parent interpreter for the given vhost and have
its own interpreter pool



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Parent/Clone?

Posted by Stas Bekman <st...@stason.org>.
What's the exact difference between the two? If parent implies Clone, 
how  is it different?


=item Parent

Create a new parent Perl interpreter for the given VirtualHost
(implies Clone).

=item Clone

Give the VirtualHost its own interpreter pool.

Thanks!

_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://apachetoday.com http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org