Spec URL: http://blues.mcgill.ca/~icon/fe/php-pear-PhpDocumentor.spec
SRPM URL: http://blues.mcgill.ca/~icon/fe/php-pear-PhpDocumentor-1.3.0-0.2.RC6.src.rpm
Description:
phpDocumentor is the current standard auto-documentation tool for the
php language. phpDocumentor has support for linking between documentation,
incorporating user level documents like tutorials and creation of
highlighted source code with cross referencing to php general
documentation.
phpDocumentor uses an extensive templating system to change your source
code comments into human readable, and hence useful, formats. This system
allows the creation of easy to read documentation in 15 different
pre-designed HTML versions, PDF format, Windows Helpfile CHM format, and
in Docbook XML.
Note:
1. The reason why phpdoc is a separate package is because FC6 separates php-cli (required for command-line execution) into a separate package. PhpDocumentor doesn't require the command-line utility -- it can be run from the web if configured to do so.
2. PHP Pear packaging guidelines are still only a draft, so I have used Remi Collet's existing php-pear modules currently already in Extras.

The release tag is off by a little bit, the %{?dist} tag should always be at
the end of the tag unless it's a sub-release bump.
0.2%{?dist}.%{rcrel} -> 0.2.%{rcrel}%{?dist}
According to Packaging/PHP, the package should also "Requires: php".
The latest version is 1.3.0 GA, but RC6 is being packaged. To be fair, GA was
released the day after I assigned the review to myself, but... :) Please
update the package to 1.3.0.
For the rpmlint warnings, the second is spurious and can be ignored. The
first, however, could be fixed, but it appears part of the pear install
process is to check the md5sums of each file. If the line ending can be fixed
w/o too much pain and mucking around with the pear-based install process, I'd
like to see it addressed.
You have php.ini both listed as source1, and created in %prep. One or the
other works nicely, but both is redundant. (Especially given nothing is ever
done with %{SOURCE0}...)
Also, while using php.ini to override the default memory and execution limits
works during the rpm build, the install fails on %post with the defaults on
the system:
[root@zeus mock]# rpm -ivh php-pear-PhpDocumentor-1.3.0-0.2.fc5.RC6.noarch.rpm
Preparing... ########################################### [100%]
1:php-pear-PhpDocumentor ########################################### [100%]
PHP Fatal error: Allowed memory size of 8388608 bytes exhausted (tried to
allocate 229 bytes) in /usr/share/pear/PEAR/Installer.php on line 283
Allowed memory size of 8388608 bytes exhausted (tried to allocate 72 bytes)
Installing the created php.ini as an additional .ini file in /etc/php.d
enables the install to suceed, e.g. in %install:
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/php.d
cp -p php.ini $RPM_BUILD_ROOT%{_sysconfdir}/php.d/phpDocumentor.ini
I don't get a good feeling from overriding core php.ini values in a package's
own modular ini file, but there doesn't seem to be another clean way to do
this. (ideas/comments from those reading are solicited :) ) Perhaps it would
be a good idea to open a bug asking for the core memory limits be updated?
The response at least from the php maintainer would be valuable.
There's nearly a meg of tutorials and examples, which is great, but that's a
bit large to not be put in a -docs subpackage.
SO, to recap:
* release tag
* check on fixing rpmlint warning
* update version
* decide how php.ini is going to be included in srpm
* deal with memory limits (a la a /etc/php.d/foo.ini file), open bug
* split %_docdir/Documentation and %_docdir/tutorials off into a -docs
package
X package meets naming and packaging guidelines. (release tag)
+ specfile is properly named, is cleanly written and uses macros consistently.
X dist tag is present (but in the wrong place)
+ build root is correct.
+ license field matches the actual license.
+ license is open source-compatible (LGPL). License text included in package.
+ source files match upstream:
e6f31c313b0b06c09acaf7047e6a5b23 PhpDocumentor-1.3.0RC6.tgz
e6f31c313b0b06c09acaf7047e6a5b23 PhpDocumentor-1.3.0RC6.tgz.srpm
X latest version is being packaged.
+ BuildRequires are proper.
X package has required requires/provides statements.
+ package builds in mock (devel/x86_64).
O rpmlint is silent. (see below)
O final provides and requires are sane (except as noted above):
** phpdoc-1.3.0-0.2.fc5.RC6.noarch.rpm
== rpmlint
== provides
phpdoc = 1.3.0-0.2.fc5.RC6
== requires
/usr/bin/php
php-cli >= 5
php-pear(PhpDocumentor) = 1.3.0
** php-pear-PhpDocumentor-1.3.0-0.2.fc5.RC6.noarch.rpm
== rpmlint
W: php-pear-PhpDocumentor wrong-file-end-of-line-encoding
/usr/share/doc/php-pear-PhpDocumentor-1.3.0/Documentation/Release-old/Release-0.3.0
W: php-pear-PhpDocumentor dangerous-command-in-%post install
== provides
php-pear(PhpDocumentor) = 1.3.0
php-pear-PhpDocumentor = 1.3.0-0.2.fc5.RC6
== requires
/bin/sh
/usr/bin/pear
php-pear(PEAR) >= 1.4.9
+ no shared libraries are present.
+ package is not relocatable.
+ owns the directories it creates.
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ %clean is present.
O %check is not present; no automated test suite
+ sane scriptlets present.
+ code, not content.
X documentation is large (~876k), so -docs subpackage is necessary.
+ %docs are not necessary for the proper functioning of the package.
+ no headers.
+ no pkgconfig files.
+ no libtool .la droppings.
+ not a GUI app.

