| Summary: | test-file-transfer-channel fails with glib 2.39 | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Sebastien Bacher <seb128> |
| Component: | tp-glib | Assignee: | Telepathy bugs list <telepathy-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | xclaesse |
| Version: | unspecified | Keywords: | patch |
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=fix-ft | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | patch | ||
|
Description
Sebastien Bacher
2013-12-04 17:02:05 UTC
Created attachment 90258 [details] [review] patch There is an ambiguity of when _accept_file_async() and _provide_file_async() are supposed to complete. The unit tests assert that in completes when the state moved to TP_FILE_TRANSFER_STATE_PENDING which is before the actual streaming of the file happens, and that's what actually happens on the success code path. However on the failure code path, TpFileTransferChannel assume the operation is still ongoing until the splice operation ends so that would mean that _accept/provide_file_async() covers the whole file transfer process and returns when the state moved to TP_FILE_TRANSFER_STATE_COMPLETED. Made a more complete fix: http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=fix-ft (In reply to comment #3) > Made a more complete fix: > http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=fix-ft You mentioned twice "tp_file_transfer_channel_accept_file_async" in the commit message. Ignoring streaming errors seem pretty nasty. :\ (In reply to comment #4) > (In reply to comment #3) > > Made a more complete fix: > > http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=fix-ft > > You mentioned twice "tp_file_transfer_channel_accept_file_async" in the > commit message. Fixed, and fixed the "steaming errors" typo Simon saw as well. > Ignoring streaming errors seem pretty nasty. :\ Better ignoring errors, than crashing on error as it always did. I follow-up branch should add an "error" signal (both in C API, and in spec IMHO), and a cancel method. OTOH, I think we should just give up on that over-engineered interface, and just give the file URI to the CM and let it do all the work. The current iface cannot even be implemented on haze... (In reply to comment #5) > I follow-up branch should add an "error" signal (both in C API, and in spec > IMHO), and a cancel method. > > OTOH, I think we should just give up on that over-engineered interface, and > just give the file URI to the CM and let it do all the work. The current > iface cannot even be implemented on haze... Oh, and I'm not volunteering to do that work atm :P (In reply to comment #5) > Better ignoring errors, than crashing on error as it always did. Fair enough. ++ > OTOH, I think we should just give up on that over-engineered interface, and > just give the file URI to the CM and let it do all the work. The current > iface cannot even be implemented on haze... I've been advocating that since the first drafts of this API... Branch merged. I opened bug #73334 and #73333 for the follow up work that can be done on TpFileTransferChannel, if someone cares. This was originally fixed in 0.23.1. I cherry-picked it for 0.22.2. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.