Bug 190101 - (php-pear-Log) Review Request: php-pear-Log
Review Request: php-pear-Log
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
:
: 196823 (view as bug list)
Depends On: 190252 php-pear-MDB2
Blocks: FE-ACCEPT 189195 pear-PHPUnit3 php-pear-PHPUnit2
  Show dependency treegraph
 
Reported: 2006-04-27 12:08 EDT by Remi Collet
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-07 16:29:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Remi Collet 2006-04-27 12:08:55 EDT
Spec URL: http://remi.collet.free.fr/rpms/extras/php-pear-Log.spec
SRPM URL: http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.4-1.fc5.src.rpm
Description: 
The Log framework provides an abstracted logging system.
It supports logging to console, file, syslog, SQL, Sqlite, mail, and mcal targets.
It also provides a subject - observer mechanism.
Comment 1 Remi Collet 2006-04-30 03:20:33 EDT
Waiting for Bug #190252
Comment 2 Remi Collet 2006-05-07 05:11:16 EDT
New spec and SRPM using %{_datadir}/pear/.pkgxml

Spec URL: http://remi.collet.free.fr/rpms/extras/php-pear-Log.spec
SRPM URL: http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.5-1.fc5.src.rpm
Comment 3 Jason Tibbitts 2006-05-10 22:37:58 EDT
I'd like to see some movement on these php-pear package reviews, but we have a
distinct lack of packaging guidelines for PHP modules.

Here are a few issues I noticed:
"PEAR" in the summary isn't really descriptive; maybe something like "Abstracted
logging facility for PHP" would make more sense.

I guess RPM won't extract the php-pear(*) provides as it will for Perl, which is
too bad.  I wonder if it could be taught.

Use "BuildArch:" instead of "BuildArchitectures:"; it lines up better.

rpmlint disagrees with the overlong line in the description.

Could you explain the comment in %prep?

Perhaps rpmlint could be taught to do the sanity check so it doesn't have to
live in the spec.

Could you explain the comment in %postun?

I think you might need Requires(post): php-pear and Requires(postun): php-pear
(or is that php-pear(PEAR)?).

Unfortunately I can't do a proper test because the updated php-pear isn't in FC5
yet and rawhide is broken at the moment.
Comment 4 Remi Collet 2006-05-11 12:30:00 EDT
There are movement, see Bug #190066.
The new php-pear-1.4.9 will allow us to progress.

Description used, is the one provide upstream on the package.xml. I must agree
it isn't very descriptive, but i don't know if it's a good idea to chance it.

For %prep. This comment is from pear template.spec
# XXX Source files location is missing here in pear cmd
It mean pear is only use to build "pearrc" (source will be provide in %install)

The Sanity check is use to check the job done by pear, because version 1.4.6
sometime left %[buildroot} relative path in .php file.
Using pear-1.4.9 and --packagingroot (insteaed of -R) solve this problem.

Of course i can remove it, but it could be useful for people who want to rebuild
the RPM for another distro.

%postun workaround ( ... || true) is for pear-1.4.6 which is unable to uninstall
somme package. No problem with pear-1.4.9.
This workaround is to avoid scriptlet (and uninstall) failure.
In this case the package is uninstalled, but not unregistred in pear extension
list. 
A solution could be to (Build)Requires pear > 1.4.7 ???

Yes php-pear(PEAR) is provide by php-pear.

