Bug 469590
| Summary: | Review Request: cpuid - Dumps information about the CPU(s) | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Fabian Affolter <mail> |
| Component: | Package Review | Assignee: | Dan Horák <dan> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, notting |
| Target Milestone: | --- | Keywords: | FutureFeature |
| Target Release: | --- | Flags: | dan:
fedora-review+
gwync: fedora-cvs+ |
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Enhancement | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2008-11-22 16:51:47 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: | 238953, 179260 | ||
|
Description
Fabian Affolter
2008-11-02 23:12:52 UTC
few notes: - better will be to use "ExclusiveArch i386 x86_64" instead of ExcludeArch, because it will not work on Sparc/S390/ARM/... - the summary should reflect that it is useful only in x86 CPUs - I see that last release was in 2006, how it works with recent CPUs? - we already have x86info - no need to mark the man page as %doc, it is done automagically (http://www.rpm.org/max-rpm-snapshot/s1-rpm-inside-files-list-directives.html) - install the non-gzipped man page (as cpuid.1), it is compressed automagically during rpm build (In reply to comment #1) > few notes: > - better will be to use "ExclusiveArch i386 x86_64" instead of ExcludeArch, > because it will not work on Sparc/S390/ARM/... fixed > - the summary should reflect that it is useful only in x86 CPUs fixed > - I see that last release was in 2006, how it works with recent CPUs? I can't test the tool with recent CPUs because I only own older hardware. > - we already have x86info and 'cat /proc/cpuinfo' > - no need to mark the man page as %doc, it is done automagically > (http://www.rpm.org/max-rpm-snapshot/s1-rpm-inside-files-list-directives.html) fixed > - install the non-gzipped man page (as cpuid.1), it is compressed automagically during rpm build fixed New Spec: http://fab.fedorapeople.org/packages/SRPMS/cpuid.spec New SRPM: http://fab.fedorapeople.org/packages/SRPMS/cpuid-20060917-2.fc9.src.rpm formal review is here, see the notes below: OK source files match upstream: 0aac0d5104e45421ce3c48f0fe2d924d0d13caf1 cpuid-20060917.src.tar.gz OK package meets naming and versioning guidelines. OK specfile is properly named, is cleanly written and uses macros consistently. OK dist tag is present. OK build root is correct. BAD license field matches the actual license. OK license is open source-compatible. License text included in package. OK latest version is being packaged. OK BuildRequires are proper. BAD compiler flags are appropriate. OK %clean is present. OK package builds in mock (Rawhide/x86_64). OK debuginfo package looks complete. BAD rpmlint is silent. OK final provides and requires look sane. N/A %check is present and all tests pass. OK no shared libraries are added to the regular linker search paths. OK owns the directories it creates. OK doesn't own any directories it shouldn't. OK no duplicates in %files. OK file permissions are appropriate. OK no scriptlets present. OK code, not content. OK documentation is small, so no -docs subpackage is necessary. OK %docs are not necessary for the proper functioning of the package. OK no headers. OK no pkgconfig files. OK no libtool .la droppings. OK not a GUI app. - license should be MIT instead of BSD (http://fedoraproject.org/wiki/Licensing/MIT) - $RPM_OPT_FLAGS are not used to compile, could be solved with make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS -D_FILE_OFFSET_BITS=64 -DVERSION=$(VERSION)" - rpmlint complains cpuid.src:48: W: macro-in-%changelog doc => make it "... %%doc ..." Thanks Dan for helping me to bring this package into Fedora. (In reply to comment #3) > - license should be MIT instead of BSD > (http://fedoraproject.org/wiki/Licensing/MIT) fixed > - $RPM_OPT_FLAGS are not used to compile, could be solved with > make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS -D_FILE_OFFSET_BITS=64 > -DVERSION=$(VERSION)" fixed > - rpmlint complains > cpuid.src:48: W: macro-in-%changelog doc => make it "... %%doc ..." fixed Update: Spec URL: http://fab.fedorapeople.org/packages/SRPMS/cpuid.spec SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/cpuid-20060917-3.fc9.src.rpm (In reply to comment #4) > > > - $RPM_OPT_FLAGS are not used to compile, could be solved with > > make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS -D_FILE_OFFSET_BITS=64 > > -DVERSION=$(VERSION)" > Hm, the VERSION is now undefined during the compile. But after reading the Makefile, the proper solution would be to use "-DVERSION=%{version}" in the CFLAGS. So, please, one more iteration is required, all other issues have already been resolved. Thanks again (In reply to comment #5) > (In reply to comment #4) > > > > > - $RPM_OPT_FLAGS are not used to compile, could be solved with > > > make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS -D_FILE_OFFSET_BITS=64 > > > -DVERSION=$(VERSION)" > > > > Hm, the VERSION is now undefined during the compile. But after reading the > Makefile, the proper solution would be to use "-DVERSION=%{version}" in the > CFLAGS. So, please, one more iteration is required, all other issues have > already been resolved. New make: make %{?_smp_mflags} CFLAGS="%{optflags} -D_FILE_OFFSET_BITS=64 -DVERSION=%{version}" Update: Spec URL: http://fab.fedorapeople.org/packages/SRPMS/cpuid.spec SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/cpuid-20060917-4.fc9.src.rpm All issues are fixed, so this package is APPROVED. Thank you for the review. New Package CVS Request ======================= Package Name: cpuid Short Description: Dumps information about the CPU(s) Owners: fab Branches: F-9 F-10 EL-5 InitialCC: cvs done. cpuid-20060917-4.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/cpuid-20060917-4.fc9 cpuid-20060917-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/cpuid-20060917-4.fc10 cpuid-20060917-4.fc9 has been pushed to the Fedora 9 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update cpuid'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-9534 ping Didn't you forget to push the update to "stable"? New packages can go directly into stable and there is special type for such update (newpackage). Yes, I did. Thanks for the hint. cpuid-20060917-4.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update cpuid'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/f10/FEDORA-2008-9937 cpuid-20060917-4.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. cpuid-20060917-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. Package Change Request ====================== Package Name: cpuid New Branches: epel7 Owners: fab InitialCC: Git done (by process-git-requests). |