A few nits:

"install -c -m ..." is better than "cp -p"
empty %doc line
%{peardir}/data/PEAR_Command_Packaging looks like it would be unowned
is: " -d doc_dir=/docs" really correct?
an empty %build can be ommitted IIRC

otherwise looks fine to me.

All fixed, with the exception of "-d doc_dir=/docs" which *is* correct, because
of the -R option when installing the package; the doc_dir in that case is
relative to the buildroot, and the docs get shuffled around before ending up in
their final, proper location - see the spec for php-pear-DB for comparison. (As
it happens, the PEAR_Command_Packaging package doesn't have any docs at the
moment, but I see that particular set of options as broadly equivalent to the
typical %configure macro, so I think it's worth leaving the doc_dir option in there)

New spec/SRPM:
http://www.timj.co.uk/linux/specs/php-pear-PEAR-Command-Packaging.spec
http://www.timj.co.uk/linux/srpms/php-pear-PEAR-Command-Packaging-0.1.0-2.src.rpm

I probably forget something, but i try this RPM and the "pear make-rpm-spec"
failed because it search for the template.spec in the %{buildroot}

I simply add, in the %install (inspired from php-pear.spec) :

# Relocate everything:
sed -si "s,%{buildroot},,g" $(find %{buildroot}%{peardir} -name \*.php)

If this is useful there, it can probably be add to the default template.spec.

Cordialy.

It's probably because I use php-pear-1.4.8.

replacing 
    pear -c pearrc install --nodeps -R %{buildroot} %{SOURCE0}
by  pear -c pearrc install --nodeps --packagingroot %{buildroot} %{SOURCE0}

solve the problem.

In this case, i also remove the
php-pear-PEAR-Command-Packaging-0.1.0-older-pear.patch

I alse add a patch to improve (a little) %file

Created attachment 126764 [details]
php-pear-PEAR-Command-Packaging-0.1.0-template.patch

Thanks for the feedback Remi.

1) The relocation problem, yes, that's probably a PEAR <1.4.7 vs PEAR 1.4.7+
thing. I think that it might be easier for this package just to wait on bug
#183359, since PEAR 1.4.7 has important packaging fixes. Or depending on how
long that takes I might investigate further making it work on older versions. It
doesn't seem like a very productive use of time though.

2) the %files thing is fixed upstream (see http://pear.php.net/bugs/7129) so
I'll also wait on a new release upstream (by me :) before proceeding with this
package.

New version of PEAR_Command_Packaging has been released
All these things should fixed in a new version:

http://www.timj.co.uk/linux/specs/php-pear-PEAR-Command-Packaging.spec
http://www.timj.co.uk/linux/srpms/php-pear-PEAR-Command-Packaging-0.1.1-1.src.rpm

php-pear use epoch=1, so i think you should have

BuildRequires:  php-pear >= 1:1.4.7

When this package is installed, the pear install and uninstall commands are
removed and the %post and %postun sections are not executed properly.

To test this, install the php-pear-PEAR-Command-Packaginag-0.1.1-1 rpm and type:
pear help

You will see that install and uninstall commands are not listed, nor do these
commands work if you try to use them.

This is a must fix item.

Also, please change the License field to say just "PHP License" this is what
rpmlint will expect.  A copy of the actual license should be included and added
to %doc

Thanks for taking on the package Chris.

(In reply to comment #9)
> When this package is installed, the pear install and uninstall commands are
> removed and the %post and %postun sections are not executed properly.

This wasn't the case previously (and indeed right now I have it quite happily
installed here without that problem) so there is something environment-specific
about this, but by sheer coincidence I noticed this myself on another machine
this morning. Fixed upstream this morning in cvs.php.net so I will build a new
version of this RPM when there is a new upstream ver.

(In reply to comment #10)
> Also, please change the License field to say just "PHP License" this is what
> rpmlint will expect. 

I do not think this should happen, for the following reasons:

a) The License field is supposed to be what the License is, not what rpmlint
thinks it should be. If rpmlint doesn't know it, rpmlint needs fixing - not the
package. That's not an argument against consistency, but is an argument for
accuracy. See also (b) below.

b) "The PHP License v3.01" is exactly what the current Core "php" package uses

