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 Geoffrey Young <ge...@modperlcookbook.org> on 2003/11/01 03:30:01 UTC
Re: [PATCH] allow implicit ServerRoot via apxs
> The easiest way I've found to do this in Apache::Test is attached. It
> extracts the "PREFIX" from apxs and uses that as the default inherited
> ServerRoot value. If a value is hard-coded into the global httpd.conf,
> it supercedes the apxs value and everything works just like before.
hmm. it looks like PREFIX is only related to the installation, whereas
ServerRoot, if not specified, is hard-coded to /usr/local/apache (according
to include/httpd.h in 2.0). so, PREFIX isn't much help if one installs then
moves the binaries to a new location.
but I suppose that a guess is better than nothing if we don't have a
ServerRoot to pull from the default httpd.conf, and PREFIX and ServerRoot
match for me :)
>
> The patch is against 1.05 and I tested it with Apache 1.3.29 and 1.3.28.
cool. I'll give it a whirl next week under a few different conditions.
thanks!
--Geoff
btw, doesn't it seem strange that prefix defaults to /usr/local/apache2 in
config.layout but ServerRoot defaults to /usr/local/apache? hmm...
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
[...]
>> I'm not sure it's a good idea to just:
>>
>> + $self->{inherit_config}->{ServerRoot} ||= $self->apxs('PREFIX');
>>
>> may be better to use PREFIX if ServerRoot is not defined in the code
>> where it's needed, so it'll be clear that we don't have ServerRoot
>> defined. And if we fail to expand paths we can tell a user the reason.
>> If we have ServerRoot=PREFIX we can't tell the real reason to the user.
>
>
> how about something like the attached patch? it seems to work for me
> under 1.3 and 2.0, and dies on a 1.3 with no ServerRoot and no mod_so.
> mike, can you give it a whirl?
>
> one thing I noticed is that if ServerRoot is not specified in httpd.conf
> then Apache-Test can't resolve relative Include directives (such as
> conf/ssl.conf), so they need to be postponed until later in order to be
> picked up as they would if ServerRoot were specified.
So my suggestion complicates things compared to the original Mike's patch.
Would it make things smoother if we did a silent ServerRoot=PREFIX and keep
the code as before, but raise a flag ServerRootMissing=1, which we can use in
the error message if failing to resolve relative paths?
__________________________________________________________________
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] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> + elsif (file_name_is_absolute($file)) {
> Shouldn't the "file_name_is_absolute" case be first?
>
indeed - I only noticed that case after I had coded the others.
clearly not enough coffee yet :)
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Mike Cramer <cr...@webkist.com>.
Geoffrey Young wrote:
> + if ($base = $self->{inherit_config}->{ServerRoot}) {
> + debug "using inherited ServerRoot $base to resolve $file";
> + }
> + elsif ($base = $self->apxs('PREFIX')) {
> + warning "using apxs-derived ServerRoot $base to resolve $file";
> + }
> + elsif (file_name_is_absolute($file)) {
> + debug "$file is already absolute";
> + return $file
> + }
> + else {
> + error "unable to resolve $file - please specify a ",
> + 'ServerRoot in your httpd.conf or use apxs';
> + die '';
> + }
> }
>
> rel2abs $file, $base;
Shouldn't the "file_name_is_absolute" case be first?
--
Mike Cramer
http://www.webkist.com/ | AIM: MikeWebkist
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Mike Cramer wrote:
> Geoffrey Young wrote:
>
>> I've tested it against everything I have. mike, if you can give CVS a
>> whirl today, that would be great.
>
>
> I finally had a chance to try this out this morning and it worked
> perfectly -- and when I forgot to feed it the path to apxs the first
> time I tried it, the error message worked as well.
>
excellent, thanks for testing.
this will be in the 1.06 release of Apache-Test, which should happen either
today or tomorrow.
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Mike Cramer <cr...@webkist.com>.
Geoffrey Young wrote:
> I've tested it against everything I have. mike, if you can give CVS a
> whirl today, that would be great.
I finally had a chance to try this out this morning and it worked
perfectly -- and when I forgot to feed it the path to apxs the first
time I tried it, the error message worked as well.
--
Mike Cramer
http://www.webkist.com/ | AIM: MikeWebkist
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
this is in now.
I've tested it against everything I have. mike, if you can give CVS a whirl
today, that would be great.
$ cvs -z9 ":pserver:anoncvs@cvs.apache.org:/home/cvspublic" checkout
mod_perl-2.0
or
$ cvs -z9 ":pserver:anoncvs@cvs.apache.org:/home/cvspublic" checkout httpd-test
thanks.
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> the call to rel2abs? since it does no check of the underlying
>> filesystem I doubt that's realistic.
>
>
> you are right, I've confused with the opposite operation abs2rel, which
> may fail if the base doesn't fit.
>
> In any case it's a good idea to check that rel2abs gives you a path that
> exists, no?
yeah, I had planned on that.
I'm off to ny.pm tomorrow, so I'll probably get back around to this on thursday.
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>>>> and it's an absolute_path.
>>>
>>>
>>>
>>>
>>> no sense checking that - rel2abs returns an absolute path by definition.
>>
>>
>>
>> what if it fails?
>
>
> the call to rel2abs? since it does no check of the underlying
> filesystem I doubt that's realistic.
you are right, I've confused with the opposite operation abs2rel, which may
fail if the base doesn't fit.
In any case it's a good idea to check that rel2abs gives you a path that
exists, no?
__________________________________________________________________
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] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>> and it's an absolute_path.
>>
>>
>>
>> no sense checking that - rel2abs returns an absolute path by definition.
>
>
> what if it fails?
the call to rel2abs? since it does no check of the underlying filesystem I
doubt that's realistic.
I'm definitely a fan of error checking where it makes sense, but I also
think it's safe to rely on APIs to do what they say. we don't go checking
calls to catfile to make sure the result has slashes in them :)
>
>>> or die otherwise.
>>
>>
>>
>> I don't think we should die here - it's not fatal to the build process
>> if 'perl Makefile.PL' can't resolve testing files.
>
>
> Hmm, look at your patch, you die if there is no base. I was just
> following your idea ;)
oops, I meant to take that out - as you can see, though, I was thinking
about whether it was ok to die here ;)
it seem, though, that everyone is kinda happy with putting the logic change
here, so I'll work these changes up and submit something new soonish.
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>
> Stas Bekman wrote:
>
>> Geoffrey Young wrote:
>> [...]
>>
>>> + elsif ($base = $self->apxs('PREFIX')) {
>>> + warning "using apxs-derived ServerRoot $base to resolve
>>> $file";
>>
>>
>>
>> May be better to say it all?
>>
>> warning "since ServerRoot is not defined, " .
>> "using apxs-derived PREFIX $base to resolve $file";
>
>
> I suppose the wording could be reworked.
sure ;)
>> While you are at it, I'd also add a check that $base exists and it's a
>> directory before you call rel2abs, because it can be a broken value.
>
>
> I think it's safe to assume if you pass in $base you know what you're
> doing (especially since nobody appears to be passing in $base :)
yes, but you attempt to do:
$base = $self->apxs('PREFIX')
what if it returns a non-existing dir? apxs may be not adjusted to the
movement of the files post-install. It doesn't hurt to check.
> as for checking the resulting ServerRoot, yeah, I thought about that. I
> suppose it can't hurt.
>
>> And also check that rel2abs gives us a an existing filename
>
>
> this function is starting to explode a bit...
So what, it's just a function. Why not making it foolproof why we are spending
time at it.
>> and it's an absolute_path.
>
>
> no sense checking that - rel2abs returns an absolute path by definition.
what if it fails?
>> or die otherwise.
>
>
> I don't think we should die here - it's not fatal to the build process
> if 'perl Makefile.PL' can't resolve testing files.
Hmm, look at your patch, you die if there is no base. I was just following
your idea ;)
__________________________________________________________________
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] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Geoffrey Young wrote:
> [...]
>
>> + elsif ($base = $self->apxs('PREFIX')) {
>> + warning "using apxs-derived ServerRoot $base to resolve
>> $file";
>
>
> May be better to say it all?
>
> warning "since ServerRoot is not defined, " .
> "using apxs-derived PREFIX $base to resolve $file";
I suppose the wording could be reworked.
>
> While you are at it, I'd also add a check that $base exists and it's a
> directory before you call rel2abs, because it can be a broken value.
I think it's safe to assume if you pass in $base you know what you're doing
(especially since nobody appears to be passing in $base :)
as for checking the resulting ServerRoot, yeah, I thought about that. I
suppose it can't hurt.
> And
> also check that rel2abs gives us a an existing filename
this function is starting to explode a bit...
> and it's an
> absolute_path.
no sense checking that - rel2abs returns an absolute path by definition.
> or die otherwise.
I don't think we should die here - it's not fatal to the build process if
'perl Makefile.PL' can't resolve testing files.
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
[...]
> + elsif ($base = $self->apxs('PREFIX')) {
> + warning "using apxs-derived ServerRoot $base to resolve $file";
May be better to say it all?
warning "since ServerRoot is not defined, " .
"using apxs-derived PREFIX $base to resolve $file";
While you are at it, I'd also add a check that $base exists and it's a
directory before you call rel2abs, because it can be a broken value. And also
check that rel2abs gives us a an existing filename and it's an absolute_path.
or die otherwise.
__________________________________________________________________
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] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Your patch works fine in my config here, but I do forsee one possible
> issue -- although this is a VERY UNLIKELY problem:
>
> <IfModule mod_foo.c>
> Include conf/foo.conf
> </IfModule>
>
> If I read your patch right, it would turn that into:
>
> <IfModule mod_foo.c>
> </IfModule>
> [rest of httpd.conf]
> [Contents of conf/foo.conf inserted here]
well, I don't think it was my patch that was doing that. the way I read the
code is that IfModule is always ignored and Include (and other) directives
are always parsed. but yes, I was moving the placement :)
>
> Obviously, Apache-Test is only really interested in a couple directives
> from the user's config file, so the chances of that reordering (and
> removal of code from the conditional block) causing problems is very small.
>
> As a bit of a side note: Apache 1.3.28 does it the simple way:
>
> 1) set "ap_server_root" to HTTPD_ROOT (http_main.c: 5396)
that's what we're discussing :)
> 2) set "ap_server_root" to "-d VAL", if present (http_main.c: 5422)
I don't think we can support that easily, since we already have -serverroot
and it means something different (it's where you run the tests from). but
if it comes up again we probably ought to consider it.
> 3) set "ap_server_root" using ServerRoot directive (via normal module
> initalization stuff)
>
> Whichever one is hit last is used, since it's a single global value.
>
> So it seems like setting things up in that order within Apache-Test
> would be reasonable. But I agree with Stas that having a record of WHICH
> ServerRoot value is being used is a good thing and would surely ease
> some confusion in edge cases like mine.
here is yet another approach. it's pretty verbose, but I think I want
_some_ warning in there if PREFIX is used, since it's kinda odd. but maybe
not on every file resolution, though. but server_file_rel2abs is the only
place I can see that uses inherit_config->{ServerRoot} so that seemed like
the logical place to put everything. I dunno...
one additional thing the patch does address is a bogus warning I was seeing
on 1.3 where the file is already absolute.
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Mike Cramer <cr...@webkist.com>.
Geoffrey Young wrote:
> one thing I noticed is that if ServerRoot is not specified in httpd.conf
> then Apache-Test can't resolve relative Include directives (such as
> conf/ssl.conf), so they need to be postponed until later in order to be
> picked up as they would if ServerRoot were specified.
Your patch works fine in my config here, but I do forsee one possible
issue -- although this is a VERY UNLIKELY problem:
<IfModule mod_foo.c>
Include conf/foo.conf
</IfModule>
If I read your patch right, it would turn that into:
<IfModule mod_foo.c>
</IfModule>
[rest of httpd.conf]
[Contents of conf/foo.conf inserted here]
Obviously, Apache-Test is only really interested in a couple directives
from the user's config file, so the chances of that reordering (and
removal of code from the conditional block) causing problems is very small.
As a bit of a side note: Apache 1.3.28 does it the simple way:
1) set "ap_server_root" to HTTPD_ROOT (http_main.c: 5396)
2) set "ap_server_root" to "-d VAL", if present (http_main.c: 5422)
3) set "ap_server_root" using ServerRoot directive (via normal module
initalization stuff)
Whichever one is hit last is used, since it's a single global value.
So it seems like setting things up in that order within Apache-Test
would be reasonable. But I agree with Stas that having a record of WHICH
ServerRoot value is being used is a good thing and would surely ease
some confusion in edge cases like mine.
--
Mike Cramer
http://www.webkist.com/ | AIM: MikeWebkist
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> But this is not a guesswork. If ServerRoot is not
> specified PREFIX should be the one, since this is what Apache is doing.
ah, ok. I didn't see how ap_config.h was pulling in the apxs prefix. cool.
> So use that and verify that the files can be found. If not, complain
> that ServerRoot can't be determined and die.
>
> I'm not sure it's a good idea to just:
>
> + $self->{inherit_config}->{ServerRoot} ||= $self->apxs('PREFIX');
>
> may be better to use PREFIX if ServerRoot is not defined in the code
> where it's needed, so it'll be clear that we don't have ServerRoot
> defined. And if we fail to expand paths we can tell a user the reason.
> If we have ServerRoot=PREFIX we can't tell the real reason to the user.
how about something like the attached patch? it seems to work for me under
1.3 and 2.0, and dies on a 1.3 with no ServerRoot and no mod_so. mike, can
you give it a whirl?
one thing I noticed is that if ServerRoot is not specified in httpd.conf
then Apache-Test can't resolve relative Include directives (such as
conf/ssl.conf), so they need to be postponed until later in order to be
picked up as they would if ServerRoot were specified.
--Geoff
Re: [PATCH] allow implicit ServerRoot via apxs
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>> The easiest way I've found to do this in Apache::Test is attached. It
>> extracts the "PREFIX" from apxs and uses that as the default inherited
>> ServerRoot value. If a value is hard-coded into the global httpd.conf,
>> it supercedes the apxs value and everything works just like before.
>
>
> hmm. it looks like PREFIX is only related to the installation, whereas
> ServerRoot, if not specified, is hard-coded to /usr/local/apache
> (according to include/httpd.h in 2.0). so, PREFIX isn't much help if
> one installs then moves the binaries to a new location.
If they move .so modules to a different location without defining ServerRoot
things won't work.
> but I suppose that a guess is better than nothing if we don't have a
> ServerRoot to pull from the default httpd.conf, and PREFIX and
> ServerRoot match for me :)
-1 on adding any new guesswork. So far it was causing more damage than it was
adding value. But this is not a guesswork. If ServerRoot is not specified
PREFIX should be the one, since this is what Apache is doing. So use that and
verify that the files can be found. If not, complain that ServerRoot can't be
determined and die.
I'm not sure it's a good idea to just:
+ $self->{inherit_config}->{ServerRoot} ||= $self->apxs('PREFIX');
may be better to use PREFIX if ServerRoot is not defined in the code where
it's needed, so it'll be clear that we don't have ServerRoot defined. And if
we fail to expand paths we can tell a user the reason. If we have
ServerRoot=PREFIX we can't tell the real reason to the user.
> btw, doesn't it seem strange that prefix defaults to /usr/local/apache2
> in config.layout but ServerRoot defaults to /usr/local/apache? hmm...
Sounds like a porting to httpd 2.0 bug. But since the majority specifies
ServerRoot nobody has noticed it. Report to httdp-dev?
__________________________________________________________________
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