Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 166063 - Review Request: perl-HTML-Mason
Summary: Review Request: perl-HTML-Mason
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jose Pedro Oliveira
QA Contact: David Lawrence
URL: http://ftp.kspei.com/pub/steve/rpms/p...
Whiteboard:
Depends On: 166059 166065
Blocks: FE-ACCEPT 168227
TreeView+ depends on / blocked
 
Reported: 2005-08-16 14:40 UTC by Steven Pritchard
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-09-16 20:19:41 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Sample Mason configuration for the conf.d directory. (2.22 KB, text/plain)
2005-08-24 18:21 UTC, Chris Grau
no flags Details
Update to 1.3101 and requirments cleanup (2.18 KB, patch)
2005-08-26 16:04 UTC, Jose Pedro Oliveira
no flags Details | Diff
Specfile patch: adds documentation, ... (1.59 KB, patch)
2005-09-09 18:31 UTC, Jose Pedro Oliveira
no flags Details | Diff

Description Steven Pritchard 2005-08-16 14:40:58 UTC
Spec Name or Url: http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason/perl-HTML-Mason.spec
SRPM Name or Url: http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason-1.30-1.src.rpm
Description:
Mason is a powerful Perl-based web site development and delivery
engine. With Mason you can embed Perl code in your HTML and construct
pages from shared, reusable components.  Mason solves the common
problems of site development: caching, debugging, templating,
maintaining development and production sites, and more.

Comment 1 Chris Grau 2005-08-20 20:17:03 UTC
HTML::Mason 1.31 was released today and is available on CPAN.  It contains a
handful of bug fixes.  Might be worth upgrading prior to a review.

Comment 2 Steven Pritchard 2005-08-20 23:31:12 UTC
Will do.

Comment 3 Ralf Corsepius 2005-08-21 05:41:10 UTC
NEEDSWORK:

* nitpicks:
- %check || :, please use "%check".
- sed'ing LD_RUN_PATH, please remove
- setting perl_vendorlib is superfluous with FC >= FC3.

* Arguable/Discussable:
- /var/www/mason and /var/www/comp
I am not sure if these are clever choices. /var/www/mason seems reasonable to
me, but /var/www/comp seems a bit too unspecific to me. However I don't have
strong opinion on this.

* Major issue:
Your /etc/http.d/conf.d/perl-HTML-Mason.conf doesn't work on FC4.
It prevents apache/httpd from startup:

# service http start
Starting httpd:                                            [FAILED]
[Sun Aug 21 07:34:47 2005] [error] Can't load Perl module Apache2 for server
..., exiting..

I am not as much an apache expert to be able to tell what is wrong with it.
AFAIS, it definitely is related to these lines in perl-HTML-Mason.conf, which
would indicate a mod_perl/mod_perl2 incompatiblity issue:
PerlModule Apache2
PerlModule Apache::compat

