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 176733 - Review Request: php-pear-DB (PEAR database abstraction layer)
Summary: Review Request: php-pear-DB (PEAR database abstraction layer)
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ignacio Vazquez-Abrams
QA Contact: David Lawrence
URL:
Whiteboard:
Depends On: 178821
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-12-31 11:19 UTC by Tim Jackson
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: 2006-02-12 10:34:01 UTC
Type: ---
Embargoed:
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Tim Jackson 2005-12-31 11:19:27 UTC
Spec Name or Url: http://www.timj.co.uk/linux/specs/php-pear-DB.spec
SRPM Name or Url: http://www.timj.co.uk/linux/srpms/php-pear-DB-1.7.6-1.src.rpm
Description:
PEAR DB is a database abstraction layer for PHP.

This package actually used to be provided by the old php-pear subpackage of PHP. It was (sensibly) dropped from that as a result of bug #173808 but as a consequence it means that this package should perhaps ultimately be moved to Core rather than Extras.

This package is testing the water as far as PEAR modules and Fedora Extras go; if this review goes OK it would be nice to get more PEAR packages into FE and to that end I have been working on getting the PHP/PEAR packages in Core to a state where this is easier to do (see bug #176725, bug #173804, bug #173806, bug #173810, bug #173814, bug #173270 and related bugs). My ultimate goal is that "pear makerpm" (at least as supplied by Fedora) should build FE-compatible spec files "out of the box" (or as close to it as possible). This is not yet the case, even with all the patches I've supplied, but the feedback from this bug will be useful in developing further patches towards that goal.

Some particular notes:

1. The package uses the new naming convention as defined by Joe Orton in bug #173806, i.e.:

Name: php-pear-DB
Provides: php-pear(DB) = 1.7.6

2. With the above in mind, the spec file has at least one nod to automated generation: the %files section is populated predominantly by "/".  This does result in the package owning lots of directories it doesn't actually need to; it doesn't cause any practical problems that I'm aware of but if it irks anyone then I will have to re-think things.

3. As a new FE contributor, I've been a bit confused about the whole license-file-in-package debate. Upstream (by convention AFAICT) does not seem to explicitly include license files in any PEAR packages. In this package I have manually brought in the license file. I'd rather not do this if I can avoid it; do I have to?

This is my first FE package; I need a sponsor.

Comment 1 Tim Jackson 2005-12-31 11:21:44 UTC
I'm adding Joe Orton to cc as Joe, your input here would be really appreciated.
(Especially about moving this to Core, in which case I presume you would take
over the package)

Comment 2 Ignacio Vazquez-Abrams 2005-12-31 12:07:33 UTC
- Use "%define peardir %(pear config-get php_dir 2> /dev/null || echo
%{_datadir}/pear)" and BR php-pear instead of hardcoding %peardir
- Drop Source1
- Wipe %{buildroot} at the beginning of %install, not of %prep
- Do NOT use / in %files; be more explicit

(In reply to comment #0)
> 3. As a new FE contributor, I've been a bit confused about the whole
license-file-in-package debate. Upstream (by convention AFAICT) does not seem to
explicitly include license files in any PEAR packages. In this package I have
manually brought in the license file. I'd rather not do this if I can avoid it;
do I have to?

You do not have to bring in the license from an external source.

Comment 3 Joe Orton 2005-12-31 12:19:49 UTC
Thanks for the submission Tim.

Other than Ignacio's comments this looks fine; I'd also trim down the
%description a bit to something more succinct.

We should probably have php-pear own /var/lib/pear too (note to self... :).

I think that Extras is the appropriate place for PEAR modules to be packaged.

Comment 4 Tim Jackson 2005-12-31 13:24:27 UTC
OK, thanks for the comments, all good! I've taken them all on board and done a
few cleanups of my own, and here goes with a new version:

Spec URL: http://www.timj.co.uk/linux/specs/php-pear-DB.spec
SRPM URL: http://www.timj.co.uk/linux/srpms/php-pear-DB-1.7.6-2.src.rpm

Comment 5 Tim Jackson 2006-01-06 16:58:43 UTC
Any further comments or anyone willing to approve this package and sponsor me?

Comment 6 Joe Orton 2006-01-06 17:19:32 UTC
That looks fine to me.  (I'm not set up to be sponsor, otherwise I would)

Comment 7 Ignacio Vazquez-Abrams 2006-01-08 11:57:16 UTC
- Builds fine in FC4 mock
- Upstream source matches
- rpmlint output:

W: php-pear-DB invalid-license The PHP License

- ignorable, rpmlint problem

E: php-pear-DB non-executable-script /usr/share/pear/tests/DB/tests/run.cvs 0644
E: php-pear-DB non-executable-script
/usr/share/pear/tests/DB/tests/driver/run.cvs 0644

- ignorable; they aren't meant to actually be run per se AFAIK

W: php-pear-DB dangerous-command-in-%post install

- bogus, but add pear to Requires(post) and Requires(postun)

- The rest looks good

The Requires(post{,un}) change can easily enough be made in CVS.

APPROVED

Comment 8 Tim Jackson 2006-01-24 17:17:10 UTC
Build fails due to php-pear missing dep on php (bug #178821)

Comment 9 Joe Orton 2006-01-30 15:50:16 UTC
Thinking about this more: this file %{_var}/lib/pear/DB.xml is just static,
right?  Or does it change?  If it doesn't change it should be in
%{_libdir}/php/pear/ instead, or something.  We can have php-pear own the
containing directory once it is decided, either way.

Comment 10 Tim Jackson 2006-02-05 21:11:02 UTC
The DB.xml file is static for a particular version of a package, so yes you're
right that maybe %_var is not the right place. libdir/php/pear sounds good to
me; any comments to the contrary? The only possible problem is that I *think*
this was formerly used in some installations as the actual installation
directory for PEAR modules; that shouldn't actually be a problem but may make a
slight mess.


Comment 11 Tim Jackson 2006-02-10 16:07:03 UTC
In the absence of any feedback I've changed the path for the package XML file to
libdir/php/pear in CVS and there is a build job running now.

Comment 12 Tim Jackson 2006-02-12 10:34:01 UTC
Built in devel.

Comment 13 Tim Jackson 2007-04-30 12:09:47 UTC
Package Change Request
======================
Package Name: php-pear-DB
Updated Fedora Owners: rpm.uk,Fedora

Comment 14 Remi Collet 2007-05-16 12:58:59 UTC
I will push actual 1.7.6 to EL-5 and 1.7.11 to devel (after F8 branch)

Package Change Request
======================
Package Name: php-pear-DB
New Branches: EL-5


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