You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by "Philippe M. Chiasson" <go...@ectoplasm.org> on 2004/12/07 22:55:17 UTC
[mp2 Patch] Static build and mod_perl include files
Stas just discovered that when building static, mod_perl's header files ended up
being installed in httpd's build tree. That had 2 bad effects. First, 3rd party
modules wouldn't find the headers. Second, thus polluted, the httpd source tree
wouldn't build anymore, confused by header files that shouldn't be there.
The following patch _fixes_ that problem, but the only way I could manage to do
it was by peeking at config_vars.mk. It did make me think about how we could make
use of config_vars.mk ourselves more directly and not rely on system(apxs) as much
in the future.
Anyways, here is the patch itself.
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: [mp2 Patch] Static build and mod_perl include files
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Index: Makefile.PL
> - MODPERL_AP_INCLUDEDIR => $build->ap_includedir(),
> + MODPERL_AP_INCLUDEDIR => $build->->install_headers_dir(),
doesn't look like you've tested it :) ->->
> MODPERL_XS_H_FILES => join(" \\\n\t", @xs_h_files),
> },
> clean => {
> Index: lib/Apache/Build.pm
> ===================================================================
[...]
> +
> + print $fh $self->canon_make_attr('AP_INCLUDEDIR',
> + $self->install_headers_dir());
>
> - for my $q (qw(LIBEXECDIR INCLUDEDIR)) {
> + for my $q (qw(LIBEXECDIR)) {
> print $fh $self->canon_make_attr("AP_$q",
> $self->apxs(-q => $q));
> }
no more need for for() loop here :)
otherwise +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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 Patch] Static build and mod_perl include files
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
>
>>Stas Bekman wrote:
>>
>>>Philippe M. Chiasson wrote:
>>>
>>>>Stas just discovered that when building static, mod_perl's header
>>>>files ended up
>>>>being installed in httpd's build tree. That had 2 bad effects. First,
>>>>3rd party
>>>>modules wouldn't find the headers. Second, thus polluted, the httpd
>>>>source tree
>>>>wouldn't build anymore, confused by header files that shouldn't be
>>>>there.
>>>>
>>>>The following patch _fixes_ that problem, but the only way I could
>>>>manage to do
>>>>it was by peeking at config_vars.mk. It did make me think about how
>>>>we could make
>>>>use of config_vars.mk ourselves more directly and not rely on
>>>>system(apxs) as much
>>>>in the future.
>>>>
>>>>Anyways, here is the patch itself.
>>>
>>> [...]
>
> OK, how about then writing a wrapper for install_headers_dir() which will
> do the right thing internally. Exposing this logic/complexity at the
> Makefile.PL level doesn't sound as a very good idea.
You are right, would be much cleaner to do so. Patch attached !
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: [mp2 Patch] Static build and mod_perl include files
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Stas Bekman wrote:
>
>> Philippe M. Chiasson wrote:
>>
>>> Stas just discovered that when building static, mod_perl's header
>>> files ended up
>>> being installed in httpd's build tree. That had 2 bad effects. First,
>>> 3rd party
>>> modules wouldn't find the headers. Second, thus polluted, the httpd
>>> source tree
>>> wouldn't build anymore, confused by header files that shouldn't be
>>> there.
>>>
>>> The following patch _fixes_ that problem, but the only way I could
>>> manage to do
>>> it was by peeking at config_vars.mk. It did make me think about how
>>> we could make
>>> use of config_vars.mk ourselves more directly and not rely on
>>> system(apxs) as much
>>> in the future.
>>>
>>> Anyways, here is the patch itself.
>>
>>
>> thanks philippe, please see comments below:
>>
>>> Index: Makefile.PL
>>> ===================================================================
>>> --- Makefile.PL (revision 111148)
>>> +++ Makefile.PL (working copy)
>>> @@ -66,6 +66,8 @@
>>>
>>> my @exe_files = map "bin/$_", qw(mp2doc mp2bug);
>>>
>>> +my $include_dir = $build->ap_exp_includedir() ||
>>> $build->ap_includedir();
>>
>>
>> why do we need expose that on the top level and complicate things? why
>> not have ap_includedir() call that internally?
>
>
> That was my original intent, but the problem is that there are really 2
> values
> for includedir, one is where to get include files from for -I and the
> other one
> is for where we need to install our header files. In the case of the
> dynamic build,
> these 2 places are one and the same.
>
> In the case of the static build, the headers we need to compile are in
> httpd's
> source tree while the location we need to install our own headers in
> elsewhere.
>
> My initial attempt was to change ap_includedir() but that quite rightly
> broke
> everything. That's why they need to be kept separate.
OK, how about then writing a wrapper for install_headers_dir() which will
do the right thing internally. Exposing this logic/complexity at the
Makefile.PL level doesn't sound as a very good idea.
>>> [...]
>>> + my $build_vars = File::Spec->catfile($self->{MP_AP_PREFIX},
>>> + 'build', 'config_vars.mk');
>>> + open my $vars, "<$build_vars" || die "Couldn't open $build_vars
>>> $!";
>>
>>
>>
>> must be 'or' and not '||', with || it's the same as:
>>
>> open my $vars, "<$build_vars";
>>
>> because of the higher precendence of ||.
>
>
> I knew that ;-) Thanks for the spot!
I know that you do, but may be other readers don't :)
--
__________________________________________________________________
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2 Patch] Static build and mod_perl include files
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
>
>>Stas just discovered that when building static, mod_perl's header files
>>ended up
>>being installed in httpd's build tree. That had 2 bad effects. First,
>>3rd party
>>modules wouldn't find the headers. Second, thus polluted, the httpd
>>source tree
>>wouldn't build anymore, confused by header files that shouldn't be there.
>>
>>The following patch _fixes_ that problem, but the only way I could
>>manage to do
>>it was by peeking at config_vars.mk. It did make me think about how we
>>could make
>>use of config_vars.mk ourselves more directly and not rely on
>>system(apxs) as much
>>in the future.
>>
>>Anyways, here is the patch itself.
>
> thanks philippe, please see comments below:
>
>>Index: Makefile.PL
>>===================================================================
>>--- Makefile.PL (revision 111148)
>>+++ Makefile.PL (working copy)
>>@@ -66,6 +66,8 @@
>>
>> my @exe_files = map "bin/$_", qw(mp2doc mp2bug);
>>
>>+my $include_dir = $build->ap_exp_includedir() || $build->ap_includedir();
>
> why do we need expose that on the top level and complicate things? why not
> have ap_includedir() call that internally?
That was my original intent, but the problem is that there are really 2 values
for includedir, one is where to get include files from for -I and the other one
is for where we need to install our header files. In the case of the dynamic build,
these 2 places are one and the same.
In the case of the static build, the headers we need to compile are in httpd's
source tree while the location we need to install our own headers in elsewhere.
My initial attempt was to change ap_includedir() but that quite rightly broke
everything. That's why they need to be kept separate.
>> [...]
>>+ my $build_vars = File::Spec->catfile($self->{MP_AP_PREFIX},
>>+ 'build', 'config_vars.mk');
>>+ open my $vars, "<$build_vars" || die "Couldn't open $build_vars $!";
>
>
> must be 'or' and not '||', with || it's the same as:
>
> open my $vars, "<$build_vars";
>
> because of the higher precendence of ||.
I knew that ;-) Thanks for the spot!
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: [mp2 Patch] Static build and mod_perl include files
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> Stas just discovered that when building static, mod_perl's header files
> ended up
> being installed in httpd's build tree. That had 2 bad effects. First,
> 3rd party
> modules wouldn't find the headers. Second, thus polluted, the httpd
> source tree
> wouldn't build anymore, confused by header files that shouldn't be there.
>
> The following patch _fixes_ that problem, but the only way I could
> manage to do
> it was by peeking at config_vars.mk. It did make me think about how we
> could make
> use of config_vars.mk ourselves more directly and not rely on
> system(apxs) as much
> in the future.
>
> Anyways, here is the patch itself.
thanks philippe, please see comments below:
> Index: Makefile.PL
> ===================================================================
> --- Makefile.PL (revision 111148)
> +++ Makefile.PL (working copy)
> @@ -66,6 +66,8 @@
>
> my @exe_files = map "bin/$_", qw(mp2doc mp2bug);
>
> +my $include_dir = $build->ap_exp_includedir() || $build->ap_includedir();
why do we need expose that on the top level and complicate things? why not
have ap_includedir() call that internally?
> ModPerl::BuildMM::WriteMakefile(
> NAME => 'mod_perl',
> VERSION => $VERSION,
> @@ -78,7 +80,7 @@
> MODPERL_MAKEFILE => basename($build->default_file('makefile')),
> PERL => $build->perl_config('perlpath'),
> MOD_INSTALL => ModPerl::BuildMM::mod_install(),
> - MODPERL_AP_INCLUDEDIR => $build->ap_includedir(),
> + MODPERL_AP_INCLUDEDIR => $include_dir,
> MODPERL_XS_H_FILES => join(" \\\n\t", @xs_h_files),
> },
> clean => {
> Index: lib/Apache/Build.pm
> ===================================================================
> --- lib/Apache/Build.pm (revision 111148)
> +++ lib/Apache/Build.pm (working copy)
> @@ -946,6 +946,22 @@
> $self->{ap_includedir} = $d;
> }
>
> +sub ap_exp_includedir {
> + my ($self) = @_;
> + return unless $self->should_build_apache;
> + my $build_vars = File::Spec->catfile($self->{MP_AP_PREFIX},
> + 'build', 'config_vars.mk');
> + open my $vars, "<$build_vars" || die "Couldn't open $build_vars $!";
must be 'or' and not '||', with || it's the same as:
open my $vars, "<$build_vars";
because of the higher precendence of ||.
--
__________________________________________________________________
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org