Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: php-xcache - yet another php cacher
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Summary: Review Request: php-xcache - yet another php cacher Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: timosha@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Description: XCache is a fast, stable PHP opcode cacher that has been tested and is now running on production servers under high load.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #1 from Timon timosha@gmail.com 2010-02-10 08:34:21 EST --- Created an attachment (id=389992) --> (https://bugzilla.redhat.com/attachment.cgi?id=389992) spec file
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #2 from Timon timosha@gmail.com 2010-02-10 08:35:02 EST --- Created an attachment (id=389994) --> (https://bugzilla.redhat.com/attachment.cgi?id=389994) srpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #3 from Timon timosha@gmail.com 2010-02-10 08:35:49 EST --- spec file: https://bugzilla.redhat.com/attachment.cgi?id=389992 srpm : https://bugzilla.redhat.com/attachment.cgi?id=389994
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |itamar@ispbrasil.com.br
--- Comment #4 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2010-02-10 08:41:49 EST --- is this your first package ?
Can you add a scratch build here ?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Matthias Runge mrunge@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mrunge@fedoraproject.org
--- Comment #5 from Matthias Runge mrunge@fedoraproject.org 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #6 from Itamar Reis Peixoto itamar@ispbrasil.com.br 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Timon timosha@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #389992|0 |1 is obsolete| |
--- Comment #7 from Timon timosha@gmail.com 2010-02-11 06:25:58 EST --- Created an attachment (id=390219) --> (https://bugzilla.redhat.com/attachment.cgi?id=390219) php-xcache.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Timon timosha@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #389994|0 |1 is obsolete| |
--- Comment #8 from Timon timosha@gmail.com 2010-02-11 06:26:54 EST --- Created an attachment (id=390220) --> (https://bugzilla.redhat.com/attachment.cgi?id=390220) php-xcache-1.3.0-2.fc12.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #9 from Timon timosha@gmail.com 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Till Maas opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |opensource@till.name
--- Comment #10 from Till Maas opensource@till.name 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #11 from Till Maas opensource@till.name 2010-02-11 06:41:10 EST --- Just noticed this: Instead of using /usr/share, please use %{_datadir}.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Timon timosha@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #390219|0 |1 is obsolete| |
--- Comment #12 from Timon timosha@gmail.com 2010-02-11 07:38:59 EST --- Created an attachment (id=390235) --> (https://bugzilla.redhat.com/attachment.cgi?id=390235) php-xcache.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Timon timosha@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #390220|0 |1 is obsolete| |
--- Comment #13 from Timon timosha@gmail.com 2010-02-11 07:40:03 EST --- Created an attachment (id=390236) --> (https://bugzilla.redhat.com/attachment.cgi?id=390236) php-xcache-1.3.0-3.fc12.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #14 from Timon timosha@gmail.com 2010-02-11 07:40:21 EST --- (In reply to comment #11)
Just noticed this: Instead of using /usr/share, please use %{_datadir}.
fixed
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Timon timosha@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Alias| |php-xcache
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@famillecollet.com
--- Comment #15 from Remi Collet fedora@famillecollet.com 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(timosha@gmail.com | |)
--- Comment #16 from Itamar Reis Peixoto itamar@ispbrasil.com.br 2010-02-15 07:38:05 EST --- Can you post a updated spec file + src.rpm ?
also can you include a koji scratch build ?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Timon timosha@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #390235|0 |1 is obsolete| | Flag|needinfo?(timosha@gmail.com | |) |
--- Comment #17 from Timon timosha@gmail.com 2010-02-16 05:29:51 EST --- Created an attachment (id=394498) --> (https://bugzilla.redhat.com/attachment.cgi?id=394498) php-xcache.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Timon timosha@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #390236|0 |1 is obsolete| |
--- Comment #18 from Timon timosha@gmail.com 2010-02-16 05:30:41 EST --- Created an attachment (id=394499) --> (https://bugzilla.redhat.com/attachment.cgi?id=394499) php-xcache-1.3.0-4.fc12.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
--- Comment #19 from Timon timosha@gmail.com 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status Whiteboard| |StalledSubmitter
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=563510
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Blocks|177841(FE-NEEDSPONSOR) |201449(FE-DEADREVIEW) Resolution| |NOTABUG Status Whiteboard|StalledSubmitter | Last Closed| |2011-01-10 18:37:50
--- Comment #20 from Jason Tibbitts tibbs@math.uh.edu 2011-01-10 18:37:50 EST --- No response for months in another of the submitter's tickets; closing them all.
package-review@lists.fedoraproject.org