Bug 455226

I am very, very apologize. I mix with the description php-pecl-parsekit.
Correct are:
Short description: php-pecl-runkit - Extension to mangle with user defined
functions and classes

Long description:
Replace, rename, and remove user defined functions and classes.
Define customized superglobal variables for general purpose use.
Execute code in restricted environment (sandboxing).

(Removing NEEDSPONSOR: bug 455067)

Small question :

Do you think it is a good idea to import a very old package which doesn't seem maintained upstream for quite a long time (june 2006).


Small notes :

Version 0.9 is available, so you cannot use a pre-release version number (0.9-0.xxx) but a post release (Official 0.9 will have a 0.9-1 number schema)

Please add in comment the "cvs export" command with the revision or date retrieved.

Also indicate where the patch come from. If they are needed to build against latest php version, you should file a bug upstream with your patch attached (but I see in CSV php 5.3.0 compatibility have been fixed for a few months).

+

Also read the PHP Guidelines (and/or read other pecl spec file)
http://fedoraproject.org/wiki/Packaging/PHP

You must have the requires for ABI version

.hu should be removed from release.

Firstly - thank you for comment.

(In reply to comment #3)
> Small question :
> 
> Do you think it is a good idea to import a very old package which doesn't seem
> maintained upstream for quite a long time (june 2006).
This is work now and have not major bugs which must be fixed by upstream. Furthermore, I do not known any alternatives by functionality yet. So, answer is yes, I think is it is a very good idea to import this package.


> Small notes :
> 
> Version 0.9 is available, so you cannot use a pre-release version number
> (0.9-0.xxx) but a post release (Official 0.9 will have a 0.9-1 number schema)
Hm. It is build from CVS. How I should number its versions - 0.9-1.CVS20080512, 0.9-2.CVS20080512, 0.9-3.CVS20080512? Or 0.9-1.1.CVS20080512, 0.9-1-2.CVS20080512, 0.9-1-3.CVS20080512?

> Please add in comment the "cvs export" command with the revision or date
> retrieved.
Excuse me, I do not understand. Date of CVS retrieving present in version and in defined macros
%define	CVS	20080512
in spec file. For what and where more I should comment it?

> Also indicate where the patch come from.
This is my patches. And it is reflected in spec changelog.

> If they are needed to build against
> latest php version, you should file a bug upstream with your patch attached
> (but I see in CSV php 5.3.0 compatibility have been fixed for a few months).
Patch not for PHP, patch for this extension to make it compatible with new PHP ABI.
So, if we speak about file bug to upstream of this pecl extension - this have not sense, because you said before what it is old and unmaintained.

> How I should number its versions

0.9-1.1.CVS20080512 seems ok.

> For what and where more I should comment it?

I'd like to see the exact commands use to build the tarball, just above the %source., p.e. (export greater than checkout) :

# cvs -d :pserver:cvsread.net/repository export -D 2009-01-22 pecl/runkit
# tar cjf runkit-CVS20090122.tar.bz2 -C pecl runkit
%Source0:		%{peclName}-CVS%{CVS}.tar.bz2

Don't use an URL which doesn't exists

I have well understood than patches are for PHP ABI.

Even if this package is unmaintained, please report the bug and post your patch(es), it will be usefull for everyone, and probably commited (last commit is only 5 weeks old on runkit.c).

I don't think restarting apache for each extension is a good idea. This should probably be removed (Have to search about this in the Guidelines).

Please :
- clean release (remove .Hu... and probably not usefull #*Hu comments)
- update to a recent CVS snapshot 
- register the extension (package2.xml is included)
- add PHP ABI check (see PHP Guidelines)
- add upstream bug reference above %patch
- use Fedora macros %pecl_xmldir, %php_extdir, %pecl_install, ...
- clean $Revision and $Log cvs status lines (spec is quite obfuscated)
- clean changelog (mainly % not acceptable)

(In reply to comment #6)
> > How I should number its versions
> 
> 0.9-1.1.CVS20080512 seems ok.
No, duedlines say what CVS, not released versions must start from 0.

I change it to 0.9-0.1.CVS20080512 enumeration.

> I'd like to see the exact commands use to build the tarball, just above the
> %source., p.e. (export greater than checkout) :
> 
> # cvs -d :pserver:cvsread.net/repository export -D 2009-01-22
> pecl/runkit
> # tar cjf runkit-CVS20090122.tar.bz2 -C pecl runkit
> %Source0:  %{peclName}-CVS%{CVS}.tar.bz2
Thank you, its done.


> I have well understood than patches are for PHP ABI.
> 
> Even if this package is unmaintained, please report the bug and post your
> patch(es), it will be usefull for everyone, and probably commited (last commit
> is only 5 weeks old on runkit.c).
Ok - http://pecl.php.net/bugs/bug.php?id=15969


> I don't think restarting apache for each extension is a good idea. This should
> probably be removed (Have to search about this in the Guidelines).
I agree. This comes as legacy. Restart is removed.


> Please :
> - clean release (remove .Hu... and probably not usefull #*Hu comments)
Done in release. In comments i think it is not necessary?

> - update to a recent CVS snapshot
Done.
> - register the extension (package2.xml is included)
Done.
> - add PHP ABI check (see PHP Guidelines)
Done.
> - add upstream bug reference above %patch
Done - http://pecl.php.net/bugs/bug.php?id=15969
> - use Fedora macros %pecl_xmldir, %php_extdir, %pecl_install, ...
Done.
> - clean $Revision and $Log cvs status lines (spec is quite obfuscated)
Done.
> - clean changelog (mainly % not acceptable)
Done.

http://hubbitus.net.ru/rpm/Fedora9/php-pecl-runkit/php-pecl-runkit-0.9-0.6.CVS20090215.fc9.src.rpm

Please, excuse me for long delay.

> Please, excuse me for long delay.

Not a problem, I'm also actually very busy

> No, duedlines say what CVS, not released versions must start from 0.

Definitely, I can't agree.
Version 0.9 have been published on 2006-06-06

So this is a post-release version
Read : http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages

I prefer using %{name}.xml rather than %{peclName}.xml (or %{peclName}2.xml) (see recent approved changes in PHP Guidelines, this is to avoid possible conflicts between pecl, pear and other channel extensions).

I need to search if %verify(not md5 mtime size) is acceptable in the Guidelines...

Is the spec encoding ok ? It seems there is UTF-8 (ru sumnary) and ISO (pl sumnary) which make my text editor crazy ?

$ file php-pecl-runkit.spec 
php-pecl-runkit.spec: Non-ISO extended-ASCII text, with LF, NEL line terminators

(In reply to comment #9)
> > No, duedlines say what CVS, not released versions must start from 0.
> Definitely, I can't agree.
> Version 0.9 have been published on 2006-06-06
> So this is a post-release version
> Read :
> http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages
Ok, you are right. Version release enumaration changed.

> I prefer using %{name}.xml rather than %{peclName}.xml (or %{peclName}2.xml)
> (see recent approved changes in PHP Guidelines, this is to avoid possible
> conflicts between pecl, pear and other channel extensions).
Ok, let it be so.

> I need to search if %verify(not md5 mtime size) is acceptable in the
> Guidelines...
> 
> Is the spec encoding ok ? It seems there is UTF-8 (ru sumnary) and ISO (pl
> sumnary) which make my text editor crazy ?
Polish summary and description recoded from iso8859-2 to UTF-8. 

> $ file php-pecl-runkit.spec 
> php-pecl-runkit.spec: Non-ISO extended-ASCII text, with LF, NEL line
> terminators
Now:
$ file php-pecl-runkit.spec
php-pecl-runkit.spec: UTF-8 Unicode text


http://hubbitus.net.ru/rpm/Fedora9/php-pecl-runkit/php-pecl-runkit-0.9-7.CVS20090215.fc9.src.rpm

Why do you use 
    install modules/%{peclName}.so %{buildroot}%{php_extdir}

Rather than (which also create the destination dir)
    make install INSTALL_ROOT=$RPM_BUILD_ROOT


Doesn't build for me :
- %doc %{peclName}-%{version}/README
+ %doc %{peclName}/README

rpmlint : 
php-pecl-runkit.src: I: checking
php-pecl-runkit.src:39: W: unversioned-explicit-obsoletes php-pear-%{peclName}
php-pecl-runkit.src: W: summary-not-capitalized runkit - mangle with user defined functions and classes
php-pecl-runkit.src: W: non-standard-group Development/Languages/PHP
php-pecl-runkit.x86_64: I: checking
php-pecl-runkit.x86_64: W: summary-not-capitalized runkit - mangle with user defined functions and classes
php-pecl-runkit.x86_64: W: non-standard-group Development/Languages/PHP
php-pecl-runkit.x86_64: W: obsolete-not-provided php-pear-runkit
php-pecl-runkit-debuginfo.x86_64: I: checking
php-pecl-runkit.spec:39: W: unversioned-explicit-obsoletes php-pear-%{peclName}
3 packages and 1 specfiles checked; 0 errors, 7 warnings.

Most of them can be fixed easily

- stuff php-pear-%{peclName} is not usefull in Fedora (php-pear-runkit don't exists AFAIK)
- %{peclName} could be removed from desc and first word capitalized.

(In reply to comment #11)
> Why do you use 
>     install modules/%{peclName}.so %{buildroot}%{php_extdir}
> 
> Rather than (which also create the destination dir)
>     make install INSTALL_ROOT=$RPM_BUILD_ROOT
I do not remember now. It have any sense?

> Doesn't build for me :
> - %doc %{peclName}-%{version}/README
> + %doc %{peclName}/README
Thank you. Fixed.

> rpmlint : 
> php-pecl-runkit.src: I: checking
> php-pecl-runkit.src:39: W: unversioned-explicit-obsoletes php-pear-%{peclName}
> php-pecl-runkit.src: W: summary-not-capitalized runkit - mangle with user
> defined functions and classes
Name deleteed from all summary and capitalized.

> php-pecl-runkit.src: W: non-standard-group Development/Languages/PHP
Changed to Development/Libraries
> php-pecl-runkit.x86_64: I: checking
> php-pecl-runkit.x86_64: W: summary-not-capitalized runkit - mangle with user
> defined functions and classes
Fixed, see before.

> - stuff php-pear-%{peclName} is not usefull in Fedora (php-pear-runkit don't
> exists AFAIK)
Excuse me, I think do not understand you. You speak about unnecessary Obsoletes: php-pear-%{peclName}? I think you are rigth, it may be safely removed in this case. I do it.

rpmlint silent now. I am sorry that is not checked immediately.

http://hubbitus.net.ru/rpm/Fedora9/php-pecl-runkit/php-pecl-runkit-0.9-8.CVS20090215.fc9.src.rpm

Hm, It build successful on my box, but failed unexpected in koji http://koji.fedoraproject.org/koji/getfile?taskID=1183646&name=build.log

As seen in log, macros %{pecl_xmldir} not replaced by it own value. Strange. More strange in this situation what php-pecl-imagick package also uses it macros and therefor built correctly...

Until now I don't known what to do...

Yes, I do that!

It is build now: http://koji.fedoraproject.org/koji/taskinfo?taskID=1232362

http://hubbitus.net.ru/rpm/Fedora9/php-pecl-runkit/php-pecl-runkit-0.9-9.CVS20090215.fc9.src.rpm

+ rpmlint is ok
php-pecl-runkit.i386: I: checking
php-pecl-runkit.src: I: checking
php-pecl-runkit-debuginfo.i386: I: checking
3 packages and 1 specfiles checked; 0 errors, 0 warnings.
+ package name
+ spec file name 
+ package meet the PHP Guidelines (except runkit.xml/php-pecl-runkit.xml)
+ License ok : PHP
+ License is upstream 
+ spec in english (+ru +pl) and legible
+ license file in sources is not provided
+ sources match the upstream sources from CVS 20090215
+ Source URL : CVS snapshot with comment
+ build  on F10.x86_64 (php 5.3)
+ BuildRequires ok
+ no locale
+ no .so
+ own all directories that it creates
+ no duplicate file
- %defattr (644,root,root,755)
+ %clean section
+ use macros consistently
+ contain code
+ small 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 (F10 i386 / php 5.2.9)
+ build in koji (rawhide / php 5.2.9)
- test suite doesn't run
- scriptlets register/unregister ok but not silent
+ Final Requires ok
/bin/sh  
/usr/bin/pecl  
php(api) = 20041225
php(zend-abi) = 20060613
+ Final Provides ok
php-pecl(runkit) = 0.9

Don't understand why you switch back to runkit.xml (rather then php-pecl-runkit.xml). Not a must (new Guidelines approved by FPC but not yet inline)
http://fedoraproject.org/wiki/PackagingDrafts/PHP

I can't run the test suite, probably because I use PHP 5.3, what are the result under PHP 5.2 ?

MUST : 
make the %post/postun scriptlet silent
use the default %defattr(-,root,root,-)

+ no .so

=> Of course I mean no shared lib which need to be in a -devel subpackage, only a PHP extension. So OK.

Ok, ok, I rename xml-file again into php-pecl-runkit.xml.

I can run test on php even 5.3 (from your repository ;) ), but I think included suite is very outdated, and not seen any passed tests. So, it is a main reason why I do not include in spec test phase.

Set %defattr(-,root,root,-)

(In reply to comment #15)
> MUST : 
> make the %post/postun scriptlet silent
It is done. BUT why??? I prefer see errors if it is present. It is also guarantee to fast bug-report if something will wrong on user system...
In provided before link to Guidelines such scripts marked only as recommended, not mandatory.

http://hubbitus.net.ru/rpm/Fedora9/php-pecl-runkit/php-pecl-runkit-0.9-10.CVS20090215.fc9.src.rpm

> I prefer see errors if it is present.

Redirection of standard output (what you've done) doesn't suppress output of error ;) but only unuseful information about (un)registration.

All MUST (and should) are fixed, so

APPROVED

Remi Collet, thank you for review.

New Package CVS Request
=======================
Package Name: php-pecl-runkit
Short Description: PHP Opcode Analyser
Owners: Hubbitus
Branches: F9 F10 EL5
InitialCC:

cvs done.

php-pecl-runkit-0.9-10.CVS20090215.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/php-pecl-runkit-0.9-10.CVS20090215.fc9

php-pecl-runkit-0.9-10.CVS20090215.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/php-pecl-runkit-0.9-10.CVS20090215.fc10

New branch please.

Package Change Request
======================
Package Name: php-pecl-runkit
New Branches: F-11
Owners: hubbitus

There already is a F-11 branch. 

Note that you may have to do a 'cvs update -d' to see it locally.

It is, but from times when it be "rawhide" if you understand me.
Now, after sucessful build, I try make update and got error:
php-pecl-runkit-0.9-10.CVS20090215.fc11 not tagged as an update candidate

Where I wrong?

You probably forget to "cvs update" to get the new branch.

In devel, package should be tagged F12.

+

No, "cvs ud -d" was runned few times.

And oof course, I known what now devel is F-12, it is a reason why I think what F-11 should be added separately.

But, when I try tag it, I get error what this tag applied on a different branch:

$ make tag
cvs tag  -c php-pecl-runkit-0_9-10_CVS20090215_fc11
ERROR: The tag php-pecl-runkit-0_9-10_CVS20090215_fc11 is already applied on a different branch
ERROR: You can not forcibly move tags between branches
php-pecl-runkit-0_9-10_CVS20090215_fc11:devel:hubbitus:1237926729
php-pecl-runkit-0_9-10_CVS20090215_fc9:F-9:hubbitus:1237929134
php-pecl-runkit-0_9-10_CVS20090215_fc10:F-10:hubbitus:1237929597
php-pecl-runkit-0_9-10_CVS20090215_el5:EL-5:hubbitus:1237933086
php-pecl-runkit-0_9-11_CVS20090215_el5:EL-5:hubbitus:1240139384
cvs tag: Pre-tag check failed
cvs [tag aborted]: correct the above errors first!
make: *** [tag] error 1

Can I move tags? Or what I should do now?

php-pecl-runkit-0.9-10.CVS20090215.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

php-pecl-runkit-0.9-10.CVS20090215.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

This package is already imported.