Bug 206266

Summary: Review Request: transmission - lightweight GTK+ BitTorrent client
Product: [Fedora] Fedora Reporter: Denis Leroy <denis>
Component: Package ReviewAssignee: Kevin Fenzi <kevin>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: admiller, b.bellec, e.bulle, panemade
Target Milestone: ---Flags: gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-09-26 07:02:09 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    

Description Denis Leroy 2006-09-13 12:12:12 UTC
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 12:59:33 UTC
{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 06:14:58 UTC
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 11:25:54 UTC
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 20:11:12 UTC
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 07:02:09 UTC
Built. thanks for your review :-)


Comment 6 Simon 2009-04-27 11:51:46 UTC
*** Bug 497806 has been marked as a duplicate of this bug. ***

Comment 7 Adam Miller 2014-03-07 04:21:40 UTC
Package Change Request
======================
Package Name: transmission
New Branches: epel7
Owners: maxamillion

Comment 8 Gwyn Ciesla 2014-03-07 12:44:26 UTC
Git done (by process-git-requests).