c) Noting the versioning of licenses is important. I'll rephrase and repeat what
I said over in bug #196281, which is:

We really ought to specify a version if the package specifies one, because
otherwise if there's (say) a PHP License v4 in future with different terms, we
would be misleading users by implying they could distribute it under the terms
of v4 whereas the authors might have only specified v3. We should respect the
author's license. Note for example that recent Core "php" packages have started
explicitly mentioning license version.

Now I will hold my hands up (as the upstream maintainer of this package) for not
enforcing consistency upstream; all the source files mention 3.01 but the web
page says "PHP License" and links to 2.02.  That will also be fixed to be
consistent for the upcoming 0.1.2.

> A copy of the actual license should be included and added to %doc

This is against the usual convention and directly conflicts with the advice I
was given in bug #176733, the first PEAR package to be added to FE.

Updated files for v0.1.2, fixing the "kill pear install" problem at:

http://www.timj.co.uk/linux/specs/php-pear-PEAR-Command-Packaging.spec
http://www.timj.co.uk/linux/srpms/php-pear-PEAR-Command-Packaging-0.1.2-1.src.rpm

rpmlint output:
W: php-pear-PEAR-Command-Packaging invalid-license The PHP License v3.01
W: php-pear-PEAR-Command-Packaging no-documentation
W: php-pear-PEAR-Command-Packaging dangerous-command-in-%post install
W: php-pear-PEAR-Command-Packaging invalid-license The PHP License v3.01
W: php-pear-PEAR-Command-Packaging patch-not-applied Patch1:
php-pear-PEAR-Command-Packaging-0.1.2-fedora-conventions2.patch
W: php-pear-PEAR-Command-Packaging patch-not-applied Patch0:
php-pear-PEAR-Command-Packaging-0.1.2-fedora-conventions1.patch
W: php-pear-PEAR-Command-Packaging patch-not-applied Patch2:
php-pear-PEAR-Command-Packaging-0.1.2-initdepname.patch

These patches are applied, they just cannot be applied in the usual way due to
PHP packaging process.