While I don't have time at the instant moment to check to see if 16MB is enough
to install this package with, I'm going to mark this bug as depending on bug
196802. (Definitely something we should validate however -- would solve a lot
of the "bad feelings" I get contemplating overriding a core php.ini value.)

Yeah, let's block on that. I've added the hacks to create a temporary local
php.ini in %post and %postun, but that's a horrible hack. I'd rather wait until
PHP moves out of 1996, when 8M was considered a lot for a process. :)

One comment :
%setup -q -n PhpDocumentor-%{version}
Is not safe because package.xml will be store in BUILD directory (can be overide
by another build running at the same time).
%setup -c -q
cd PhpDocumentor-%{version}
Seem safer
This should probably be add to the PHP Guidelines (for PEAR and PECL)
See /etc/rpmdevtools/spectemplate-php-pear.spec from latest rpmdevtools-5.1-1.fc5

Is there a reason that one package is called phpdoc and not
php-pear-PhpDocumentor? The source looks to all be from PhpDocumentor.tgz. I
realize phpdoc is the name of the binary, and can be run from the command line,
but it seems odd to me to have one source rpm produce packages of completely
different names.
Also, is the web interface enabled by default? I didn't see anyting in
/etc/httpd/conf.d. I guess I thought something like
#Created for PhpDocumentor package
Alias /phpdoc '/usr/share/pear/data/PhpDocumentor'
<Directory /phpdoc>
Options -Indexes
</Directory>
I am not sure if enabling this on the web by default is allowed (encouraged)
though.

1. Because php-pear-PhpDocumentor is a library that phpdoc uses. Think of them
as "curl" and "libcurl".
2. I don't think enabling web access by default is a good idea -- it's not a
common usage, and that would make it depend on httpd, which is not necessary for
most uses of phpdoc.

Note -docs now picks up a rpmlint warning:
W: php-pear-PhpDocumentor-docs wrong-file-end-of-line-encoding
/usr/share/doc/php-pear-PhpDocumentor-docs-1.3.1/Documentation/Release-old/Release-0.3.0
Please fix this before building; everything else looks to have been addressed...
APPROVED

I think most of us feel that way at this point :)
The fedora-review flag / review procedure is _not_ in effect and has not yet
been approved by FESCo. (It is currently in use for merge reviews, but as this
is not a core package this is not a merge review.)
Note that to import/branch & add to owners.list, the fedora-cvs flag procedure
has been approved by FESCo and should be used here. (basically, just set
fedora-cvs to '?' and at the same time add a comment as described in the
procedure with owner/branch info.) see
http://www.fedoraproject.org/wiki/CVSAdminProcedure