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

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

[Ludovic Courtès]

This is not news to us, but as
<https://distrowatch.com/weekly.php?issue=20190624#guixsd> notes, the
application menu of desktop environments is not automatically updated
when a package is installed or removed.  It’d be great if we could
somehow notify the desktop environment.

Ludo’.

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

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

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

> This is not news to us, but as
> <https://distrowatch.com/weekly.php?issue=20190624#guixsd> notes, the
> application menu of desktop environments is not automatically updated
> when a package is installed or removed.  It’d be great if we could
> somehow notify the desktop environment.

We’ve investigated today on IRC with Maxim and Leo P. and here’s the
summary of our findings:

  • GNOME Shell, in ‘appDisplay.js’, “listens” to the
    ‘installed-changed’ GLib signals and uses that to rebuild its
    application menu.

  • In ‘shell-app-system.c’, ‘installed-changed’ is emitted when the
    GAppInfoMonitor emits ‘changed’:

      monitor = g_app_info_monitor_get ();
      g_signal_connect (monitor, "changed", G_CALLBACK (installed_changed), self);
      installed_changed (monitor, self);

  • GLib emits the ‘changed’ signal when ‘g_app_info_monitor_fire’ is
    called from ‘desktop_file_dir_changed’, itself called when one of
    the directories in $XDG_DATA_DIRS (among others) changes.  It uses
    ‘GFileMonitor’ under the hood, which is essentially inotify.

The GLib patch below is an attempt to monitor ~/.guix-profile and to
treat changes to that symlink as if they were changes to
~/.guix-profile/share/applications (which contains ‘.desktop’ files.)
It actually builds but I haven’t tested it yet.  :-)

WDYT?

If we take that route, we could add a ‘replacement’ for GLib.

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2821 bytes --]

diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c
index f1e2fdd..96dcc32 100644
--- a/gio/gdesktopappinfo.c
+++ b/gio/gdesktopappinfo.c
@@ -161,6 +161,7 @@ static DesktopFileDir *desktop_file_dir_user_config = NULL;  /* (owned) */
 static DesktopFileDir *desktop_file_dir_user_data = NULL;  /* (owned) */
 static GMutex          desktop_file_dir_lock;
 static const gchar    *gio_launch_desktop_path = NULL;
+static GFileMonitor   *guix_profile_monitor = NULL;
 
 /* Monitor 'changed' signal handler {{{2 */
 static void desktop_file_dir_reset (DesktopFileDir *dir);
@@ -230,6 +231,22 @@ desktop_file_dir_get_alternative_dir (DesktopFileDir *dir)
   return parent;
 }
 
