[gnome-software: 2/9] gs-debug: Split out environment variables from GsDebug state

[Phaedrus Leeds <mwleeds src gnome org>]

commit d5c6bbb5d28c82995e7f1019fd765fdddd2f123b
Author: Philip Withnall <pwithnall endlessos org>
Date:   Thu Feb 18 09:42:01 2021 +0000

    gs-debug: Split out environment variables from GsDebug state
    
    Allow the `GsDebug` state to be initialised from the environment
    variables, but also allow other initialisation sources. This should make
    unit testing a little easier, and reduce reliance on `setenv()` (which
    is not thread-safe).
    
    The internal mutex can be dropped and atomic variables used for anything
    which might change at runtime, instead. (See following commits which
    make use of this.)
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>

 lib/gs-cmd.c   |  2 +-
 lib/gs-debug.c | 94 ++++++++++++++++++++++++++++++++++++++++++++--------------
 lib/gs-debug.h |  6 +++-
 src/gs-main.c  |  2 +-
 4 files changed, 78 insertions(+), 26 deletions(-)
---
diff --git a/lib/gs-cmd.c b/lib/gs-cmd.c
index 73af14973..7254eda24 100644
--- a/lib/gs-cmd.c
+++ b/lib/gs-cmd.c
@@ -296,7 +296,7 @@ main (int argc, char **argv)
        g_autoptr(GError) error = NULL;
        g_autoptr(GsAppList) list = NULL;
        g_autoptr(GPtrArray) categories = NULL;
-       g_autoptr(GsDebug) debug = gs_debug_new ();
+       g_autoptr(GsDebug) debug = gs_debug_new_from_environment ();
        g_autofree gchar *plugin_blocklist_str = NULL;
        g_autofree gchar *plugin_allowlist_str = NULL;
        g_autofree gchar *refine_flags_str = NULL;
diff --git a/lib/gs-debug.c b/lib/gs-debug.c
index 6854b6c35..69a763eae 100644
--- a/lib/gs-debug.c
+++ b/lib/gs-debug.c
@@ -16,9 +16,11 @@
 
 struct _GsDebug
 {
-       GObject          parent_instance;
-       GMutex           mutex;
-       gboolean         use_time;
+       GObject           parent_instance;
+
+       gchar           **domains;  /* (owned) (nullable), read-only after construction, guaranteed to be 
%NULL if empty */
+       gboolean          verbose;  /* (atomic) */
+       gboolean          use_time;  /* read-only after construction */
 };
 
 G_DEFINE_TYPE (GsDebug, gs_debug, G_TYPE_OBJECT)
@@ -30,22 +32,21 @@ gs_log_writer_console (GLogLevelFlags log_level,
                       gpointer user_data)
 {
        GsDebug *debug = GS_DEBUG (user_data);
-       const gchar *domains = NULL;
-       const gchar *gs_debug = NULL;
+       gboolean verbose;
+       const gchar * const *domains = NULL;
        const gchar *log_domain = NULL;
        const gchar *log_message = NULL;
        g_autofree gchar *tmp = NULL;
-       g_autoptr(GMutexLocker) locker = NULL;
        g_autoptr(GString) domain = NULL;
 
-       domains = g_getenv ("G_MESSAGES_DEBUG");
-       gs_debug = g_getenv ("GS_DEBUG");
+       domains = (const gchar * const *) debug->domains;
+       verbose = g_atomic_int_get (&debug->verbose);
 
        /* check enabled, fast path without parsing fields */
        if ((log_level == G_LOG_LEVEL_DEBUG ||
             log_level == G_LOG_LEVEL_INFO) &&
-           gs_debug == NULL &&
-           (domains == NULL || *domains == '\0'))
+           !verbose &&
+           debug->domains == NULL)
                return G_LOG_WRITER_HANDLED;
 
        /* get data from arguments */
@@ -63,10 +64,10 @@ gs_log_writer_console (GLogLevelFlags log_level,
        /* check enabled, slower path */
        if ((log_level == G_LOG_LEVEL_DEBUG ||
             log_level == G_LOG_LEVEL_INFO) &&
-           gs_debug == NULL &&
-           domains != NULL &&
-           g_strcmp0 (domains, "all") != 0 &&
-           (log_domain == NULL || !strstr (domains, log_domain)))
+           !verbose &&
+           debug->domains != NULL &&
+           g_strcmp0 (debug->domains[0], "all") != 0 &&
+           (log_domain == NULL || !g_strv_contains (domains, log_domain)))
                return G_LOG_WRITER_HANDLED;
 
        /* this is really verbose */
@@ -77,10 +78,6 @@ gs_log_writer_console (GLogLevelFlags log_level,
            log_level == G_LOG_LEVEL_DEBUG)
                return G_LOG_WRITER_HANDLED;
 
