Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 563510 (php-xcache) - Review Request: php-xcache - yet another php cacher
Summary: Review Request: php-xcache - yet another php cacher
Keywords:
Status: CLOSED NOTABUG
Alias: php-xcache
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2010-02-10 13:33 UTC by Timon
Modified: 2012-08-18 10:20 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-01-10 23:37:50 UTC


Attachments (Terms of Use)
spec file (deleted)
2010-02-10 13:34 UTC, Timon
no flags Details
srpm (deleted)
2010-02-10 13:35 UTC, Timon
no flags Details
php-xcache.spec (deleted)
2010-02-11 11:25 UTC, Timon
no flags Details
php-xcache-1.3.0-2.fc12.src.rpm (deleted)
2010-02-11 11:26 UTC, Timon
no flags Details
php-xcache.spec (deleted)
2010-02-11 12:38 UTC, Timon
no flags Details
php-xcache-1.3.0-3.fc12.src.rpm (deleted)
2010-02-11 12:40 UTC, Timon
no flags Details
php-xcache.spec (deleted)
2010-02-16 10:29 UTC, Timon
no flags Details
php-xcache-1.3.0-4.fc12.src.rpm (deleted)
2010-02-16 10:30 UTC, Timon
no flags Details

Description Timon 2010-02-10 13:33:10 UTC
Description: XCache is a fast, stable PHP opcode cacher that has been tested and is now running on production servers under high load.

Comment 1 Timon 2010-02-10 13:34:21 UTC
Created attachment 389992 [details]
spec file

Comment 2 Timon 2010-02-10 13:35:02 UTC
Created attachment 389994 [details]
srpm

Comment 4 Itamar Reis Peixoto 2010-02-10 13:41:49 UTC
is this your first package ?

Can you add a scratch build here ?

Comment 5 Matthias Runge 2010-02-10 20:09:06 UTC
As I'm not approved, I could only provide an unofficial review:

rpmlint ../RPMS/i686/php-xcache-1.3.0-1.fc12.i686.rpm ../RPMS/i686/php-xcache-debuginfo-1.3.0-1.fc12.i686.rpm php-xcache.spec 
2 packages and 1 specfiles checked; 0 errors, 0 warnings. (ok)

* name meets naming convention (ok)