The dangerous command can be ignored (you can surpress this warning by adding
quote around the install, like 'install', if you decide to do this you should
add a comment to your spec file saying you are doing this to surpress spurious
rpmlint warnings.

The License discussion is below I am going to highly recommend you do as I say
about the license issue, I talked to several FESCo members about this subject.

- package conforms to PHP package naming guidelines
- spec file name matches %{name}
- package meets packaging guidelines
- package licensed with open source compatible license
- package does not include license text
- spec file written in American english
- spec file is legible
- sources match upstream
b8b3e3791c687e8ddaaeac7c65732233  PEAR_Command_Packaging-0.1.2.tgz
- package successfully compiles and builds on FC-5 x86_64
- Not all BuildRequires are sane
Should use:
BuildRequires: php-pear >= 1.4.9
Requires(post): php-pear >= 1.4.9
Requires(pre):  php-pear >= 1.4.9
- package does not use locales
- package does not put .so files in standard directories (no need to run ldconfig)
- package owns all directories it creates
- defattr permissions are okay
- package contains proper %clean section
- macro usage is consistant
- package contains permissible content
- no large documentation
- doc files do not affect runtime
- package does not need a devel sub package
- package does not contain any libtool archives
- package does not need a .desktop file
- package does not own any files or directories owned by other packages

=========================================
  SUMMARY
=========================================
SHOULD FIX:
- Change license tag to just "PHP License"
- Include actual license in %doc

This has the benefit of having an _actual_ license to look at, not just a
license name.  It also kills off three rpmlint warnings.  It also makes it
absolutely clear about the licensing agreement.

Comment from a FESCo member about this (Ville Skyttä):
scop |  I'd use just "PHP License"
scop |  the License tag is not legally binding anyway, and including specific
version number has potential for bitrot

I am not going to make this a blocker, but I strongly recommend you follow my
guidelines on this.  An actual license is _always_ better than just a version
number in the header.


MUST FIX:
- The package contains /usr/share/pear/PEAR/Command/Packaging.php.orig
This looks like a leftover from a patch operation or something.  It should not
be included as one of the files in the package.
- Use the following Requires:
BuildRequires: php-pear >= 1.4.9
Requires(post): php-pear >= 1.4.9
Requires(pre):  php-pear >= 1.4.9
Your current "Requires:" line looks okay
- Include the following macro at the top of your spec:
%define datadir %(pear config-get data_dir 2> /dev/null || echo
%{_datadir}/pear/data)
and use %{datadir} in your %files section

Additional SHOULD item:
- Add ||: to your %post and %postun scriptlets, see:
http://www.fedoraproject.org/wiki/ScriptletSnippets

Ooops, just noticed something else, Requires(pre) is wrong, it should look like
this:

BuildRequires: php-pear >= 1.4.9
Requires(post): php-pear >= 1.4.9
Requires(postun): php-pear >= 1.4.9

This is a must fix.

I've posted to fedora-packaging about a couple of the issues there, but just to
pick up on something:

(In reply to comment #15)
> Ooops, just noticed something else, Requires(pre) is wrong, it should look 
> like  this:
> BuildRequires: php-pear >= 1.4.9
> Requires(post): php-pear >= 1.4.9
> Requires(postun): php-pear >= 1.4.9

Why? I don't understand what was wrong with the original spec.  What bit of the
build process and post and postun scripts requires PEAR 1.4.9? And even if it
did, shouldn't that be 1:1.4.9?

Ooops yes, you are correct. The version numbers are not needed for these
scriptlets.  However you should have Requires(postun) and not Requires(pre).

You are also correct about the Epoch, I did not notice this before.

Remi has been using 1.4.9 on his packages for the BuildRequires, there might be
some packaging fixes in that version, if 1.4.7 is sufficient then this is okay,
I was just being safe than sorry.

I read your post on fedora-packaging, did you check the fedora-packaging
archives?  Your question comes up about once a month.  Here is a similar thread
that comes to the conclusion I suggested:

https://www.redhat.com/archives/fedora-packaging/2006-March/msg00004.html

"Best Practice" is to keep the License tag as simple as possible and to place
the license text in %doc as I suggested.

Note also that this is a SHOULD and not a MUST.  If you feel you do not want to
follow "best practices" then feel free not to.

Updated packages to conform with the standards currently being proposed on
fedora-packaging:

http://www.timj.co.uk/linux/specs/php-pear-PEAR-Command-Packaging.spec
http://www.timj.co.uk/linux/srpms/php-pear-PEAR-Command-Packaging-0.1.2-2.src.rpm

Hi Tim, can you update this spec file per the latest template:

https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=135472

More info at:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198706

You can also have a look on a ready template for "pear makerpm" at 
http://remi.collet.free.fr/rpms/SPEC/php-pear-1.4.11-template.spec

I have updated the package to support the latest template as generated by
rpmdevtools-5.1-1 (build Sat 09 Sep 2006 06:05:21) from bug #198706.

http://www.timj.co.uk/linux/specs/php-pear-PEAR-Command-Packaging.spec
http://www.timj.co.uk/linux/srpms/php-pear-PEAR-Command-Packaging-0.1.2-2.src.rpm

The package is now effectively self-consistent; if you install it and do "pear
makerpm PEAR_Command_Packaging-0.1.2.tgz" then the output spec file you get is
very similar to the one used to build the package itself. (It doesn't of course
have the patches, but mostly the differences are trivial)

Tim, the spec file in the srpm is not the same as the other spec file and
contains some errors and ommissions.

From the other spec file, please remove the "PEAR:" from the summary, use
install -pm 644 instead of cp in %install, set %setup section does not match the
template although not technically necessary.  Please shorten description to just
the first paragraph, %build is missing a cd from the template, but not
technically required.  Please include a copy of the php license in %doc.

Oh I see, you linked to version 2 above, I got version 3.  Other comments: 
Remove requires on php, this is not needed if php < 5.

I checked out the version 3.  Please remove "PEAR:" from the summary in the
template.spec, also I'm not sure why you have BuildArch in the template.spec
because all pear packages are noarch.

rpmlint doesnt like Source1 ending in .spec, can you rename this to something
that doesnt end in .spec?

Sorry about the version number cock-up.

> Please remove "PEAR:" from the summary in the template.spec

I've debated with myself about this one and put it in/took it out several times.
My general feeling is that it gives the "right" answer more times with it in
than out. For example, PEAR::DB has "Database Abstraction Layer" as its summary
from the XML. "PEAR: Database Abstraction Layer" makes more sense to me. Ditto
for various other packages.

See also owners.list for other PEAR packages.

> also I'm not sure why you have BuildArch in the template.spec
> because all pear packages are noarch.

Huh? You still need BuildArch in there! See also the rpmdevtools template.
If you mean why has it got an expanded @arch@ macro, well:

a) it doesn't matter, since the output is what's important
b) it matters for PECL. As it happens, you will not currently get a very useful
PECL spec out of PCP, but you should do eventually, and ideally the spec
template should be shared as much as possible.

