GNOME Bugzilla – Bug 555332
Use shadow mounts instead of adoption
Last modified: 2018-09-21 16:27:54 UTC
I'm working on a "Connected Server" feature for gvfs/Nautilus. This bug is about the needed gvfs bits. See http://mail.gnome.org/archives/gvfs-list/2008-October/msg00002.html for details.
Created attachment 120079 [details] [review] proposed patch
After some discussion and analysis it turns out we probably want to do connected-server via bookmarks and not GConf. We still need the shadow mounts for gphoto2 and afc backends though. Will attach a patch with only the shadow mounts stuff.
Created attachment 120589 [details] [review] patch for using shadow mounts I also sneaked in the gvfs-mount --monitor option. Let me know if you want that as a separate commit or not at all. Thanks.
Separate commit sounds great to me.
Separate commits please. In general I like this patch. However, I think it leaves undue work on the side of the user wrt filtering out the shadowed mounts. Not only is it inefficient in that you generally need to filter each mount against all other mounts, its also lots of code being duplicated in lots of places. I think a better approach would be to add g_mount_is_shadowed() to be used in these places. Then we add g_mount_shadow() and g_mount_unshadow() which increment/decrement a per-mount int stored in gobject user data. is_shadowed() just returns TRUE if this is set and != 0. For the case of Gtk+ where we can't rely on this new API, we can temporarily add some code that just looks at the per user data directly. (i.e. cut and paste g_mount_is_shadowed)
(In reply to comment #5) > Separate commits please. Will do. > In general I like this patch. However, I think it leaves undue work on the side > of the user wrt filtering out the shadowed mounts. Not only is it inefficient > in that you generally need to filter each mount against all other mounts, its > also lots of code being duplicated in lots of places. > > I think a better approach would be to add g_mount_is_shadowed() to be used in > these places. Then we add g_mount_shadow() and g_mount_unshadow() which > increment/decrement a per-mount int stored in gobject user data. is_shadowed() > just returns TRUE if this is set and != 0. Can do. > For the case of Gtk+ where we can't rely on this new API, we can temporarily > add some code that just looks at the per user data directly. (i.e. cut and > paste g_mount_is_shadowed) Yeah. I'll try to fix this up; will attach new patches soon. On a more tangential note, re all the duplication of work to generate side-bar-ish things, I was talking about this with Matthias the other day. Today, we do this in a couple of places - the sidebar in Nautilus - the sidebar in the GTK+ file chooser - the panel's Places menu - there's a "File Browser" plug-in for GEdit http://www.andphp.com/wp-content/uploads/gedit_1.png Have you ever noticed that - the items in the file chooser sidebar are ordered differently compared to the Nautilus one? (Also, the Nautilus sidebar has useful eject icons) - sometimes the name and icons differ (Nautilus and the File Chooser seems to agree, but sometimes it's different in the panel) So what I proposed to Matthias was a function in GIO to enumerate the items one wants to put in a user visible sidebar. This includes drives, volumes, mounts and bookmarks. A naive attempt to do this would be something like gboolean g_places_enumerate (GPlacesEnumerateFunc callback, gpointer user_data); with typedef gboolean (*GPlacesEnumerate) (GObject *object, const gchar *name, GIcon *icon, gpointer user_data); where object is a GDrive, GVolume, GMount or GBookmark object. If called with object==NULL it means "insert a separator". The return value is used to short-circuit iteration. Maybe this API is too simple, I don't know. I *think* we need something like this. We have a lot of powerful API in GIO right now for managing user visible objects such as drives, volumes, mounts and bookmarks. I don't think the API is too complex, the problem space is just that complex. But I think it's hard to get it wrong when doing sidebar-ish of things, definitely current usage suggests that. Funny enough, Matthias' reaction was that we should instead have a GtkPlacesSidebar widget (or something) which I'm all for too. Then we'd have the same behavior in the file chooser and Nautilus which might be a win. However, this won't work for the Places menu in the panel (unless we also provide a GtkPlacesMenu widget). So I think we want this in GIO too. Anyway, just an idea, I don't have code for this yet, wanted to pick your brain about it first. It shouldn't be hard to do the GIO bits, the GTK+ bits would definitely be harder.
re comment 5: s/hard to get wrong/easy to get wrong/
Not sure enumerate is enough. You need to handle additions, removal, allowing the user to reorder things, etc. Not sure about the idea of an API for this though... I'll think about it. For sharing a widget, i think that is kinda hard. Nautilus will want to do its own async file i/o stuff anyway, so won't really be able to use this, and as you say, it doesn't work for menus.
Created attachment 121043 [details] [review] gio patch Here's the gio bits of the shadowing patch. It also updates the docs for g_volume_get_activation_root() and attempts to explain what shadow mounts are. When we add the bookmarking code, we want to expand the docs for g_mount_is_shadowed() to mention that bookmarks are other user-visible objects that may shadow a mount. Btw, am not sure it's kosher how I create the private data for a mount; is there a better way to do this for a GInterface? (can't use g_type_class_add_private). $ diffstat gio-shadow-r7616-20081021.patch docs/reference/gio/gio-sections.txt | 3 gio/gio.symbols | 3 gio/gmount.c | 134 ++++++++++++++++++++++++++++++++++++ gio/gmount.h | 8 +- gio/gunionvolumemonitor.c | 3 gio/gvolume.c | 34 --------- 6 files changed, 151 insertions(+), 34 deletions(-)
Created attachment 121044 [details] [review] gvfs patch Updated gvfs patch. Also, another enhancement to gvfs-mount to print out the type name of the proxy monitor, e.g. Volume(0): Apple, Inc. iPhone Type: GProxyVolume (GProxyVolumeMonitorGPhoto2) $ diffstat gvfs-shadow-r2071-20081021.patch client/gdaemonmount.c | 148 --------- client/gdaemonvolumemonitor.c | 8 monitor/proxy/Makefile.am | 1 monitor/proxy/gproxydrive.c | 3 monitor/proxy/gproxymount.c | 3 monitor/proxy/gproxyshadowmount.c | 441 +++++++++++++++++++++++++++ monitor/proxy/gproxyshadowmount.h | 59 +++ monitor/proxy/gproxyvolume.c | 282 ++++++++++++++--- monitor/proxy/gproxyvolume.h | 4 monitor/proxy/gproxyvolumemonitor.c | 72 +--- monitor/proxy/gproxyvolumemonitor.h | 1 monitor/proxy/remote-volume-monitor-module.c | 2 programs/gvfs-mount.c | 252 +++++++++++++++ 13 files changed, 1016 insertions(+), 260 deletions(-)
(In reply to comment #8) > Not sure enumerate is enough. You need to handle additions, removal, allowing > the user to reorder things, etc. Not sure about the idea of an API for this > though... I'll think about it. Sounds like you're thinking about a complete GPlacesMonitor class to unify GVolumeMonitor and (future) GBookmarkMonitor functionality. Not sure that is needed, seems overkill anyway, what I'm proposing is that the user would need to listen on these signals himself and just use the utility *function* (emphasis on function) to redo his UI (the UI could be gvfs-mount(1)). > For sharing a widget, i think that is kinda hard. Nautilus will want to do its > own async file i/o stuff anyway, so won't really be able to use this, I'm talking about src/nautilus-places-sidebar.c ("Places") not the other sidebar in src/file-manager/fm-tree-view.c ("Tree"). Looking at the code Places code, I don't see too much Nautilus specific stuff. What kind of i/o did you have in mind? Either way, the widget could be written in a way so it's easy for Nautilus to plug in it's own stuff for e.g. mounting/unmount/ejecting and so forth. > and as you say, it doesn't work for menus. I think that's just because the panel decides to be special. The drive applet wouldn't be able to use it either, should we care about that? Either way, having *just* the GTK+ file chooser and Nautilus use the same code base would be a huge win I think. Both maintenance, performance and usability wise.
I commited both of these. However, we need to also look at is_shadowed in nautilus to make this fully work.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gvfs/issues/64.