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