Spec URL: http://people.redhat.com/wcohen/libpfm/libpfm.spec SRPM URL: http://people.redhat.com/wcohen/libpfm/libpfm-4.2.0-2.fc18.src.rpm Description: libpfm4 is a library to help encode events for use with operating system kernels performance monitoring interfaces. The current version provides support for the perf_events interface available in upstream Linux kernels since v2.6.31.
The current version of papi builds libpfm bundled into the rpm. This review is being submitted to unbundle libpfm from papi and allow libpfm to a separate rpm.
* https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package * https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries * https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag * https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean > %{!?with_python: %global with_python 1} You may want to look into using "%bcond_without python" instead. > %defattr(644,root,root,755) > %doc README https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > %attr(755,root,root) %{_libdir}/lib*.so* Ownership is root:root by default. If the mode is not 0755 for the installed lib, I highly recommend fixing it upstream and temporarily during %install. Usage of the eye-catching %attr should be limited to non-ordinary file permissions as not to overuse it.
Thanks for the review. I have attempted to address each of the points mentioned in the review in libpfm-4.2.0-3.fc16.src.rpm and libpfm.spec available at: http://people.redhat.com/wcohen/libpfm/ A scratch build is also available at: http://koji.fedoraproject.org/koji/taskinfo?taskID=3940897 * https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package Change requires to: Requires: %{name}%{?_isa} = %{version}-%{release} * https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries There are cases where HPC developer want static versions of the library (the compute node may not have shared libraries installed). I have split the static library into a libpfm-static sub package. * https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag Removed the unneeded Buildroot tag * https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean Removed the unneeded %clean section > %{!?with_python: %global with_python 1} You may want to look into using "%bcond_without python" instead. convered the libpfm.spec to use %bcond_without. For the file permissions it looks like the permissions were correct so removed the %defattr. > %defattr(644,root,root,755) > %doc README https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > %attr(755,root,root) %{_libdir}/lib*.so* Ownership is root:root by default. If the mode is not 0755 for the installed lib, I highly recommend fixing it upstream and temporarily during %install. Usage of the eye-catching %attr should be limited to non-ordinary file permissions as not to overuse it.
Note rpmlint complains about a couple things in libpfm-python rpm: libpfm-python.x86_64: E: non-executable-script /usr/lib64/python2.6/site-packages/perfmon/pmu.py 0644L /usr/bin/env libpfm-python.x86_64: E: non-executable-script /usr/lib64/python2.6/site-packages/perfmon/session.py 0644L /usr/bin/env Checking with upstream developers whether those should be executable files or whether #! should be removed from the files.
Would it be possible to get this package reviewed and into Rawhide? Updated the libpfm.spec file to include a patch for the python files. The new libpfm.spec and libpfm-4.2.0-4.fc17.src.rpm are available at http://people.redhat.com/wcohen/libpfm/ The results of rpmlint are pretty minor: $ rpmlint ../SRPMS/libpfm-4.2.0-4.fc17.src.rpm ../RPMS/x86_64/libpfm-* libpfm.src: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm.src: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm.src:70: W: rpm-buildroot-usage %build %global python_config CONFIG_PFMLIB_NOPYTHON=n PYTHON_PREFIX=$RPM_BUILD_ROOT/%{python_prefix} libpfm.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libpfm.so libpfm-devel.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm-devel.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm-python.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm-python.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm-python.x86_64: W: no-documentation libpfm-static.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm-static.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm-static.x86_64: W: no-documentation 6 packages and 0 specfiles checked; 0 errors, 14 warnings.
Good: - rpmlint checks return: The spelling errors are no problem, but rpm-build-root and devel-file-in-non-devel-package should be fixed. Move the libpfm.so to -devel, and remove $RPM_BUILD_ROOT from line 70. Still builds with this change. - package meets naming guidelines - package meets packaging guidelines - license ( MIT ) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream FIX package compiles on devel (x86_64) Builds fine on f17, but dies in mock for rawhide. End of relevant log section: cc -g -Wall -Werror -I. -I/builddir/build/BUILD/libpfm-4.2.0/perf_examples/../include -DCONFIG_PFMLIB_DEBUG -DCONFIG_PFMLIB_OS_LINUX -I. -D_GNU_SOURCE -pthread -c notify_self.c notify_self.c:50:29: error: 'struct siginfo' declared inside parameter list [-Werror] notify_self.c:50:29: error: its scope is only this definition or declaration, which is probably not what you want [-Werror] notify_self.c: In function 'sigio_handler': notify_self.c:59:10: error: dereferencing pointer to incomplete type notify_self.c:67:10: error: dereferencing pointer to incomplete type notify_self.c:70:39: error: dereferencing pointer to incomplete type notify_self.c:72:49: error: dereferencing pointer to incomplete type notify_self.c:93:18: error: dereferencing pointer to incomplete type notify_self.c: In function 'main': notify_self.c:133:39: error: assignment from incompatible pointer type [-Werror] cc1: all warnings being treated as errors - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file - devel package ok - no .la files - post/postun ldconfig ok - devel requires base package n-v-r So it's the two things above. Also I was wondering, do you really need to ship the static libs, since it builds a solib? Does papi or something else need it?
The newer version of glibc does not have struct siginfo, and ust siginfo_t should be used instead. Made a patch and sent it to the upstream perfmon2-devel.net mailing list. The patched version of the libpfm-4.2.0 now compile successfully on Fedora Rawhide. The revised libpfm.spec and libpfm-4.2.0-5.fc17.src.rpm are at http://people.redhat.com/wcohen/libpfm/ Most people will just use the shared libraries, but there are people in High Performance Computing (HPC) that may not have much in the way of shared libraries on the compute nodes and they would like to have static versions of the papi/libpfm to support that environment.
Patch on perfmon2-devel mailing list archive: http://sourceforge.net/mailarchive/forum.php?thread_name=1339095973-12372-1-git-send-email-wcohen%40redhat.com&forum_name=perfmon2-devel
Ok, rawhide build is good, and I concur on static libs, but you still need to remove the buildroot tag from %build and move the .so to -devel.
Fixed up the buildroot tag and moved the .so to -devel in libpfm-4.2.0-6.fc17.src.rpm on people.redhat.com/wcohen/libpfm/. Results of rpmlint: $ rpmlint ../SRPMS/libpfm-4.2.0-6.fc17.src.rpm ../RPMS/x86_64/libpfm-*4.2.0-6* libpfm.src: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm.src: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm-devel.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm-devel.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm-python.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm-python.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm-python.x86_64: W: no-documentation libpfm-static.x86_64: W: spelling-error Summary(en_US) perf -> pref, per, perv libpfm-static.x86_64: W: spelling-error %description -l en_US perf -> pref, per, perv libpfm-static.x86_64: W: no-documentation 6 packages and 0 specfiles checked; 0 errors, 12 warnings.
Excellent, APPROVED.
New Package SCM Request ======================= Package Name: libpfm Short Description: Library to encode performance events for use by perf tool Owners: wcohen Branches: InitialCC:
Turns out this already existed but was retired. I unretired it, take ownership in pkgdb, and request branches other than rawhide if you need them.
Okay I have taken ownership of it and imported the updated srpm. When trying to build in f18 I get: (x86-07.phx2.fedoraproject.org) -> FAILED: BuildError: package libpfm is blocked for tag f18 Is there some flag that needs to be set for F18/rawhide?
Oh, I forgot, you need to log a trac with rel-eng to get it unblocked. https://fedorahosted.org/rel-eng/
Package Change Request ====================== Package Name: libpfm New Branches: f18 Owners: wcohen Unretiring the libpfm package so can avoid having papi build a bundled version of libpfm.
Invalid branch, f18==rawhide==devel==master.
The libpfm package is now in fedora rawhide. http://koji.fedoraproject.org/koji/buildinfo?buildID=324030