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 190007 (php-pecl-zip) - Review Request: php-pecl-zip
Summary: Review Request: php-pecl-zip
Keywords:
Status: CLOSED NEXTRELEASE
Alias: php-pecl-zip
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-04-26 16:50 UTC by Remi Collet
Modified: 2012-04-20 06:07 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-27 17:55:48 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Remi Collet 2006-04-26 16:50:47 UTC
Spec URL: http://remi.collet.free.fr/rpms/extras/php-pecl-zip.spec
SRPM URL: http://remi.collet.free.fr/rpms/extras/php-pecl-zip-1.2.3-1.fc5.src.rpm
Description: Zip is an extension to create and read zip files.

----------------------------------------
This is my first RPM proposal for extras.

I provides some useful RPM on my little repo which i think most could/should be include in the Extras.

Remi.

Comment 1 Brian Pepple 2006-04-28 14:21:21 UTC
Note: This is not a formal review.

1. Package doesn't build in Mock.  Looks like your missing some BuildRequires. 
For information on Mock, refer to http://fedoraproject.org/wiki/Extras/MockTricks.
2. Duplicate BuildRequires:  automake (by php-devel), php (by php-devel),
autoconf (by php-devel), libtool & gcc-c++ (provided in base install in Mock)

Comment 2 Remi Collet 2006-04-28 21:18:07 UTC
Thank for your comment.

Here is the new spec.

http://remi.collet.free.fr/rpms/extras/php-pecl-zip.spec
http://remi.collet.free.fr/rpms/extras/php-pecl-zip-1.3.1-2.fc5.src.rpm

1/ upgrade to new version
2/ BuildRoot and Requires corrected
3/ Build with mock succed.

Comment 3 Christopher Stone 2006-06-27 21:17:41 UTC
MUST ITEM CHECKLIST:
- rpmlint output:
W: php-pecl-zip invalid-license PHP License
W: php-pecl-zip incoherent-version-in-changelog 1.3.0-8.fc5 1.3.1-2.fc5
W: php-pecl-zip invalid-license PHP License
W: php-pecl-zip-debuginfo invalid-license PHP License

Fix Changelog version numbers.  Do not put dist in version numbers unless the
package is specific to that dist only.

The license file warnings are okay to ignore, rpmlint will recognize "PHP
License" in a future release.

- package is named according to php packaging naming guidelines
- spec file name matches %{name}
- package meets packaging guidelines
- package is licensed with open source compatible license
- license field matches actual license
- source does not contain license file
- spec file is in American English (and french)
- spec file is legible
- sources match upstream
d78c8d076b8ced344b3950b1e5299411  zip-1.3.1.tgz
- package successfully compiles and builds on FC-5 x86_64
- All build dependencies are listed in BuildRequires
- package does not use locales
- package does not contain shared libraries in default paths (no need to run
ldconfig)
- package is not relocatable
- package owns all directories it creates
- package does not contain any duplicate %files
- permissions are set properly
- package contains proper %clean section
- macro usage is consistant
- package contains permissible content
- package does not have large documentation
- package does not include header files or static libraries
- package does not use pkgconfig files
- package does not contain library with suffix
- package does not require a devel subpackage
- package does not contain any .la files
- package is not a gui and does not need a .desktop file
- package does not own files or directories owned by other packages

MUST
- Must add Provides: php-pecl(zip)
- Must remove unneeded Requires: zlib

SHOULD
- Investigate what package re2c is used for in this package and determine if it
would be useful to add:
configure: WARNING: You will need re2c 0.9.11 or later if you want to regenerate
 PHP parsers.
- Attach the PHP License file as a source file and include it in %doc
- Use %defattr(-,root,root,-) the defaults are the same

Comment 4 Remi Collet 2006-06-29 18:44:44 UTC
I don't see any use of re2c in the makefile. Probably come from a generic pecl
configure.

Here is new spec :
SPEC : http://remi.collet.free.fr/rpms/extras/php-pecl-zip.spec
SRPM : http://remi.collet.free.fr/rpms/extras/php-pecl-zip-1.4.1-1.fc5.src.rpm
Mock : http://remi.collet.free.fr/rpms/extras/php-pecl-zip-build.log

