Bug 675914 - Review Request: flush - GTK-based BitTorrent client
Summary: Review Request: flush - GTK-based BitTorrent client
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-02-08 09:06 UTC by Oxana Kurysheva
Modified: 2011-03-25 19:05 UTC (History)
5 users (show)

Fixed In Version: flush-0.9.10-1.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-03-16 04:08:10 UTC
lemenkov: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Oxana Kurysheva 2011-02-08 09:06:25 UTC
Spec URL: http://www.ossportal.ru/sites/default/files/packages/flush.spec
SRPM URL: http://www.ossportal.ru/sites/default/files/packages/flush-0.9.9-1.fc14.src.rpm
Description: 
Flush - A GTK-based BitTorrent client.
Features:
 * Controlling running instance by command line interface.
 * Running many instances with different configs from the same user.
 * Automatic copying finished downloads to specified directory.
 * Setting custom download path for each file of the torrent.
 * Ability to choose torrent file's character set encoding.
 * Automatic torrents loading from specified directory.
 * Automatic pausing and removing old torrents.
 * Temporary pausing and resuming torrents.
 * Overall and current session statistics.
 * Creating your own torrent files.
 * IP filter.

Comment 1 Peter Lemenkov 2011-02-08 11:28:18 UTC
I'll review it

Comment 2 Peter Lemenkov 2011-02-08 11:40:24 UTC
Unblocking FE-NEEDSPONSOR - I just sponsored Oksana.

Comment 3 Peter Lemenkov 2011-02-08 11:44:00 UTC
Koji scratch build for Rawhide (still wip at the moment of writing this):

http://koji.fedoraproject.org/koji/taskinfo?taskID=2782701

Comment 4 Peter Lemenkov 2011-02-08 12:03:43 UTC
Unfotrunately it failed to build in Koji. Please, fix this issue.

There are other issues:

1) The directory %{_datadir}/%{name} is left unowned. Please fix the %files section in the following  way:

