bug#35594: bug#36376: Application menu of desktop environment not automatically updated

[Maxim Cournoyer]

Hello Ludovic!

Ludovic Courtès <ludo@gnu.org> writes:

[...]

> So with this new patch it’s monitoring /var/guix/profiles/per-user/USER
> and /var/guix/profiles instead, which correctly detects changes.  It
> detects a bit “too much” (for instance, running ‘guix pull’ triggers the
> inotify hook because it changes
> /var/guix/profiles/per-user/USER/current-guix) but that’s probably OK.
>
> If you’re using GNOME, Xfce, or another GLib-based desktop environment,
> I’d welcome tests on the bare metal!  Just apply the patch on a checkout
> of ‘master’ and run:

I've now done so!  It works :-)!  I do have noticed something a bit off,
see the comments below.

[...]

> diff --git a/gnu/packages/patches/glib-appinfo-watch.patch 
> b/gnu/packages/patches/glib-appinfo-watch.patch
> new file mode 100644
> index 0000000000..638a5e0949
> --- /dev/null
> +++ b/gnu/packages/patches/glib-appinfo-watch.patch
> @@ -0,0 +1,92 @@
> +This patch lets GLib's GDesktopAppInfo API watch and notice changes
> +to the Guix user and system profiles.  That way, the list of available
> +applications shown by the desktop environment is immediately updated
> +when the user runs "guix install", "guix remove", or "guix system
> +reconfigure" (see <https://issues.guix.gnu.org/35594>).
> +
> +It does so by monitoring /var/guix/profiles (for changes to the system
> +profile) and /var/guix/profiles/per-user/USER (for changes to the user
> +profile) and crawling their share/applications sub-directory when
> +changes happen.
> +
> +diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c
> +index f1e2fdd..095c110 100644
> +--- a/gio/gdesktopappinfo.c
> ++++ b/gio/gdesktopappinfo.c
> +@@ -148,6 +148,7 @@ typedef struct
> +   gchar                      *alternatively_watching;
> +   gboolean                    is_config;
> +   gboolean                    is_setup;
> ++  gchar                      *guix_profile_watch_dir;
> +   GFileMonitor               *monitor;
> +   GHashTable                 *app_names;
> +   GHashTable                 *mime_tweaks;
> +@@ -180,6 +181,7 @@ desktop_file_dir_unref (DesktopFileDir *dir)
> +     {
> +       desktop_file_dir_reset (dir);
> +       g_free (dir->path);
> ++      g_free (dir->guix_profile_watch_dir);
> +       g_free (dir);
> +     }
> + }
> +@@ -204,6 +206,13 @@ desktop_file_dir_get_alternative_dir (DesktopFileDir 
> *dir)
> + {
> +   gchar *parent;
> +
> ++  /* If DIR is a profile, watch the specified directory--e.g.,
> ++   * /var/guix/profiles/per-user/$USER/ for the user profile.  Do not watch
> ++   * ~/.guix-profile or /run/current-system/profile because GFileMonitor 
> does
> ++   * not pass IN_DONT_FOLLOW and thus cannot notice any change.  */
> ++  if (dir->guix_profile_watch_dir != NULL)
> ++    return g_strdup (dir->guix_profile_watch_dir);
> ++
> +   /* If the directory itself exists then we need no alternative. */
> +   if (g_access (dir->path, R_OK | X_OK) == 0)
> +     return NULL;
> +@@ -249,11 +258,11 @@ desktop_file_dir_changed (GFileMonitor      *monitor,
> +    *
> +    * If this is a notification for a parent directory (because the
> +    * desktop directory didn't exist) then we shouldn't fire the signal
> +-   * unless something actually changed.
> ++   * unless something actually changed or it's in /var/guix/profiles.
> +    */
> +   g_mutex_lock (&desktop_file_dir_lock);
> +
> +-  if (dir->alternatively_watching)
> ++  if (dir->alternatively_watching && dir->guix_profile_watch_dir == NULL)
                                      ^^^^^^
                                      Why is this needed/desirable?

OK, I think I get it.  The API seems to imply that users (such as
gnome-shell) need to keep asking about a desktop directory entry as they
are otherwise "forgotten".  Since we are inserting entries at the level
of glib ourselves, these will never be asked for by gnome-shell, so must
not be cleared.  Is that correct?

> +     {
> +       gchar *alternative_dir;
> +
> +@@ -1555,6 +1564,32 @@ desktop_file_dirs_lock (void)
> +       for (i = 0; dirs[i]; i++)
> +         g_ptr_array_add (desktop_file_dirs, desktop_file_dir_new (dirs[i]));
> +
> ++      {
> ++        /* Monitor the system and user profile under /var/guix/profiles and
> ++         * treat modifications to them as if they were modifications to 
> their
> ++         * /share sub-directory.  */
> ++        const gchar *user;
> ++        DesktopFileDir *system_profile_dir, *user_profile_dir;
> ++
> ++        system_profile_dir =
> ++          desktop_file_dir_new ("/var/guix/profiles/system/profile/share");
> ++        system_profile_dir->guix_profile_watch_dir = g_strdup 
> ("/var/guix/profiles");
> ++        g_ptr_array_add (desktop_file_dirs, desktop_file_dir_ref 
> (system_profile_dir));
> ++
> ++        user = g_get_user_name ();

This seems to get the user of the running glib application; e.g. for
GNOME Shell it returns 'gdm'...

> ++        if (user != NULL)
> ++          {
> ++            gchar *profile_dir, *user_data_dir;
> ++
> ++            profile_dir = g_build_filename ("/var/guix/profiles/per-user", 
> user, NULL);
> ++            user_data_dir = g_build_filename (profile_dir, "guix-profile", 
> "share", NULL);
> ++            user_profile_dir = desktop_file_dir_new (user_data_dir);
> ++            user_profile_dir->guix_profile_watch_dir = profile_dir;
> ++            g_ptr_array_add (desktop_file_dirs, desktop_file_dir_ref 
> (user_profile_dir));
> ++            g_free (user_data_dir);
> ++          }
> ++      }
> ++

...which means the above puts the watch on the
"/var/guix/profiles/per-user/gdm" directory, which doesn't exist.

sudo strace -f -s200 -p$(pgrep gnome-shell | head -n1) reports entries
such as:

--8<---------------cut here---------------start------------->8---
92   00:48:47 poll([{fd=6, events=POLLIN}, {fd=7, events=POLLIN}, {fd=9, 
events=POLLIN}, {fd=19, events=POLLIN}, {fd=28, events=POLLIN}], 5, -1 
<unfinished ...>
390   00:48:47 <... poll resumed>)      = 0 (Timeout)
390   00:48:47 inotify_add_watch(17, "/var/guix/profiles/per-user/gdm", 
IN_MODIFY|IN_ATTRIB|IN_CLOSE_WRITE|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT|IN_ONLYDIR)
 = -1 ENOENT (No such file or directory)
390   00:48:47 poll([{fd=3, events=POLLIN}, {fd=17, events=POLLIN}], 2, 3996) = 
0 (Timeout)
390   00:48:51 inotify_add_watch(17, "/var/guix/profiles/per-user/gdm", 
IN_MODIFY|IN_ATTRIB|IN_CLOSE_WRITE|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT|IN_ONLYDIR)
 = -1 ENOENT (No such file or directory)
390   00:48:51 poll([{fd=3, events=POLLIN}, {fd=17, events=POLLIN}], 2, 3998) = 
0 (Timeout)
390   00:48:55 inotify_add_watch(17, "/var/guix/profiles/per-user/gdm", 
IN_MODIFY|IN_ATTRIB|IN_CLOSE_WRITE|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT|IN_ONLYDIR)
 = -1 ENOENT (No such file or directory)
--8<---------------cut here---------------end--------------->8---

The fallback mechanism should have been disabled in
desktop_file_dir_changed (dir->guix-profile_watch_dir != NULL so
desktop_file_dir_get_alternative_dir doesn't get called), so it seems
this shouldn't work.

Could it be that the watches used by glib implement a recursive
inotify-based watch and that it works because any changes under the
/var/guix/profiles directory get picked up, including those of user
profiles.  The sources seem to suggest so, for example in
gio/inotify/inotify-path.c.  If that is the case, it could lead to a new
interesting problems: applications from other users could end up being
shown inadvertently.

I'll try to validate the above hypothesis, but this takes time.

In any case, the current situation is already an improvement, so thank
you for your efforts!

Maxim