Bug 525346

PERF_HEAD=`ls /usr/src/kernels/*/include/linux/perf_counter.h|sort |tail -n 1` || exit 1
ln -s $PERF_HEAD perf_counter.h

Ew. How often do you expect the ABI to change here?

Why wouldn't that be in kernel-headers, actually?

perf_counter.h is not expected to change. However, there could be multiple perf_counter.h files. The PERF_HEAD was written so that it would just pick one for the symbolic link.

The kernel-headers would be more appropriate. However, kernel-headers doesn't currently include perf_counter.h. Need to get that added.

Revised the spec file:

To separate out the static library files.
To run tests when building the package.

Spec URL: http://people.redhat.com/wcohen/papi/papi.spec
SRPM URL: http://people.redhat.com/wcohen/papi/papi-3.7.0-1.src.rpm
Description:

Correction:

Spec URL: http://people.redhat.com/wcohen/papi/papi.spec
SRPM URL: http://people.redhat.com/wcohen/papi/papi-3.7.0-3.src.rpm

Updated srpm to be buildable on non-x86 machines;

Spec URL: http://people.redhat.com/wcohen/papi/papi.spec
SRPM URL: http://people.redhat.com/wcohen/papi/papi-3.7.0-6.src.rpm

Updated srpm for a couple SPEC file cleanups (proper URL for download location and requires for -devel RPM);

Spec URL: http://people.redhat.com/wcohen/papi/papi.spec
SRPM URL: http://people.redhat.com/wcohen/papi/papi-3.7.0-7.src.rpm

A scratch build for papi-3.7.0-7.src.rpm is at:

https://koji.fedoraproject.org/koji/taskinfo?taskID=1722841

Taking for review.

Created attachment 364225 [details]
papi.spec.patch

Patch to apply to papi.spec which fixes a number of RPM and
rpmlint issues.

* Fri Oct 09 2009 Richard W.M. Jones <rjones> - 3.7.0-8
- Fix URL and Source0.
- Grammatical corrections to the description sections.
- Remove RPATHs from shared libraries.
- RPM shouldn't own directories.
- Add soname patch so soname is libpapi.so.3.
- Add exit patch so library doesn't call exit directly.

Created attachment 364227 [details]
papi-3.7.0-soname.patch

This patch to the source makes PAPI use a library
soname like 'libpapi.so.3' instead of 'libpapi.so'.
This is to fix an rpmlint error.

Created attachment 364228 [details]
papi-3.7.0-exit.patch

This patch to the source makes PAPI call abort(3)
instead of exit(3).  Calling exit in shared libraries
provokes an rpmlint warning.

I'm now continuing the review based on my patched version ...

Created attachment 364229 [details]
papi.spec.patch

Further change to papi.spec, this replaces the
previous papi.spec.patch.

+ rpmlint output

papi-static.x86_64: W: no-documentation

I think this warning can be ignored, because the base package
contains plenty of documentation.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
  BSD
+ license matches the actual package license
+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
+ package successfully builds on at least one architecture
  x86_64
n/a ExcludeArch bugs filed
+ BuildRequires list all build dependencies
  http://koji.fedoraproject.org/koji/taskinfo?taskID=1722841
n/a %find_lang instead of %{_datadir}/locale/*
+ binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
+ header files should be in -devel
+ static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
+ libfoo.so must go in -devel
+ -devel must require the fully versioned base
+ packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8
+ use %global instead of %define

Optional:

+ if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if available
+ reviewer should build the package in mock
- the package should build into binary RPMs on all supported architectures
- review should test the package functions as described
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin

-------------------------------------------------------

THIS PACKAGE IS APPROVED by rjones

(Note: This applies only the -9 version containing my additional changes)

If I may ask, what's the justification of having static libraries?

I'll defer to the packager on this question.  I did
some Googling and it seems like people use both the
dynamic and static versions of this library, and I can't
see any down side to using the dynamic version, but
I'm no expert on this.

PAPI builds the static library. This might have been used because of the lack
of share library versioning:

http://www.ncsa.illinois.edu/UserInfo/Resources/Software/Tools/PAPI/

Or because some PAPI targets do not support shared libraries (IBM blue gene):

http://ipm-hpc.sourceforge.net/installation.html

However, on Fedora it would probably be reasonable to remove the static
libraries as mentioned at:

https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries

Yeah, I'd prefer to not ship static libs if we can get away with it.

Updated srpm to get rid of the static libraries;

Spec URL: http://people.redhat.com/wcohen/papi/papi.spec
SRPM URL: http://people.redhat.com/wcohen/papi/papi-3.7.0-10.src.rpm

New Package CVS Request
=======================
Package Name: papi
Short Description: Performance Application Programming Interface
Owners: wcohen
Branches: F-12 EL-6
InitialCC: lwang

William, I don't think EL-6 exists yet...

It doesn't. ;) 

Also, we can't add arbitrary email addresses to initialcc. It has to be a fedora account system account name. Can you revist and reset the fedora-cvs flag when you are ready?

New Package CVS Request
=======================
Package Name: papi
Short Description: Performance Application Programming Interface
Owners: wcohen
Branches: F-12
InitialCC:

cvs done.

This package is in Fedora now, and so I think this
bug should be closed.

https://admin.fedoraproject.org/pkgdb/acls/name/papi