[Bug 251019] Review Request: lshw - Hardware lister

bugzilla at redhat.com bugzilla at redhat.com
Sat Aug 11 16:56:27 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 lyonel at ezix.org  2007-08-11 12:56 EST -------
(In reply to comment #8)
> (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}.

The problem is that the reported information will be highly inaccurate (and, on
certain platforms like x86, not only incomplete but *different* from what you
get as root) when lshw is run as normal user. Maybe I should add a warning in
the GUI (like what you get when running the CLI as non-root user). What do
others think?
 
> 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'  

I have added to lshw's SVN some drawings without logos and also "replacement"
logos for intel, amd, powerpc (cf.
http://dev.ezix.org/source/packages/artwork/nologo/ comments welcome).

>   - 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 ?

ppc is OK, that's my test/dev machine.
 
> ./ 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 ?

as discussed earlier (cf. duplicate BR), lshw makes use of *both* sources
(system's hwdata and bundled data files) so it always uses the most recent data.
 It'd be good to have Fedora's hwdata updated once a month but it's currently
very old and updated infrequently. I may add an option (in the GUI) to download
fresher files in the future and cache them in $XDG_CACHE_HOME/lshw (usually
$HOME/.cache/lshw).



-- 
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