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 684290 - Download using libsoup, not gvfs
Download using libsoup, not gvfs
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
3.5.x (unsupported)
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-09-18 12:46 UTC by Alexander Larsson
Modified: 2016-03-31 13:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Download http directly via libsoup (3.12 KB, patch)
2012-09-18 12:46 UTC, Alexander Larsson
committed Details | Review

Description Alexander Larsson 2012-09-18 12:46:33 UTC
We currently download via a GVfs file copy, which is a rather complicated affair that spawns a separate http mount process and talks to it via dbus. The http backend uses libsoup anyway, so we might as well just use that directly ourselves. This gives us an easier, more direct dependency which is easier to bundle, and faster/more robust.
Comment 1 Alexander Larsson 2012-09-18 12:46:49 UTC
Created attachment 224616 [details] [review]
Download http directly via libsoup

The GVfs http support works for this, but is quite overcomplicated for
pure file downloads. Directly using libsoup means less dependencies
and more direct codepaths (library calls instead of multiple
cross-process dbus calls).
Comment 2 Christophe Fergeau 2012-09-19 21:09:14 UTC
Review of attachment 224616 [details] [review]:

While I understand the reasoning behind this patch, the diffstat does not make it very compelling ( 3 files changed, 17 insertions(+), 1 deletion(-) ), ie it makes the code more complex for not much gain, so not really sure what to do with it :-/
Comment 3 Christophe Fergeau 2012-09-19 21:09:14 UTC
Review of attachment 224616 [details] [review]:

While I understand the reasoning behind this patch, the diffstat does not make it very compelling ( 3 files changed, 17 insertions(+), 1 deletion(-) ), ie it makes the code more complex for not much gain, so not really sure what to do with it :-/
Comment 4 Alexander Larsson 2012-09-20 14:26:05 UTC
The complexity in the code is one thing, the complexity during runtime is another. GVfs is not really a http downloader type of library, and the http support in it is sorta thrown in because its possible. And in the end, the same libsoup code will be run in the gvfs http backend.
Comment 5 Zeeshan Ali 2012-09-25 12:25:39 UTC
(In reply to comment #4)
> The complexity in the code is one thing, the complexity during runtime is
> another. GVfs is not really a http downloader type of library, and the http
> support in it is sorta thrown in because its possible. And in the end, the same
> libsoup code will be run in the gvfs http backend.

I would like to keep this code as generic as possible so that we can download from any source (HTTP, FTP, SSH etc) that gio on the system can handle.
Comment 6 Alexander Larsson 2012-10-02 13:29:44 UTC
> I would like to keep this code as generic as possible so that we can download
> from any source (HTTP, FTP, SSH etc) that gio on the system can handle.

If so you need to be able to mount such uris, do authentication and whatnot. Thats will also result in a bunch of user-visible mounts that may surprise the user.
Comment 7 Zeeshan Ali 2012-10-02 17:09:10 UTC
Review of attachment 224616 [details] [review]:

Looks good otherwise. It doesn't seem to complicate the code much so ACK from me given these small issues are addressed.

::: src/downloader.vala
@@ +47,3 @@
+            yield;
+            if (msg.status_code != Soup.KnownStatusCode.OK) {
+                throw new Boxes.Error.INVALID ("Not ok");

* "Not ok" -> msg.reason_phrase ?
* No need for curly braces.
Comment 8 Alexander Larsson 2012-10-04 13:40:42 UTC
Attachment 224616 [details] pushed as 16cb6ed - Download http directly via libsoup