Spec URL: http://kalev.fedorapeople.org/sparsehash.spec SRPM URL: http://kalev.fedorapeople.org/sparsehash-1.7-1.fc14.src.rpm Description: The Google SparseHash project contains several C++ template hash-map implementations with different performance characteristics, including an implementation that optimizes for space and one that optimizes for speed.
Two suggestions: 1. move all files too -devel subpackage 2. It'll better to leave this package as noarch, I think test in not very useful for development libs. You can add %check to packages which require sparsehash.
-Unsponsored reviewer- [makerpm@localhost SPECS]$ rpmlint -v sparsehash.spec sparsehash.spec: I: checking-url http://google-sparsehash.googlecode.com/files/sparsehash-1.7.tar.gz (timeout 10 seconds) 0 packages and 1 specfiles checked; 0 errors, 0 warnings. * Compiled and Built successfully on 2.6.33.5-124.fc13.x86_64 * No %find_lang macros (none needed) * All requirements met in https://fedoraproject.org/wiki/Packaging:ReviewGuidelines
(In reply to comment #1) > Two suggestions: > 1. move all files too -devel subpackage When I was creating the package, I couldn't make up my mind whether to put everything in -devel or not. If you think it's better that way, I'll move all files over to -devel. > 2. It'll better to leave this package as noarch, I think test in not very > useful for development libs. You can add %check to packages which require > sparsehash. I respectfully disagree here. Building the package as noarch means that it's built only once for every primary architecture we have. For F-12, it's only one build for i686, x86_64, ppc, and ppc64. That would mean not running self tests on 3/4 architectures. If we have a package which comes with self tests, we should try to run them if possible. You are saying these tests should be moved to packages which require sparsehash; I disagree. Every component should test itself if it is possible. And besides that, a large part of sparsehash package consists of self tests; if they aren't necessary why do you think upstream wrote them in the first place? The package is also less than 0.5 MB, which wouldn't be much of a saving on mirrors either.
* Sat Jul 03 2010 Kalev Lember <kalev> - 1.7-2 - Move all files to -devel (#609728) Spec URL: http://kalev.fedorapeople.org/sparsehash.spec SRPM URL: http://kalev.fedorapeople.org/sparsehash-1.7-2.fc14.src.rpm
YES - Package meets naming and packaging guidelines YES - Spec file matches base package name. YES - Spec has consistant macro usage. YES - Meets Packaging Guidelines. YES - License YES - License field in spec matches YES - License file included in package YES - Spec in American English YES - Spec is legible. YES - Sources match upstream md5sum: NA - Package needs ExcludeArch YES - BuildRequires correct NA - Spec handles locales/find_lang NA - Package is relocatable and has a reason to be. YES - Package has %defattr and permissions on files is good. YES - Package has a correct %clean section. YES - Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) YES - Package is code or permissible content. NA - Doc subpackage needed/used. YES - Packages %doc files don't affect runtime. YES - Headers/static libs in -devel subpackage. NA - Spec has needed ldconfig in post and postun NA - .pc files in -devel subpackage/requires pkgconfig NA - .so files in -devel subpackage. YES - -devel package Requires: %{name} = %{version}-%{release} NA - .la files are removed. NA - Package is a GUI app and has a .desktop file YES - Package compiles and builds on at least one arch. YES - Package has no duplicate files in %files. YES - Package doesn't own any directories other packages own. YES - Package owns all the directories it creates. NO - No rpmlint output.--> RPMLINT false W about invalid URL, verified with spectool - final provides and requires are sane: (include output of for i in *rpm; do echo $i; rpm -qp --provides $i; echo =; rpm -qp --requires $i; echo; done manually indented after checking each line. I also remove the rpmlib junk and anything provided by glibc.) [16:56:18][adam@turnip][result]+ for i in *rpm; do echo $i; rpm -qp --provides $i; echo =; rpm -qp --requires $i; echo; done sparsehash-1.7-2.fc13.src.rpm = rpmlib(FileDigests) <= 4.6.0-1 rpmlib(CompressedFileNames) <= 3.0.4-1 sparsehash-devel-1.7-2.fc13.x86_64.rpm sparsehash-devel = 1.7-2.fc13 sparsehash-devel(x86-64) = 1.7-2.fc13 = rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(PayloadIsXz) <= 5.2-1 SHOULD Items: YES - Should build in mock. YES - Should build on all supported archs NA - Should function as described. NA - Should have sane scriptlets. - Should have subpackages require base package with fully versioned depend. YES - Should have dist tag YES - Should package latest version - check for outstanding bugs on package. (For core merge reviews) No issues found. APPROVED.
Thanks for the quick review, Adam! New Package CVS Request ======================= Package Name: sparsehash Short Description: Extremely memory-efficient C++ hash_map implementation Owners: kalev Branches: F-13 InitialCC:
(In reply to comment #3) > (In reply to comment #1) > > Two suggestions: > > 1. move all files too -devel subpackage > > When I was creating the package, I couldn't make up my mind whether to put > everything in -devel or not. If you think it's better that way, I'll move all > files over to -devel. > It's a convension, see https://bugzilla.redhat.com/show_bug.cgi?id=609788 > > > 2. It'll better to leave this package as noarch, I think test in not very > > useful for development libs. You can add %check to packages which require > > sparsehash. > > I respectfully disagree here. > > Building the package as noarch means that it's built only once for every > primary architecture we have. For F-12, it's only one build for i686, x86_64, > ppc, and ppc64. That would mean not running self tests on 3/4 architectures. > > If we have a package which comes with self tests, we should try to run them if > possible. You are saying these tests should be moved to packages which require > sparsehash; I disagree. Every component should test itself if it is possible. > And besides that, a large part of sparsehash package consists of self tests; if > they aren't necessary why do you think upstream wrote them in the first place? > > The package is also less than 0.5 MB, which wouldn't be much of a saving on > mirrors either. You can add noarch to -devel subpackage, the test will still run on all arch.
s/conversion/convention
CVS done (by process-cvs-requests.py).
(In reply to comment #7) > You can add noarch to -devel subpackage, the test will still run on all arch. Oh, this is really clever: keep main package archful but mark the -devel subpackage (where all files end up in) as noarch. The end result is that the package build + tests run on each separate architecture, but we only get a single, noarch rpm for the mirrors. Thanks Chen. I've imported and built the updated package, closing the ticket.
sparsehash-1.7-3.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/sparsehash-1.7-3.fc13
Added missing 'assigned to'
sparsehash-1.7-3.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: sparsehash New Branches: el6 Owners: than
Git done (by process-git-requests).
Package Change Request ====================== Package Name: sparsehash New Branches: el5 Owners: besser82 Please create the el5-branch based onto the el6-branch. Many thanks!