Bug 458204
Summary: | Review Request: coredumper - Library to help applications create core dumps | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Rakesh Pandit <rpandit> |
Component: | Package Review | Assignee: | Debarshi Ray <debarshir> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | debarshir, fedora-package-review, notting, rpandit |
Target Milestone: | --- | Flags: | debarshir:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2008-09-06 07:57:27 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 461340, 461341, 461342 |
Description
Rakesh Pandit
2008-08-06 22:50:13 UTC
MUST Items: OK - rpmlint is clean OK - follows Naming Guidelines OK - spec file is named as %{name}.spec xx - package does not meet Packaging Guidelines + Why does the -devel subpackage have 'Requires: automake'? The sample Spec included in the upstream tarball (packages/rpm/rpm.spec) does not have it too. + Please add a %check stanza as: %check make check According to the README, a 'BuildRequires: binutils gdb' will be needed to run the tests. + Moving the library headers from %{_includedir}/google to %{_includedir}/%{name} will break the builds of programs which contain '#include <google/coredumper.h>'. If similar Google projects are also using %{_includedir}/google, then you might not need to move the files. Otherwise, if you want to move them there are a few options: - Provide a README.Fedora in %doc as mentioned in https://fedoraproject.org/wiki/Packaging/Guidelines#Summary_and_description to document this. - Provide a pkgconfig file which has 'Cflags: -I${includedir}/coredumper' - Initiate a discussion on fedora-packaging - It is better to pass --includedir=DIR to %configure instead of using the mkdir & mv dance to change the location of the headers. + No need to 'find $RPM_BUILD_ROOT -type f -name "*.a" -delete' in %install since --enable-static=no was passed to %configure. + You could consider shipping ChangeLog under %doc, and putting README in the -devel subpackage since it talks about the examples and manual pages. README also mentions 'src/coredump_unittest.c' so it might be a good idea to ship this single file under %doc in the -devel subpackage. OK - license meets Licensing Guidelines OK - License field meets actual license OK - upstream license file included in %doc OK - spec file uses American English OK - spec file is legible OK - sources match upstream sources OK - package builds successfully xx - ExcludeArch is needed This library has been tested only on %{ix86} x86_64 and arm (see README), and it has been specifically mentioned that it does not work on ppc (see ChangeLog). You MUST ExcludeArch atleast ia64, ppc and ppc64. Work is on to port Fedora to arm and sparc, so you might want to exclude them as well. Once the package is approved please file a bug each for ia64, ppc and ppc64, and make them block the following trackers respectively: https://bugzilla.redhat.com/show_bug.cgi?id=FE-ExcludeArch-ia64 https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=FE-ExcludeArch-ppc https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=FE-ExcludeArch-ppc64 Also add a comment in the Spec above the ExcludeArch line mentioning the bug numbers for ia64, ppc and ppc64. OK - build dependencies correctly listed OK - no locales OK - %post and %postun invoke ldconfig OK - package is not relocatable xx - file and directory ownership + The %files stanza for the -devel subpackage is broken. It should be fixed to have '%dir %{_includedir}/%{name}'. Otherwise the %{_includedir}/coredumper directory will not be owned by the package: [rishi@ginger SPECS]$ rpm -qf /usr/include/coredumper file /usr/include/coredumper is not owned by any package [rishi@ginger SPECS]$ + It might be better not to mention the compression suffix (ie. .gz) for the manual pages. OK - no duplicates in %file OK - file permissions set properly OK - %clean present OK - macros used consistently OK - contains code and permissable content OK - -doc is not needed OK - contents of %doc does not affect the runtime OK - header files in -devel OK - no static libraries OK - no pkgconfig files OK - library files without suffix in -devel OK - -devel requires base package OK - no libtool archives OK - %{name}.desktop file not needed OK - does not own files or directories owned by other packages OK - buildroot correctly prepped OK - all file names valid UTF-8 SHOULD Items: OK - upstream provides license text xx - no translations for description and summary OK - package builds in mock successfully OK - package builds on all supported architectures xx - package functions as expected + Only works on %{ix86}, x86_64 and arm. Other architectures should be, especially ia64, ppc and ppc64, should be excluded. OK - scriptlets are sane OK - subpackages other than -devel are not needed OK - no pkgconfig files OK - no file dependencies The file %{_includedir}/linux/dirent.h was dropped from kernel-headers on rawhide. So, package fails on koji. So, this package wouldn't build on rawhide. I have fixed other issues. SPEC: http://rakesh.fedorapeople.org/spec/coredumper.spec SRPM: http://rakesh.fedorapeople.org/srpm/coredumper-1.2.1-3.fc9.src.rpm For linux/dirent.h I will contact upstream. I have reported upstream with a patch. SPEC: http://rakesh.fedorapeople.org/spec/coredumper.spec SRPM: http://rakesh.fedorapeople.org/srpm/coredumper-1.2.1-4.fc9.src.rpm Build successfully: http://koji.fedoraproject.org/koji/taskinfo?taskID=785059 http://code.google.com/p/google-coredumper/source/detail?r=31 <-- Fixed upstream I have applied the same upstream patch to package(removing old patch). As soon as next version is released patch will be removed. Build successfully http://koji.fedoraproject.org/koji/taskinfo?taskID=797003 SPEC: http://rakesh.fedorapeople.org/spec/coredumper.spec SRPM: http://rakesh.fedorapeople.org/srpm/coredumper-1.2.1-5.fc9.src.rpm README also mentions 'src/coredump_unittest.c' so did you consider shipping this single file under %doc in the -devel subpackage? Also add alpha, arm, s390 and sparc to the list of excluded architectures. Everything else looks fine. Don't forget to file a bug for each of the excluded architectures and make them block on the individual architecture trackers. The Spec should have the bug numbers in a comment. +---------------------------------+ | This package is APPROVED by me. | +---------------------------------+ Thanks, I will incorporate all changes before moving on. In the meantime I have requested for cvs. New Package CVS Request ======================= Package Name: coredumper Short Description: Library to help applications create core dumps. Owners: rakesh Branches: F-8 F-9 InitialCC: rakesh Cvsextras Commits: yes cvs done. coredumper-1.2.1-6.fc8 has been submitted as an update for Fedora 8. http://admin.fedoraproject.org/updates/coredumper-1.2.1-6.fc8 coredumper-1.2.1-6.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/coredumper-1.2.1-6.fc9 coredumper-1.2.1-6.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. coredumper-1.2.1-6.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report. |