Spec URL: http://0pointer.de/public/mutrace.spec SRPM URL: http://0pointer.de/public/mutrace-0.1-1.fc12.src.rpm Description: Mutex Tracer mutracer is a mutex tracer/profiler. The package should be easy to review. See this for more information: http://0pointer.de/blog/projects/mutrace.html If you run this through rpmlint you'll get invalid-soname and shared-lib-calls-exit warnings. Both are safe to be ignored. The former is expected since this is not a library applications are supposed to link to, it's intended only for LD_PRELOAD. This is similar to libpulsedsp.so. The latter too is expected since we actually hook into the different variations of exit() to print our statistics summary. After printing that we call the real implementations of those calls.
As far as I can see, there's not much to correct. In the %files section, you should change the %defattr and %attr settings like this: %files %defattr(-,root,root,-) %doc README LGPL %{_bindir}/mutrace %attr(0644,root,root) %{_libdir}/libmutrace.so
Umh, shared libraries should have 755 attributes.
(In reply to comment #2) > Umh, shared libraries should have 755 attributes. Yes, I also wanted to write that. But as the library is intended to be used with LD_PRELOAD only, it's not absolutely necessary. At least, LD_PRELOAD also works with libraries having 0644 permissions.
Well, LD_PRELOAD'ed library can have 0644 permission. The point is that: on Fedora every ELF binaries must have executionable permission when %install ends, otherwide find-debuginfo.sh won't work correctly. So at the stage %install ends, libmutrace.so cannot be 0644 permission. We can change the permission by rpm's %attr explicitly after find-debuginfo.sh strips binaries like this, however we usually just say "shared libraries should have 755 attributes" because it is much simpler. By the way I think it is better to move libmutrace.so to under %_libexecdir/%name or so because this does not seem to be system-wide library.
(In reply to comment #4) > By the way I think it is better to move libmutrace.so to under > %_libexecdir/%name > or so because this does not seem to be system-wide library. We cannot do this due to the madness that is multilib. We need to make sure that ld.so uses /usr/lib/libmutrace.so for 32bit and /usr/lib/libmutrace.so for 64bit processes. If we moved that .so into a subdir then we had to hardcode its path in $LD_PRELOAD which would break ld.so's automatic selection of the module to load. So, no can do. Sorry.
(In reply to comment #5) > We cannot do this due to the madness that is multilib. We need to make sure > that ld.so uses /usr/lib/libmutrace.so for 32bit and /usr/lib/libmutrace.so for Oops, that second path should have been /usr/lib64/libmutrace.so of course. Sorry for the confusion. > 64bit processes. If we moved that .so into a subdir then we had to hardcode its > path in $LD_PRELOAD which would break ld.so's automatic selection of the module > to load. So, no can do. Sorry.
.spec file is now fixed according to the comments: Spec URL: http://0pointer.de/public/mutrace.spec SRPM URL: http://0pointer.de/public/mutrace-0.1-2.fc12.src.rpm
Ping. I'd be thanful if someone could do the second review before the freeze! Thanks!
Presumably you mean the first review, since nobody appears to be officially reviewing this? Will be done in a few hours -- got to run right now.
ReviewTemplate MUST ? rpmlint source clean. binary not produced due to errors. OK package name OK spec file name OK package guideline-compliant ? license complies with guidelines complies with guidelines, yes. However, I recall a discussion in -devel about the licensing of Fedora Infrastructure projects.These are supposed to be LGPLv2+ for libraries and GPLv2+ for applications. Yours most likely is not bound by these guidelines, and should the infrastructure team decide to use mutrace, they can (the combination becomes GPLv3+), so this is up to you. OK license field accurate OK license file not deleted OK spec in US English OK spec legible FIX source matches upstream $ sha1sum mutrace-0.1.tar.gz ../SOURCES/mutrace-0.1.tar.gz dcb16f9a80262cb608f641ee5c960b15def6ad83 mutrace-0.1.tar.gz 6017fc40158663eeffec4c70ba69cf04cde11ef2 ../SOURCES/mutrace-0.1.tar.gz Looks like the source in the SRPM is old: -rw-rw-r--. 1 michel michel 339432 2009-09-22 00:09 mutrace-0.1.tar.gz -rw-rw-r--. 1 michel michel 328641 2009-09-15 18:20 ../SOURCES/mutrace-0.1.tar.gz OK builds under >= 1 archs, others excluded rpmbuild -bb attempted on x86_64 (see also "no dupes in %files") FIX build dependencies complete missing binutils-devel OK library -> ldconfig OK own all directories FIX no dupes in %files no dupes, but some unpackaged files: %{_libdir}/libmatrace.so and libmutrace-backtrace-symbols.so OK permission OK %clean RPM_BUILD_ROOT OK macros used consistently OK Package contains code NA large docs => -doc NA doc not runtime dependent OK clean buildroot before install OK filenames UTF-8 SHOULD FIX if license text missing, ask upstream to include it corresponding GPL license text not included FIX package build in mock on all architectures not using mock yet, BR problem OK package functioned as described FIX require package not files require util-linux-ng rather than /usr/bin/getopt ?
Lennart? Would be nice to see this packaged soon-ish, especially since building for F-12 now require rel-eng to manually tag a package.
Yepp, the .spec file was actually belonging to an older tarball. Since I didn't really do a proper release of mutarce yet I just updated the tarball, but didn't update the spec file.
(In reply to comment #10) > ? license complies with guidelines > complies with guidelines, yes. However, I recall a discussion in -devel about > the licensing of Fedora Infrastructure projects.These are supposed to be > LGPLv2+ for libraries and GPLv2+ for applications. Yours most likely is not > bound by these guidelines, and should the infrastructure team decide to use > mutrace, they can (the combination becomes GPLv3+), so this is up to you. I don't see how mutrace should ever be used for Fedora infrastructure. > OK license field accurate > OK license file not deleted > OK spec in US English > OK spec legible > FIX source matches upstream > $ sha1sum mutrace-0.1.tar.gz ../SOURCES/mutrace-0.1.tar.gz > dcb16f9a80262cb608f641ee5c960b15def6ad83 mutrace-0.1.tar.gz > 6017fc40158663eeffec4c70ba69cf04cde11ef2 ../SOURCES/mutrace-0.1.tar.gz Fixed now. > FIX build dependencies complete > missing binutils-devel Fixed. > FIX no dupes in %files > no dupes, but some unpackaged files: %{_libdir}/libmatrace.so and > libmutrace-backtrace-symbols.so Fixed. > SHOULD > FIX if license text missing, ask upstream to include it > corresponding GPL license text not included It is included. > FIX package build in mock on all architectures > not using mock yet, BR problem Uh? > OK package functioned as described > FIX require package not files > require util-linux-ng rather than /usr/bin/getopt ? /usr/bin/getopt is pretty generic and changed packages a couple of times in the past, which is is why I decided to stick with a path dependency instead of package dependency. Spec file and srpm nowupdated: Spec URL: http://0pointer.de/public/mutrace.spec SRPM URL: http://0pointer.de/public/mutrace-0.1-3.fc12.src.rpm
Just a small side note: this newer snapshot of mutrace now includes a copy of libacktrace-symbols which is licensed GPLv2+. I have hence updated the spec file license tag accordingly. Oh, and I now did the proper release of mutrace 0.1, so the confusion between tarballs and srpms shouldn't happen anymore.
(In reply to comment #13) > (In reply to comment #10) > > > ? license complies with guidelines > I don't see how mutrace should ever be used for Fedora infrastructure. Fair enough. > > FIX source matches upstream > > $ sha1sum mutrace-0.1.tar.gz ../SOURCES/mutrace-0.1.tar.gz > > dcb16f9a80262cb608f641ee5c960b15def6ad83 mutrace-0.1.tar.gz > > 6017fc40158663eeffec4c70ba69cf04cde11ef2 ../SOURCES/mutrace-0.1.tar.gz > > Fixed now. > Verified $ sha1sum mutrace-0.1.tar.gz ../SOURCES/mutrace-0.1.tar.gz ea972f8666933fb297b9791187858ccca3fecff7 mutrace-0.1.tar.gz ea972f8666933fb297b9791187858ccca3fecff7 ../SOURCES/mutrace-0.1.tar.gz > > FIX build dependencies complete > > missing binutils-devel > > Fixed. OK > > > FIX no dupes in %files > > no dupes, but some unpackaged files: %{_libdir}/libmatrace.so and > > libmutrace-backtrace-symbols.so > > Fixed. OK > > > SHOULD > > FIX if license text missing, ask upstream to include it > > corresponding GPL license text not included > > It is included. Em, I don't see it. I see these license files: GPL ==> GPL v2 (needed) LGPL ==> LGPL v3 (needed) per the LGPL file, """ This version of the GNU Lesser General Public License incorporates the terms and conditions of version 3 of the GNU General Public License, supplemented by the additional permissions listed below. """ Per the FSF's instructions: http://www.fsf.org/licensing/licenses/gpl-howto.html """ You should also include a copy of the license itself somewhere in the distribution of your program. All programs, whether they are released under the GPL or LGPL, should include the text version of the GPL. In GNU programs the license is usually in a file called COPYING. """ since you use LGPLv3+, you must bundle the GPLv3 license text as well. > > FIX package build in mock on all architectures > > not using mock yet, BR problem > > Uh? I didn't build the package in mock, since the build dependencies were not complete (and there were unpackaged files), and I figured I'd have to test the fixed package anyway. There are some failures on PPC, both with F-11 and F-12: http://koji.fedoraproject.org/koji/taskinfo?taskID=1732211 In mutrace.c, you define DEBUG_TRAP on non-x86 architectures as raise(SIGTRAP), but you are not #ifdef-including signal.h > > OK package functioned as described > > FIX require package not files > > require util-linux-ng rather than /usr/bin/getopt ? > > /usr/bin/getopt is pretty generic and changed packages a couple of times in the > past, which is is why I decided to stick with a path dependency instead of > package dependency. Fair enough I'm attaching the patch, which allows building on non-x86 architectures; incorporate that and add the missing GPLv3 text and this package should be ready to go.
Created attachment 363922 [details] Patches mutrace.c to include signal.c on non-x86 archs
Thanks. Merged your patch (thanks for prepping that!), and added the GPL3 license text. Spec URL: http://0pointer.de/public/mutrace.spec SRPM URL: http://0pointer.de/public/mutrace-0.2-1.fc12.src.rpm
Koji build (F-12): http://koji.fedoraproject.org/koji/taskinfo?taskID=1738488 $ rpmlint SRPMS/mutrace-0.2-1.fc12.src.rpm ~/Download/mutrace*0.2* mutrace.x86_64: E: invalid-soname /usr/lib64/libmatrace.so libmatrace.so mutrace.x86_64: W: shared-lib-calls-exit /usr/lib64/libmatrace.so exit mutrace.x86_64: W: shared-lib-calls-exit /usr/lib64/libmatrace.so _exit mutrace.x86_64: E: invalid-soname /usr/lib64/libmutrace.so libmutrace.so mutrace.x86_64: W: shared-lib-calls-exit /usr/lib64/libmutrace.so exit mutrace.x86_64: W: shared-lib-calls-exit /usr/lib64/libmutrace.so _exit mutrace.x86_64: E: invalid-soname /usr/lib64/libmutrace-backtrace-symbols.so libmutrace-backtrace-symbols.so mutrace.x86_64: W: shared-lib-calls-exit /usr/lib64/libmutrace-backtrace-symbols.so exit.5 mutrace.x86_64: W: shared-lib-calls-exit /usr/lib64/libmutrace-backtrace-symbols.so _exit.5 3 packages and 0 specfiles checked; 3 errors, 6 warnings. The invalid-soname is OK, really, as the libraries are not supposed to be linked to by applications. The shared-lib-calls-exit warnings are just warnings, and presumably there's a good reason for using exit. That took care of all the previous problems -- APPROVED.
New Package CVS Request ======================= Package Name: mutrace Short Description: Mutex Tracer Owners: lennart Branches: F-12 devel InitialCC:
cvs done.
Commited, built and asked for a tag for f12.