[Bug 478504] Review Request: gget - Download Manager for the GNOME desktop.

Thu Jan 15 03:56:59 UTC 2009

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=478504

--- Comment #23 from Christoph Wickert <fedora at christoph-wickert.de>  2009-01-14 22:56:58 EDT ---
Although we are still waiting for the epipahny bug to be fixed, here is a 
preliminary review:

Review for 969acdd5a5ca3e849221489021fdc9f5  gget-0.0.4-5.fc10.src.rpm

FIX - MUST: $ rpmlint /var/lib/mock/fedora-rawhide-i386/result/gget-*
gget.i386: E: no-binary
gget.i386: W: conffile-without-noreplace-flag /etc/gconf/schemas/gget.schemas
gget-debuginfo.i386: E: empty-debuginfo-package
gget-epiphany-extension.i386: W: no-documentation
gget-epiphany-extension.i386: E: only-non-binary-in-usr-lib
4 packages and 0 specfiles checked; 3 errors, 2 warnings.

- First error is because of python and can be ignored. rpmlint expects python
packages to be noarch, but we need to make this one arch dependent because of
the epiphany extension.
- Warning about the gconf schema can be ignored, see comment # 9
- The debuginfo package is empty because of python. You need to prevent it from
being built by adding
  %define debug_package %{nil}
somewhere at the beginning of the spec. See 
https://fedoraproject.org/wiki/Packaging/Debuginfo for more info
- no doc warning for extension package can be ignored, doc is in the base
package
- the last error is also because of python and can be ignored.

OK - MUST: The package is named according to the Package Naming Guidelines.
OK - MUST: The spec file name matches the base package %{name}, in the format
%{name}.spec.
OK - MUST: The package meets the Packaging Guidelines (except for the issues
mentioned)
OK - MUST: The package is licensed with a Fedora approved license (GPLv2+) and
meets the Licensing Guidelines.
FIX - MUST: The License field in the package spec file does not match the
actual license: The license included is GPLv2, but if you take a look at the py
files you will see the typical "or any later version", so this is GPLv2+

FIX - MUST: The license file from the source package is not included in %doc.

OK - MUST: The spec file is in American English.
OK - MUST: The spec file for the package is legible.
OK - MUST: The sources used to build the package match the upstream source by
MD5 914d2d51186f6d24237409e59f8f9cde
OK - MUST: The package successfully compiles and builds into binary rpms on
i386
N/A - MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch.
OK - MUST: All build dependencies are listed in BuildRequires, but there are a
couple of redundant packages that don't need to be listed explicitly:  
  python is required by python-devel
  pygobject2-devel is required by pygtk2-devel

OK - MUST: The spec file handles locales properly with the %find_lang macro.
N/A - MUST: Every binary RPM package (or subpackage) which stores shared
library files (not just symlinks) in any of the dynamic linker's default paths,
must call ldconfig in %post and %postun.
N/A - MUST: If the package is designed to be relocatable, the packager must
state this fact in the request for review, along with the rationalization for
relocation of that specific package.
OK - MUST: The package owns all directories that it creates.
OK - MUST: The package does not contain any duplicate files in the %files
listing.
OK - MUST: Permissions on files are set properly. Every %files section includes
a %defattr(...) line.
OK - MUST: The package has a %clean section, which contains ( or
$RPM_BUILD_ROOT ).
OK - MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines.
OK - MUST: The package contains code.
N/A - MUST: Large documentation files should go in a -doc subpackage.
OK - MUST: Files included as %doc do not affect the runtime of the application.
N/A - MUST: Header files must be in a -devel package.
N/A - MUST: Static libraries must be in a -static package.
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires:
pkgconfig'.
N/A - MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), then library files that end in .so (without suffix) must go in
a -devel package.
N/A - MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency: Requires: %{name} =
%{version}-%{release}
OK - MUST: The package does not contain any .la libtool archives.
FIX - MUST: The package contains a GUI application and includes a
%{name}.desktop file, but that file is not properly installed with
desktop-file-install in the %install section. See
https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

FIX - MUST: The package does not own files or directories already owned by
other packages, but you need to wait until bug # 479921 is fixed and add
"%define epimajor 2.24", "Requires: epiphany(abi) = %{epimajor}" and 
"BuildRequires: epiphany-devel >= %{epimajor}" to the extension package again.
BTW: Instead of 
  %{_libdir}/epiphany/*/extensions/gget*
I suggest
  %{_libdir}/epiphany/*/extensions/gget.py*
to make sure you don't package unwanted files, but shouldn't really matter.

OK - MUST: At the beginning of %install, the package runs $RPM_BUILD_ROOT.
OK - MUST: All filenames in rpm packages are valid UTF-8.
N/A - SHOULD: If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to include it.
N/A - SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.
OK - SHOULD: The the package builds in mock.
OK - SHOULD: The package should compile and build into binary rpms on all
supported architectures.
OK - SHOULD: The package basically functions as described, but tends to crash
if the downloads.xml is malformed. Traceback mailed to upstream with Ant cc'ed.
OK - SHOULD: Scriptlets are sane, but remember the mimeinfo stuff
N/A - SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.
N/A - SHOULD: The placement of pkgconfig(.pc) files depends on their usecase,
and this is usually for development purposes, so should be placed in a -devel
pkg.
N/A - SHOULD: If the package has file dependencies outside of /etc, /bin,
/sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the
file instead of the file itself.

Other issues:
- Changelog is incomplete, there is a gap between 0.0.4-1 and 0.0.4-5. Please
fix it up with the specs from the pastebin.
- ChangeLog from source needs to be included in %doc
- "chmod +x ..." of the py files during install is not needed, remove it.
- There are some issues with the desktop file:
  - Categories includes "Application" which is no longer valid in latest
freedesktop.org specs. Use "--remove-category="Application" in
desktop-file-install
  - the desktop file contains a mimetype, so you have to run
update-mime-database, see
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#mimeinfo
- Add "Requires: dbus" because dbus-python and notify-python will only pull in
dbus-libs automatically
- Timestamp of Source0 does not match. Please download it again with wget or
alike, see
https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

Suggestions:
- Summary of the epiphany extension could be improved a little: "GGet extension
for the Epiphany web browser"
- lowercase some words in the descriptions: download manager, package. Only
names (such as Gnome or GGet) should be upper case.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.