You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2005/02/16 22:18:42 UTC

Re: [mp2] [PATCH] DESTDIR does not apply to mod_perl.so and includes

Redirecting to the dev list, so others can vote on this:

>>That's why the only sensible thing I can suggest is a second DESTDIR 
>>argument, which is apache-specific and of course it needs to be called 
>>differently. I've earlier suggested MP_AP_DESTDIR.
> 
> 
> Attached is a patch which will add a new build time option
> MP_AP_DESTDIR, which will be prefixed to all of the Apache bits
> for installation.  This works for me, building SysV packages on
> Solaris 8 in a non-root environment.

I'm +1. An excellent patch, Cory!

But doesn't it need to deal with the trailing slash? Is it ok if someone 
adds one and then it'll becomes foo//bar? I guess we also assume that the 
right part always has the leading slash, since it's a real path to the 
installed Apache dir, right? In the case of windows where it will become 
fooc:\bar is probably irrelevant here.

> ------------------------------------------------------------------------
> 
> diff --speed-large-files --minimal -Nru mod_perl-2.0.0-RC4.orig/Makefile.PL mod_perl-2.0.0-RC4/Makefile.PL
> --- mod_perl-2.0.0-RC4.orig/Makefile.PL	2005-02-16 15:22:21.709509000 -0500
> +++ mod_perl-2.0.0-RC4/Makefile.PL	2005-02-16 14:50:44.791344000 -0500
> @@ -79,6 +79,7 @@
>          PERL               => $build->perl_config('perlpath'),
>          MOD_INSTALL        => ModPerl::BuildMM::mod_install(),
>          MODPERL_AP_INCLUDEDIR  => $build->install_headers_dir(),
> +        MODPERL_AP_DESTDIR => $build->{MP_AP_DESTDIR},
>          MODPERL_XS_H_FILES => join(" \\\n\t", @xs_h_files),
>      },
>      clean     => {
> @@ -672,8 +673,8 @@
>  	cd "$(MODPERL_SRC)" && $(MAKE) install
>  
>  modperl_xs_h_install:
> -	@$(MKPATH) $(MODPERL_AP_INCLUDEDIR)
> -	$(CP) $(MODPERL_XS_H_FILES) $(MODPERL_AP_INCLUDEDIR)
> +	@$(MKPATH) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_INCLUDEDIR)
> +	$(CP) $(MODPERL_XS_H_FILES) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_INCLUDEDIR)
>  
>  modperl_src_clean:
>  	cd "$(MODPERL_SRC)" && $(MAKE) clean
> diff --speed-large-files --minimal -Nru mod_perl-2.0.0-RC4.orig/docs/user/install/install.pod mod_perl-2.0.0-RC4/docs/user/install/install.pod
> --- mod_perl-2.0.0-RC4.orig/docs/user/install/install.pod	2005-02-16 15:22:17.271960000 -0500
> +++ mod_perl-2.0.0-RC4/docs/user/install/install.pod	2005-02-16 15:21:45.448926000 -0500
> @@ -511,6 +511,17 @@
>  
>    % t/TEST -httpd /home/stas/httpd/prefork/bin/httpd
>  
> +=head4 MP_AP_DESTDIR
> +
> +Apache installation destination directory.  This path will be prefixed to the
> +installation paths for all Apache specific files during C<make install>.  For
> +instance, if C<MP_AP_DESTDIR> is set to C</tmp/foo>, the Apache module will be
> +installed to:
> +
> +  /tmp/foo/path/to/httpd-2.0/libexec/mod_perl.so
> +
> +This option exists to make the lives of package maintainers easier.
> +
>  =head4 MP_APR_CONFIG
>  
>  If APR wasn't installed under the same file tree as httpd, you may
> diff --speed-large-files --minimal -Nru mod_perl-2.0.0-RC4.orig/lib/Apache/Build.pm mod_perl-2.0.0-RC4/lib/Apache/Build.pm
> --- mod_perl-2.0.0-RC4.orig/lib/Apache/Build.pm	2005-02-16 15:22:18.718177000 -0500
> +++ mod_perl-2.0.0-RC4/lib/Apache/Build.pm	2005-02-16 14:50:45.142351000 -0500
> @@ -1569,16 +1569,16 @@
>      if (!$self->should_build_apache) {
>          $install .= <<'EOI';
>  # install mod_perl.so
> -	@$(MKPATH) $(MODPERL_AP_LIBEXECDIR)
> +	@$(MKPATH) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_LIBEXECDIR)
>  	$(MODPERL_TEST_F) $(MODPERL_LIB_DSO) && \
> -	$(MODPERL_CP) $(MODPERL_LIB_DSO) $(MODPERL_AP_LIBEXECDIR)
> +	$(MODPERL_CP) $(MODPERL_LIB_DSO) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_LIBEXECDIR)
>  EOI
>      }
>      
>      $install .= <<'EOI';
>  # install mod_perl .h files
> -	@$(MKPATH) $(MODPERL_AP_INCLUDEDIR)
> -	$(MODPERL_CP) $(MODPERL_H_FILES) $(MODPERL_AP_INCLUDEDIR)
> +	@$(MKPATH) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_INCLUDEDIR)
> +	$(MODPERL_CP) $(MODPERL_H_FILES) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_INCLUDEDIR)
>  EOI
>  
>      my $mf = $self->default_file('makefile');
> @@ -1617,9 +1617,9 @@
>          print $fh $self->canon_make_attr('lib_symbols', $symbols);
>          $install .= <<'EOI';
>  # install mod_perl symbol file
> -	@$(MKPATH) $(MODPERL_AP_LIBEXECDIR)
> +	@$(MKPATH) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_LIBEXECDIR)
>  	$(MODPERL_TEST_F) $(MODPERL_LIB_SYMBOLS) && \
> -	$(MODPERL_CP) $(MODPERL_LIB_SYMBOLS) $(MODPERL_AP_LIBEXECDIR)
> +	$(MODPERL_CP) $(MODPERL_LIB_SYMBOLS) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_LIBEXECDIR)
>  EOI
>      }
>  
> @@ -1629,9 +1629,9 @@
>                                           "$self->{MP_AP_PREFIX}/lib");
>          $install .= <<'EOI';
>  # install mod_perl.lib
> -	@$(MKPATH) $(MODPERL_AP_LIBDIR)
> +	@$(MKPATH) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_LIBDIR)
>  	$(MODPERL_TEST_F) $(MODPERL_LIB_LOCATION) && \
> -	$(MODPERL_CP) $(MODPERL_LIB_LOCATION) $(MODPERL_AP_LIBDIR)
> +	$(MODPERL_CP) $(MODPERL_LIB_LOCATION) $(MODPERL_AP_DESTDIR)$(MODPERL_AP_LIBDIR)
>  EOI
>      }
>  
> @@ -1645,6 +1645,9 @@
>          print $fh $self->canon_make_attr('libperl', $libperl);
>      }
>  
> +    print $fh $self->canon_make_attr('ap_destdir', $self->{MP_AP_DESTDIR})
> +        if $self->{MP_AP_DESTDIR};
> +
>      for my $method (qw(ccopts ldopts inc)) {
>          print $fh $self->canon_make_attr($method, $self->$method());
>      }
> diff --speed-large-files --minimal -Nru mod_perl-2.0.0-RC4.orig/lib/ModPerl/BuildOptions.pm mod_perl-2.0.0-RC4/lib/ModPerl/BuildOptions.pm
> --- mod_perl-2.0.0-RC4.orig/lib/ModPerl/BuildOptions.pm	2005-02-16 15:22:20.166130000 -0500
> +++ mod_perl-2.0.0-RC4/lib/ModPerl/BuildOptions.pm	2005-02-16 14:50:45.262146000 -0500
> @@ -230,6 +230,7 @@
>  OPTIONS_FILE   0    Read options from given file
>  STATIC_EXTS    0    Build Apache::*.xs as static extensions
>  APXS           0    Path to apxs
> +AP_DESTDIR     1    Destination for Apache specific mod_perl bits
>  AP_PREFIX      0    Apache installation or source tree prefix
>  AP_CONFIGURE   0    Apache ./configure arguments
>  APR_CONFIG     0    Path to apr-config


