From debbugs-submit-bounces@debbugs.gnu.org Tue Nov 17 04:09:01 2020 Received: (at 35594) by debbugs.gnu.org; 17 Nov 2020 09:09:01 +0000 Received: from localhost ([127.0.0.1]:58279 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kewyz-0002o8-7f for submit@debbugs.gnu.org; Tue, 17 Nov 2020 04:09:01 -0500 Received: from eggs.gnu.org ([209.51.188.92]:41954) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kewyx-0002nq-08; Tue, 17 Nov 2020 04:08:59 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]:50810) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kewyp-0002hE-Pi; Tue, 17 Nov 2020 04:08:51 -0500 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=53968 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1kewyp-0004n9-Bb; Tue, 17 Nov 2020 04:08:51 -0500 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Maxim Cournoyer Subject: Re: bug#36376: Application menu of desktop environment not automatically updated References: <871rzhn483.fsf@gnu.org> <87o8keawju.fsf@gnu.org> <87d00swboe.fsf@gmail.com> <878sbexylp.fsf@gnu.org> <87r1p6tiut.fsf@gmail.com> <87lff9nsl0.fsf@gnu.org> <87d00iin6w.fsf@gnu.org> <875z64d1hr.fsf@gmail.com> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 27 Brumaire an 229 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Tue, 17 Nov 2020 10:08:49 +0100 In-Reply-To: <875z64d1hr.fsf@gmail.com> (Maxim Cournoyer's message of "Mon, 16 Nov 2020 23:57:20 -0500") Message-ID: <87eekstkny.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 35594 Cc: Leo Prikler , 35594@debbugs.gnu.org, 36376@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi Maxim, Maxim Cournoyer skribis: > Ludovic Court=C3=A8s writes: > > [...] > >> So with this new patch it=E2=80=99s monitoring /var/guix/profiles/per-us= er/USER >> and /var/guix/profiles instead, which correctly detects changes. It >> detects a bit =E2=80=9Ctoo much=E2=80=9D (for instance, running =E2=80= =98guix pull=E2=80=99 triggers the >> inotify hook because it changes >> /var/guix/profiles/per-user/USER/current-guix) but that=E2=80=99s probab= ly OK. >> >> If you=E2=80=99re using GNOME, Xfce, or another GLib-based desktop envir= onment, >> I=E2=80=99d welcome tests on the bare metal! Just apply the patch on a = checkout >> of =E2=80=98master=E2=80=99 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 *moni= tor, >> + * >> + * 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 =3D= =3D NULL) > ^^^^^^ > Why is this needed/desirable? As the comment above states it, the =E2=80=98if=E2=80=99 is here so that th= e =E2=80=9Cchanged=E2=80=9D signal is not fired when we=E2=80=99re just watching a parent directory. However, in our case, =E2=80=98dir->alternatively_watching !=3D NULL=E2=80= =99 possibly means that we=E2=80=99re watching a Guix profile, in which case we do want = to fire the =E2=80=9Cchanged=E2=80=9D signal. This =E2=80=9C&&=E2=80=9D allows us to disambiguate between =E2=80=9Cwatchi= ng a parent directory=E2=80=9D and =E2=80=9Cwatching a Guix profile=E2=80=9D. >> +@@ -1555,6 +1564,32 @@ desktop_file_dirs_lock (void) >> + for (i =3D 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/profile= s and >> ++ * treat modifications to them as if they were modifications t= o their >> ++ * /share sub-directory. */ >> ++ const gchar *user; >> ++ DesktopFileDir *system_profile_dir, *user_profile_dir; >> ++ >> ++ system_profile_dir =3D >> ++ desktop_file_dir_new ("/var/guix/profiles/system/profile/sha= re"); >> ++ system_profile_dir->guix_profile_watch_dir =3D g_strdup ("/var= /guix/profiles"); >> ++ g_ptr_array_add (desktop_file_dirs, desktop_file_dir_ref (syst= em_profile_dir)); >> ++ >> ++ user =3D 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 !=3D NULL) >> ++ { >> ++ gchar *profile_dir, *user_data_dir; >> ++ >> ++ profile_dir =3D g_build_filename ("/var/guix/profiles/per-= user", user, NULL); >> ++ user_data_dir =3D g_build_filename (profile_dir, "guix-pro= file", "share", NULL); >> ++ user_profile_dir =3D desktop_file_dir_new (user_data_dir); >> ++ user_profile_dir->guix_profile_watch_dir =3D 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=3D6, events=3DPOLLIN}, {fd=3D7, events=3DPOLLIN},= {fd=3D9, events=3DPOLLIN}, {fd=3D19, events=3DPOLLIN}, {fd=3D28, events=3D= POLLIN}], 5, -1 > 390 00:48:47 <... poll resumed>) =3D 0 (Timeout) > 390 00:48:47 inotify_add_watch(17, "/var/guix/profiles/per-user/gdm", I= N_MODIFY|IN_ATTRIB|IN_CLOSE_WRITE|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DE= LETE|IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT|IN_ONLYDIR) =3D -1 ENOENT (No s= uch file or directory) > 390 00:48:47 poll([{fd=3D3, events=3DPOLLIN}, {fd=3D17, events=3DPOLLIN= }], 2, 3996) =3D 0 (Timeout) > 390 00:48:51 inotify_add_watch(17, "/var/guix/profiles/per-user/gdm", I= N_MODIFY|IN_ATTRIB|IN_CLOSE_WRITE|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DE= LETE|IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT|IN_ONLYDIR) =3D -1 ENOENT (No s= uch file or directory) > 390 00:48:51 poll([{fd=3D3, events=3DPOLLIN}, {fd=3D17, events=3DPOLLIN= }], 2, 3998) =3D 0 (Timeout) > 390 00:48:55 inotify_add_watch(17, "/var/guix/profiles/per-user/gdm", I= N_MODIFY|IN_ATTRIB|IN_CLOSE_WRITE|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DE= LETE|IN_DELETE_SELF|IN_MOVE_SELF|IN_UNMOUNT|IN_ONLYDIR) =3D -1 ENOENT (No s= uch file or directory) > > The fallback mechanism should have been disabled in > desktop_file_dir_changed (dir->guix-profile_watch_dir !=3D NULL so > desktop_file_dir_get_alternative_dir doesn't get called), so it seems > this shouldn't work. What shouldn=E2=80=99t work? We keep adding a watch on a non-existent directory, but I think that=E2=80= =99s 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=E2=80=99re watching /var/guix/profiles (for the system profile) and /var/guix/profiles/per-user/USER, but .desktop files are loaded from the user=E2=80=99s own profile, so that should be fine. Thanks a lot for looking into this! Ludo=E2=80=99.