GNOME Bugzilla – Bug 675333
Cannot forget association in Open With dialog: program not found
Last modified: 2018-05-24 14:12:29 UTC
Created attachment 213335 [details] screenshot Once I had wine installed, then I have removed it. But it polluted many of mime associations :) I have a lot of "Wine core exe" and other wine apps associated with some common file extensions. So I started to search for a way to remove them and found this bug, to reproduce: * have some file association with its target app not existing (wine in my case) * right click on that file and select "Open With" => "Other..." * in the associated app list right click on the association of the program in question (wine) * Context menu with "Forget association" will pop up, click it * boom! it says "Could not find application"! So in the end I cannot remove this association in a user friendly way :) See also: attached screenshot.
-> glib I think what we're seeing here is the combination of a number of bugs: The dialog checks whether the executable (specified in the Exec line of the app desktop file) is correctly parsable and exists in PATH [1], upon calls to gtk_app_chooser_get_app_info(). I don't think that's good behavior (also because those error messages are quite random), and those dialogs should not exist at that level (it would be responsibility of the application that tries to launch a GAppInfo to display an eventual error in the best way to the user). Anyway, gtk_app_chooser_get_app_info() is called by the code that can remove the association, and that's why you get the error dialog. There's no reason why we should show those broken applications in GtkAppChooserDialog, since there's nothing possibly useful the user can do with them. GIO already globally filters out applications for which the value specified in TryExec does not exist in path [2], since the Desktop Entry spec says so [3], but it doesn't apply the same check for the value of the Exec field. Instead, I think it should. I spoke with Owen about this yesterday (since he is one of the authors of the original spec), and he agreed with me, as long as applying such a filter is not too expensive, so I went ahead and wrote the following patch. Note that in the tests I made *there is* an actual slowdown (around 5%) with the patch applied, when calling g_app_info_get_all(). Since these functions are very specific and kinda slow already (and we do similar checks for TryExec just a few lines before) I think the patch is worth committing. Another nice effect is that things such as GtkAppChooserDialog would be guaranteed to always get valid applications. [1] http://git.gnome.org/browse/gtk+/tree/gtk/gtkappchooserdialog.c#n220 [2] http://git.gnome.org/browse/glib/tree/gio/gdesktopappinfo.c#n306 [3] http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s05.html
Created attachment 213471 [details] [review] desktopappinfo: check whether the specified executable is valid Before declaring the desktop file as valid, make sure the referenced application actually exists in path and the commandline is not malformed.
Created attachment 213472 [details] [review] appchooserdialog: remove redundant checks GTK patch
In general this seems right to me, we should not by default pass what is essentially broken desktop files to apps. However, I worry slightly about cases where we need to handle such broken desktop files, like e.g. to fix them or something when our libraries can't even *load* the desktop file. Maybe you should use a lower level API (i.e. GKeyFile) for that kind of thing though.
Alex, should we get these in for 3.8?
Yeah, that makes sense to me. Please do.
Attachment 213471 [details] pushed as f641699 - desktopappinfo: check whether the specified executable is valid
For the record, this commit caused bug 695191.
I want to revert this change. With the desktop file index, we now do g_app_info_get_all() with zero disk IO whatsoever. This patch has therefore become extremely expensive as a part of this operation. Here's a snapshot of what strace output looks like for this operation. Note that this patch is now responsible for 100% of the syscalls here.... [pid 18288] access("/home/desrt/.cache/lcl/bin/nautilus", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/home/desrt/.cache/jhbuild/install/bin/nautilus", X_OK) = 0 [pid 18288] getuid() = 1000 [pid 18288] stat("/home/desrt/.cache/jhbuild/install/bin/nautilus", {st_mode=S_IFREG|0755, st_size=6832191, ...}) = 0 [pid 18288] access("/home/desrt/.cache/lcl/bin/nm-connection-editor", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/home/desrt/.cache/jhbuild/install/bin/nm-connection-editor", X_OK) = 0 [pid 18288] getuid() = 1000 [pid 18288] stat("/home/desrt/.cache/jhbuild/install/bin/nm-connection-editor", {st_mode=S_IFREG|0755, st_size=2470376, ...}) = 0 [pid 18288] access("/usr/libexec/nm-vpnc-auth-dialog", X_OK) = 0 [pid 18288] getuid() = 1000 [pid 18288] stat("/usr/libexec/nm-vpnc-auth-dialog", {st_mode=S_IFREG|0755, st_size=55592, ...}) = 0 [pid 18288] access("/home/desrt/.cache/lcl/bin/userinfo", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/home/desrt/.cache/jhbuild/install/bin/userinfo", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/lib/lightdm/lightdm/userinfo", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/sbin/userinfo", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/bin/userinfo", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/sbin/userinfo", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/bin/userinfo", X_OK) = 0 [pid 18288] getuid() = 1000 [pid 18288] stat("/usr/bin/userinfo", {st_mode=S_IFREG|0755, st_size=40288, ...}) = 0 [pid 18288] access("/home/desrt/.cache/lcl/bin/usermount", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/home/desrt/.cache/jhbuild/install/bin/usermount", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/lib/lightdm/lightdm/usermount", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/sbin/usermount", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/bin/usermount", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/sbin/usermount", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/bin/usermount", X_OK) = 0 [pid 18288] getuid() = 1000 [pid 18288] stat("/usr/bin/usermount", {st_mode=S_IFREG|0755, st_size=23656, ...}) = 0 [pid 18288] access("/home/desrt/.cache/lcl/bin/userpasswd", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/home/desrt/.cache/jhbuild/install/bin/userpasswd", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/lib/lightdm/lightdm/userpasswd", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/sbin/userpasswd", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/bin/userpasswd", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/sbin/userpasswd", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/bin/userpasswd", X_OK) = 0 [pid 18288] getuid() = 1000 [pid 18288] stat("/usr/bin/userpasswd", {st_mode=S_IFREG|0755, st_size=32040, ...}) = 0 [pid 18288] access("/home/desrt/.cache/lcl/bin/rhythmbox-client", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/home/desrt/.cache/jhbuild/install/bin/rhythmbox-client", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/lib/lightdm/lightdm/rhythmbox-client", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/sbin/rhythmbox-client", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/local/bin/rhythmbox-client", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/sbin/rhythmbox-client", X_OK) = -1 ENOENT (No such file or directory) [pid 18288] access("/usr/bin/rhythmbox-client", X_OK) = 0 [pid 18288] getuid() = 1000 [pid 18288] stat("/usr/bin/rhythmbox-client", {st_mode=S_IFREG|0755, st_size=42600, ...}) = 0
A possible compromise: I think this is only a problem for desktop files installed under ~/.local/share/applications/, so maybe we could reduce the size of the problem by an order of magnitude or two by only doing the check for those ones.... I guess we're quite unlikely to find broken desktop files in /usr unless something is seriously wrong.
(In reply to comment #9) > I want to revert this change. > > > With the desktop file index, we now do g_app_info_get_all() with zero disk IO > whatsoever. This patch has therefore become extremely expensive as a part of > this operation. Here's a snapshot of what strace output looks like for this > operation. Note that this patch is now responsible for 100% of the syscalls > here.... (In reply to comment #10) > A possible compromise: I think this is only a problem for desktop files > installed under ~/.local/share/applications/, so maybe we could reduce the size > of the problem by an order of magnitude or two by only doing the check for > those ones.... > > I guess we're quite unlikely to find broken desktop files in /usr unless > something is seriously wrong. I am not familiar with how the desktop file index works, but can you move the I/O to the moment when the index is created and cache that information? I agree that this is mostly a problem for files in ~/.local/share/applications, but I think the system should be resistant to defective desktop files in all cases.
So it's not 100%. Even after testing a revert, there are still a few apps with TryExec= lines that also cause checks to occur.... (In reply to comment #11) > I am not familiar with how the desktop file index works, but can you move the > I/O to the moment when the index is created and cache that information? I agree Jasper suggested the same... I guess it might make sense to exclude applications with a missing TryExec entirely.... I don't like this, though because it doesn't take into account that people may be using this more dynamically than we expect. I think storing this information in the cache is basically a bad idea because we have no way to know when it goes stale. With desktop files, we can do the usual tricks of comparing mtimes of the cache with its containing directory, but if we bring the existence (or not) of files in /usr/bin into the equation, we have to then somehow include the mtime on all the directories in the PATH into that calculation, and that's getting ridiculous (since anyone who installs a binary would have to run the update tool, not just those installing desktop files). > that this is mostly a problem for files in ~/.local/share/applications, but I > think the system should be resistant to defective desktop files in all cases. I don't consider installing a desktop file in /usr/share/applications/ with an Exec= line pointing to a file that you didn't install to be substantially different than installing a python application with a missing file that causes it not to show its UI on startup, with the exception that we have a better chance of giving meaningful feedback to the user in the second case. I really think the essence of this problem is that of dynamically-created desktop files in ~/.local/share/applications/, and maybe our focus should be on another solution to clean these things up periodically instead of suffering performance degradation every time we perform a fairly common operation... Maybe we just need to tell the wine people to put a TryExec= line in their desktop files....
(In reply to comment #12) > Jasper suggested the same... I guess it might make sense to exclude > applications with a missing TryExec entirely.... I don't like this, though > because it doesn't take into account that people may be using this more > dynamically than we expect. I think you have to do the check for TryExec in any case if you want to be compatible with the desktop entry spec... > I think storing this information in the cache is basically a bad idea because > we have no way to know when it goes stale. With desktop files, we can do the > usual tricks of comparing mtimes of the cache with its containing directory, > but if we bring the existence (or not) of files in /usr/bin into the equation, > we have to then somehow include the mtime on all the directories in the PATH > into that calculation, and that's getting ridiculous (since anyone who installs > a binary would have to run the update tool, not just those installing desktop > files). Another (not so pretty) thing you could do is monitor directories in $PATH and treat the addition/removal/modification of an executable file in any of those locations the same way you would for a file in the various $XDG_DATA_DIRS/applications. Or just ignore the case where an application enumerating apps is running and a binary goes from being installed to not being installed - but with a desktop file referencing it still present. That way, the state would be consistent the next time the application starts, which is still better than showing forever a broken desktop entry. > I don't consider installing a desktop file in /usr/share/applications/ with an > Exec= line pointing to a file that you didn't install to be substantially > different than installing a python application with a missing file that causes > it not to show its UI on startup, with the exception that we have a better > chance of giving meaningful feedback to the user in the second case. > > > I really think the essence of this problem is that of dynamically-created > desktop files in ~/.local/share/applications/, and maybe our focus should be on > another solution to clean these things up periodically instead of suffering > performance degradation every time we perform a fairly common operation... > > Maybe we just need to tell the wine people to put a TryExec= line in their > desktop files.... I don't disagree in principle, but still it's a nice property for a system to make the wrong thing - executing a non-existent binary - impossible to happen, instead of delegating the problem to error UIs.
-- 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/glib/issues/543.