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@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).