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 691823 - Support driver and logo files from non-HTTP locations
Support driver and logo files from non-HTTP locations
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other All
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-01-16 02:49 UTC by Zeeshan Ali
Modified: 2016-03-31 13:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
util-app: Make copy_file() available to other classes (2.25 KB, patch)
2013-01-16 02:49 UTC, Zeeshan Ali
committed Details | Review
downloader: (Try to) Support non-HTTP cases (4.79 KB, patch)
2013-01-16 02:49 UTC, Zeeshan Ali
needs-work Details | Review
downloader: (Try to) Support non-HTTP cases (4.83 KB, patch)
2013-01-22 20:06 UTC, Zeeshan Ali
committed Details | Review

Description Zeeshan Ali 2013-01-16 02:49:28 UTC
Attaching patches that add support for driver and logo files from non-HTTP sources.
Comment 1 Zeeshan Ali 2013-01-16 02:49:30 UTC
Created attachment 233559 [details] [review]
util-app: Make copy_file() available to other classes
Comment 2 Zeeshan Ali 2013-01-16 02:49:33 UTC
Created attachment 233560 [details] [review]
downloader: (Try to) Support non-HTTP cases

This is not guaranteed to work for every kind of filesystem but at least
now we don't assume HTTP blindly and support caching from local
filesystem. Its a perfectly valid usecase for distros to provide product
logos or driver files from local filesystem and with this patch we'll
support that.
Comment 3 Alexander Larsson 2013-01-22 08:44:26 UTC
Review of attachment 233559 [details] [review]:

ack
Comment 4 Alexander Larsson 2013-01-22 08:47:33 UTC
Review of attachment 233560 [details] [review]:

::: src/downloader.vala
@@ +62,2 @@
         try {
+            if (uri.has_prefix ("http://"))

What about https? and upper case strings?

I think you should use remote_file.has_uri_scheme() instead.
Comment 5 Zeeshan Ali 2013-01-22 14:12:12 UTC
Review of attachment 233560 [details] [review]:

::: src/downloader.vala
@@ +62,2 @@
         try {
+            if (uri.has_prefix ("http://"))

Wouldn't HTTPS require some kind of authentication that we haven't yet implemented?

And g_file_has_uri_scheme takes care of case?
Comment 6 Alexander Larsson 2013-01-22 16:20:42 UTC
nah, https is not authentication, just encryption, although we will not be doing any certificate checks, so its not MITM proof.

has_uri_scheme takes care of case.
Comment 7 Christophe Fergeau 2013-01-22 18:25:23 UTC
(In reply to comment #2)
> Its a perfectly valid usecase for distros to provide product
> logos or driver files from local filesystem and with this patch we'll
> support that.

Is this the initial motivation for this patch series? Can't we look for files in some well-known location in addition to ~/.cache if we want to let distros ship the various logos/drivers?
Comment 8 Zeeshan Ali 2013-01-22 19:51:38 UTC
(In reply to comment #7)
> (In reply to comment #2)
> > Its a perfectly valid usecase for distros to provide product
> > logos or driver files from local filesystem and with this patch we'll
> > support that.
> 
> Is this the initial motivation for this patch series? Can't we look for files
> in some well-known location in addition to ~/.cache if we want to let distros
> ship the various logos/drivers?

Its not about us looking for the cache but rather the source files being on the local (or non-HTTP) filesystem already. i-e we'll get file:///some_path.jpg for a logo from (perhaps a custom) libosinfo database. Same goes for drivers. As you know I'm trying to get qxl drivers installed by default for windows and I did this for one of my approaches for that. Then I realized these patches would be nice to have anyways.
Comment 9 Zeeshan Ali 2013-01-22 20:06:51 UTC
Created attachment 234134 [details] [review]
downloader: (Try to) Support non-HTTP cases

v2: Make use of g_file_has_uri_scheme() and also handle HTTPS through libsoup.
Comment 10 Alexander Larsson 2013-01-23 08:50:58 UTC
Using a local cache is imho important too, but thats bug 691495. This simple patch seems to make sense too.
Comment 11 Alexander Larsson 2013-01-23 08:51:37 UTC
Review of attachment 234134 [details] [review]:

ack
Comment 12 Christophe Fergeau 2013-01-23 11:09:55 UTC
Yup, it makes sense to be able to download from ftp or whatever. 
But it's a very convoluted way of letting distros preinstall the drivers (ie they have to install them somewhere on the FS and then write custom libosinfo database files, or patch the existing ones to use the correct drivers).
Comment 13 Zeeshan Ali 2013-01-23 14:09:43 UTC
(In reply to comment #12)
> Yup, it makes sense to be able to download from ftp or whatever. 
> But it's a very convoluted way of letting distros preinstall the drivers (ie
> they have to install them somewhere on the FS and then write custom libosinfo
> database files, or patch the existing ones to use the correct drivers).

This would be mainly a way for them to install new drivers, not patch/update existing ones (though they can use it that way too). There could also be a third-party package that adds additional stuff to libosinfo/Boxes, e.g some proprietary drivers.
Comment 14 Zeeshan Ali 2013-01-23 14:12:34 UTC
Attachment 233559 [details] pushed as 22d3cb1 - util-app: Make copy_file() available to other classes
Attachment 234134 [details] pushed as 9ce057b - downloader: (Try to) Support non-HTTP cases