Re: gio, gvfs and nautilus fixes

[Alexander Larsson <alexl redhat com>]

On Thu, 2008-02-21 at 02:46 -0500, David Zeuthen wrote:
> Hey,
> 
> Here's a couple of patches for gio, gvfs and Nautilus. The description
> is in the ChangeLog for each module. Some comments
> 
>  - I'm unsure my fix for nautilus_file_get_mount() is correct and I'm
>    not sure what it's supposed to do. My assumption is that it will
>    return a GMount for every file on a GMount. The older version only
>    did this for the root folder. My fix is provably not correct; we
>    may not have created NautilusFile objects to get to the root. One way
>    of testing this is to have some removable media with a DCIM/
>    directory in the root; for every folder you go into you should see
>    a cluebar. Without this fix you will only see it for the root folder
>    of the mount.

This is due to the check in lacks_mount:

static gboolean
lacks_mount (NautilusFile *file)
{
	return (!file->details->mount_is_up_to_date &&
		(
		 /* Unix mountpoint, could be a GMount */
		 file->details->is_mountpoint ||
		 

Without this we would be calling get_enclosing_mount() and doing the
related stat-walk and mtab reading for each file nautilus displays, as
we get the mount attribute for all files displayed (so we get the
unmount/eject etc context menu items). This is gonna be pretty slow, and
furthermore it sort of changes what nautilus_file_get_mount means. Atm
it means "this location represents a mount", either by being a
mountpoint or by being something like a desktop icon for a mount, a
smb://server/share file or a file in computer:, but with something like
what you propose it means "is on this mount".

For instance, it would put an eject menu item on all files on a cd,
which i'm not sure is a good idea.

>  - Autorunning was broken; this patch unbreaks it by moving the call
>    to inhibit into only selected call sites. One thing we can't fix
>    is when you mount from computer:// because it runs in another
>    process. Ideas on how to fix this?

None atm, but we need to figure out some way to do global autorun
inhibiting, because atm nautilus kicks in when you mount something from
the panel (or other app) too.

>  - g_file_find_enclosing_mount() for GDaemonFile was broken. We just
>    cannot construct a new GMount on the fly as it won't have the
>    the reference to a foreign GVolume that adopted it. My attempt
>    to fix this involves a global variable; it's probably safe as
>    GDaemonVolumeMonitor is a singleton anyway. It's not pretty though.
>    But it works.

This is totally unthreadsafe, and called from the i/o thread in the
async_find_enclosing_mount case...

>  - Before the feature freeze I commited half of the gphoto2 support
>    namely the bits in the volume monitor to create, maintain and
>    destroy GVolume for connected cameras. This patch is the second
>    half, the actual filesystem code. It's lacking write support
>    at the moment but is otherwise in pretty good shape (barring
>    crashes in libgphoto2; it doesn't like my Sandisk Sansa MTP
>    device that much).
> 
>    So this part of the patch set falls into a grey area wrt feature
>    freeze; technically it's just a bug fix (shipping gvfs without
>    it will appear broken as you can see the GVolume objects for
>    the cameras.. but you have no filesystem code for it!) but I guess
>    asking the release team whether it's OK to commit may be in order.
>    Shrug.

In fact, a lot of changes in the gvfs patch adds strings, and we're in
string freeze period right now, so you have to request permission on
gnome-i18n to commit most of it.

I commited the gio code and the nautilus code (sans the get_mount
change). For the gvfs code you need to request string break permissions
and handle the threadsafety issue.