From: "Ludovic Courtès" <ludo@gnu.org>
To: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Cc: Leo Prikler <leo.prikler@student.tugraz.at>,
35594@debbugs.gnu.org, 36376@debbugs.gnu.org
Subject: bug#36376: Application menu of desktop environment not automatically updated
Date: Tue, 17 Nov 2020 10:08:49 +0100 [thread overview]
Message-ID: <87eekstkny.fsf@gnu.org> (raw)
In-Reply-To: <875z64d1hr.fsf@gmail.com> (Maxim Cournoyer's message of "Mon, 16 Nov 2020 23:57:20 -0500")
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’.
next prev parent reply other threads:[~2020-11-17 9:09 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-25 14:12 bug#36376: Application menu of desktop environment not automatically updated Ludovic Courtès
2020-11-03 22:46 ` Ludovic Courtès
2020-11-03 23:28 ` Leo Prikler
2020-11-04 21:41 ` bug#35594: " Maxim Cournoyer
2020-11-04 22:15 ` Ludovic Courtès
2020-11-05 6:38 ` Maxim Cournoyer
2020-11-06 16:02 ` Ludovic Courtès
2020-11-06 18:56 ` Maxim Cournoyer
2020-11-10 15:23 ` Ludovic Courtès
2020-11-10 17:48 ` bug#35594: " zimoun
2020-11-10 21:23 ` Ludovic Courtès
2020-11-12 15:56 ` bug#35594: " Ludovic Courtès
2020-11-13 20:38 ` bug#36376: " Ludovic Courtès
2020-11-17 4:57 ` Maxim Cournoyer
2020-11-17 9:08 ` Ludovic Courtès [this message]
2020-11-18 4:34 ` Maxim Cournoyer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87eekstkny.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=35594@debbugs.gnu.org \
--cc=36376@debbugs.gnu.org \
--cc=leo.prikler@student.tugraz.at \
--cc=maxim.cournoyer@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).