After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 663682 - remove EmpathyTpFile
remove EmpathyTpFile
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: File Transfer
unspecified
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks:
 
 
Reported: 2011-11-09 11:03 UTC by Jonny Lamb
Modified: 2011-11-29 14:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libempathy: remove empathy-tp-file from headers (926 bytes, patch)
2011-11-10 11:43 UTC, Guillaume Desmottes
accepted-commit_now Details | Review
ft-handler: start using TpFTChannel instead of EmpathyTpFile (16.08 KB, patch)
2011-11-10 11:43 UTC, Guillaume Desmottes
reviewed Details | Review
client-factory: stop creating EmpathyTpFile objects (5.07 KB, patch)
2011-11-10 11:43 UTC, Guillaume Desmottes
accepted-commit_now Details | Review
tp-file: remove EmpathyTpFile (32.16 KB, patch)
2011-11-10 11:44 UTC, Guillaume Desmottes
accepted-commit_now Details | Review

Description Jonny Lamb 2011-11-09 11:03:24 UTC
http://cgit.freedesktop.org/~jonny/empathy/log/?h=ft

Here's a branch to do just that. Unfortunately it can't be merged yet as my tp-glib changes to add Provide/Accept implementations to TpFTChannel haven't been merged or released yet but I thought I'd just get on and do it. See https://bugs.freedesktop.org/show_bug.cgi?id=39188 for the tp-glib bug.

I tried to split up the patches as much as possible but some were bigger than I'd have hoped.
Comment 1 Guillaume Desmottes 2011-11-10 11:43:53 UTC
Created attachment 201142 [details] [review]
libempathy: remove empathy-tp-file from headers

This is just a little hack so GEnums aren't created for enums in
empathy-tp-file.h. We'll remove the files in a sec but I want to try
and avoid breaking bisect.

Signed-off-by: Jonny Lamb <jonny.lamb@collabora.co.uk>
Comment 2 Guillaume Desmottes 2011-11-10 11:43:57 UTC
Created attachment 201143 [details] [review]
ft-handler: start using TpFTChannel instead of EmpathyTpFile

Signed-off-by: Jonny Lamb <jonny.lamb@collabora.co.uk>
Comment 3 Guillaume Desmottes 2011-11-10 11:43:59 UTC
Created attachment 201144 [details] [review]
client-factory: stop creating EmpathyTpFile objects

TpAutomaticClientFactory will create TpFileTransferChannels for us and
they're, like, way better.

Signed-off-by: Jonny Lamb <jonny.lamb@collabora.co.uk>
Comment 4 Guillaume Desmottes 2011-11-10 11:44:02 UTC
Created attachment 201145 [details] [review]
tp-file: remove EmpathyTpFile

TpFTChannel!

Signed-off-by: Jonny Lamb <jonny.lamb@collabora.co.uk>
Comment 5 Guillaume Desmottes 2011-11-10 11:44:51 UTC
Review of attachment 201142 [details] [review]:

++
Comment 6 Guillaume Desmottes 2011-11-10 11:53:08 UTC
Review of attachment 201143 [details] [review]:

::: libempathy/empathy-ft-handler.c
@@ +266,3 @@
 
+  if (priv->channel != NULL) {
+    tp_channel_close_async (TP_CHANNEL (priv->channel), NULL, NULL);

channel_closed_cb in tp-file is doing some extra work to cancel the cancellable, if needed. Can we safely miss that?

@@ +1516,3 @@
     g_cancellable_cancel (priv->cancellable);
   else
+    tp_channel_close_async (TP_CHANNEL (priv->channel), NULL, NULL);

same question as above regarding the cancellable.
Comment 7 Guillaume Desmottes 2011-11-10 11:56:54 UTC
Review of attachment 201144 [details] [review]:

++
Comment 8 Guillaume Desmottes 2011-11-10 12:00:08 UTC
Review of attachment 201145 [details] [review]:

++ assuming "make check" passes.
Comment 9 Guillaume Desmottes 2011-11-10 12:01:52 UTC
For some value of "accepted-commit_now" meaning "commit once the tp-glib bits have been released and we depend on the right tp-glib version" :)
Comment 10 Jonny Lamb 2011-11-10 13:22:23 UTC
(In reply to comment #6)
> Review of attachment 201143 [details] [review]:
> 
> ::: libempathy/empathy-ft-handler.c
> @@ +266,3 @@
> 
> +  if (priv->channel != NULL) {
> +    tp_channel_close_async (TP_CHANNEL (priv->channel), NULL, NULL);
> 
> channel_closed_cb in tp-file is doing some extra work to cancel the
> cancellable, if needed. Can we safely miss that?
> 
> @@ +1516,3 @@
>      g_cancellable_cancel (priv->cancellable);
>    else
> +    tp_channel_close_async (TP_CHANNEL (priv->channel), NULL, NULL);
> 
> same question as above regarding the cancellable.

Yes, the cancellable is still present and will be cancelled when the channel is invalidated (or when the proxy is disposed).
Comment 11 Jonny Lamb 2011-11-10 13:22:40 UTC
(In reply to comment #9)
> For some value of "accepted-commit_now" meaning "commit once the tp-glib bits
> have been released and we depend on the right tp-glib version" :)

Groovy, thanks!
Comment 12 Guillaume Desmottes 2011-11-10 13:47:06 UTC
(In reply to comment #10)
> (In reply to comment #6)
> > Review of attachment 201143 [details] [review] [details]:
> > 
> > ::: libempathy/empathy-ft-handler.c
> > @@ +266,3 @@
> > 
> > +  if (priv->channel != NULL) {
> > +    tp_channel_close_async (TP_CHANNEL (priv->channel), NULL, NULL);
> > 
> > channel_closed_cb in tp-file is doing some extra work to cancel the
> > cancellable, if needed. Can we safely miss that?
> > 
> > @@ +1516,3 @@
> >      g_cancellable_cancel (priv->cancellable);
> >    else
> > +    tp_channel_close_async (TP_CHANNEL (priv->channel), NULL, NULL);
> > 
> > same question as above regarding the cancellable.
> 
> Yes, the cancellable is still present and will be cancelled when the channel > is invalidated (or when the proxy is disposed).

I see; works for me then!
Comment 13 Guillaume Desmottes 2011-11-23 15:41:37 UTC
Is this branch still blocked?
Comment 14 Guillaume Desmottes 2011-11-29 14:59:53 UTC
Merged to master /o/

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.