> Please shorten description to just the first paragraph

Done

> %build is missing a cd from the template, but not
> technically required. 

Yes, I took it out because it's utterly pointless when there is no build
section. Remember: the template is a template, not the finished article. It's
intended to have human attention although of course minimising that is a bonus.
(The *output* of php-PCP aims to be something slightly closer to the finished
article, but even that needs human review)

> Please include a copy of the php license in %doc.

Done. NB: fixed upstream in CVS.

> rpmlint doesnt like Source1 ending in .spec, can you rename this to
> something that doesnt end in .spec?

Yes, done, though I'm pondering whether this should be filed as an rpmlint bug.


New version (with right version number in URL this time):

http://www.timj.co.uk/linux/specs/php-pear-PEAR-Command-Packaging.spec
http://www.timj.co.uk/linux/srpms/php-pear-PEAR-Command-Packaging-0.1.2-4.src.rpm

"PEAR" in the summary doesn't make sense at all.  It is already plainly obvious
from the package name that it is a pear package.  It especially does not make
sense for this package. You even mention the word "PEAR" twice in your summary.
 A summary is supposed to be as short as possible.  Even a summary that starts
with the word "The" should be shortened.  For this summary I would use:

Summary: Creates spec files from PEAR modules

We definately do not want to have "PEAR:" as a standard in our summaries, and
packages that already have it should remove it.

----

From the BuildArch comment, I meant to say that it didn't make sense to me to
specify an arch, in other words, I was saying just hard code BuildArch: noarch.
But if this is going to happen automatically anyway that is fine, it was just
confusing to me.

----

The reason we add the cd to the %build is incase anyone wants to add something
to the %build they will not have to remember to add the cd there.  It's
definately not required, just mentioning that this was different in your spec
vs. the template spec.

----

APPROVED pending the removal of "PEAR:" from Summary field in spec file and
template.spec.

Why hasn't this package been imported and built?

Changing this back to FE-REVIEW and NEEDINFO status since it's been several
weeks since this was approved and nothing has been done.  Let me know when you
are no longer AWOL.

I'm not AWOL. This one has just slipped under the radar, sorry. I'll import and
build ASAP.

Thanks for all the comments and review. successfully built in devel (job #18300)