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@xxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review