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 Stas Bekman <st...@stason.org> on 2004/01/15 00:24:23 UTC

[Fwd: Re: Modperl 2.0 Not finding correct *.conf]

any objections for this patch?

-------- Original Message --------
Subject: Re: Modperl 2.0 Not finding correct *.conf
Date: Sun, 11 Jan 2004 14:13:42 -0800
From: Stas Bekman <st...@stason.org>
Organization: Hope, Humanized
CC: steve larson <ex...@yahoo.com>,	mod_perl Mailing List 
<mo...@perl.apache.org>
References: <20...@web40706.mail.yahoo.com> 
<40...@stason.org>

Stas Bekman wrote:
> steve larson wrote:
> 
>> Hello,
>> ulimit -c unlimited; t/TEST -conf
>> *** file /etc/httpd/conf.d/*.conf does not exist
>>  make test produced error, looking for
>> /etc/httpd/*.conf 
> 
> it's a warning, not an error. It'd have died if it was an error. I can 
> see where the problem is, I'll fix that soonish. It's unrelated to the 
> problem preventing you from running the test suite.

Please try this patch:

Index: lib/Apache/TestConfigParse.pm
===================================================================
RCS file:
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigParse.pm,v
retrieving revision 1.41
diff -u -r1.41 TestConfigParse.pm
--- lib/Apache/TestConfigParse.pm       9 Dec 2003 14:53:58 -0000       1.41
+++ lib/Apache/TestConfigParse.pm       11 Jan 2004 21:59:59 -0000
@@ -9,7 +9,7 @@
  use Apache::TestTrace;

  use File::Spec::Functions qw(rel2abs splitdir file_name_is_absolute);
-use File::Basename qw(basename);
+use File::Basename qw(dirname basename);

  sub strip_quotes {
      local $_ = shift || $_;
@@ -43,7 +43,8 @@
      $self->$where($directive => $val);
  }

-#resolve relative files like Apache->server_root_relative
+# resolve relative files like Apache->server_root_relative
+# this function doesn't test whether the resolved file exists
  sub server_file_rel2abs {
      my($self, $file, $base) = @_;

@@ -87,14 +88,20 @@
              # return early, skipping file test below
              return $file;
          }
-
      }

-    if (-e $result) {
-        debug "$file successfully resolved to existing file $result";
+    my $dir = dirname $result;
+    # $file might not exist (e.g. if it's a glob pattern like
+    # "conf/*.conf" but what we care about here is to check whether
+    # the base dir was successfully resolved. we don't check whether
+    # the file exists at all. it's the responsibility of the caller to
+    # do this check
+    if (defined $dir && -e $dir && -d _) {
+        debug "$file successfully resolved to $result";
      }
      else {
-        warning "file $result does not exist";
+        $dir ||= '';
+        warning "dir '$dir' does not exist (while resolving '$file')";

          # old behavior was to return the resolved but non-existent
          # file.  preserve that behavior and return $result anyway.

__________________________________________________________________
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


-- 
Reporting bugs: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html

-- 


__________________________________________________________________
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: [Fwd: Re: Modperl 2.0 Not finding correct *.conf]

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
>>In any case, adding a check for a file is fine with me as long as you
>>s/warning/debug/, so it won't appear in the normal output and users
>>don't send false complaints. How about adding it as an extra check on
>>top of this patch?
> 
> 
> that all sounds fine.

Thanks Geoff. I've added that change.

Feel free to improve it if it looks not so good.

__________________________________________________________________
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: [Fwd: Re: Modperl 2.0 Not finding correct *.conf]

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

> In any case, adding a check for a file is fine with me as long as you
> s/warning/debug/, so it won't appear in the normal output and users
> don't send false complaints. How about adding it as an extra check on
> top of this patch?

that all sounds fine.

--Geoff


Re: [Fwd: Re: Modperl 2.0 Not finding correct *.conf]

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
> Stas Bekman wrote:
> 
>>any objections for this patch?
> 
> 
>>-    if (-e $result) {
>>-        debug "$file successfully resolved to existing file $result";
> 
> 
> well, you're removing the file check in favor of a directory check.  in the
> interests of debugging, I'd probably like to preserve the check and throw a
> warning when $dir exists but the resolved file does not.  I think the caller
> is responsible for making sure the file exists because it may or may not
> care and result of the call is the same no matter what.  but it was nice to
> have the debug output the last time I worked on this chunk of code.

yes, but we don't want users to complain about:

*** file /etc/httpd/conf.d/*.conf does not exist

and several people did already. Obviously /etc/httpd/conf.d/*.conf doesn't 
exist, so what good does this test make? The caller already checks whether the 
output of glob() exists before using it. So I thought that the purpose of this 
function is the same as rel2abs (Which does not check whether the resulting 
file or whatever exists) the only difference is that it uses extra heuristics 
to get the value of the base. Therefore I think checking for the directory is 
the right thing to do.

In any case, adding a check for a file is fine with me as long as you 
s/warning/debug/, so it won't appear in the normal output and users don't send 
false complaints. How about adding it as an extra check on top of this patch?

__________________________________________________________________
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: [Fwd: Re: Modperl 2.0 Not finding correct *.conf]

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

Stas Bekman wrote:
> any objections for this patch?

> -    if (-e $result) {
> -        debug "$file successfully resolved to existing file $result";

well, you're removing the file check in favor of a directory check.  in the
interests of debugging, I'd probably like to preserve the check and throw a
warning when $dir exists but the resolved file does not.  I think the caller
is responsible for making sure the file exists because it may or may not
care and result of the call is the same no matter what.  but it was nice to
have the debug output the last time I worked on this chunk of code.

--Geoff