+static void
+guix_profile_changed  (GFileMonitor      *monitor,
+                       GFile             *file,
+                       GFile             *other_file,
+                       GFileMonitorEvent  event_type,
+                       gpointer           user_data)
+{
+  DesktopFileDir *dir = user_data;
+
+  desktop_file_dir_reset (dir);
+
+  /* When ~/.guix-profile changes, emit the 'changed' signal so everyone
+     knows.  */
+  g_app_info_monitor_fire ();
+}
+
 static void
 desktop_file_dir_changed (GFileMonitor      *monitor,
                           GFile             *file,
@@ -1531,6 +1548,7 @@ desktop_file_dirs_lock (void)
 
   if (desktop_file_dirs == NULL || desktop_file_dirs->len == 0)
     {
+      const gchar *home;
       const char * const *dirs;
       gint i;
 
@@ -1555,6 +1573,27 @@ desktop_file_dirs_lock (void)
       for (i = 0; dirs[i]; i++)
         g_ptr_array_add (desktop_file_dirs, desktop_file_dir_new (dirs[i]));
 
+      home = g_get_home_dir ();
+      if (guix_profile_monitor == NULL && home != NULL)
+        {
+          DesktopFileDir *dir;
+          const gchar *profile, *data_dir;
+          profile = g_build_filename (home, ".guix-profile", NULL);
+          data_dir = g_build_filename (profile, "share", NULL);
+          dir = desktop_file_dir_new (data_dir);
+
+          /* Monitor ~/.guix-profile and treat modifications to
+             ~/.guix-profile as if they were modifications to
+             ~/.guix-profile/share.  */
+          guix_profile_monitor =
+            g_local_file_monitor_new_in_worker (profile, FALSE, G_FILE_MONITOR_NONE,
+                                                guix_profile_changed,
+                                                desktop_file_dir_ref (dir),
+                                                closure_notify_cb, NULL);
+
+          g_ptr_array_add (desktop_file_dirs, desktop_file_dir_ref (dir));
+        }
+
       /* The list of directories will never change after this, unless
        * g_get_user_config_dir() changes due to %G_TEST_OPTION_ISOLATE_DIRS. */
       desktop_file_dirs_config_dir = user_config_dir;

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

[Leo Prikler]

Hi Ludo,

Am Dienstag, den 03.11.2020, 23:46 +0100 schrieb Ludovic Courtès:
> Ludovic Courtès <ludo@gnu.org> skribis:
> 
> > This is not news to us, but as
> > <https://distrowatch.com/weekly.php?issue=20190624#guixsd> notes,
> > the
> > application menu of desktop environments is not automatically
> > updated
> > when a package is installed or removed.  It’d be great if we could
> > somehow notify the desktop environment.
> 
> We’ve investigated today on IRC with Maxim and Leo P. and here’s the
> summary of our findings:
Seeing my name thrown around more and more lately makes me blush a
little.
> [...]
> 
> The GLib patch below is an attempt to monitor ~/.guix-profile and to
> treat changes to that symlink as if they were changes to
> ~/.guix-profile/share/applications (which contains ‘.desktop’ files.)
> It actually builds but I haven’t tested it yet.  :-)
> 
> WDYT?
Not having tested it either, I think that we should also listen on
/run/current-system/ (if it exists), so that changes to the system as
done by `reconfigure` are picked up.  This includes most importantly
changes to the GNOME ecosystem itself.

Regards, Leo

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

[Maxim Cournoyer]

Hello Ludovic and Leo,

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> Hi Ludo,
>
> Am Dienstag, den 03.11.2020, 23:46 +0100 schrieb Ludovic Courtès:
>> Ludovic Courtès <ludo@gnu.org> skribis:
>> 
>> > This is not news to us, but as
>> > <https://distrowatch.com/weekly.php?issue=20190624#guixsd> notes,
>> > the
>> > application menu of desktop environments is not automatically
>> > updated
>> > when a package is installed or removed.  It’d be great if we could
>> > somehow notify the desktop environment.
>> 
>> We’ve investigated today on IRC with Maxim and Leo P. and here’s the
>> summary of our findings:
> Seeing my name thrown around more and more lately makes me blush a
> little.
>> [...]
>> 
>> The GLib patch below is an attempt to monitor ~/.guix-profile and to
>> treat changes to that symlink as if they were changes to
>> ~/.guix-profile/share/applications (which contains ‘.desktop’ files.)
>> It actually builds but I haven’t tested it yet.  :-)
>> 
>> WDYT?

Pretty cool!  It'd keep on working even if we even clean up /etc/profile
from the hardcoded defauts environment variables (say, if we merge one
of the proposed solution of https://issues.guix.gnu.org/22138).

> Not having tested it either, I think that we should also listen on
> /run/current-system/ (if it exists), so that changes to the system as
> done by `reconfigure` are picked up.  This includes most importantly
> changes to the GNOME ecosystem itself.

I agree that it seems useful and important to add a monitor on the
/run/current-system as well.

Thank you!

Maxim

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

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

Hi!

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

> Am Dienstag, den 03.11.2020, 23:46 +0100 schrieb Ludovic Courtès:
>> Ludovic Courtès <ludo@gnu.org> skribis:
>> 
>> > This is not news to us, but as
>> > <https://distrowatch.com/weekly.php?issue=20190624#guixsd> notes,
>> > the
>> > application menu of desktop environments is not automatically
>> > updated
>> > when a package is installed or removed.  It’d be great if we could
>> > somehow notify the desktop environment.
>> 
>> We’ve investigated today on IRC with Maxim and Leo P. and here’s the
>> summary of our findings:
> Seeing my name thrown around more and more lately makes me blush a
> little.

Heheh, you provided useful hints!

>> The GLib patch below is an attempt to monitor ~/.guix-profile and to
>> treat changes to that symlink as if they were changes to
>> ~/.guix-profile/share/applications (which contains ‘.desktop’ files.)
>> It actually builds but I haven’t tested it yet.  :-)
>> 
>> WDYT?
> Not having tested it either, I think that we should also listen on
> /run/current-system/ (if it exists), so that changes to the system as
> done by `reconfigure` are picked up.  This includes most importantly
> changes to the GNOME ecosystem itself.