-- 
__________________________________________________________________
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] DESTDIR does not apply to mod_perl.so and includes

Posted by Stas Bekman <st...@stason.org>.
Cory Omand wrote:
[...]
>>That's better, Cory, but why introducing new variables? you still kept the 
>>old ones, but they aren't used anymore, are they? (e.g. AP_LIBEXECDIR) 
>>Looks like a lot of extra noise.
>>
>>May be keep the old makefile vars unchanged but merge the new prefix 
>>transparently into those vars if the prefix exists? or at least drop the 
>>not-used vars?
> 
> 
> My initial thought was that this should behave like DESTDIR for
> other packages -- it is only set on installation.  Therefore, when
> you are using AP_LIBEXECDIR, you know it is pointing to where things
> will be when installed, but AP_LIBEXECDESTDIR is not referenced
> anywhere in the compilation portions of the makefile.  As I have
> another look through the makefile, however, it appears that the
> only purpose of the MODPERL_AP_* variables is for installation, so
> maybe moving to use the existing variable names is fine...
> 
> Another patch is attached --

Looks perfect to me, Cory

(it will need a few tiny style tweaks, but don't worry about that).

+1

once others vote on this it can go in.

-- 
__________________________________________________________________
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] DESTDIR does not apply to mod_perl.so and includes

Posted by Stas Bekman <st...@stason.org>.
Cory Omand wrote:

With a few minor tweaks it's now committed. Thanks Cory.

You can see the detail of commit here:
http://svn.apache.org/viewcvs.cgi?rev=154318&view=rev

> +AP_DESTDIR     1    Destination for Apache specific mod_perl bits

I've changed this to 0. I don't think this option should be appendable.

-- 
__________________________________________________________________
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] DESTDIR does not apply to mod_perl.so and includes

Posted by Stas Bekman <st...@stason.org>.
Cory Omand wrote:
> On Wed, 2005-02-16 at 13:18, Stas Bekman wrote:
> 
>>But doesn't it need to deal with the trailing slash? Is it ok if someone 
>>adds one and then it'll becomes foo//bar? I guess we also assume that the 
>>right part always has the leading slash, since it's a real path to the 
>>installed Apache dir, right? In the case of windows where it will become 
>>fooc:\bar is probably irrelevant here.
> 
> 
> Stas,
> 
> It shouldn't matter if someone specifies the trailing slash, as all
> that this path is used for is as an argument to $(MODPERL_CP).  You
> are correct about the right portion -- it always will have the
> leading slash, because that path comes back from apxs. I hadn't
> considered the windows case, however.  I've attached a new version
> of the patch which is 'volume aware', provides canonical paths, and
> would properly handle the case where apxs returns a path without a
> leading slash.

That's better, Cory, but why introducing new variables? you still kept the 
old ones, but they aren't used anymore, are they? (e.g. AP_LIBEXECDIR) 
Looks like a lot of extra noise.

May be keep the old makefile vars unchanged but merge the new prefix 
transparently into those vars if the prefix exists? or at least drop the 
not-used vars?


-- 
__________________________________________________________________
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