From c62643adc3fdfda4b68c9bb80d0a1db7f98ff881 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Mar 2023 14:30:39 +0000 Subject: [PATCH 1/7] gappinfo: Clarify one-shot behaviour of GAppInfoMonitor::changed in docs Signed-off-by: Philip Withnall Fixes: #799 --- gio/gappinfo.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/gio/gappinfo.c b/gio/gappinfo.c index 2a8ca8481..42feb9d7c 100644 --- a/gio/gappinfo.c +++ b/gio/gappinfo.c @@ -1658,19 +1658,34 @@ g_app_launch_context_launch_failed (GAppLaunchContext *context, * @short_description: Monitor application information for changes * * #GAppInfoMonitor is a very simple object used for monitoring the app - * info database for changes (ie: newly installed or removed - * applications). + * info database for changes (newly installed or removed applications). * * Call g_app_info_monitor_get() to get a #GAppInfoMonitor and connect - * to the "changed" signal. + * to the #GAppInfoMonitor::changed signal. The signal will be emitted once when + * the app info database changes, and will not be emitted again until after the + * next call to g_app_info_get_all() or another `g_app_info_*()` function. This + * is because monitoring the app info database for changes is expensive. + * + * The following functions will re-arm the #GAppInfoMonitor::changed signal so + * it can be emitted again: + * - g_app_info_get_all() + * - g_app_info_get_all_for_type() + * - g_app_info_get_default_for_type() + * - g_app_info_get_fallback_for_type() + * - g_app_info_get_recommended_for_type() + * - g_desktop_app_info_get_implementations() + * - g_desktop_app_info_new() + * - g_desktop_app_info_new_from_filename() + * - g_desktop_app_info_new_from_keyfile() + * - g_desktop_app_info_search() * * In the usual case, applications should try to make note of the change * (doing things like invalidating caches) but not act on it. In * particular, applications should avoid making calls to #GAppInfo APIs * in response to the change signal, deferring these until the time that - * the data is actually required. The exception to this case is when + * the updated data is actually required. The exception to this case is when * application information is actually being displayed on the screen - * (eg: during a search or when the list of all applications is shown). + * (for example, during a search or when the list of all applications is shown). * The reason for this is that changes to the list of installed * applications often come in groups (like during system updates) and * rescanning the list on every change is pointless and expensive. @@ -1728,8 +1743,10 @@ g_app_info_monitor_class_init (GAppInfoMonitorClass *class) /** * GAppInfoMonitor::changed: * - * Signal emitted when the app info database for changes (ie: newly installed - * or removed applications). + * Signal emitted when the app info database changes, when applications are + * installed or removed. + * + * Since: 2.40 **/ g_app_info_monitor_changed_signal = g_signal_new (I_("changed"), G_TYPE_APP_INFO_MONITOR, G_SIGNAL_RUN_FIRST, 0, NULL, NULL, g_cclosure_marshal_VOID__VOID, G_TYPE_NONE, 0); @@ -1747,6 +1764,10 @@ g_app_info_monitor_class_init (GAppInfoMonitorClass *class) * thread-default main context whenever the list of installed * applications (as reported by g_app_info_get_all()) may have changed. * + * The #GAppInfoMonitor::changed signal will only be emitted once until + * g_app_info_get_all() (or another `g_app_info_*()` function) is called. Doing + * so will re-arm the signal ready to notify about the next change. + * * You must only call g_object_unref() on the return value from under * the same main context as you created it. * From 0a10851faa1e3a9563df03dd591e3ecc02c9fcfb Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Mar 2023 14:36:57 +0000 Subject: [PATCH 2/7] tests: Add copyright/licensing header to appmonitor tests This is put together through git archaeology: ``` git log gio/tests/appmonitor.c ``` The following commits were too trivial to have meaningful copyright: - 54047080e963b2d6c3f966340dcd9d788b73ac9c - 4e7d22e268a4e06beb1c09585a48288c31004da5 - f2c1cfe8c770f73367a021044bcdda550348714c - f8f344923eba57ca13d82e53f3e7b82633179f43 - 3ce00b29ec0bb240412506affb5c65bdb18c7ad3 - 346836962521cce76f3cbd53259a985d3bef6a2a - e9d9edde824df1229bc5539610b98c98013e93aa Signed-off-by: Philip Withnall Helps: #1415 --- gio/tests/appmonitor.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/gio/tests/appmonitor.c b/gio/tests/appmonitor.c index 9db8c4dea..076e7c8a9 100644 --- a/gio/tests/appmonitor.c +++ b/gio/tests/appmonitor.c @@ -1,3 +1,24 @@ +/* GLib testing framework examples and tests + * + * Copyright © 2013 Red Hat, Inc. + * Copyright © 2015, 2017, 2018 Endless Mobile, Inc. + * + * SPDX-License-Identifier: LGPL-2.1-or-later + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, see . + */ + #include #include From 9279f3b0f9f08b63ea89245c175c5f872e9947e6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Mar 2023 14:38:04 +0000 Subject: [PATCH 3/7] tests: Use g_assert_*() rather than g_assert() in appmonitor tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It won’t get compiled out with `G_DISABLE_ASSERT`. Signed-off-by: Philip Withnall --- gio/tests/appmonitor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gio/tests/appmonitor.c b/gio/tests/appmonitor.c index 076e7c8a9..50a0c5863 100644 --- a/gio/tests/appmonitor.c +++ b/gio/tests/appmonitor.c @@ -118,7 +118,7 @@ test_app_monitor (Fixture *fixture, g_timeout_add_seconds (3, quit_loop, loop); g_main_loop_run (loop); - g_assert (changed_fired); + g_assert_true (changed_fired); changed_fired = FALSE; /* FIXME: this shouldn't be required */ @@ -130,7 +130,7 @@ test_app_monitor (Fixture *fixture, g_main_loop_run (loop); - g_assert (changed_fired); + g_assert_true (changed_fired); g_main_loop_unref (loop); g_remove (app_path); From c0ca3f995ba7755c797f15981429ba4156afc8ed Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Mar 2023 14:42:22 +0000 Subject: [PATCH 4/7] tests: Fix a FIXME in the appmonitor test The test thought that calling `g_app_info_get()` was a bit of a hack, but actually it (or calling another `g_app_info_*()` function) is the right way to use the `GAppInfoMonitor` API. See the documentation improvements a couple of commits back for details. The remaining FIXME higher up in the test should probably be fixed by getting `g_app_info_monitor_get()` to arm the signal. That requires changes in `g_app_info_monitor_get()` to call `desktop_file_dir_init()`. That will have to happen another time. Signed-off-by: Philip Withnall Helps: #799 --- gio/tests/appmonitor.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/gio/tests/appmonitor.c b/gio/tests/appmonitor.c index 50a0c5863..e973b990d 100644 --- a/gio/tests/appmonitor.c +++ b/gio/tests/appmonitor.c @@ -22,6 +22,10 @@ #include #include +#ifdef G_OS_UNIX +#include +#endif + typedef struct { gchar *applications_dir; @@ -45,6 +49,7 @@ teardown (Fixture *fixture, g_clear_pointer (&fixture->applications_dir, g_free); } +#ifdef G_OS_UNIX static gboolean create_app (gpointer data) { @@ -90,19 +95,17 @@ quit_loop (gpointer data) return G_SOURCE_REMOVE; } +#endif /* G_OS_UNIX */ static void test_app_monitor (Fixture *fixture, gconstpointer user_data) { +#ifdef G_OS_UNIX gchar *app_path; GAppInfoMonitor *monitor; GMainLoop *loop; - -#ifdef G_OS_WIN32 - g_test_skip (".desktop monitor on win32"); - return; -#endif + GDesktopAppInfo *app = NULL; app_path = g_build_filename (fixture->applications_dir, "app.desktop", NULL); @@ -121,8 +124,11 @@ test_app_monitor (Fixture *fixture, g_assert_true (changed_fired); changed_fired = FALSE; - /* FIXME: this shouldn't be required */ - g_list_free_full (g_app_info_get_all (), g_object_unref); + /* Check that the app is now queryable. This has the side-effect of re-arming + * the #GAppInfoMonitor::changed signal for the next part of the test. */ + app = g_desktop_app_info_new ("app.desktop"); + g_assert_nonnull (app); + g_clear_object (&app); g_timeout_add_seconds (3, quit_loop, loop); @@ -138,6 +144,9 @@ test_app_monitor (Fixture *fixture, g_object_unref (monitor); g_free (app_path); +#else /* if !G_OS_UNIX */ + g_test_skip (".desktop monitor on win32"); +#endif /* !G_OS_UNIX */ } int From 666c72c9c5bff66c023aad806dc8f46f3da89cb4 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Mar 2023 14:50:55 +0000 Subject: [PATCH 5/7] tests: Fix non-removal of a timeout in appmonitor test If the first part of the test takes less than 3s (which is normal), the timeout for it is not removed, and could spuriously fire during the second part of the test, causing a false failure. Instead of relying on source IDs, just use (and explicitly destroy) a `GSource` for the timeouts. Signed-off-by: Philip Withnall --- gio/tests/appmonitor.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/gio/tests/appmonitor.c b/gio/tests/appmonitor.c index e973b990d..e20860cec 100644 --- a/gio/tests/appmonitor.c +++ b/gio/tests/appmonitor.c @@ -105,6 +105,8 @@ test_app_monitor (Fixture *fixture, gchar *app_path; GAppInfoMonitor *monitor; GMainLoop *loop; + GMainContext *context = NULL; /* use the global default main context */ + GSource *timeout_source = NULL; GDesktopAppInfo *app = NULL; app_path = g_build_filename (fixture->applications_dir, "app.desktop", NULL); @@ -113,24 +115,31 @@ test_app_monitor (Fixture *fixture, g_list_free_full (g_app_info_get_all (), g_object_unref); monitor = g_app_info_monitor_get (); - loop = g_main_loop_new (NULL, FALSE); + loop = g_main_loop_new (context, FALSE); g_signal_connect (monitor, "changed", G_CALLBACK (changed_cb), loop); g_idle_add (create_app, app_path); - g_timeout_add_seconds (3, quit_loop, loop); + timeout_source = g_timeout_source_new_seconds (3); + g_source_set_callback (timeout_source, quit_loop, loop, NULL); + g_source_attach (timeout_source, NULL); g_main_loop_run (loop); g_assert_true (changed_fired); changed_fired = FALSE; + g_source_destroy (timeout_source); + g_clear_pointer (&timeout_source, g_source_unref); + /* Check that the app is now queryable. This has the side-effect of re-arming * the #GAppInfoMonitor::changed signal for the next part of the test. */ app = g_desktop_app_info_new ("app.desktop"); g_assert_nonnull (app); g_clear_object (&app); - g_timeout_add_seconds (3, quit_loop, loop); + timeout_source = g_timeout_source_new_seconds (3); + g_source_set_callback (timeout_source, quit_loop, loop, NULL); + g_source_attach (timeout_source, NULL); delete_app (app_path); @@ -138,6 +147,8 @@ test_app_monitor (Fixture *fixture, g_assert_true (changed_fired); + g_source_destroy (timeout_source); + g_clear_pointer (&timeout_source, g_source_unref); g_main_loop_unref (loop); g_remove (app_path); From e4546614a8f59793d89d201279dd1fd9f3db99d3 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Mar 2023 14:56:08 +0000 Subject: [PATCH 6/7] tests: Port appmonitor test from GMainLoop to using GMainContext directly This makes the exit conditions for each main loop clearer, and eliminates use of global variables. It introduces no functional changes. Signed-off-by: Philip Withnall --- gio/tests/appmonitor.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/gio/tests/appmonitor.c b/gio/tests/appmonitor.c index e20860cec..6b7c1f723 100644 --- a/gio/tests/appmonitor.c +++ b/gio/tests/appmonitor.c @@ -76,22 +76,24 @@ delete_app (gpointer data) g_remove (path); } -static gboolean changed_fired; - static void -changed_cb (GAppInfoMonitor *monitor, GMainLoop *loop) +changed_cb (GAppInfoMonitor *monitor, + gpointer user_data) { - changed_fired = TRUE; - g_main_loop_quit (loop); + gboolean *changed_fired = user_data; + + *changed_fired = TRUE; + g_main_context_wakeup (g_main_context_get_thread_default ()); } static gboolean -quit_loop (gpointer data) +timeout_cb (gpointer user_data) { - GMainLoop *loop = data; + gboolean *timed_out = user_data; - if (g_main_loop_is_running (loop)) - g_main_loop_quit (loop); + g_assert (!timed_out); + *timed_out = TRUE; + g_main_context_wakeup (g_main_context_get_thread_default ()); return G_SOURCE_REMOVE; } @@ -104,10 +106,11 @@ test_app_monitor (Fixture *fixture, #ifdef G_OS_UNIX gchar *app_path; GAppInfoMonitor *monitor; - GMainLoop *loop; GMainContext *context = NULL; /* use the global default main context */ GSource *timeout_source = NULL; GDesktopAppInfo *app = NULL; + gboolean changed_fired = FALSE; + gboolean timed_out = FALSE; app_path = g_build_filename (fixture->applications_dir, "app.desktop", NULL); @@ -115,16 +118,17 @@ test_app_monitor (Fixture *fixture, g_list_free_full (g_app_info_get_all (), g_object_unref); monitor = g_app_info_monitor_get (); - loop = g_main_loop_new (context, FALSE); - g_signal_connect (monitor, "changed", G_CALLBACK (changed_cb), loop); + g_signal_connect (monitor, "changed", G_CALLBACK (changed_cb), &changed_fired); g_idle_add (create_app, app_path); timeout_source = g_timeout_source_new_seconds (3); - g_source_set_callback (timeout_source, quit_loop, loop, NULL); + g_source_set_callback (timeout_source, timeout_cb, &timed_out, NULL); g_source_attach (timeout_source, NULL); - g_main_loop_run (loop); + while (!changed_fired && !timed_out) + g_main_context_iteration (context, TRUE); + g_assert_true (changed_fired); changed_fired = FALSE; @@ -138,18 +142,18 @@ test_app_monitor (Fixture *fixture, g_clear_object (&app); timeout_source = g_timeout_source_new_seconds (3); - g_source_set_callback (timeout_source, quit_loop, loop, NULL); + g_source_set_callback (timeout_source, timeout_cb, &timed_out, NULL); g_source_attach (timeout_source, NULL); delete_app (app_path); - g_main_loop_run (loop); + while (!changed_fired && !timed_out) + g_main_context_iteration (context, TRUE); g_assert_true (changed_fired); g_source_destroy (timeout_source); g_clear_pointer (&timeout_source, g_source_unref); - g_main_loop_unref (loop); g_remove (app_path); g_object_unref (monitor); From afcd0bb9008c2d5b677bea80752f17098c2038fb Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Mar 2023 14:58:02 +0000 Subject: [PATCH 7/7] tests: Add some explanatory comments to appmonitor test This should split the code up into logical blocks a bit better, and make it a bit easier to see what the test is doing at a glance. Signed-off-by: Philip Withnall --- gio/tests/appmonitor.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gio/tests/appmonitor.c b/gio/tests/appmonitor.c index 6b7c1f723..c1d68b889 100644 --- a/gio/tests/appmonitor.c +++ b/gio/tests/appmonitor.c @@ -117,6 +117,8 @@ test_app_monitor (Fixture *fixture, /* FIXME: this shouldn't be required */ g_list_free_full (g_app_info_get_all (), g_object_unref); + /* Create an app monitor and check that its ::changed signal is emitted when + * a new app is installed. */ monitor = g_app_info_monitor_get (); g_signal_connect (monitor, "changed", G_CALLBACK (changed_cb), &changed_fired); @@ -141,6 +143,7 @@ test_app_monitor (Fixture *fixture, g_assert_nonnull (app); g_clear_object (&app); + /* Now check that ::changed is emitted when an app is uninstalled. */ timeout_source = g_timeout_source_new_seconds (3); g_source_set_callback (timeout_source, timeout_cb, &timed_out, NULL); g_source_attach (timeout_source, NULL);