* can not verify license (BSD given in SPEC, couldn't find this in code, esp. COPYING does not mention BSD)

* Mixing $RPM_BUILD_ROOT and %{buildroot} should not be done (cf. Packaging guidelines in Fedora wiki)

* %global preferred over %define (used both)

* some code in SPEC is commented out, decreases readability. Why don't you remove those lines? Why is code for RHEL 4 and RHEL 5 included?

* Package compiles succesful into binary RPMS

Comment 6 Itamar Reis Peixoto 2010-02-10 20:24:53 UTC
I recommend you to put this out of spec file.

%{__cat} > %{buildroot}%{_sysconfdir}/php.d/xcache.ini << 'EOF'
[xcache-common]
zend_extension = %{php_extdir}/xcache.so

[xcache.admin]
xcache.admin.enable_auth = On
xcache.admin.user = "mOo"
; xcache.admin.pass = md5($your_password)
xcache.admin.pass = ""

[xcache]
xcache.shm_scheme =        "mmap"
xcache.size  =               60M
xcache.count =                 1
xcache.slots =                8K
xcache.ttl   =              3600
xcache.gc_interval =         300

xcache.var_size  =            4M
xcache.var_count =             1
xcache.var_slots =            8K
xcache.var_ttl   =             0
xcache.var_maxttl   =          0
xcache.var_gc_interval =     300
EOF

Comment 7 Timon 2010-02-11 11:25:58 UTC
Created attachment 390219 [details]
php-xcache.spec

Comment 8 Timon 2010-02-11 11:26:54 UTC
Created attachment 390220 [details]
php-xcache-1.3.0-2.fc12.src.rpm

Comment 9 Timon 2010-02-11 11:27:59 UTC
(In reply to comment #4)
> is this your first package ?
exactly, no. I'm not approved yet, but already have two review requests (#464141,#464117).

(In reply to comment #5)
> As I'm not approved, I could only provide an unofficial review:
thanks 

> * can not verify license (BSD given in SPEC, couldn't find this in code, esp.
> COPYING does not mention BSD)
http://xcache.lighttpd.net/changeset/45
I added COPYING to %doc dir.
 
> * Mixing $RPM_BUILD_ROOT and %{buildroot} should not be done (cf. Packaging
> guidelines in Fedora wiki)
fixed

> * some code in SPEC is commented out, decreases readability. Why don't you
> remove those lines? Why is code for RHEL 4 and RHEL 5 included?
fixed

(In reply to comment #6)
> I recommend you to put this out of spec file.
> 
> %{__cat} > %{buildroot}%{_sysconfdir}/php.d/xcache.ini << 'EOF'
> [xcache-common]
> zend_extension = %{php_extdir}/xcache.so
> 
> [xcache.admin]
> xcache.admin.enable_auth = On
> xcache.admin.user = "mOo"
> ; xcache.admin.pass = md5($your_password)
> xcache.admin.pass = ""
> 
> [xcache]
> xcache.shm_scheme =        "mmap"
> xcache.size  =               60M
> xcache.count =                 1
> xcache.slots =                8K
> xcache.ttl   =              3600
> xcache.gc_interval =         300
> 
> xcache.var_size  =            4M
> xcache.var_count =             1
> xcache.var_slots =            8K
> xcache.var_ttl   =             0
> xcache.var_maxttl   =          0
> xcache.var_gc_interval =     300
> EOF    
fixed

spec: https://bugzilla.redhat.com/attachment.cgi?id=390219
srpm: https://bugzilla.redhat.com/attachment.cgi?id=390220

Comment 10 Till Maas 2010-02-11 11:39:24 UTC
(In reply to comment #9)
> (In reply to comment #4)

> > * some code in SPEC is commented out, decreases readability. Why don't you
> > remove those lines? Why is code for RHEL 4 and RHEL 5 included?
> fixed

It's good to remove the commented out lines, but if there is a chance that this package can be included in EPEL[0], then the rhel conditionals are helpful.

[0] https://fedoraproject.org/wiki/EPEL

Comment 11 Till Maas 2010-02-11 11:41:10 UTC
Just noticed this:
Instead of using /usr/share, please use %{_datadir}.

Comment 12 Timon 2010-02-11 12:38:59 UTC
Created attachment 390235 [details]
php-xcache.spec

Comment 13 Timon 2010-02-11 12:40:03 UTC
Created attachment 390236 [details]
php-xcache-1.3.0-3.fc12.src.rpm

Comment 14 Timon 2010-02-11 12:40:21 UTC
(In reply to comment #11)
> Just noticed this:
> Instead of using /usr/share, please use %{_datadir}.    

fixed

Comment 15 Remi Collet 2010-02-13 16:49:50 UTC
Quick notes

- don't understand why you try to register this as a pecl extension (%post / %postun). 
This is not a pecl extension, and you don't have the package.xml file required for this.


- so, could also be removed
%global pecl_name xcache
Requires(post): %{__pecl}
Requires(postun): %{__pecl}
Provides:      php-pecl(%{pecl_name}) = %{version}


- xcache.ini 
zend_extension = /EXT_DIR/xcache.so
no need to give full path.


- %defattr(-, root, root, 0755)
(-,root,root,-) must be enough. php script should be 644 
This will remove a lot of rpmlint message
php-xcache.x86_64: E: script-without-shebang /usr/share/php-xcache/admin/...


- admin / coverager
You need to create a alias (httpd/conf.d) to give access to admin URL.
probably also need a writable dir for xcache.coveragedump_directory

If you don't want to make this available, don't install it, just add it in %doc

- INSTALL
don't need this file in RPM

- Buildroot
is acceptable, but read
http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot

Comment 16 Itamar Reis Peixoto 2010-02-15 12:38:05 UTC
Can you post a updated spec file + src.rpm ?

also can you include a koji scratch build ?

Comment 17 Timon 2010-02-16 10:29:51 UTC
Created attachment 394498 [details]
php-xcache.spec

Comment 18 Timon 2010-02-16 10:30:41 UTC
Created attachment 394499 [details]
php-xcache-1.3.0-4.fc12.src.rpm

Comment 19 Timon 2010-02-16 10:34:02 UTC
(In reply to comment #15)
> Quick notes
> 
> - don't understand why you try to register this as a pecl extension (%post /
> %postun). 
> This is not a pecl extension, and you don't have the package.xml file required
> for this.
> - so, could also be removed
> %global pecl_name xcache
> Requires(post): %{__pecl}
> Requires(postun): %{__pecl}
> Provides:      php-pecl(%{pecl_name}) = %{version}
I used php-pecl-apc as template.
fixed

> - xcache.ini 
> zend_extension = /EXT_DIR/xcache.so
> no need to give full path.
http://ru2.php.net/manual/en/ini.core.php#ini.zend-extension
Absolute path required.

> - %defattr(-, root, root, 0755)
> (-,root,root,-) must be enough. php script should be 644 
> This will remove a lot of rpmlint message
> php-xcache.x86_64: E: script-without-shebang /usr/share/php-xcache/admin/...
fixed

> - admin / coverager
> You need to create a alias (httpd/conf.d) to give access to admin URL.
> probably also need a writable dir for xcache.coveragedump_directory
> If you don't want to make this available, don't install it, just add it in %doc
Fixed. I move them to %doc.

> - INSTALL
> don't need this file in RPM
fixed

> - Buildroot
> is acceptable, but read
> http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot
fixed. 
BuildRoot:     %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)


(In reply to comment #16)
> Can you post a updated spec file + src.rpm ?
php-xcache.spec: https://bugzilla.redhat.com/attachment.cgi?id=394498 
php-xcache-1.3.0-4.fc12.src.rpm: https://bugzilla.redhat.com/attachment.cgi?id=394499

> also can you include a koji scratch build ?    
[timon@timon rpmbuild]$ koji build --scratch dist-f12 SRPMS/php-xcache-1.3.0-4.fc12.src.rpm Uploading srpm: SRPMS/php-xcache-1.3.0-4.fc12.src.rpm
[====================================] 100% 00:00:05 107.71 KiB  21.09 KiB/sec
Created task: 1990279
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=1990279
None
Watching tasks (this may be safely interrupted)...
1990279 build (dist-f12, php-xcache-1.3.0-4.fc12.src.rpm): open (x86-06.phx2.fedoraproject.org)
  1990280 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, ppc): free
  1990281 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, x86_64): free
  1990280 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, ppc): free -> open (ppc06.phx2.fedoraproject.org)
  1990281 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, x86_64): free -> open (xb-01.phx2.fedoraproject.org)
  1990283 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, i686): open (x86-05.phx2.fedoraproject.org)
  1990282 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, ppc64): open (ppc10.phx2.fedoraproject.org)
  1990283 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, i686): open (x86-05.phx2.fedoraproject.org) -> closed
  0 free  4 open  1 done  0 failed
  1990281 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, x86_64): open (xb-01.phx2.fedoraproject.org) -> closed
  0 free  3 open  2 done  0 failed
  1990282 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, ppc64): open (ppc10.phx2.fedoraproject.org) -> closed
  0 free  2 open  3 done  0 failed
  1990280 buildArch (php-xcache-1.3.0-4.fc12.src.rpm, ppc): open (ppc06.phx2.fedoraproject.org) -> closed
  0 free  1 open  4 done  0 failed
1990279 build (dist-f12, php-xcache-1.3.0-4.fc12.src.rpm): open (x86-06.phx2.fedoraproject.org) -> closed
  0 free  0 open  5 done  0 failed

1990279 build (dist-f12, php-xcache-1.3.0-4.fc12.src.rpm) completed successfully

Comment 20 Jason Tibbitts 2011-01-10 23:37:50 UTC
No response for months in another of the submitter's tickets; closing them all.


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