[Bug 251019] Review Request: lshw - Hardware lister

bugzilla at redhat.com bugzilla at redhat.com
Sat Aug 11 14:15:13 UTC 2007


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lshw - Hardware lister


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=251019





------- Additional Comments From dtimms at iinet.net.au  2007-08-11 10:15 EST -------
(In reply to comment #7)
> After talking with spot I have removed some logos, package should be safe now.
> 
> lshw-gui seems to work ok without the logos.
> 
> SPEC: http://terjeros.fedorapeople.org/lshw/lshw.spec
> SRPM: http://terjeros.fedorapeople.org/lshw/lshw-B.02.11.01-2.fc7.src.rpm

In terms of the requiring admin privileges to run: lshw can run as a normal user
- if so, only hardware information available as a normal user will be shown.
More information is show if run as root {or through consolehelper}.

Pre-review of lshw-B.02.11.01-2.fc7.src.rpm:
===== key:
.x = Not ok
.? = Either I don't understand enough to confirm that the element is acceptable,
or I don't understand what the spec is doing. I would request some comment to
explain if you believe that the item is OK.
./ = tick - OK. No work required. Usually a comment was included after: to
describe what I found.
=====
MUST Items:
./ rpmlint result: no warnings nor errors.
./ named according to the Package Naming Guidelines: matches upstream project
and source download name.
./ spec file name matches the base package: lshw.spec lshw 
.? package must meet the Packaging Guidelines.
./ package must be licensed with an open-source compatible license:
  - web site indicates GPL and upstream source includes GPLv2.
./ License field in the package spec file must match the actual license:
  - GPLv2 as required by new licensing guideline
./ source package includes the text of the license, so text of the license(s)
for the package must be included in %doc:
  - COPYING is included as required.
  - 6* Trademark svg graphics are included in the source tarball, but the spec
removes them. Discussion on fedora-devel and with spot lead to this requirement.
  - build log shows successful removal:
+ for f in powermacg5 intel powermac amd mini powerpc
+ rm -fv
/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/powermacg5.svg
removed
`/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/powermacg5.svg'
+ for f in powermacg5 intel powermac amd mini powerpc
+ rm -fv
/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/intel.svg
removed
`/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/intel.svg'
+ for f in powermacg5 intel powermac amd mini powerpc
+ rm -fv
/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/powermac.svg
removed
`/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/powermac.svg'
+ for f in powermacg5 intel powermac amd mini powerpc
+ rm -fv
/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/amd.svg
removed
`/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/amd.svg'
+ for f in powermacg5 intel powermac amd mini powerpc
+ rm -fv
/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/mini.svg
removed
`/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/mini.svg'
+ for f in powermacg5 intel powermac amd mini powerpc
+ rm -fv
/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/powerpc.svg
removed
`/var/tmp/lshw-B.02.11.01-2.fc7-root-mockbuild/usr/share/lshw/artwork/powerpc.svg'  

  - emac.svg {artistic view of apple logo} is still included - is it OK to
include in Fedora ?
  
./ The spec file must be written in American English.
./ spec file for the package must be legible:
./ source in .src.rpm matches upstream md5sum:
  - $ md5sum lshw-B.02.11.01.tar.gz
23debbc3c0a719f301861cfc079b3f4b  lshw-B.02.11.01.tar.gz  {web source}
23debbc3c0a719f301861cfc079b3f4b  /usr/src/redhat/SOURCES/lshw-B.02.11.01.tar.gz

./ successfully compiles and builds into binary rpms: i386 {athlon}
.? If the package does not successfully compile, build or work on an
architecture - :
  - I tried only on i386{i686/athlon}
  - no excludearchs listed.
  - have you tested on x86_64 or mac ?

./ build dependencies must be listed in BuildRequires:
  - no listed BR is in the auto included list
  - the package built on my system
  - package built in mock
  - built package runs OK, works as expected
./ spec file MUST handle locales properly:
  - neither find_lang macro nor %{_datadir}/locale are used.
./ has no shared library files: 
  - contains only standalone executables with external text lookup files
./ not relocatable and does not use Prefix: /usr
./ package must own all directories that it creates:
  - 
./ A package must not contain any duplicate files in the %files listing:
  - does not appear to.
./ Permissions on files must be set properly. 
  - Executables are set with executable permissions
  - other files aren't
  - %files section includes a %defattr(...) line.
.x must have a %clean section, containing rm -rf %{buildroot} (or $RPM_BUILD_ROOT):
  - %{__rm} -rf %{buildroot} differs from this requirement.
  
.? Each package must consistently use macros:
  - no new macros are defined.
  - %{x} are actually %{__X}, again, why ?
  
./ The package must contain code, or permissable content.
  - contains a GUI app
./ Large documentation files:
  - no. total doc is 43kB
? - COPYING is included in both lshw and lshw-gui. However, lshw-gui Requires 
lshw, so the COPYING is guaranteed to be installed already. I don't know if
that is normal, or whether it could / should be removed.

./ %doc files must not affect the runtime of the application:
  - OK.
./ Header files must be in a -devel package:
  - no header files.
./ Static libraries must be in a -static package:
  - no static libraries.
./ has no pkgconfig(.pc) files.
./ library files with a suffix: no libraries
./ devel packages must require the base package: 
  - no -devel package
./ Packages must NOT contain any .la libtool archives
  - no .la's
./ Packages containing GUI applications must include a %{name}.desktop file, and
that file must be properly installed with desktop-file-install
  - done and the menu entry works.
./ Packages must not own files or directories already owned by other packages. 
.x At the beginning of %install, each package MUST run rm -rf %{buildroot} (or
$RPM_BUILD_ROOT):
  - the command is: %{__rm} -rf %{buildroot}, which is not standard, why ?
  
./ All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
./ If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it: included.
./ The description and summary sections in the package spec file should contain
translations for supported Non-English languages:
  - no other translations available 
.?  The package should compile and build into binary rpms on all supported
architectures:
  - only tested on i386 {athlon}  
  
./ package functions as described
  - both GUI and CLI provide a lot of hw information as the summary describes
./  If scriptlets are used, those scriptlets must be sane: 
  - none used.
./  subpackages other than devel should require the base package using a
fully versioned dependency.
./ no pkgconfig(.pc) files.
./ no file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin.
=====
Personally, I think console helper should be saying ~"this program performs
better when run with root privileges", and give the normal user a chance to ~run
 unprivileged". This makes the app at least somewhat useful if you do not have
root rights on the system. This might also means not placing in /usr/sbin ?

I also suggest: that even though lshw includes a copy of hw data, that any such
files included by fedora hwdata should not be packaged. This has the advantages of:
- removes any confusion / discrepancy that would otherwise occur if the result
of other fedora hardware tools is compared to lshw that included different
hwdata files.
- shouldn't require updates to lshw when only hw data has been updated.

disadvantages:
- fedora's hwdata does not seem to be updated very often - but perhaps it should
get updated say once a month or so to take into account newly released hardware.
- if lshw is updated {upstream updates all hw data before a release}, the fedora
lshw would not be able to immediately use new data. This is countered by the
fact that a package should be essentially static during a distribution's release
life. If the main change is to hw data - then only a new hwdata should be released.

What do others think ?

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.




More information about the package-review mailing list