Bug 187818 - (ktorrent) Review Request: ktorrent : KDE bittorrent client
Review Request: ktorrent : KDE bittorrent client
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Package Reviews List
: Reopened
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-03 15:17 EDT by Roland Wolters
Modified: 2009-09-27 19:40 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-14 14:43:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Roland Wolters 2006-04-03 15:17:03 EDT
Spec Name or Url: http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent.spec
SRPM Name or Url: http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent-1.2-1.src.rpm
Description:

I finished packaging ktorrent and would appreciate a review so that it can find its way into fedora extras.

Ktorrent is a KDE-integrated GUI for bittorrent. It features, among other things, upnp support, udp trackers and speed capping. More information can be found at http://ktorrent.pwsp.net/ .
Comment 1 Rex Dieter 2006-04-03 15:45:38 EDT
Off the top of my head:

* use --vendor='' instead of --vendor="fedora", as the .desktop file already has
a vendor of 'kde'

* See "GTK+ icon cache" in
  http://fedoraproject.org/wiki/ScriptletSnippets

* If the .desktop file contains a MimeType= entry, see "desktop-database" on
same ScriptletSnippets page.

* See "Exclusion of Static Libraries" on
http://fedoraproject.org/wiki/Packaging/Guidelines regarding libtool archives. 
In short, you want to delete/omit
%{_libdir}/lib*.la

