GNOME Bugzilla – Bug 551403
Support Reading Volume Icon/Name Information from .xdg-volume-info
Last modified: 2018-09-21 16:26:16 UTC
The attached patch allows a volume's icon and name information to be read from the .xdg-volume-info file inside the mount. This allows volumes which are not handled by HAL to have custom icons and volume names. The primary target this patch is trying to provide a solution for, is user-local fuse mounts, which a web service might provide for data storage, and would be an appropriate place to have a custom icon specified for the mount, to distinguish it from other sftp/ftp/dav, or local disk mounts.
Created attachment 118315 [details] [review] Support for .xdg-volume-info on storage volumes
Is there any chance this could get reviewed?
I'm pretty sure we don't want to do io to guess an icon. Alex, whats your opinion ?
Hrmm. GVFS is already doing IO to get an icon, though most of that code should actually be in GIO and not GVFS. We should be able to support autorun.inf without requiring HAL to do it, especially if GIOs goals include "work on Windows" and "do approximately the same as the OS it's running on would do naturally." Alex? If you have a suggestion for a better way of doing the IO async or something, I am all for that, but I really want to get this sort of functionality in GIO and not require evil hacks in GVFS to make things work.
There are two risks with accessing mount info like that, first of all the app can hang if it comes across a hanged NFS mount, secondly we can cause secondary effects to the filesystems when we probe them (mount autofs filesystems, etc). This is quite a bit more likely to happen here as opposed to the normal user file i/o case because we're explicitly doing this to all mounted filesystems. So, the core unix mount handling is designed to be very carefully about what i/o it does. We don't want to do any i/o like that from gio, as that would multiply the number of times we do this, and risk hanging all the users apps. Instead it should go in ghalmount.c with the other icon sniffing. This code runs in a single daemon per session to avoid excessive i/o and has full control over its context, so we can easily schedule the i/o done async in a thread to avoid hanging. I'm sorry you think this is an "evil hack", but its how the gio/gvfs split was designed. The gio part has the interfaces and very lowlevel common code and the gvfs code has the implementation for "modern unix systems". I don't know how "works on windows" is related to this bug, but for win32 we're using gwin32volumemonitor instead of the gvfs ones which uses the native win32 APIs to enumerate volumes (which gets icons via SHGetFileInfoW using SHGFI_ICONLOCATION. Plus, we shouldn't read .xdg-volume-info on win32 anyway.
Alex, this can't really be done in ghalmount.c because HAL does not enumerate mounts, it only enumerates hardware. For example, the primary place where this would be most useful is with FUSE mounts, however, they do not show up via the HAL monitoring. Nor do loopback mounts. Does the win32 monitor run in a separate process? It is doing synchronous I/O, no?
The hal monitor also has the information from fstab available, so it could show all sorts of mounts. It is rather that the set of mounts that we've decited that GVolumeMonitor should show don't include that sort of lowlevel stuff. The win32 monitor isn't in a separate process, but i don't think the shell calls actually do the sniffing, they just ask the shell process what the sniffed results are.
Created attachment 128128 [details] [review] Patch to read .xdg-volume-info files on volumes not handled by hal Here is an updated patch against gvfs which supports reading the volume information from .xdg-volume-info for volumes not enumerated by hal.
Hi, I think .xdg-volume-info is a useful thing. In particular I want to use it for the "Rename..." feature (on devices) in Nautilus (it would a. use DeviceKit-disks to set the FS label; and b. update the .xdg-volume-info file). Where do you envision we standardize this file format? Probably xdg-list. It looks like this could race against the autorun.inf code that we use to set the icon in the (unlikely) event that you have both autorun.inf and .xdg-volume-info on an optical disc. > + /* Scan through for an "icon=" line. Can't use GKeyFile, > + * because .inf files aren't always valid key files > + **/ Looks like copy-paste from the autorun.inf code; I think we can say .xdg-volume-info files must be in a format so they can be parsed by GKeyFile. Then we get UTF8 validation for free. Btw, it would be nice to use a GFileMonitor to watch the .xdg-volume-info file for changes; this is fine since the volume monitor is a single process. I think in general it would be useful to have both your code and the autorun.inf code in libgvfs-common and the rework how the HAL monitor uses it. Then it's also trivial to make the new DeviceKit-disks GVfs monitor (that I'm currently working on) use it. I'll look into doing a patch for merging this into libgvfs-common today.
(In reply to comment #9) > Hi, > > I think .xdg-volume-info is a useful thing. In particular I want to use it for > the "Rename..." feature (on devices) in Nautilus (it would a. use > DeviceKit-disks to set the FS label; and b. update the .xdg-volume-info file). I don't know that we should be using .xdg-volume-info for this. In most cases, I think we want to stick to just using the filesystem attributes for custom names for volumes, to retain consistency when a user plugs their USB disk into a Windows PC for example. The main utility I see for this .xdg-volume-info file is so that custom filesystems on Linux, such as gmailfs and other FUSE filesystems, can provide useful names and icons. > Where do you envision we standardize this file format? Probably xdg-list. I suppose that is the appropriate place, yes. > It looks like this could race against the autorun.inf code that we use to set > the icon in the (unlikely) event that you have both autorun.inf and > .xdg-volume-info on an optical disc. I don't think so because the autorun.inf case only currently gets run if it is handled by HAL, and not otherwise, while my patch only gets hit for filesystems not handled by HAL. The two methods also set the same variable in the struct to TRUE, and test for it, so I think that should also avoid any potential race? > > + /* Scan through for an "icon=" line. Can't use GKeyFile, > > + * because .inf files aren't always valid key files > > + **/ > > Looks like copy-paste from the autorun.inf code; I think we can say > .xdg-volume-info files must be in a format so they can be parsed by GKeyFile. > Then we get UTF8 validation for free. Yeah, I sort of overlooked that when i copy/pasted the code. I think the GKeyFile implementation is less code too, so that's fine with me. > Btw, it would be nice to use a GFileMonitor to watch the .xdg-volume-info file > for changes; this is fine since the volume monitor is a single process. I'm not sure we really want to have the icon/name suddenly changing in the UI? It could be handled I guess, but having the disk shuffle around while the user is trying to access it, seems like it could be pretty annoying? I think it would also cause the icon to move on the user's desktop, which would be bad. I think we're just envisioning different use cases here, and there is some conflict. What I envision the functionality being useful for, I don't think lends to having the file data be changed while in use, and so the monitor doesn't make much sense to me to have. > I think in general it would be useful to have both your code and the > autorun.inf code in libgvfs-common and the rework how the HAL monitor uses it. > Then it's also trivial to make the new DeviceKit-disks GVfs monitor (that I'm > currently working on) use it. > > I'll look into doing a patch for merging this into libgvfs-common today. Great. I agree it would be nice to have some of this "check for icon info on the volume we're mounting" to be simpler and easier to extend. My patch was a little more complex than I would have liked, if only due to the many code paths that exist in ghalmount.c :). Thanks for looking at it.
Created attachment 129415 [details] [review] Patch that loads correct file name Apparently the patch I uploaded previously was looking for the wrong file name. It should be .xdg-volume-info, not .volume-info (which I was using as a test point, and apparently got pulled in the patch somehow, without me noticing until now).
I've submitted a patch to the mailing list http://mail.gnome.org/archives/gvfs-list/2009-February/msg00024.html that does this. It's a bit simpler than your patch - there's not really any need to do a case-insensitive search, we just require that the file is named .xdg-volume-info. - it uses GKeyFile so locale / UTF-8 etc. stuff works - requires the icon and name keys are in the [XDG Volume Info] group Here's an example of a .xdg-volume-info file [XDG Volume Info] name=Awesome Stuff name[da]=Satans Godt Kram icon=web-browser icon[da]=logviewer
The latest patch changes name to Name and icon to Icon, e.g. [XDG Volume Info] Name=Awesome Stuff Name[da]=Satans Godt Kram Icon=web-browser Icon[da]=logviewer and the implementation makes both of these keys optional. There's also some concern that we should spec this out before landing it in GVfs. Thoughts?
I think it might be better to just use [Volume Info] for the header. The file is already obviously XDG specific, having xdg in the file name. I also don't really see the point of stuffing translations in the file, especially for Icon. We don't even translate Icon in .desktop files. The main place I see this being useful, is for things like FUSE mounts, where the file would actually be synthetic, and the Name string would be translated at runtime anyway.
I've committed this for now. 2009-03-02 David Zeuthen <davidz@redhat.com> Bug 551403 – Support Reading Volume Icon/Name Information from .xdg-volume-info * common/Makefile.am: * common/gvfsmountinfo.[ch]: Move autorun file detection to common library. Also add routines for detecting .xdg-volume-info files. * monitor/hal/ghalmount.c: Use g_mount_info*() functions for detecting autorun and .xdg-volume-info files.
Rodney: Its also useful to e.g. put a custom icon on a usb stick image. However, in that case its more useful to give an icon file instead of a themed icon name, so I propose adding IconFile=relative/path/to/imagefile.png too. I agree however that translating the icon seems wrong.
(In reply to comment #14) > I think it might be better to just use [Volume Info] for the header. The file > is already obviously XDG specific, having xdg in the file name. I also don't > really see the point of stuffing translations in the file, especially for Icon. > We don't even translate Icon in .desktop files. > > The main place I see this being useful, is for things like FUSE mounts, where > the file would actually be synthetic, and the Name string would be translated > at runtime anyway. I can see this being useful for lots of other things than FUSE mounts. For example, if I'm doing an embedded device I can put a .xdg-volume-info file on the disk of my device. For that use case it's useful to have both localized Name and Icon, you can't really assume that the host bits of the device can generate this on demand. Not sure it matters whether it's [Volume Info] or [XDG Volume Info]. I committed the patch (because Alex wants to do a GVfs release) before reading your comment so right now it's the latter. Not sure it matters much. Anyway, we probably want a spec for this before releasing GVfs 2.26. If we don't get one we can also just turn this feature off. Btw, Alex also mentioned on IRC that it might be useful to support loadable icons (in addition to themed icons), e.g. something like a relative path to the mount point Icon=.my-company-icon.svg Icon=data/my-company.png or something (we'd determine it's a relative icon by seeing if the Icon value has a file extension). So we probably want this in the spec too.
davidz: I don't think its a good idea to do heuristics on the "Icon" value to see what type of icon it is. I'd rather have a separate IconFile key.
I'm OK with a separate key. Do you also want me to remove the translation support for Icon (and IconFile)?
I think if you want to specify an icon file which is also on your usb stick, you should probably be using autorun.inf anyway, so that it will work in windows. And again, users aren't going to translate their own custom name strings to lots of different languages anyway, and it would be better for them to be using the actual volume label in the file system for real usb disks, memory cards, etc... using FAT.
I don't think its only useful for "users". For instance, it seems highly useful for a distro live cd to have a png with a logo and a translated title. In that case you want title + IconFile, so having to use both autorun.inf and .xdg-volume-info seems weird. So, my opinion is that we: a) Rename group to [Volume Info] No other desktop-file-style xdg spec has xdg in the group name b) Don't make icon translatable Not in desktop file spec and doesn't seem that useful c) Add IconFile support
+1 for IconFile - just implemented a fuse-fs that fakes a .xdg-volume-info and tried to provide a custom icon.
A related issue is that GVolume has in the meantime gained support for symbolic icons, and that is not reflected in the existing .xdg-volume-info code.
I wonder why this bug report is still opened, because .xdg-volume-info is already supported, even IconFile is supported, see: https://git.gnome.org/browse/gvfs/tree/common/gvfsmountinfo.c#n321 But maybe you can't see your icon, because symbolic icon is used instead as pointed Matthias... Btw is .xdg-volume-info standardized already somewhere?
Changing component to udisks2 volume monitor, because hal volume monitor is obsolete currently and this issue seems to be valid also for udisks2 volume monitor...
-- 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/60.