Comment 4 Chris Grau 2005-08-21 06:00:27 UTC
(In reply to comment #3)
[snip]
> - /var/www/mason and /var/www/comp
> I am not sure if these are clever choices. /var/www/mason seems reasonable to
> me, but /var/www/comp seems a bit too unspecific to me. However I don't have
> strong opinion on this.

My (unrequested) opinion would be to do what conf.d/perl.conf does and leave
every option commented as examples of what could be done.  For example, I use
/var/cache/mason (like mod_proxy) for MasonDataDir and /var/www/mason as
MasonCompRoot.  Also, since libapreq2 is available in extras, an example of
using the apache args method would be nice in addition to the cgi args method.

[snip]
> I am not as much an apache expert to be able to tell what is wrong with it.
> AFAIS, it definitely is related to these lines in perl-HTML-Mason.conf, which
> would indicate a mod_perl/mod_perl2 incompatiblity issue:
> PerlModule Apache2
> PerlModule Apache::compat

Neither of these lines are used with current versions of mod_perl and
HTML::Mason.  A physical Apache2 module was removed in mod_perl 2.0.0rc5, and
Apache::compat isn't needed in HTML::Mason >= 1.30.


Comment 5 Ralf Corsepius 2005-08-21 06:32:57 UTC
(In reply to comment #4)
> (In reply to comment #3)
> [snip]
> My (unrequested) opinion would be to do what conf.d/perl.conf does and leave
> every option commented as examples of what could be done.  For example, I use
> /var/cache/mason (like mod_proxy) for MasonDataDir and /var/www/mason as
> MasonCompRoot.
Agreed, /var/cache/mason seems to be a better choice to me.

Comment 6 Steven Pritchard 2005-08-24 15:03:05 UTC
Sorry, I rushed this one out.  I'll be more careful in the future.

I still haven't tested this well, but httpd will at least start with this:

 http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason-1.31-2.src.rpm
 http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason/perl-HTML-Mason.spec

Comment 7 Steven Pritchard 2005-08-24 15:06:39 UTC
The sample config file definitely still could use some work, so suggestions on
that are welcome.  (I see I missed a couple of things in comment #4.)

Comment 9 Chris Grau 2005-08-24 18:21:03 UTC
Created attachment 118085 [details]
Sample Mason configuration for the conf.d directory.

Here's my stab at a Mason configuration file for Apache 2.0.x.	It's based on
what I've been using in a production environment for some time.

It should be enough to get Mason up and running with basic functionality, and
I've included a pointer to the full list of configuration options.

Comment 10 Jose Pedro Oliveira 2005-08-26 16:04:10 UTC
Created attachment 118164 [details]
Update to 1.3101 and requirments cleanup

Steven,

Could you update HTML::Mason to version 1.3101 and clean the requirements and
build requirements list (patch attached)?

The following requirments are all core perl modules (perl > 5.8.0) [1]
  BuildRequires:  perl(CGI) >= 2.46
  BuildRequires:  perl(File::Spec) >= 0.8
  BuildRequires:  perl(Scalar::Util) >= 1.01
  BuildRequires:  perl(Test)
  BuildRequires:  perl(Test::Builder)
  Requires:	  perl(CGI) >= 2.46
  Requires:	  perl(File::Spec) >= 0.8
  Requires:	  perl(Scalar::Util) >= 1.01

References:
[1] Source: script corelist from perl-Module-CoreList

Comment 11 Steven Pritchard 2005-08-29 20:58:15 UTC
How does this look?

http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason-1.3101-1.src.rpm
http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason/perl-HTML-Mason.spec

%changelog
* Mon Aug 29 2005 Steven Pritchard <steve> 1.3101-1
- Update to 1.3101
- Spec cleanup (jpo)
- Include sample config file from Chris Grau


Comment 12 Jose Pedro Oliveira 2005-09-09 18:31:36 UTC
Created attachment 118655 [details]
Specfile patch: adds documentation, ...

Changelog:
* adds the html documentation pages
* adds the samples
  (the eg/ appliaction brings a new requirement which is filtered)
* changes the mason dir from /var/www/mason/html to /var/www/mason
* changes the "perl Makefile.PL" statement

Comment 13 Ralf Corsepius 2005-09-15 16:58:17 UTC
Do we have consensus on how to continue?

What I can tell: Steve's rpm perl-HTML-Mason-1.3101-1.src.rpm works fine with
rt3 under FC4 ;)

Did anybody test this under FC3?

I'd approve it for FC4 and FC5, if we have consensus on the mason dirs 
(FWIW: Debian seems to use /var/cache/mason).

Comment 14 Steven Pritchard 2005-09-15 18:12:09 UTC
+# Requires: filter perl(MasonX::Request::PlusApacheSession)
+cat <<__EOF__ > %{name}-perlreq
+#!/bin/sh
+/usr/lib/rpm/perl.req \$* | grep -v 'perl(MasonX::Request::PlusApacheSession)'
+__EOF__
+%define __perl_requires %{_builddir}/HTML-Mason-%{version}/%{name}-perlreq
+chmod +x %{__perl_requires}

Is this really necessary?  I don't see this dependency.

-echo '!' | %{__perl} Makefile.PL INSTALLDIRS=vendor
+%{__perl} Makefile.PL INSTALLDIRS=vendor

Interesting...  That must have changed since the last build I did by hand.

Fixed.
 
-mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/www/mason/html
+mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/www/mason

Shouldn't DocumentRoot (/var/www/mason/html) be under MasonCompRoot
(/var/www/mason)?  I see that's not what the config file I'm including does now.
 My old one did...

I'll change it, but I'm still curious.  :-)
 
+%doc htdocs/ eg/ samples/
-%{_localstatedir}/www/mason/
+%dir %{_localstatedir}/www/mason

Done.

http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason-1.3101-2.src.rpm
http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason/perl-HTML-Mason.spec

Comment 15 Chris Grau 2005-09-15 18:24:40 UTC
(In reply to comment #14)
> +# Requires: filter perl(MasonX::Request::PlusApacheSession)
> +cat <<__EOF__ > %{name}-perlreq
> +#!/bin/sh
> +/usr/lib/rpm/perl.req \$* | grep -v 'perl(MasonX::Request::PlusApacheSession)'
> +__EOF__
> +%define __perl_requires %{_builddir}/HTML-Mason-%{version}/%{name}-perlreq
> +chmod +x %{__perl_requires}
> 
> Is this really necessary?  I don't see this dependency.

I seem to recall that perl(MasonX::Request::PlusApacheSession) gets pulled in by
one of the examples.  I had to do something like this with an older custom
package of HTML::Mason.

> -mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/www/mason/html
> +mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/www/mason
> 
> Shouldn't DocumentRoot (/var/www/mason/html) be under MasonCompRoot
> (/var/www/mason)?  I see that's not what the config file I'm including does now.

When I set up a Mason site, I usually use /var/www/html as the document root and
/var/www/mason as the private component root.  YMMV, of course.


Comment 16 Steven Pritchard 2005-09-15 18:31:23 UTC
Yeah, the examples introduce new provides and requires.  I'm fixing that now. 
(I noticed right after I hit "Save Changes".  Oops.)

Comment 17 Steven Pritchard 2005-09-15 18:52:08 UTC
http://ftp.kspei.com/pub/steve/rpms/perl-HTML-Mason-1.3101-3.src.rpm filters out
the bogus requires and provides.

Comment 18 Jose Pedro Oliveira 2005-09-15 18:59:25 UTC
Steven,

Can you replace the perl(mod_perl2) BR and R by mod_perl only?

This way perl-HTML-Mason will work for FC-3 and FC-4. Otherwise it will only
work with FC-4 (perl(mod_perl2) is only provided by mod_perl >= 2.0.0 RC5).

/jpo

Comment 19 Jose Pedro Oliveira 2005-09-15 19:06:55 UTC
Hum!  Please ignore my last comment.  I don't know if it is possible to support
mod_perl 1.99 (judging ny the comments in ApacheHandler.pm).  

Comment 20 Steven Pritchard 2005-09-15 19:11:48 UTC
I'm going to attempt to support FC-3, but I'd rather not worry about it until
after we get this into CVS.  Initially I'll only request builds for FC-4 and devel.

Comment 21 Chris Grau 2005-09-15 19:22:18 UTC
(In reply to comment #20)
> I'm going to attempt to support FC-3, but I'd rather not worry about it until
> after we get this into CVS.  Initially I'll only request builds for FC-4 and
devel.

I'd have to look at the code to be sure, but I think Mason went the same route
as other modules that depend on mod_perl.  Mason won't work with anything before
mod_perl 2.0.0rc5 and thus it won't work on FC-3.

If we really want Mason on FC-3, I think someone should package up 1.28.  Either
that version or one of the 1.29 dev releases supported mod_perl 1.99.


Comment 22 Jose Pedro Oliveira 2005-09-15 19:26:37 UTC
I have nothing further to add. Publish++ (FC-4 and devel)

Comment 23 Ralf Corsepius 2005-09-15 19:50:08 UTC
I am still having doubts on /var/www/mason and therefore don't want to approve
the package (Did anybody consider the FHS?).

As JPO seems to be convinced, I am assigning this package request to him.

Comment 24 Chris Grau 2005-09-15 20:17:22 UTC
(In reply to comment #23)
> I am still having doubts on /var/www/mason and therefore don't want to approve
> the package (Did anybody consider the FHS?).

I think if we were to follow the FHS, the entire www tree would be moved to
/srv.  However, I'm sure that's a flame war that's been faught elsewhere.

The /var/www/mason directory is not without precedent.  Consider, for example,
/var/www/{cgi-bin,error,icons,manual} owned by httpd and httpd-manual.  Also
/var/www/usage owned by webalizer.  Granted, all of these are usually aliased to
public URLs, but I think the real point is to prevent them from interfering with
the document root.

I don't know how others go about this, but my use of /var/www/mason is as a
private component root so that components/pages in the document root can call
them, but they are not accessible through a public URL.


Comment 25 Ralf Corsepius 2005-09-16 02:45:34 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > I am still having doubts on /var/www/mason and therefore don't want to approve
> > the package (Did anybody consider the FHS?).
> 
> I think if we were to follow the FHS, the entire www tree would be moved to
> /srv.  However, I'm sure that's a flame war that's been faught elsewhere.
Yes, but you are aware, that the move to /srv already is taking place, even on
RH based distributions?

> The /var/www/mason directory is not without precedent.  Consider, for example,
> /var/www/{cgi-bin,error,icons,manual} owned by httpd and httpd-manual.  Also
> /var/www/usage owned by webalizer. 

> I don't know how others go about this, but my use of /var/www/mason is as a
> private component root so that components/pages in the document root can call
> them, but they are not accessible through a public URL.
Let me ask differently: Can /var/www/mason be read-only?

Comment 26 Chris Grau 2005-09-16 15:54:53 UTC
(In reply to comment #25)
> (In reply to comment #24)
> > (In reply to comment #23)
> > > I am still having doubts on /var/www/mason and therefore don't want to approve
> > > the package (Did anybody consider the FHS?).
> > 
> > I think if we were to follow the FHS, the entire www tree would be moved to
> > /srv.  However, I'm sure that's a flame war that's been faught elsewhere.
> Yes, but you are aware, that the move to /srv already is taking place, even on
> RH based distributions?

No, I was not aware.  Glad to hear it.  I wonder how long it will take me to get
used to typing /srv instead of /var.

> > The /var/www/mason directory is not without precedent.  Consider, for example,
> > /var/www/{cgi-bin,error,icons,manual} owned by httpd and httpd-manual.  Also
> > /var/www/usage owned by webalizer. 
> 
> > I don't know how others go about this, but my use of /var/www/mason is as a
> > private component root so that components/pages in the document root can call
> > them, but they are not accessible through a public URL.
> Let me ask differently: Can /var/www/mason be read-only?

If, after developing some code and installing it on a production system, the
admin chooses to make /var/www/mason read-only, everything will still work. 
Mason writes its cache files to MasonCacheDir, which we are pushing as
/var/cache/mason in this package.

I hope I understood correctly what you were asking here.


Comment 27 Ville Skyttä 2005-09-16 18:31:52 UTC
Just happened to notice that the summary of this package could be improved 
somewhat, for example: 
 
Summary: Powerful Perl-based web site development and delivery engine 

Comment 28 Steven Pritchard 2005-09-16 20:19:41 UTC
I've updated the summary.

Comment 29 Ralf Corsepius 2005-09-17 05:03:06 UTC
(In reply to comment #26)

> > Let me ask differently: Can /var/www/mason be read-only?
> 
> If, after developing some code and installing it on a production system, the
> admin chooses to make /var/www/mason read-only, everything will still work. 
> Mason writes its cache files to MasonCacheDir, which we are pushing as
> /var/cache/mason in this package.
> 
> I hope I understood correctly what you were asking here.
Yes, this is what I meant, because IMO, it would not be a good idea to have
dynamically changing files under /var/www.

Comment 30 Ralf Corsepius 2005-10-06 16:23:42 UTC
Housekeeping - Package has already been released



Note You need to log in before you can comment on or make changes to this bug.