Changes :
- update to 1.4.1
- bundle the v3.01 PHP LICENSE file
- Suppr. Requires zip, Add Provides php-pecl(zip) and php-zip
- change defattr

php-zip is provided according to new PHP guidelines.


Comment 5 Christopher Stone 2006-06-29 21:32:21 UTC
All items fixed.

Please add %{version}-%{release} to php-pecl(zip) in Provides or else let me
know why it should not be there.

APPROVED

Comment 6 Christopher Stone 2006-06-30 23:58:56 UTC
I think the Group tags for pecl and pear packages are completely reversed as
they currently stand.  I believe pecl packages should be Development/Libraries
and pear packages should be Development/Languages.  I'm waiting to hear back
from Fedora-Packaging on this.

Comment 7 Christopher Stone 2006-07-02 21:45:41 UTC
Don't let the group tag thing prevent you from checking this into cvs, or is
there another reason why this has not been checked into cvs yet?

Comment 8 Ville Skyttä 2006-07-02 22:09:41 UTC
https://www.redhat.com/archives/fedora-extras-list/2006-June/msg01130.html:

      * As a result of the PHP guidelines not being approved, a
        moratorium was placed on all php packages. JasonTibbs announced
        this on the fedora-extras and fedora-maintainers lists.

Comment 9 Ville Skyttä 2006-07-02 22:11:17 UTC
Comment 8 doesn't mean this can't be checked to CVS though, but that it
shouldn't be shipped yet before the guidelines are finalized and approved.

Comment 10 Christopher Stone 2006-07-03 05:06:19 UTC
After checking this into CVS, please add:

%define apiver %((phpize --version 2>/dev/null || echo 'PHP Api Version:
20041225' ) | sed -n '/PHP Api Version/ s/.*:  *//p')

And change Requires: php to:
Requires: php-api >= %{apiver}


Comment 11 Remi Collet 2006-07-03 20:53:31 UTC
Approved version 1.4.1-1 push to CVS.
Waiting for PHP guidelines being approved before building it and adding the
suggestion (Comment #5 and #10)

Comment 12 Remi Collet 2006-08-27 17:55:13 UTC
Version 1.7.2 push to CVS and build for devel.

Comment 13 Remi Collet 2007-03-25 09:13:54 UTC
Package Change Request
======================
Package Name: php-pecl-zip
New Branches: EL-5


Comment 14 Dennis Gilmore 2007-03-25 17:36:56 UTC
branched for EL-5

Comment 15 Ville Skyttä 2007-03-25 21:17:31 UTC
The entry added to owners.epel.list is malformed, it's missing a trailing "|"
and causes tracebacks on commits.

https://admin.fedoraproject.org/tickets/customer.pl?Action=CustomerTicketZoom&TicketID=1044

[...]
Checking in sources;
/cvs/extras/rpms/perl-Set-IntSpan/devel/sources,v  <--  sources
new revision: 1.6; previous revision: 1.5
done
Traceback (most recent call last):
  File "/cvs/extras/CVSROOT/getnotifylist", line 26, in ?
    owners = owners.OwnerList(populate_all = 0)
   File "/cvs/extras/CVSROOT/owners.py", line 47, in __init__
    self.populate_owners()
  File "/cvs/extras/CVSROOT/owners.py", line 121, in populate_owners
    parse_file(self.owners_epel_list)
  File "/cvs/extras/CVSROOT/owners.py", line 103, in parse_file
    (product, component, desc, owner, qa, cclist) = l.split("|")
ValueError: unpack list of wrong size
Running syncmail...
Mailing cvsextras.com ...
...syncmail done.
[...]

Comment 16 Dennis Gilmore 2007-03-25 23:05:41 UTC
thanks.  fixed 

Comment 17 Remi Collet 2008-07-09 18:44:43 UTC
Package Change Request
======================
Package Name: php-pecl-zip
New Branches: EL-4 


Comment 18 Kevin Fenzi 2008-07-10 23:05:10 UTC
cvs done.

Comment 19 Jan ONDREJ 2012-04-20 06:07:48 UTC
Any chance to get this into EL-6 an FC16, where zip extensions has been removed from php package? See bz#551513


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