Bug 206266 - Review Request: transmission - lightweight GTK+ BitTorrent client
Review Request: transmission - lightweight GTK+ BitTorrent client
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
: 497806 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-13 08:12 EDT by Denis Leroy
Modified: 2014-03-07 07:44 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-26 03:02:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Denis Leroy 2006-09-13 08:12:12 EDT
Spec URL: http://www.poolshark.org/src/transmission.spec
SRPM URL: http://www.poolshark.org/src/transmission-0.6.1-1.src.rpm

Description: 
Transmission is a free, lightweight BitTorrent client. It features a
simple, intuitive interface on top on an efficient, cross-platform
back-end.

Notes from packager:
- Transmission is primarily a MacOSX application but provides a nice simple GTK+ backend.
- It uses a custom simplistic build system that requires a patch to make install things at the expected locations
- configure needs a patch to fix an incorrect gtk+ version comparison (numerically 2.10 < 2.6 :-) )
Comment 1 Parag AN(पराग) 2006-09-13 08:59:33 EDT
{Not Official Reviewer}
packaging looks ok.
+ Mockbuild is successfull for i386 FC6 with some warnings 
+ rpmlint on binary rpm is silent
+ dist tag is present
+ Buildroot is correct
+ source URL is correct
+ BR is correct
+ License used is MIT
+ License file LICENSE is included
+ desktop file is handled correctly
+ MD5 sum on tarball is matching upstream tarball
1156a88c77ab71782b9261881ea13811  Transmission-0.6.1.tar.gz
+ No duplicate files

Application got installed correclty. Desktop icon is visible also.
Comment 2 Kevin Fenzi 2006-09-23 02:14:58 EDT
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (MIT)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
1156a88c77ab71782b9261881ea13811  Transmission-0.6.1.tar.gz
1156a88c77ab71782b9261881ea13811  Transmission-0.6.1.tar.gz.1
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Spec handles locales/find_lang
See below - Spec has needed ldconfig in post and postun
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package is a GUI app and has a .desktop file
OK - Package doesn't own any directories other packages own.
OK - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
OK - Should build in mock.

Issues:

1. You have:
Requires(post):   /sbin/ldconfig
Requires(postun): /sbin/ldconfig

But there are no libraries in this package, and you don't call
ldconfig in post/postun. Should remove those?

2. Your desktop-file-install and update-desktop-database
calls don't seem to quite match the guidelines on:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
and
http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

3. You shouldn't use
%makeinstall
if you can avoid it.
See:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-
fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

Can you use make DESTDIR=%{buildroot} install or make DESTDIR=$RPM_BUILD_ROOT 
install?
Comment 3 Denis Leroy 2006-09-25 07:25:54 EDT
Thanks for the review :-). Your points addressed below:

1. Fixed.

2. update-desktop-database calls fixed! The desktop-file-install looks good to
me. It doesn't have the --add-category Fedora option since the category is
already present in the desktop file (which is embedded into the spec). Many
packages embed the desktop file into the spec directly, while others check it
into CVS and add it as an extra Source: line. I'm not sure if one way is
preferred over the other...

3. The use of %makeinstall here is intentional. Transmission does not use
autoconf/automake and therefore does not honor DESTDIR, but rather uses a custom
makefile system. My understanding was that %makeinstall is targeted at cases
like this one, i.e. because it provides more variable definitions, more than
what is usually needed for autoconf/automake... ?

http://www.poolshark.org/src/transmission-0.6.1-2.src.rpm
http://www.poolshark.org/src/transmission.spec
Comment 4 Kevin Fenzi 2006-09-25 16:11:12 EDT
1. ok. Looks good. 

2. Looks fine. I can't see that it matters too much either way if the desktop 
is a source file or part of the spec. 

3. Well, %makeinstall should be avoided where possible for the reasons outlined 
above, but in this case it looks like you do need to use it. ;( 

All the blockers I see are fixed, this package is APPROVED. 
Don't forget to close this bug NEXTRELEASE once it's been imported and built. 
Comment 5 Denis Leroy 2006-09-26 03:02:09 EDT
Built. thanks for your review :-)
Comment 6 Simon 2009-04-27 07:51:46 EDT
*** Bug 497806 has been marked as a duplicate of this bug. ***
Comment 7 Adam Miller 2014-03-06 23:21:40 EST
Package Change Request
======================
Package Name: transmission
New Branches: epel7
Owners: maxamillion
Comment 8 Jon Ciesla 2014-03-07 07:44:26 EST
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.