GNOME Bugzilla – Bug 765900
Add g_drive_is_removable() support
Last modified: 2016-05-20 11:11:58 UTC
Created attachment 327142 [details] [review] gio: Add g_drive_is_removable() support Nautilus wants to show entries in the sidebar only for removable devices. It uses currently sort of conditions to determine which devices should be shown. Those condition fails in some cases unfortunatelly. Lets provide g_drive_is_removable() which uses udisks Removable property to determine which devices should be shown. It should return true for all drives with removable media, or flash media, or drives on usb and firewire buses.
Review of attachment 327142 [details] [review]: ::: gio/gdrive.c @@ +287,3 @@ + iface = G_DRIVE_GET_IFACE (drive); + + **/ You cannot really do this unconditionally: what happens if a newer GLib is used with an older version of GVfs? This should be: iface = G_DRIVE_GET_IFACE (drive); if (iface->is_removable != NULL) return iface->is_removable (drive); return FALSE; Also, don't use the `(* vtable->vfunc)` style: it's just noise. ::: gio/gdrive.h @@ +174,3 @@ GList * g_drive_get_volumes (GDrive *drive); +GLIB_AVAILABLE_IN_2_50 +gboolean g_drive_is_removable (GDrive *drive); Coding style: tab vs spaces.
Created attachment 327717 [details] [review] gio: Add g_drive_is_removable() support Thanks for the review!
Review of attachment 327717 [details] [review]: Looks good to me, and it's simple enough, but I'm not the GIO maintainer so I'll just put 'reviewed' in the status, and a +1 from me for pushing it to master.
Review of attachment 327717 [details] [review]: Please go ahead. Thanks for the look, Emmanuele. ::: gio/gdrive.c @@ +286,3 @@ + + iface = G_DRIVE_GET_IFACE (drive); + if (iface->is_removable != NULL) Yuck. Normally I'd complain about how this should be done with a default "real_" vfunc instead but, well, the entire file is this way, so, +1 for consistency. :)
Thanks for review, I want to push also the GTK/GVfs patches which depends on it, I added requirement for GLib 2.49.1, but it seems GLib is using prerelease bump, so it is 2.49.0 currently. Should I wait with GTK/GVFS patches for the release, or it is possible to bump the GLib version earlier, or what is recommended workflow in such case?
sure, we can bump the release in git, if it helps
Comment on attachment 327717 [details] [review] gio: Add g_drive_is_removable() support commit 7b3f6da
(In reply to Matthias Clasen from comment #6) > sure, we can bump the release in git, if it helps Thanks, so I bumped the version by the commit 931483a.
#gtk+: gregier oholy: hey looking at commit: https://git.gnome.org/browse/glib/commit/?id=7b3f6da30718c443d64169813b4ae1b3a28ba934 the new vfunc should be added to the end otherwise the ABI is broken
Created attachment 328249 [details] [review] Fix ABI compatibility Is this enough to fix the ABI? Should I move also another code to corresponding places?
Review of attachment 328249 [details] [review]: Yes this is good. For interfaces, you can add new functions all you want (without requiring padding), but they must be at the end of the interface otherwise previously compiled code against that interface will have the wrong pointer offset.
Comment on attachment 328249 [details] [review] Fix ABI compatibility Thanks, pushed as commit 17e5281.