[Bug 523540] Review Request: opentracker - BitTorrent Tracker

Sat May 22 19:04:34 UTC 2010

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=523540

--- Comment #61 from Dominic Hopf <dmaphy at fedoraproject.org> 2010-05-22 15:04:20 EDT ---
Guys, thanks very much for your efforts. Here's another try of a formal review.
There are at least two more small issues which need to be fixed (scroll down
for those) before I will approve the package.

$ rpmlint opentracker.spec
opentracker.spec: W: invalid-url Source0: opentracker-0.tar.bz2
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

This warning can be ignored, since opentracker is rolling release.

$ rpmlint opentracker-0-0.12.20100410cvs.fc12.src.rpm
opentracker.src: W: invalid-url Source0: opentracker-0.tar.bz2
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

See above.

$ rpmlint opentracker-debuginfo-0-0.12.20100410cvs.fc12.x86_64.rpm \
          opentracker-ipv4-0-0.12.20100410cvs.fc12.x86_64.rpm \
          opentracker-ipv6-0-0.12.20100410cvs.fc12.x86_64.rpm \
          opentracker-rootdir-0-0.12.20100410cvs.fc12.noarch.rpm

opentracker-ipv4.x86_64: W: spelling-error Summary(en_US) ipv -> iv, IPA, iPod
opentracker-ipv4.x86_64: W: no-manual-page-for-binary opentracker-ipv4
opentracker-ipv6.x86_64: W: spelling-error Summary(en_US) ipv -> iv, IPA, iPod
opentracker-ipv6.x86_64: W: no-manual-page-for-binary opentracker-ipv6
opentracker-rootdir.noarch: W: spelling-error Summary(en_US) dir -> deer, rid,
Dir
opentracker-rootdir.noarch: W: spelling-error %description -l en_US Filesystem
-> File system, File-system, Systematic
opentracker-rootdir.noarch: W: spelling-error %description -l en_US chroot ->
cheroot, ch root, ch-root
opentracker-rootdir.noarch: W: no-documentation
opentracker-rootdir.noarch: W: non-standard-gid /var/opentracker opentracker
4 packages and 0 specfiles checked; 0 errors, 9 warnings.

Manpages should be provided by upstream. Spelling issues can be ignored,
I think the wording is okay. The other rootdir package warnings can also be
ignored.

Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines
 [x] Specfile name matches %{name}.spec
 [x] Package seems to meet Packaging Guidelines
 [x] Package successfully compiles and builds into binary RPMs on at least one
     supported architecture.
     Tested on: Fedora 12/x86_64
 [x] Rpmlint output:
     source RPM: see above
     binary RPM: see above
 [x] Package is not relocatable.
 [x] License in specfile matches actual License and meets Licensing Guidelines
     License: Beerware
 [-] License file is included in %doc.
 [x] Specfile is legible and written in AE
 [x] Sourcefile in the Package is the same as provided in the mentioned Source
     diff and meld do not show any differences between an own checkout and
     provided tarball
 [x] Package compiles successfully
 [x] All build dependencies are listed in BuildRequires
 [-] Specfile handles locales properly
 [-] ldconfig called in %post and %postun if required
 [x] Package owns directorys it creates
 [-] Package requires other packages for directories it uses.
 [x] Package does not list a file more than once in the %files listing
 [x] %files section includes %defattr and permissions are set properly
 [x] %clean section is there and contains rm -rf %{buildroot}
 [x] Macros are consistently used
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage
 [x] Program runs properly without files listed in %doc
 [-] Header files are in a -devel package
 [-] Static libraries are in a -static package
 [-] Package requires pkgconfig if .pc files are present
 [-] .so-files are put into a -devel subpackage
 [-] Subpackages include fully versioned dependency for the base package
 [-] Any libtool archives (*.la) are removed
 [-] contains desktop file (%{name}.desktop) if it is a GUI application
 [x] Package does not own files or directories owned by other packages.
 [x] %{buildroot} is removed at beginning of %install
 [-] Filenames are encoded in UTF-8

=== SUGGESTED ITEMS ===
 [-] Package contains latest upstream version
 [x] Package does not include license text files separate from upstream.
 [-] non-English translations for description and summary
 [x] Package builds in mock
     Tested on: F12/x86_64
 [x] Package should compile and build into binary RPMs on all supported
architectures.
     tested build with koji
 [!] Program runs
  The opentracker-ipv6 service does not start. See below.

 [-] Scriptlets must be sane, if used.
 [-] pkgconfig (*.pc) files are placed in a -devel package
 [-] require package providing a file instead of the file itself
     no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required

=== Issues to point out ===
==== 1 ====
The user change for the directory /var/%{name} doesn't seem to work as
intended.
The user opentracker is also needed for the opentracker-rootdir subpackage, but
you create it just with the %pre ipv4 and %pre ipv6 commands. This results in a
warning when installing the package with yum:

 Running Transaction
  Installing     : opentracker-rootdir-0-0.12.20100410cvs.fc12.noarch          
                                                               1/4
 warning: group opentracker does not exist - using root

/var/opentracker is not owned by the user opentracker then, but by the user
root.

Since the user creation is the same for both subpackages, I suggest to create
the user once, when installing the rootdir subpackage (since it is
installed as the first package).

==== 2 ====
I get an error when trying to start the opentracker-ipv6 service:
# LANG=C service opentracker-ipv6 start
opentracker-ipv6 starten: /etc/init.d/opentracker-ipv6: Zeile 36: -f: Kommando
nicht gefunden.
                                                           [FEHLGESCHLAGEN]

I can not do any deeper testing for IPv6, unfortunately.

==== 3 ====
I think there is no need for two different configuration directories (when the
ipv4 and ipv6 package is installed) in /etc/. It should be enough to have one
directory (/etc/opentracker/) with two different files (opentracker-ipv4.conf,
opentracker-ipv6.conf) in it. However, this one shall not block the review.

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