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(a)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.