-       /* make threadsafe */
-       locker = g_mutex_locker_new (&debug->mutex);
-       g_assert (locker != NULL);
-
        /* time header */
        if (debug->use_time) {
                g_autoptr(GDateTime) dt = g_date_time_new_now_utc ();
@@ -180,7 +177,7 @@ gs_debug_finalize (GObject *object)
 {
        GsDebug *debug = GS_DEBUG (object);
 
-       g_mutex_clear (&debug->mutex);
+       g_clear_pointer (&debug->domains, g_strfreev);
 
        G_OBJECT_CLASS (gs_debug_parent_class)->finalize (object);
 }
@@ -195,15 +192,66 @@ gs_debug_class_init (GsDebugClass *klass)
 static void
 gs_debug_init (GsDebug *debug)
 {
-       g_mutex_init (&debug->mutex);
-       debug->use_time = g_getenv ("GS_DEBUG_NO_TIME") == NULL;
        g_log_set_writer_func (gs_debug_log_writer,
                               g_object_ref (debug),
                               (GDestroyNotify) g_object_unref);
 }
 
+/**
+ * gs_debug_new:
+ * @domains: (transfer full) (nullable): a #GStrv of debug log domains to output,
+ *     or `{ "all", NULL }` to output all debug log domains; %NULL is equivalent
+ *     to an empty array
+ * @verbose: whether to output log debug messages
+ * @use_time: whether to output a timestamp with each log message
+ *
+ * Create a new #GsDebug with the given configuration.
+ *
+ * Ownership of @domains is transferred to this function. It will be freed with
+ * g_strfreev() when the #GsDebug is destroyed.
+ *
+ * Returns: (transfer full): a new #GsDebug
+ * Since: 40
+ */
 GsDebug *
-gs_debug_new (void)
+gs_debug_new (gchar    **domains,
+              gboolean   verbose,
+              gboolean   use_time)
 {
-       return GS_DEBUG (g_object_new (GS_TYPE_DEBUG, NULL));
+       g_autoptr(GsDebug) debug = g_object_new (GS_TYPE_DEBUG, NULL);
+
+       /* Strictly speaking these should be set before g_log_set_writer_func()
+        * is called, but threads probably haven’t been started at this point. */
+       debug->domains = (domains != NULL && domains[0] != NULL) ? g_steal_pointer (&domains) : NULL;
+       debug->verbose = verbose;
+       debug->use_time = use_time;
+
+       return g_steal_pointer (&debug);
+}
+
+/**
+ * gs_debug_new_from_environment:
+ *
+ * Create a new #GsDebug with its configuration loaded from environment
+ * variables.
+ *
+ * Returns: (transfer full): a new #GsDebug
+ * Since: 40
+ */
+GsDebug *
+gs_debug_new_from_environment (void)
+{
+       g_auto(GStrv) domains = NULL;
+       gboolean verbose, use_time;
+
+       if (g_getenv ("G_MESSAGES_DEBUG") != NULL) {
+               domains = g_strsplit (g_getenv ("G_MESSAGES_DEBUG"), " ", -1);
+               if (domains[0] == NULL)
+                       g_clear_pointer (&domains, g_strfreev);
+       }
+
+       verbose = (g_getenv ("GS_DEBUG") != NULL);
+       use_time = (g_getenv ("GS_DEBUG_NO_TIME") == NULL);
+
+       return gs_debug_new (g_steal_pointer (&domains), verbose, use_time);
 }
diff --git a/lib/gs-debug.h b/lib/gs-debug.h
index 6dd529d7f..2abbe9303 100644
--- a/lib/gs-debug.h
+++ b/lib/gs-debug.h
@@ -8,6 +8,7 @@
 
 #pragma once
 
+#include <glib.h>
 #include <glib-object.h>
 
 G_BEGIN_DECLS
@@ -16,6 +17,9 @@ G_BEGIN_DECLS
 
 G_DECLARE_FINAL_TYPE (GsDebug, gs_debug, GS, DEBUG, GObject)
 
-GsDebug                *gs_debug_new           (void);
+GsDebug                *gs_debug_new           (gchar          **domains,
+                                        gboolean         verbose,
+                                        gboolean         use_time);
+GsDebug                *gs_debug_new_from_environment  (void);
 
 G_END_DECLS
diff --git a/src/gs-main.c b/src/gs-main.c
index 76837ab79..ed0be685f 100644
--- a/src/gs-main.c
+++ b/src/gs-main.c
@@ -26,7 +26,7 @@ main (int argc, char **argv)
        int status = 0;
        g_autoptr(GDesktopAppInfo) appinfo = NULL;
        g_autoptr(GsApplication) application = NULL;
-       g_autoptr(GsDebug) debug = gs_debug_new ();
+       g_autoptr(GsDebug) debug = gs_debug_new_from_environment ();
 
        g_set_prgname ("org.gnome.Software");
        setlocale (LC_ALL, "");