Bug 486009 - Review Request: php-pear-Crypt-Blowfish - Quick two-way blowfish encryption
Review Request: php-pear-Crypt-Blowfish - Quick two-way blowfish encryption
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Remi Collet
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 486010
  Show dependency treegraph
 
Reported: 2009-02-17 16:12 EST by Xavier Bachelot
Modified: 2009-04-17 12:30 EDT (History)
4 users (show)

See Also:
Fixed In Version: 1.1.0-0.3.rc2.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-25 11:22:38 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
fedora: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Xavier Bachelot 2009-02-17 16:12:10 EST
Spec URL: http://www.bachelot.org/fedora/SPECS/php-pear-Crypt_Blowfish.spec
SRPM URL: http://www.bachelot.org/fedora/SRPMS/php-pear-Crypt_Blowfish-1.1.0-0.1.rc2.fc10.src.rpm
Description: 
This package allows you to perform two-way blowfish encryption on the fly using only PHP. This package does not require the MCrypt PHP extension to work, although it can make use of it if available.
Comment 1 Remi Collet 2009-02-22 02:17:20 EST
First notes :
- It requires PEAR >= 1.7.2 (according to upstream and to xml) : this will block this package for EPEL.

- rpmlint warning :
W: summary-not-capitalized quick two-way blowfish encryption

- %file must be fixed
%files
%defattr(-,root,root,-)
%{pear_xmldir}/%{pear_name}.xml
%{pear_testdir}/%{pear_name}
%{pear_phpdir}/Crypt

- I would prefer using %{name}.xml rather than %{pear_name}.xml (see recently approved PHP Guidelines, this is usefull to avoid conflict between package from various channel)

- a comment about running the tests (which must be done as root after install) will be usefull :
=> pear run-tests -p Crypt_Blowfish
Comment 2 Christopher Stone 2009-02-22 12:47:35 EST
Shouldn't this be renamed to php-pear-Crypt-Blowfish to meet Fedora package naming guidelines?
Comment 3 Remi Collet 2009-02-22 13:00:17 EST
Yes Christopher, you're right, I've miss this error (but it's only first notes ;) not the formal review).
Comment 4 Xavier Bachelot 2009-02-22 18:08:42 EST
Renamed and updated to follow suggestions :
Spec URL: http://www.bachelot.org/fedora/SPECS/php-pear-Crypt-Blowfish.spec
SRPM URL: http://www.bachelot.org/fedora/SRPMS/php-pear-Crypt-Blowfish-1.1.0-0.2.rc2.fc10.src.rpm
Comment 5 Remi Collet 2009-02-23 11:29:32 EST
You should remove (probably a paste error, already defined above)
----------------------------
BuildRequires: php-pear(PEAR)
Requires: php-pear(PEAR)
Requires(post): %{__pear}
Requires(postun): %{__pear}
Provides:     php-pear(foo) = %{version}
----------------------------
Comment 6 Remi Collet 2009-02-23 11:54:21 EST
REVIEW:

+ rpmlint is ok
php-pear-Crypt-Blowfish.src: I: checking
php-pear-Crypt-Blowfish.noarch: I: checking
php-pear-Crypt-Blowfish.noarch: W: no-documentation
2 packages and 1 specfiles checked; 0 errors, 1 warnings.
+ package name
+ spec file name 
+ package meet the PHP Guidelines 
+ License ok : BSD
+ License is upstream 
+ spec in english and legible
+ no license file in sources
+ sources match the upstream sources
09f0e38a4d524ba4102db5d11b07ffe9  Crypt_Blowfish-1.1.0RC2.tgz
+ Source URL ok
+ build / run on F10.x86_64
+ BuildRequires (php-pear >= 1:1.4.9-1.2) ok
+ no locale
+ no .so
+ own all directories that it creates
+ no duplicate file
+ %defattr ok
+ %clean section
+ use macros consistently
+ contain code
+ no documentation
+ no devel
+ no pkgconfig
+ no sub-package
+ no GUI
+ don't own files or directories already owned by other packages
+ %install start with rm -rf 
+ valid UTF-8
+ build in mock (fedora-rawhide-x86_64)
+ test suite ok (2 PASSED TESTS)
+ scriptlets ok
- Final Requires ok, but see previous comments
- Final Provides ko, see previous comments
php-pear(foo) = 1.1.0

Just fix the previous comment and I will approve this package.
Comment 7 Xavier Bachelot 2009-02-23 14:27:32 EST
Thanks for the review, Remi. This is indeed a cut and paste error...

Fixed version :
Spec URL: http://www.bachelot.org/fedora/SPECS/php-pear-Crypt-Blowfish.spec
SRPM URL:
http://www.bachelot.org/fedora/SRPMS/php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10.src.rpm
Comment 8 Remi Collet 2009-02-24 00:50:14 EST
+ Requires
php-pear(PEAR)

+ Provides
php-pear(Crypt_Blowfish) = 1.1.0
php-pear-Crypt-Blowfish = 1.1.0-0.3.rc2.fc8

APPROVED
Comment 9 Xavier Bachelot 2009-02-24 03:43:42 EST
New Package CVS Request
=======================
Package Name: php-pear-Crypt-Blowfish
Short Description: Quick two-way blowfish encryption
Owners: xavierb
Branches: F-9 F-10
InitialCC:
Comment 10 Kevin Fenzi 2009-02-24 15:42:58 EST
cvs done.
Comment 11 Fedora Update System 2009-02-24 19:10:55 EST
php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10
Comment 12 Xavier Bachelot 2009-02-24 19:13:46 EST
Imported and built for F-10 and devel.
Doesn't build on F-9 because php-pear version is too old (available 1.7.1, required 1.7.2)
Comment 13 Fedora Update System 2009-02-25 11:22:33 EST
php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 14 Xavier Bachelot 2009-04-15 06:20:26 EDT
Remi, the earlier 1.1.0 rc1 or 1.0.1 versions don't have a require on pear >= 1.7.2, this would allow to build for F9 and EL5. Would you object if I build either version ?
Comment 15 Xavier Bachelot 2009-04-16 05:21:48 EDT
Package Change Request
======================
Package Name: php-pear-Crypt-Blowfish
New Branches: EL-5
Owners: xavierb
Comment 16 Kevin Fenzi 2009-04-17 12:30:45 EDT
cvs done.

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