Bug 469590 - Review Request: cpuid - Dumps information about the CPU(s)
Review Request: cpuid - Dumps information about the CPU(s)
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dan Horák
Fedora Extras Quality Assurance
: FutureFeature
Depends On:
Blocks: FE-ExcludeArch-ppc64/F-ExcludeArch-ppc64 F-ExcludeArch-ppc
  Show dependency treegraph
 
Reported: 2008-11-02 18:12 EST by Fabian Affolter
Modified: 2014-09-29 07:56 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-11-22 11:51:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dan: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Fabian Affolter 2008-11-02 18:12:52 EST
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/cpuid.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/cpuid-20060917-1.fc9.src.rpm

Project URL: http://www.etallen.com/cpuid.html

Description:
cpuid dumps detailed information about the CPU(s) gathered from the CPUID
instruction, and also determines the exact model of CPU(s). It supports Intel,
AMD, and VIA CPUs, as well as older Transmeta, Cyrix, UMC, NexGen, and Rise
CPUs.

Koji scratch builds:
http://koji.fedoraproject.org/koji/taskinfo?taskID=915108

rpmlint output:
[fab@laptop024 i386]$ rpmlint -i cpuid*
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

[fab@laptop024 SRPMS]$ rpmlint -i cpuid*
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

'ExcludeArch: ppc ppc64' because cpuid is not able to detect those CPUs
Comment 1 Dan Horák 2008-11-03 04:42:57 EST
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
Comment 2 Fabian Affolter 2008-11-04 18:25:00 EST
(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
Comment 3 Dan Horák 2008-11-06 11:12:01 EST
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 ..."
Comment 4 Fabian Affolter 2008-11-08 16:09:22 EST
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
Comment 5 Dan Horák 2008-11-09 05:03:32 EST
(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.
Comment 6 Fabian Affolter 2008-11-09 06:28:31 EST
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
Comment 7 Dan Horák 2008-11-09 10:55:15 EST
All issues are fixed, so this package is APPROVED.
Comment 8 Fabian Affolter 2008-11-09 11:04:22 EST
Thank you for the review.
Comment 9 Fabian Affolter 2008-11-09 11:05:02 EST
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:
Comment 10 Kevin Fenzi 2008-11-10 11:43:48 EST
cvs done.
Comment 11 Fedora Update System 2008-11-10 14:19:34 EST
cpuid-20060917-4.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/cpuid-20060917-4.fc9
Comment 12 Fedora Update System 2008-11-10 14:19:37 EST
cpuid-20060917-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/cpuid-20060917-4.fc10
Comment 13 Fedora Update System 2008-11-11 21:52:49 EST
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
Comment 14 Dan Horák 2008-11-21 12:57:31 EST
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).
Comment 15 Fabian Affolter 2008-11-21 16:13:51 EST
Yes, I did. Thanks for the hint.
Comment 16 Fedora Update System 2008-11-22 11:47:00 EST
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
Comment 17 Fedora Update System 2008-11-22 11:51:44 EST
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.
Comment 18 Fedora Update System 2008-11-26 21:09:47 EST
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.
Comment 19 Fabian Affolter 2014-09-28 09:39:38 EDT
Package Change Request
======================
Package Name: cpuid
New Branches: epel7
Owners: fab
InitialCC:
Comment 20 Gwyn Ciesla 2014-09-29 07:56:37 EDT
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.