[Bug 226295] Merge Review: php-pear

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: php-pear
Alias: php-pear

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226295





------- Additional Comments From chris.stone at gmail.com  2007-02-05 14:45 EST -------
(In reply to comment #8)
> Thanks for the review!

You're welcome, but we are only about 1/2 way done ;-)

> 
> Fixed in -4:
> - upstream do not provide a permanent archive URL for each version of the .phar
> installer, so there is no URL to provide (yes, I know this sucks).  I'll add a 
> comment to this effect

See comment #9, I need to know why using a .phar is required instead of using
the .tgz

> - bogus Group

Thanks

> - pear.conf marked noreplace; /etc/rpm/macros.* should never be noreplace (and
> that should be documented by standard not per spec file)

Thanks. Please add a comment, something like:
# macros.* should be replaced on updates
Remember, you are not the only one who reads the spec file, and should you get
hit by a bus, the next php-pear maintainer will appreciate the extra documentation.

> 
> Won't fix:
> - passing -q to %setup has no effect when -T is also used

Filed a bug against rpmlint (bug #227389).  Thanks for pointing this out.

> - providing an noop %build adds no value

Yet, rpm has "unpredictable results if not added".  You can email Ville Skytta
if you need further clarification on this, he has real world examples where this
has caused problems.  Better to play safe than sorry.  *ALL* pear packages
contain empty %build sections, php-pear should not be an exception.  Again, this
is something that takes 5 seconds to add, and causes no harm and helps
documentation in the spec for other users.  Please add.

> - no idea what the issue with $RPM_SOURCE_DIR is, this has been used in spec
> files forever

You should install source files using %{SOURCEx} notation, please change your
for loop to something like this:
install -pm 755 %{SOURCE10} $RPM_BUILD_ROOT%{_bindir}/pear
install -pm 755 %{SOURCE11} $RPM_BUILD_ROOT%{_bindir}/pecl
install -pm 755 %{SOURCE12} $RPM_BUILD_ROOT%{_bindir}/peardev

In addition, your reference to macros.pear should use %{SOURCE13}


> - the patches are applied using %{PATCHn} syntax

My mistake.  You are right in this case, pear packages are installed funky and
patches have to be done this way.

> - the license specified in every PEAR class file is indeed v3 not v2.02.  There
> is no accepted policy for the License tag in Fedora yet; there is no point
> tweaking this on a whim until there is, rpmlint is not definitive on that front

Okay, can you give me some kind of proof that _all_ pear classes are
automatically licensed with PHP License 3.0?  I know of many pear classes that
use BSD for example.

The upstream home page actually has a link to the license file which is a
version 2.02 license.

I simply cannot approve this until this matter is cleared up.  The licenses
**MUST** match upstream.  If upstream's home page is pointing to the wrong
license, then please attach to this bug report an e-mail from upstream saying
their home page is incorrect.

Please also remove the "The" and "v" in the License tag.  Why not atleast make
the licenses consistent to help people who run shell scripts and such parsing
license tags?  A license of "PHP License 3.0" is no different than "The PHP
License v3.0", however the first case is consistent with all other packages
licensed with a PHP license.

> 
> Again, the upgrade to the latest upstream is not relevant to the review of the
> current packaging and need not block the review process.  Of course the
> packaging may change with each new upstream releases, that is always going to be
> true.

Right, so what is the point in reviewing a package that is going to change
significantly immediately after it is reviewed?  It makes far greater sense to
make the significant changes _before_ the review.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.