* The %{_libdir}/kde3/* bits should be in the main pkg, not -devel

* actually, since the only thing that's going to end up in -devel is
%{_libdir}/lib*.so, there's probably not much point in even making a -devel
subpkg.  Might as well fold everything into the main pkg.

* Not much point in including
%doc INSTALL
Since that's (usually) talks about compiling/building, and installing from
source, which isn't relevent here.
Comment 2 Roland Wolters 2006-04-04 18:19:58 EDT
Thank you very much for your detailed and very helpful comments, I corrected  
almost all mentioned problems, except one, which I think is the most  
important: the static libraries.  
  
Although I compiled with --disable-static and --enable-shared it still built  
the *.la and complained about the fact that these were installed but not  
packed.  
  
I try now to contact the ktorrent people in the hope that they can help me to  
correct their program. Is their any chance of correcting the problem through  
rpm like just not installing these files?  
Comment 3 Matthew Truch 2006-04-04 19:48:50 EDT
(In reply to comment #2)
> Although I compiled with --disable-static and --enable-shared it still built  
> the *.la and complained about the fact that these were installed but not  
> packed.  
>   
> I try now to contact the ktorrent people in the hope that they can help me to  
> correct their program. Is their any chance of correcting the problem through  
> rpm like just not installing these files?  


Actually, this is a known bug and affects many KDE apps.  Currently, you'll have
to include the .la files.  

From the fedora page: http://fedoraproject.org/wiki/Packaging/Guidelines under
the heading "Exclusion of Static Libraries" you'll see the comment: "[Comment
from mschwendt: It is not that easy, unfortunately, to kill libtool dependency
bloat this way. Some software needs libtool archives at run-time because it uses
an old libltdl to dlopen DSOs or uses a broken libltdl (like KDE bug #93359).]"

The KDE bug referenced is http://bugs.kde.org/show_bug.cgi?id=93359 for more info.  
Comment 4 Rex Dieter 2006-04-04 22:20:33 EDT
Roland, --disable-static doesn't disable libtool archives, notabug.  The
ktorrent devs won't (and shouldn't) care.

However, you still really should omit %{_libdir}/lib*.la by either doing and the
end of %install:
rm -f $RPM_BUILD_ROOT%{_libdir}/lib*.la

or in %files section:
%exclude %{_libdir}/lib*.la

libtool archives in other places (like %{_libdir}/kde3/*.la for loadable modules
or plugins) aren't harmful like those found at %{_libdir}/lib*.la that refer to
shared libraries.  So... they ought to be left alone.  Besides, as noted, many
apps (kde ones notably) actually *need* the .la files for the loadable
modules/plugins to properly function.
Comment 5 Roland Wolters 2006-04-05 06:55:09 EDT
Ok, I followed your advices, and excluded the files in the files section.   
I also moved the library file from development to the main package.   
FYI: the %{_libdir}/kde3/* files have to be in the main package since they are  
providing plugins, it was my error to pack them into the devel package. 
  
The new files are here:  
Spec: http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent.spec 
SRPM: 
http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent-1.2-2.src.rpm 
The old spec file is renamed to ktorrent.spec.release1 
 
One problem still left: when I now install/deinstall the package I get the 
error: 
> No theme index file in '/usr/share/apps/ktorrent/icons/hicolor'. 
> If you really want to create an icon cache here, use --ignore-theme-index. 
 
Any ideas? 
Comment 6 Rex Dieter 2006-04-05 06:59:35 EDT
Replace

touch --no-create %{_datadir}/apps/ktorrent/icons/hicolor || :
gtk-update-icon-cache --quiet %{_datadir}/apps/ktorrent/icons/hicolor || :

with

touch --no-create %{_datadir}/icons/hicolor || :
gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor 2> /dev/null || :

and you should be golden.
Comment 7 Rex Dieter 2006-04-05 09:43:47 EDT
This doesn't look right in %files:
%{_datadir}/applications/kde/ktorrent.desktop
%{_datadir}/applications/ktorrent.desktop
%{_datadir}/applnk/Internet/ktorrent.desktop
You only need *one* .desktop file.

Change:

/usr/bin/desktop-file-install --vendor=""			\
        --dir=${RPM_BUILD_ROOT}%{_datadir}/applications		\
        --add-category=X-Fedora					\
	%{buildroot}%{_datadir}/applications/kde/ktorrent.desktop
to

rm -f $RPM_BUILD_ROOT%{_datadir}/applnk/Internet/ktorrent.desktop
/usr/bin/desktop-file-install --vendor=""			\
        --dir=${RPM_BUILD_ROOT}%{_datadir}/applications/kde	\
        --add-category=X-Fedora					\
	%{buildroot}%{_datadir}/applications/kde/ktorrent.desktop
Comment 8 Roland Wolters 2006-04-05 11:54:21 EDT
Thanks again for the helpful comments, I made the requested changes, as usual  
the files can be found here:  
Spec: http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent.spec  
SRPM: 
http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent-1.2-3.src.rpm 
 
The old spec file has been changed to ktorrent.spec.release2 if you want to 
compare it. 
 
rpmlint had one warning since there is one development file in the main 
package (see also above): 
> W: ktorrent devel-file-in-non-devel-package /usr/lib/libktorrent.so 
 
Everything else is running without problems. 
Comment 9 Roland Wolters 2006-04-06 08:43:35 EDT
Just to be clear about this: I do NOT have a sponsorship at the moment and 
therefore I need someone to sponsor me so that the package can be accpeted. 
Comment 10 Hugo Cisneiros 2006-05-22 16:29:50 EDT
BTW, you should consider updating it to version 2.0beta1, as it contains some 
more neat features and is stable as well. This kde-apps page shows more info:

http://www.kde-apps.org/content/show.php?content=26353

Also, looking at KTorrent's Download Page:

http://ktorrent.pwsp.net/index.php?page=downloads

We can see some packages for FC5, that maybe could help in building the 
Extras-compliant specfile. Stay well! Thanks.
Comment 11 Roland Wolters 2006-05-23 10:38:10 EDT
The new version is still in beta, so I would prefer to wait until it is 
released as a final version because Fedora Core 5 is a final version.
It would be nice if there are some documents which point out when it is ok to 
update to beta versions.

About the FC5 packages on the homepage: these have been build by me. Where do 
you think do we still need help in compliance of the spec file? The above spec 
file was discussed and every problem has been solved as far as I see it.
Please be more specific about problems or areas where you see a lack of 
compliance.
Comment 12 Rex Dieter 2006-05-23 10:50:26 EDT

I'd say this is approve-worthy (or very close to it).  
Now, all you need is a sponsor.

p.s. 
Small item: you can safely omit
Requires(post): desktop-file-utils
Requires(postun): desktop-file-utils
IMO, the Scriptlets wiki is wrong to suggest including this.
I'll leave it as an exercise for the reader to determine what happens when/if
desktop-file-utils isn't installed when ktorrent is.  (:
Comment 13 Roland Wolters 2006-05-23 10:51:36 EDT
To add a note about the fc5-files: the ktorrent-1.2 files are done be me, I 
just saw that there are now also packages for ktorrent 2.0beta, these are not 
done by me.

I had a look into the spec files though, but I do not really see where these 
are superior by the ones I provided here:
- there is no hicolor handling
- the *.la files are installed
- no desktop-database handling because of mimetype in ktorrent.desktop
- the buildrequires are missing gmp-devel

Please be more specific about why you think that the spec files here provided 
shouldn't be good enough.
Comment 14 Hugo Cisneiros 2006-05-23 11:01:36 EDT
(In reply to comment #13)
> Please be more specific about why you think that the spec files here
> provided shouldn't be good enough.

Sorry, I didn't mean to say this. I was suggesting that you could update your 
specfile to ktorrent's 2.0beta version, not to replace your spec with the 
other :) But as you explained, let's wait for the final version to update. 
Thanks!
Comment 15 Jason Tibbitts 2006-06-03 00:36:06 EDT
Roland, did you get your account set up?  I recall from another open ticket that
John Mahowald was going to sponsor you, but I didn't see your request cross the
account system.
Comment 16 Roland Wolters 2006-06-06 19:06:48 EDT
My account role status is described as "in progress", the Sponsor field says 
"unneeded". I added myself to the cvsextras group already, but that didn't 
change anything in the groupname, this still says "cla_done".
According to http://www.fedoraproject.org/wiki/Extras/Contributors I have to 
wait and will get further information.

John Mahowald offered me in Bug 189263 (rsibreak) to sponsor me, and I applied 
to cvsextras as described - nothing happened. I can contact John in the 
mentioned bug report as well to point out that there is maybe an error or 
something.
Comment 17 Jason Tibbitts 2006-06-06 21:33:13 EDT
What is your ID in the Fedora account system?
Comment 18 Roland Wolters 2006-06-07 05:15:04 EDT
My username their is "liquidat", the full name is of course the same as here 
given, Roland Wolters. There are no other ID's mentioned on 
https://admin.fedora.redhat.com/accounts/userbox.cgi
Comment 19 Roland Wolters 2006-06-21 16:30:08 EDT
Thanks to John Mahowald I'm now sponsored and the packages are build:
http://buildsys.fedoraproject.org/build-status/job.psp?uid=11382
http://buildsys.fedoraproject.org/build-status/job.psp?uid=11383

Thanks everyone for the help and the very useful comments! As soon as ktorrent 
version 2.0 is out I will try to update the package.

Closed as nextrelease.
Comment 20 Hugo Cisneiros 2006-06-21 16:43:21 EDT
Great! Thanks for packaging this :-)
Comment 21 Jason Tibbitts 2006-06-22 10:31:04 EDT
If this is closed, why is is still blocking both FE-NEW and FE-NEEDSPONSOR? 
Blocking FE-ACCEPT instead.
Comment 22 Rex Dieter 2006-06-22 10:56:31 EDT
One problem here (correct me if I'm missing it), but I see no 
official "Reviewer" nor an official "APPROVED".  

Now my comment #12 
"I'd say this is approve-worthy (or very close to it)." may have been 
misconstrued as such, and if so, my apologies.
Comment 23 Roland Wolters 2006-06-22 12:52:52 EDT
Hm, then I misunderstood something: if I need a sponsor for each package than 
I missed it. The consequence is that the built package (listed above) is not 
allowed to be in fedora extras.

"The Fedora Extras package submission process requires that new package 
submitters be sponsored before they are granted the access necessary to check 
in their packages." http://www.fedoraproject.org/wiki/Extras/HowToGetSponsored
I understood this as there is only one sponsor needed.
Additionally, http://www.fedoraproject.org/wiki/Extras/Contributors gives the 
impression that the whole process is only necessary once.

Sorry for the problems I caused, how do I delete the at least not legal 
packages now in the extras and devel tree? Should I just remove both branches 
in the cvs or the complete package? Or is there manual action necessary to 
delete the package from the main mirror?
Comment 24 Rex Dieter 2006-06-22 13:08:12 EDT
So, you were sponsored, but you missed the review/APPROVED process.  Your
sponsor, John Mahowald, was/is supposed to that part.

You only need to be sponsored once, yes, and it is your sponsor who
needs-to(should?) be the one to officially review (and APPROVE) your first
submission.  See
http://www.fedoraproject.org/wiki/Extras/Contributors: "Get Sponsored"
"When the package is APPROVED by the reviewer, that person will then sponsor you

Now, once you're sponsored and have your first submission APPROVED, you are now
free to submit other packages for review.  These additional packages can be
reviewed by *anyone*, but these submissions still need to be APPROVED by a
reviewer before they can be imported/built for Extras.

Does that help?  If you have more questions regarding the process, direct them
to your Sponsor (that's what Sponsors are for)... (:

Comment 25 Rex Dieter 2006-06-22 13:10:09 EDT
It occured to me that there's a chance that ktorrent *was* reviewed/APPROVED by
John, but that it may have been lost in the recent bugzilla crash/dataloss that
occurred recently.

If that is the case, please ignore my rambling.
Comment 26 Jason Tibbitts 2006-06-22 14:03:38 EDT
I checked back through my archive and indeed I don't see how this was ever even
reviewed.  I will change the blocker back to FE-NEW and reopen.

Comment 27 Roland Wolters 2006-06-22 14:10:27 EDT
The problem for me was that my second submission, rsibreak, was approved and 
sponsored - together with the feeling that this ktorrent submission is also 
approved (although, as it turned now out that wasn't so) I submitted it to 
build.

However, now we have one unapproved package which is in extras, how can I 
delete it? Is it deleted automatically if I delete the package in cvs?
And, again: where do I find information about approving of packages? If only 
my sponsor is the one who is allowed to do that how can I make sure that he 
sees this submission?
Comment 28 Jason Tibbitts 2006-06-22 14:28:27 EDT
I will ask an admin to delete the package.

When you are not sponsored, a sponsor must do your first review.  Once that's
done, you are sponsored and FE-NEEDSPONSOR will be removed from all of your
package review tickets.  At that point and from then on, any of the fedora
extras package owners (besides yourself) can review and approve your packages.
Anyone at all can comment on your packages, of course.

You must not check in a package until it has been approved.  You will know this
when a reviewer indicates that the package has been "APPROVED" and the blocker
is moved to FE-ACCEPT.  If this is not done, the package is not approved and
must not be checked in.

The process is listed step-by-step in
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Comment 29 Rex Dieter 2006-06-22 14:31:32 EDT
Roland,  I'll review this one (what's currently in cvs), and hopefully we can
get this mess sorted out quickly.
Comment 30 Rex Dieter 2006-06-22 14:40:38 EDT
NEEDSWORK:
* %{_datadir}/apps/ktorrent unowned
* including pkg name, "KTorrent", in Summary is redundant.

SUGGESTED (feel free to ignore, my personal opinion only)
* drop Requires: desktop-file-utils, not *really* needed.

Either post an updated specfile or update cvs with (at least) NEEDSWORK items
addressed, and I'll mark it APPROVED.
Comment 31 Roland Wolters 2006-06-22 14:52:27 EDT
Jason, I *am* sponsored. But as you can see, FE-NEEDSPONSOR was not removed 
from this package. But it was removed from the package where I was sponsored 
and which was APPROVED by the sponsor, rsibreak; see this bug: #189263.

About the linked step-by-step guideline - I read that before to create this 
bug report, but then used 
http://www.fedoraproject.org/wiki/Extras/Contributors to go through the whole 
process of uploading and the stuff. The other guide is linked there but I 
missed it somehow.
Thanks for deleting and taking care of the problem.

Rex, thanks for the comments, I will try to correct that asap.
Comment 32 Roland Wolters 2006-06-23 10:39:10 EDT
I removed the redundant ktorrent name, and added %{_datadir}/apps/ktorrent to 
the %files section to sort out the problem of missing ownership. Please 
comment on that.

About desktop-file-util - I'm not completly sure what all happens and what not 
if I drop this. Before I do not understand it 100% I keep myself strict to the 
guidelines, that's safer for me.

The new files:
Spec Url: http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent.spec
SRPM Url: 
http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent-1.2-5.src.rpm
Comment 33 Rex Dieter 2006-06-23 10:52:38 EDT
Now, you'll get duplicates... (only a warning, but still), and still have some
unowned (icon) dirs under %_datadir/apps/ktorrent.  I'd suggest simplifying to a
single line in %files

%{_datadir}/apps/ktorrent/

which will own the dir and (recursively) everything under it, or use

%dir %{_datadir}/apps/ktorrent/
%{_datadir}/apps/ktorrent/*

to explictly name/own the parent dir, and then use the wildcard to get
everything under it.
Comment 34 Roland Wolters 2006-06-23 11:49:00 EDT
Thanks for the fast comment - I changed the spec file according to your first 
suggestion. However, I checked the build log again, and found some suspicious 
things:

cpio: ktorrent-1.2/apps/ktcachecheck/<built-in>: No such file or directory
cpio: ktorrent-1.2/apps/ktorrent/<built-in>: No such file or directory
cpio: ktorrent-1.2/apps/kttorinfo/<built-in>: No such file or directory
cpio: ktorrent-1.2/libktorrent/<built-in>: No such file or directory
cpio: ktorrent-1.2/libktorrent/interfaces/<built-in>: No such file or 
directory
cpio: ktorrent-1.2/libktorrent/kademlia/<built-in>: No such file or directory
cpio: ktorrent-1.2/libktorrent/migrate/<built-in>: No such file or directory
cpio: ktorrent-1.2/libktorrent/torrent/<built-in>: No such file or directory
cpio: ktorrent-1.2/libktorrent/util/<built-in>: No such file or directory
cpio: ktorrent-1.2/plugins/infowidget/<built-in>: No such file or directory
cpio: ktorrent-1.2/plugins/ipfilter/<built-in>: No such file or directory
cpio: ktorrent-1.2/plugins/logviewer/<built-in>: No such file or directory
cpio: ktorrent-1.2/plugins/partfileimport/<built-in>: No such file or 
directory
cpio: ktorrent-1.2/plugins/search/<built-in>: No such file or directory
cpio: ktorrent-1.2/plugins/upnp/<built-in>: No such file or directory

I'm not sure how to deal with this.

Also a huge amount of hints like this appeared:
file_contexts:  invalid context system_u:object_r:bin_t (instead of bin_t also 
ending on shlib_t, locale_t, usr_t, src_t

Selinux is disabled for this machine at the moment, could that be the reason?
Comment 35 Rex Dieter 2006-06-23 13:04:21 EDT
Dunno much about selinux sorry.

I am pretty sure the ".../<built-in>: No such file or directory
warnings are harmless.  It's just verbosity from rpm's creating of the
-debuginfo pkg(s).

-- Rex
Comment 36 Jason Tibbitts 2006-06-23 14:37:19 EDT
The selinux stuff is a generic bug in libselinux (or one of the selinux
packages, at least) which will show up pretty much any time you build a package.
 It is fixed in the next version.  The messages are not caused by any problem in
this package and you're free to ignore them.
Comment 37 Roland Wolters 2006-06-23 19:06:38 EDT
Jason, thanks for the comment, I was worried because there were quite many of 
these lines.

Rex, I installed the debuginfo package on my system and did not get any error 
message so that package looks fine...
Comment 38 Rex Dieter 2006-07-27 07:19:36 EDT
Waiting to see a final update (per comment #32 and comment #33), then we can 
probably approve this.
Comment 39 Roland Wolters 2006-07-27 09:27:06 EDT
The actual version of the spec file was changed according to comment #33.

The only errors I see are the rpmlint error because I do not create a devel 
package (see above).
Comment 40 Rex Dieter 2006-07-27 10:24:04 EDT
OK, looks good then, APPROVED/FE-ACCEPT.
Comment 41 Roland Wolters 2006-08-08 23:02:35 EDT
There is one problem now: before I was able to upload the ktorrent to cvs and 
start packaging, a new major version was released. It is a jump from version 
1.2 to 2.0 with new features:
- Support for distributed hash tables (mainline version)
- Protocol encryption
- Bandwith scheduling
- Directory scanner to automatically load torrents in certain directories
- Trackers can now be added to torrents
- File prioritization for multi file torrents

The old spec file works when I modify the file list and the version number as 
well. However, the installation of the 2.0 rpm has a file conflict with 
kdelibs, and I added
%exclude %{_datadir}/mimelnk/application/x-bittorrent.desktop
in the spec file.

The new spec file and SRPM can be found here:
Spec: http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent.spec  
SRPM: 
http://www.personal.uni-jena.de/~p1woro/fedorarpms/ktorrent-2.0-1.src.rpm

Is the package in this form still approved/accepted?
Comment 42 Rex Dieter 2006-08-08 23:16:26 EDT
Sure, (still) approved, though I don't see any 
%exclude %{_datadir}/mimelnk/application/x-bittorrent.desktop
in the referenced specfile.  (:
Comment 43 Roland Wolters 2006-08-10 08:05:34 EDT
Thanks for your hint, you're right. I somehow copied the old version instead 
of the new one. Please give me short feedback about the newest version and the 
exclude line, after that I will import ktorrent 2.0 to the cvs.
Comment 44 Rex Dieter 2006-08-10 10:09:11 EDT
(Still) looks good, good ahead and import.
Comment 45 Roland Wolters 2006-08-14 14:43:16 EDT
Had to add a build require, gmp-devel, but now it builds in devel, I will ask 
for a FC5 branch also.

Closed as NEXTRELEASE according to 
http://www.fedoraproject.org/wiki/Extras/Contributors
Comment 46 Roland Wolters 2007-03-26 17:48:57 EDT
Package Change Request
======================
Package Name: ktorrent
Updated Fedora CC: Rex Dieter (rdieter@math.unl.edu)

Please make him co-maintainer.

This review is used for this procedure because it was stated to do so: 
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
Comment 47 Rex Dieter 2008-10-29 10:34:42 EDT
Package Change Request
======================
Package Name: ktorrent
New Branches: EL-5
Owners: rdieter
Comment 48 Kevin Fenzi 2008-10-29 17:38:15 EDT
cvs done.

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