php-pear-1.4.9 is in rawhide and in FC5-testing (see Bug #190252)
Comment 5 Jason Tibbitts 2006-05-11 12:40:44 EDT
> Description used, is the one provide upstream on the package.xml. I must agree
> it isn't very descriptive, but i don't know if it's a good idea to chance it.

Upstream can be broken in many ways.  We have to change the descriptions for
Perl modules as well.  Since the summary is the first thing the users will see,
it must be as descriptive as possible in the 60 or so characters available.

> For %prep. This comment is from pear template.spec

Do we have a pear template in fedora-rpmdevtools?  I don't see one.

[sanity check]
> Of course i can remove it, but it could be useful for people who want to rebuild
> the RPM for another distro.

We don't usually worry about that, but my point is that rpmlint is our sanity
checker and it's worth discussing whether it should be taught to check for
things like that.

Are you targeting FC4 with these packages?  If not, we should just require the
unbuggy php-pear version once it has been released.

I'm still waiting for either a buildable rawhide or the updated php-pear package
in FC5 to do a full review.  If anyone can answer the question of whether the
scriptlets need Requires(post) and Requires(postun) dependencies, please chip in.
Comment 6 Remi Collet 2006-05-11 13:24:26 EDT
> Do we have a pear template in fedora-rpmdevtools?  I don't see one.

/usr/share/pear/data/PEAR/template.spec provides by php-pear (old command : pear
makerpm).
or
/usr/share/pear/data/PEAR_Command_Packaging/template.spec provides by 
php-pear-PEAR-Command-Packaging (Bug #185423, new command pear make-rpm-spec,
soon in Extras).

I think this template is not really suitable for Extras and could really by
improve, but i'm not the packager for this.
Comment 7 Remi Collet 2006-05-15 14:30:55 EDT
Spec URL: http://remi.collet.free.fr/rpms/extras/php-pear-Log.spec
SRPM URL: http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.4-3.fc5.src.rpm
Mock build log : http://remi.collet.free.fr/rpms/extras/php-pear-Log-build.log

- Require pear >= 1.4.9
- Requires(hint): (only comment actually) + description
- bundle the v3.01 PHP LICENSE file (as in php-pear)
- use --packagingroot (instead of -R)
- check from install to check (as in php-pear)

Comment 8 Remi Collet 2006-05-15 14:32:43 EDT
ooops.
SRPM URL: http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.5-3.fc5.src.rpm
Comment 10 Christopher Stone 2006-06-27 01:03:47 EDT
Hey guys, I did not know this review request existed.  I created a php-pear-MDB2
review request and php-pear-Log review request.

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196823
Comment 11 Christopher Stone 2006-06-27 01:16:20 EDT
php-pecl-xdebug request:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196749

php-pear-MDB2 reqeust:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196793

NOTE that my php-pear-Log spec file uses these two packages.
Comment 12 Christopher Stone 2006-06-27 20:06:57 EDT
*** Bug 196823 has been marked as a duplicate of this bug. ***
Comment 13 Christopher Stone 2006-06-27 20:17:59 EDT
This package should require php-pear-MDB2 since I already went through the
trouble of making this package.  Please review this package so that it may be
closed.
Comment 14 Remi Collet 2006-07-12 13:22:52 EDT
SPEC : http://remi.collet.free.fr/rpms/extras/php-pear-Log.spec
SRPM : http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.7-1.fc5.src.rpm
Mock : http://remi.collet.free.fr/rpms/extras/php-pear-Log-build.log

Changes : 
- update to 1.9.7
- use new macros from /etc/rpm/macros.pear
- add Requires php-pear(DB) and php-pear(MDB2)

MDB2/DB are defined as optional upstream and i'm using Log extension without
them for a while. But you're probably right that we must include "optional"
requires as mandatory while they are not handled by rpm/yum.
Comment 16 Christopher Stone 2006-09-03 20:17:39 EDT
Hi Remi, can you update the %setup and %install for this guy too?  Thanks.
Comment 17 Remi Collet 2006-09-04 12:10:06 EDT
SPEC : http://remi.collet.free.fr/rpms/extras/php-pear-Log.spec
SRPM : http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.8-3.fc5.src.rpm
Mock : http://remi.collet.free.fr/rpms/extras/php-pear-Log-build.log

%changelog
* Mon Sep 04 2006 Remi Collet <Fedora@FamilleCollet.com> 1.9.8-3
- new and simpler %%prep and %%install

Note : i've also update php-pear-Mail and php-pear-HTTP (already approved) on
the CVS.
Comment 18 Christopher Stone 2006-09-06 18:22:47 EDT
Hi Remi, since I need this built for FC-5, can you update the spec file to match
as closely as possible the latest template:

http://cvs.fedora.redhat.com/viewcvs/*checkout*/fedora-rpmdevtools/spectemplate-php-pear.spec?root=fedora&rev=1.4&sortby=date

You may also want to include changes that hanvn't been officially committed to
cvs yet:

https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=135697
Comment 19 Remi Collet 2006-09-07 13:04:39 EDT
SPEC : http://remi.collet.free.fr/rpms/extras/php-pear-Log.spec
SRPM : http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.8-4.fc5.src.rpm
Mock : http://remi.collet.free.fr/rpms/extras/php-pear-Log-build.log

Note :
- i didn't use %{ClassName} as it's very useful on a template but not on a
"generated" specfile.
- i didn't check for package.xml/package2.xml as auto-generation knows which one use
- i create docdir in the main build directory (cleaner, i think)
Comment 20 Christopher Stone 2006-09-07 14:24:41 EDT
Looking good, please change:

- Summary should not have "PEAR", a summary should be as short as possible while
still being descriptive enough to convey what it is.  Extra stuff like "A" or
"The" or "PEAR" should be removed from summaries.

- Set BuildRequires:  php-pear >= 1:1.4.9-1.2

Normally I would approve it now and request you make the changes in CVS, but the
BuildRequires is a blocker and must be fixed before I can approve it.

Comment 21 Remi Collet 2006-09-07 14:49:19 EDT
SPEC : http://remi.collet.free.fr/rpms/extras/php-pear-Log.spec
SRPM : http://remi.collet.free.fr/rpms/extras/php-pear-Log-1.9.8-5.fc5.src.rpm

I've change then BR, but i really think it's not a BR
- 1.4.9 is required to have --packagingroot working
- 1.4.9-1.1 is required for memory limit, but 8M enough for this extension
- 1.4.9-1.2 is required for macros, which are embeded in the spec.
so...
Comment 22 Christopher Stone 2006-09-07 15:54:21 EDT
Yeah, I just logged in to approve this anyway because you had already defined
the macros.  Looks good now anyway, approved.
Comment 23 Christopher Stone 2006-09-07 15:58:49 EDT
BTW, could you be so kind and sync and build this package for FC5?  I'm going to
need this for some of my packages which I want to build on FC5.  Thanks for all
your work on this and the other php packages.  :D
Comment 24 Remi Collet 2007-05-16 09:14:46 EDT
Package Change Request
======================
Package Name: php-pear-Log
New Branches: EL-5

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