This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 563510 - (php-xcache) Review Request: php-xcache - yet another php cacher
Review Request: php-xcache - yet another php cacher
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2010-02-10 08:33 EST by Timon
Modified: 2012-08-18 06:20 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-01-10 18:37:50 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
spec file (4.55 KB, text/plain)
2010-02-10 08:34 EST, Timon
no flags Details
srpm (108.88 KB, application/octet-stream)
2010-02-10 08:35 EST, Timon
no flags Details
php-xcache.spec (3.50 KB, text/plain)
2010-02-11 06:25 EST, Timon
no flags Details
php-xcache-1.3.0-2.fc12.src.rpm (109.27 KB, application/octet-stream)
2010-02-11 06:26 EST, Timon
no flags Details
php-xcache.spec (3.49 KB, text/plain)
2010-02-11 07:38 EST, Timon
no flags Details
php-xcache-1.3.0-3.fc12.src.rpm (109.27 KB, application/octet-stream)
2010-02-11 07:40 EST, Timon
no flags Details
php-xcache.spec (2.17 KB, text/plain)
2010-02-16 05:29 EST, Timon
no flags Details
php-xcache-1.3.0-4.fc12.src.rpm (107.71 KB, application/octet-stream)
2010-02-16 05:30 EST, Timon
no flags Details

  None (edit)
Description Timon 2010-02-10 08:33:10 EST
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 08:34:21 EST
Created attachment 389992 [details]
spec file
Comment 2 Timon 2010-02-10 08:35:02 EST
Created attachment 389994 [details]
srpm
Comment 4 Itamar Reis Peixoto 2010-02-10 08:41:49 EST
is this your first package ?

Can you add a scratch build here ?
Comment 5 Matthias Runge 2010-02-10 15:09:06 EST
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 15:24:53 EST
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 06:25:58 EST
Created attachment 390219 [details]
php-xcache.spec
Comment 8 Timon 2010-02-11 06:26:54 EST
Created attachment 390220 [details]
php-xcache-1.3.0-2.fc12.src.rpm
Comment 9 Timon 2010-02-11 06:27:59 EST
(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 06:39:24 EST
(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 06:41:10 EST
Just noticed this:
Instead of using /usr/share, please use %{_datadir}.
Comment 12 Timon 2010-02-11 07:38:59 EST
Created attachment 390235 [details]
php-xcache.spec
Comment 13 Timon 2010-02-11 07:40:03 EST
Created attachment 390236 [details]
php-xcache-1.3.0-3.fc12.src.rpm
Comment 14 Timon 2010-02-11 07:40:21 EST
(In reply to comment #11)
> Just noticed this:
> Instead of using /usr/share, please use %{_datadir}.    

fixed
Comment 15 Remi Collet 2010-02-13 11:49:50 EST
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 07:38:05 EST
Can you post a updated spec file + src.rpm ?

also can you include a koji scratch build ?
Comment 17 Timon 2010-02-16 05:29:51 EST
Created attachment 394498 [details]
php-xcache.spec
Comment 18 Timon 2010-02-16 05:30:41 EST
Created attachment 394499 [details]
php-xcache-1.3.0-4.fc12.src.rpm
Comment 19 Timon 2010-02-16 05:34:02 EST
(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 18:37:50 EST
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.