Like I wrote in the related bug, I think /run/current-system is less
important because it typically doesn’t change much:

  https://issues.guix.gnu.org/35594#6

But I guess I’m also being a bit lazy…

Anyhow, I’ve tested the patch in ‘guix system vm
gnu/system/examples/desktop.tmpl’, with a ‘glib’ replacement as shown
below.  I strace’d the user’s gnome-shell and ran:

  ln -s /run/current-system/profile ~/.guix-profile

The trace showed that this led gnome-shell to traverse files in
~/.guix-profile/share (not just the applications/ sub-directory.)

I wanted to test ‘guix install’ for real, which meant doing it on ‘guix
system vm-image’, but that took too long; so I tried ‘disk-image -t
qcow2’ instead but partition.img.drv fails.  So I haven’t been able to
actually test with ‘guix install’.

Anyway, here’s the Guix patch.  You need to drop the GLib patch in the
right place.

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2120 bytes --]

diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index bba9461d44..1c0de6eaed 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -181,6 +181,7 @@ shared NFS home directories.")
   (package
    (name "glib")
    (version "2.62.6")
+   (replacement glib-with-gio-patch)
    (source (origin
             (method url-fetch)
             (uri (string-append "mirror://gnome/sources/"
@@ -387,6 +388,14 @@ dynamic loading, and an object system.")
    (home-page "https://developer.gnome.org/glib/")
    (license license:lgpl2.1+)))
 
+(define glib-with-gio-patch
+  (package
+    (inherit glib)
+    (source (origin
+              (inherit (package-source glib))
+              (patches (append (search-patches "glib-appinfo-watch.patch")
+                               (origin-patches (package-source glib))))))))
+
 (define-public glib-with-documentation
   ;; glib's doc must be built in a separate package since it requires gtk-doc,
   ;; which in turn depends on glib.
diff --git a/gnu/system/examples/desktop.tmpl b/gnu/system/examples/desktop.tmpl
index 716b9feb8d..4797111c67 100644
--- a/gnu/system/examples/desktop.tmpl
+++ b/gnu/system/examples/desktop.tmpl
@@ -4,7 +4,7 @@
 
 (use-modules (gnu) (gnu system nss))
 (use-service-modules desktop xorg)
-(use-package-modules certs gnome)
+(use-package-modules certs gnome linux)
 
 (operating-system
   (host-name "antelope")
@@ -57,7 +57,8 @@
                      ;; for HTTPS access
                      nss-certs
                      ;; for user mounts
-                     gvfs)
+                     gvfs
+                     strace)
                     %base-packages))
 
   ;; Add GNOME and Xfce---we can choose at the log-in screen
@@ -65,7 +66,6 @@
   ;; include the X11 log-in service, networking with
   ;; NetworkManager, and more.
   (services (append (list (service gnome-desktop-service-type)
-                          (service xfce-desktop-service-type)
                           (set-xorg-configuration
                            (xorg-configuration
                             (keyboard-layout keyboard-layout))))

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

[Maxim Cournoyer]

[-- Attachment #1: Type: text/plain, Size: 2549 bytes --]

Hi Ludovic!

> +static void
> +guix_profile_changed  (GFileMonitor      *monitor,
> +                       GFile             *file,
> +                       GFile             *other_file,
> +                       GFileMonitorEvent  event_type,
> +                       gpointer           user_data)
> +{
> +  DesktopFileDir *dir = user_data;
> +
> +  desktop_file_dir_reset (dir);
> +
> +  /* When ~/.guix-profile changes, emit the 'changed' signal so everyone
> +     knows.  */
> +  g_app_info_monitor_fire ();
> +}
> +
>  static void
>  desktop_file_dir_changed (GFileMonitor      *monitor,
>                            GFile             *file,
> @@ -1531,6 +1548,7 @@ desktop_file_dirs_lock (void)
>
>    if (desktop_file_dirs == NULL || desktop_file_dirs->len == 0)
>      {
> +      const gchar *home;
>        const char * const *dirs;
>        gint i;
>
> @@ -1555,6 +1573,27 @@ desktop_file_dirs_lock (void)
>        for (i = 0; dirs[i]; i++)
>          g_ptr_array_add (desktop_file_dirs, desktop_file_dir_new (dirs[i]));
>
> +      home = g_get_home_dir ();
> +      if (guix_profile_monitor == NULL && home != NULL)
> +        {
> +          DesktopFileDir *dir;
> +          const gchar *profile, *data_dir;
> +          profile = g_build_filename (home, ".guix-profile", NULL);
> +          data_dir = g_build_filename (profile, "share", NULL);
> +          dir = desktop_file_dir_new (data_dir);
> +
> +          /* Monitor ~/.guix-profile and treat modifications to
> +             ~/.guix-profile as if they were modifications to
> +             ~/.guix-profile/share.  */
> +          guix_profile_monitor =
> +            g_local_file_monitor_new_in_worker (profile, FALSE, G_FILE_MONITOR_NONE,
> +                                                guix_profile_changed,
> +                                                desktop_file_dir_ref (dir),
> +                                                closure_notify_cb, NULL);
> +
> +          g_ptr_array_add (desktop_file_dirs, desktop_file_dir_ref (dir));
> +        }
> +
>        /* The list of directories will never change after this, unless
>         * g_get_user_config_dir() changes due to %G_TEST_OPTION_ISOLATE_DIRS. */
>        desktop_file_dirs_config_dir = user_config_dir;

I think the attached patch is an improvement over the above, by also
supporting /run/current-system/share, being a bit simpler, and not
adding extra monitors.

I've reused the cool trick of passing FALSE to
g_local_file_monitor_new_in_worker to force it to use its polling mode
fallback.


[-- Attachment #2: glib-appinfo-watch.patch --]
[-- Type: text/x-patch, Size: 2697 bytes --]

From a79645c565e56ac201e66936d9f9883ad9387b06 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Thu, 5 Nov 2020 00:24:29 -0500
Subject: [PATCH] gdesktopappinfo: Fix monitoring of a Guix profile
 XDG_DATA_DIR.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes <https://issues.guix.gnu.org/35594>.

Treat the $HOME/.guix-profile/share and /run/current-system/share
XDG_DATA_DIRS file names specially so that the inotify-based monitors
placed by GLib monitor their parent link rather than an immutable
directory.

Co-authored by Ludovic Courtès <ļudo@gnu.org>.
---
 gio/gdesktopappinfo.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c
index f1e2fdd65..e659034d5 100644
--- a/gio/gdesktopappinfo.c
+++ b/gio/gdesktopappinfo.c
@@ -1380,7 +1380,8 @@ closure_notify_cb (gpointer  data,
 static void
 desktop_file_dir_init (DesktopFileDir *dir)
 {
-  const gchar *watch_dir;
+  const gchar *watch_dir, *guix_profile_data, *system_profile_data;
+  gboolean is_directory = TRUE;
 
   g_assert (!dir->is_setup);
 
@@ -1390,13 +1391,25 @@ desktop_file_dir_init (DesktopFileDir *dir)
   dir->alternatively_watching = desktop_file_dir_get_alternative_dir (dir);
   watch_dir = dir->alternatively_watching ? dir->alternatively_watching : dir->path;
 
+  /* Workaround for Guix: watch the link of a profile, not its
+   * immutable 'share' sub-directory. */
+  guix_profile_data = g_build_filename (g_get_home_dir (), ".guix-profile", "share", NULL);
+  system_profile_data = "/run/current-system/share";
+
+  if (g_str_equal (watch_dir, guix_profile_data)
+      || g_str_equal (watch_dir, system_profile_data))
+  {
+      is_directory = FALSE;
+      watch_dir = g_path_get_dirname (watch_dir);
+  }
+
   /* There is a very thin race here if the watch_dir has been _removed_
    * between when we checked for it and when we establish the watch.
    * Removes probably don't happen in usual operation, and even if it
    * does (and we catch the unlikely race), the only degradation is that
    * we will fall back to polling.
    */
-  dir->monitor = g_local_file_monitor_new_in_worker (watch_dir, TRUE, G_FILE_MONITOR_NONE,
+  dir->monitor = g_local_file_monitor_new_in_worker (watch_dir, is_directory, G_FILE_MONITOR_NONE,
                                                      desktop_file_dir_changed,
                                                      desktop_file_dir_ref (dir),
                                                      closure_notify_cb, NULL);
-- 
2.28.0


[-- Attachment #3: Type: text/plain, Size: 165 bytes --]


It's untested because 'guix vm' is taking more time to complete than I'm
willing to wait for in the middle of the night :-).  I'll try it
tomorrow.

Thanks,

Maxim

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

[Ludovic Courtès]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> From a79645c565e56ac201e66936d9f9883ad9387b06 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Thu, 5 Nov 2020 00:24:29 -0500
> Subject: [PATCH] gdesktopappinfo: Fix monitoring of a Guix profile
>  XDG_DATA_DIR.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Fixes <https://issues.guix.gnu.org/35594>.
>
> Treat the $HOME/.guix-profile/share and /run/current-system/share
> XDG_DATA_DIRS file names specially so that the inotify-based monitors
> placed by GLib monitor their parent link rather than an immutable
> directory.
>
> Co-authored by Ludovic Courtès <ļudo@gnu.org>.
                                  ^
This is LATIN SMALL LETTER L WITH CEDILLA.  :-)

> ---
>  gio/gdesktopappinfo.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

I like that it’s short and sweet, nice!

Thanks,
Ludo’.

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

[Maxim Cournoyer]

Hey Ludovic,

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

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> From a79645c565e56ac201e66936d9f9883ad9387b06 Mon Sep 17 00:00:00 2001
>> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> Date: Thu, 5 Nov 2020 00:24:29 -0500
>> Subject: [PATCH] gdesktopappinfo: Fix monitoring of a Guix profile
>>  XDG_DATA_DIR.
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> Fixes <https://issues.guix.gnu.org/35594>.
>>
>> Treat the $HOME/.guix-profile/share and /run/current-system/share
>> XDG_DATA_DIRS file names specially so that the inotify-based monitors
>> placed by GLib monitor their parent link rather than an immutable
>> directory.
>>
>> Co-authored by Ludovic Courtès <ļudo@gnu.org>.
>                                   ^
> This is LATIN SMALL LETTER L WITH CEDILLA.  :-)
>
>> ---
>>  gio/gdesktopappinfo.c | 17 +++++++++++++++--
>>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> I like that it’s short and sweet, nice!

I thought too, but it doesn't work, because these entries never make it
to XDG_DATA_DIRS in the first place, at least for the system profile.

I also gave a try to 1) a modified version of your patch that added
support for /run/current-system and 2) your patch unmodified with
inconclusive results so far.

I did see things being scanned in response to /run/current-system
changing, but I believe this was because I goofed up the dir path to
"/run/current-system/share" (it should have been
""/run/current-system/profile/share"), and there's some code
gdesktopappinfo.c (desktop_file_dir_get_alternative_dir) that sets the
monitored directory to a parent when the directory doesn't exist.

In either version, strace failed to show any activity upon recreating
the ~/.guix-profile (that is /root/.guix-profile since I was testing as
root in the VM) link, different to what you had found.

So, to be continued...

Maxim

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

[Ludovic Courtès]

Hi Maxim,

So I went further to test the initial GLib patch I posted in “real
conditions”.

‘guix system vm-image’ takes an awful lot of time for big images; I
patiently waited for completion for an image of ‘desktop.tmpl’, but then
its root file system turned out to be too small for me to run ‘guix
install’.

So instead I tried this:

  1. ‘guix system vm desktop.tmpl’;

  2. ‘guix install inkscape -p /tmp/test’ on my actual machine;

  3. Start the GNOME VM created above;

  4. Inside the VM, search for Inkscape, notice it’s not there;

  5. Inside the VM, have ~/.guix-profile point to the profile created on
     the host above, and notice that Inkscape becomes available.

Video!

  mpv http://web.fdn.fr/~lcourtes/tmp/gnome-desktop-app-detection.webm

So I think we should include this fix in 1.2.  What do people think?

The main downside is extra grafting, which means extra disk and CPU
usage for our users.  However, libx11 (11K dependents) is also grafted,
so it shouldn’t make things worse (GLib has 6K dependents).

Maxim, did you have a variant of the GLib patch that also checks
/run/current-system?

Thanks,
Ludo’.

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

[zimoun]

On Tue, 10 Nov 2020 at 16:25, Ludovic Courtès <ludo@gnu.org> wrote:

> Video!

Héhé!  Nice :-)

> So I think we should include this fix in 1.2.  What do people think?

You mean only the initial patch.  If it works in "real conditions" for
bob on 'antelope' (desktop.tmpl), then it would nice to have for 1.2,


> The main downside is extra grafting, which means extra disk and CPU
> usage for our users.  However, libx11 (11K dependents) is also grafted,
> so it shouldn’t make things worse (GLib has 6K dependents).

No free lunch. :-)
Is it really slow or acceptable?


All the best,
simon

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

[Ludovic Courtès]

Hi,

zimoun <zimon.toutoune@gmail.com> skribis:

> On Tue, 10 Nov 2020 at 16:25, Ludovic Courtès <ludo@gnu.org> wrote:

[...]

>> The main downside is extra grafting, which means extra disk and CPU
>> usage for our users.  However, libx11 (11K dependents) is also grafted,
>> so it shouldn’t make things worse (GLib has 6K dependents).
>
> No free lunch. :-)
> Is it really slow or acceptable?

If you’re installing a graphical applications, it’s already being
grafted, so the extra graft doesn’t make a difference.

Non-graphical applications that depend on GLib will now need to be
grafted, but I think there aren’t many of them, and they too might also
be grafted for other reasons.

Ludo’.

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

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

Hi!

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

> So I went further to test the initial GLib patch I posted in “real
> conditions”.

I did some more testing using a similar workflow: this time I repeatedly
changed the profile (installing/removing packages), which showed that
the first patch was buggy (it would only notice the creation of
~/.guix-profile and nothing beyond that).

This is in part due to the fact that ‘GFileMonitor’ follows symlinks (it
doesn’t pass the ‘IN_DONT_FOLLOW’ inotify flag).  Thus, when it monitors
~/.guix-profile, it’s really monitoring /gnu/store/…-profile, which
never changes.

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:

  sudo -E ./pre-inst-env guix system reconfigure …

If there are no objections, I’d like to apply this patch shortly so we
can go ahead with a last round of pre-release tests.

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: the patch --]
[-- Type: text/x-patch, Size: 7112 bytes --]

From 7d03b2b823951219ece46c7f34e25ae36a8ba12c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Thu, 12 Nov 2020 16:35:24 +0100
Subject: [PATCH] gnu: glib: Graft patch to detect changes to the installed
 applications.

Fixes <https://bugs.gnu.org/35594>.
Reported by sirgazil <sirgazil@zoho.com> and others.

* gnu/packages/patches/glib-appinfo-watch.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/glib.scm (glib)[replacement]: New field.
(glib-with-gio-patch): New variable.
(glib-with-documentation): Use 'package/inherit'.
---
 gnu/local.mk                                  |  1 +
 gnu/packages/glib.scm                         | 14 ++-
 gnu/packages/patches/glib-appinfo-watch.patch | 92 +++++++++++++++++++
 3 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 gnu/packages/patches/glib-appinfo-watch.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index d5a13cbdbd..2301a04d2f 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1050,6 +1050,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/ghostscript-no-header-id.patch		\
   %D%/packages/patches/ghostscript-no-header-uuid.patch		\
   %D%/packages/patches/ghostscript-no-header-creationdate.patch \
+  %D%/packages/patches/glib-appinfo-watch.patch			\
   %D%/packages/patches/glib-tests-timer.patch			\
   %D%/packages/patches/glibc-CVE-2018-11236.patch		\
   %D%/packages/patches/glibc-CVE-2018-11237.patch		\
diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index 901222476a..43523e516d 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -181,6 +181,7 @@ shared NFS home directories.")
   (package
    (name "glib")
    (version "2.62.6")
+   (replacement glib-with-gio-patch)
    (source (origin
             (method url-fetch)
             (uri (string-append "mirror://gnome/sources/"
@@ -387,11 +388,20 @@ dynamic loading, and an object system.")
    (home-page "https://developer.gnome.org/glib/")
    (license license:lgpl2.1+)))
 
+(define glib-with-gio-patch
+  ;; GLib with a fix for <https://bugs.gnu.org/35594>.
+  ;; TODO: Fold into 'glib' above in the next rebuild cycle.
+  (package
+    (inherit glib)
+    (source (origin
+              (inherit (package-source glib))
+              (patches (cons (search-patch "glib-appinfo-watch.patch")
+                             (origin-patches (package-source glib))))))))
+
 (define-public glib-with-documentation
   ;; glib's doc must be built in a separate package since it requires gtk-doc,
   ;; which in turn depends on glib.
-  (package
-    (inherit glib)
+  (package/inherit glib
     (properties (alist-delete 'hidden? (package-properties glib)))
     (outputs (cons "doc" (package-outputs glib))) ; 20 MiB of GTK-Doc reference
     (native-inputs
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)
+     {
+       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 ();
++        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);
++          }
++      }
++
+       /* The list of directories will never change after this, unless
+        * g_get_user_config_dir() changes due to %G_TEST_OPTION_ISOLATE_DIRS. */
+       desktop_file_dirs_config_dir = user_config_dir;
-- 
2.29.2

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

[Ludovic Courtès]

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

>>From 7d03b2b823951219ece46c7f34e25ae36a8ba12c Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
> Date: Thu, 12 Nov 2020 16:35:24 +0100
> Subject: [PATCH] gnu: glib: Graft patch to detect changes to the installed
>  applications.
>
> Fixes <https://bugs.gnu.org/35594>.
> Reported by sirgazil <sirgazil@zoho.com> and others.
>
> * gnu/packages/patches/glib-appinfo-watch.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/glib.scm (glib)[replacement]: New field.
> (glib-with-gio-patch): New variable.
> (glib-with-documentation): Use 'package/inherit'.

Pushed as ae10ec441aa524bf267f9cefd4a319b44d0b8b44 (version-1.2.0).

Ludo’.

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

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

[Ludovic Courtès]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> 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.

Thanks for testing!

>> +@@ -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?

As the comment above states it, the ‘if’ is here so that the “changed”
signal is not fired when we’re just watching a parent directory.

However, in our case, ‘dir->alternatively_watching != NULL’ possibly
means that we’re watching a Guix profile, in which case we do want to
fire the “changed” signal.

This “&&” allows us to disambiguate between “watching a parent
directory” and “watching a Guix profile”.

>> +@@ -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.

Yes, that profile is typically never populated.

> sudo strace -f -s200 -p$(pgrep gnome-shell | head -n1) reports entries
> such as:
>
> 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)
>
> 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.

What shouldn’t work?

We keep adding a watch on a non-existent directory, but I think that’s
fine.

> 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.

We’re watching /var/guix/profiles (for the system profile) and
/var/guix/profiles/per-user/USER, but .desktop files are loaded from the
user’s own profile, so that should be fine.

Thanks a lot for looking into this!

Ludo’.

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

[Maxim Cournoyer]

Hello Ludovic!

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

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>>> +@@ -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?
>
> As the comment above states it, the ‘if’ is here so that the “changed”
> signal is not fired when we’re just watching a parent directory.
>
> However, in our case, ‘dir->alternatively_watching != NULL’ possibly
> means that we’re watching a Guix profile, in which case we do want to
> fire the “changed” signal.
>
> This “&&” allows us to disambiguate between “watching a parent
> directory” and “watching a Guix profile”.

Yes, this makes sense.  I got confused by the wording of the (existing)
comment that mentions "unless something actually changed".  I used to
think this meant the contents of whatever directory is being watched,
but looking more closely, it's really just about if the parent directory
of a non-existing data directory changed...  Makes senses, we don't want
that.

>>> +@@ -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.
>
> Yes, that profile is typically never populated.

Ah, I now understand the source of my confusion; there are multiple
gnome-shell instances (components?) running, one apparently started by
the greeter (GDM), which doesn't have a profile, and others started by
the users which logged in.

Thanks again for the fix and explanations!

Maxim