- %{_datadir}/%{name}/*
+ %{_datadir}/%{name}/

Note that I stripped trailing asterisk thus claiming ownership on the entire directory (not only on their contents).

2) The following change *should* be done with patch instead of sed. 

sed -i -e 's|strerror(e.system_error())|EE(e)|g' \
        src/create_torrent_dialog.cpp

3) Please, ensure that none of bundled libaries (dbus-c++, libconfig, libtorrent) are used during building. I suggest you to remove both of then in the %prep section.

Comment 5 Oxana Kurysheva 2011-02-09 17:10:20 UTC
Fixed everything.

There were some problems with new API of new libnotify in rawhide.
http://koji.fedoraproject.org/koji/taskinfo?taskID=2823012
Spec URL: http://www.ossportal.ru/sites/default/files/packages/flush.spec
SRPM URL:
http://www.ossportal.ru/sites/default/files/packages/flush-0.9.9-1.fc14.src.rpm

Comment 6 Oxana Kurysheva 2011-02-09 22:35:49 UTC
Sorry, not everything. I forgot about the third point. I will check deps tomorrow, they aren't good now. Don't check a package before this, please..

Comment 8 Peter Lemenkov 2011-02-11 11:51:53 UTC
The package still uses internal bundled copies of the following libraries:

* dbus-c++
* libconfig
* libtorrent

You should try to add --enable-system-libconfig and --enable-system-libtorrent to the %configure. Unfortunately, it seems that these commandline switches conflicts with --disable-bundle-package. Also there is no way to drop dependency on  dbus-c++ w/o changing configure.ac (and reconfiguring it).

Also there is one more issue - with updating icons cache. Take a look at the build logs:

...
*** Icon cache not updated. After install please run:
***   gtk-update-icon-cache -f -t /usr/share/icons/hicolor
...

this also must be fixed. Otherwise looks good.

Comment 9 Peter Lemenkov 2011-03-04 10:01:47 UTC
Ping

Comment 10 Peter Lemenkov 2011-03-09 09:10:02 UTC
FYI version 0.9.10 has been released.

Comment 11 Oxana Kurysheva 2011-03-09 09:58:05 UTC
Thanks, will try..

Comment 14 Peter Lemenkov 2011-03-11 09:19:18 UTC
Looks much better now!

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

- rpmlint is almost silent

work ~/Desktop: rpmlint flush-0.9.10-1.fc15.x86_64.rpm 
flush.x86_64: W: spelling-error %description -l en_US configs -> con figs, con-figs, configures

^^^ This one may be omitted.

flush.x86_64: W: file-not-utf8 /usr/share/man/ru/man1/flush.1.gz

^^^ This one should be converted from koi8-r to UTF-8 before installing.

1 packages and 0 specfiles checked; 0 errors, 2 warnings.
work ~/Desktop: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.

- The package doesn't meet the Packaging Guidelines fully (still there is an issue with icon cache). Take a look at this link for best practices with updating icon's cache:

http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

Just copypaste this example into your spec (replacing %post section entirely).

+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.

- The License field in the package spec file MUST match the actual license (GPLv3 or later). Thus the License tag must be set to GPLv3+.

+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, matches the upstream source, as provided in the spec URL.

work ~/Desktop: sha256sum flush-0.9.10.tar.bz2*
9c2605bb5c9e8daabfbe1a63fbceb1029bad3b679a3e023a6f2e73c2b8c16253  flush-0.9.10.tar.bz2
9c2605bb5c9e8daabfbe1a63fbceb1029bad3b679a3e023a6f2e73c2b8c16253  flush-0.9.10.tar.bz2.orig
work ~/Desktop:

+ The package successfully compiles and builds into binary rpms on at least one primary architecture. See koji link above
+ All build dependencies are listed in BuildRequires.
+ The spec file handles locales properly (by using the %find_lang macro).
0 No shared library files in some of the dynamic linker's default paths.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
0 The package DOESN'T have a %clean section, so it won't build cleanly on systems with old rpm (EL-4 and EL-5, not sure about EL-6). Beware.
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files without a suffix (e.g. libfoo.so).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
+ The package includes a %{name}.desktop file, and this file is properly installed with desktop-file-install in the %install section.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Ok, so the only remaining issues are:

* Convert russian man-page to UTF-8
* Change Licence tag to GPLv3+
* Properly update icon's cache

Comment 17 Peter Lemenkov 2011-03-11 11:36:50 UTC
Ok, good. I can't find any other issues, so this package is

APPROVED.

Comment 18 Oxana Kurysheva 2011-03-11 13:01:21 UTC
New Package CVS Request
=======================
Package Name: flush
Short Description: A GTK-based BitTorrent client.
Owners: avienda
Branches: F-13 F-14 F-15
InitialCC:

Comment 19 Jason Tibbitts 2011-03-11 14:38:32 UTC
Git done (by process-git-requests).

Comment 20 Fedora Update System 2011-03-11 20:25:48 UTC
flush-0.9.10-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/flush-0.9.10-1.fc14

Comment 21 Fedora Update System 2011-03-11 20:26:30 UTC
flush-0.9.10-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/flush-0.9.10-1.fc13

Comment 22 Fedora Update System 2011-03-11 20:27:03 UTC
flush-0.9.10-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/flush-0.9.10-1.fc15

Comment 23 Fedora Update System 2011-03-12 04:25:15 UTC
flush-0.9.10-1.fc15 has been pushed to the Fedora 15 testing repository.

Comment 24 Fedora Update System 2011-03-16 04:08:04 UTC
flush-0.9.10-1.fc15 has been pushed to the Fedora 15 stable repository.

Comment 25 Fedora Update System 2011-03-20 21:24:34 UTC
flush-0.9.10-1.fc13 has been pushed to the Fedora 13 stable repository.

Comment 26 Fedora Update System 2011-03-20 21:26:43 UTC
flush-0.9.10-1.fc14 has been pushed to the Fedora 14 stable repository.

Comment 27 Michael Schwendt 2011-03-23 10:32:21 UTC
It defaults to "[x] Hide main window to try at startup". It this normal? It is very confusing to start the app via "Applications > Internet > Flush" without any window getting displayed.

Comment 28 Oxana Kurysheva 2011-03-25 19:05:54 UTC
Thank you, Michael. Will try to solve this with upstream. There are some strange bugs with this option.


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