From 49fafec020ba40494738824ed97ddbef938f4725 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Thu, 9 Nov 2017 14:30:27 +0000 Subject: [PATCH] Set environment variables to allow easy per-plugin debugging This allows end-users testing a specific plugin to start fwupd with an extra command line parameter, e.g. `--plugin-verbose=unifying` to output a lot of debugging information to the console for that specific plugin. This replaces a lot of ad-hoc environment variables with different naming conventions. --- docs/libfwupd/libfwupd-docs.xml | 13 ++++ plugins/dfu/dfu-target.c | 4 +- plugins/ebitdo/fu-ebitdo-device.c | 6 +- plugins/nitrokey/fu-nitrokey-device.c | 2 +- plugins/unifying/lu-device.c | 4 +- src/fu-debug.c | 88 ++++++++++++++------------- src/fu-debug.h | 3 - src/fu-main.c | 2 +- 8 files changed, 69 insertions(+), 53 deletions(-) diff --git a/docs/libfwupd/libfwupd-docs.xml b/docs/libfwupd/libfwupd-docs.xml index 9c28bea9c..db379ef8a 100644 --- a/docs/libfwupd/libfwupd-docs.xml +++ b/docs/libfwupd/libfwupd-docs.xml @@ -401,6 +401,19 @@ fu_plugin_update_reload (FuPlugin *plugin, FuDevice *device, GError **error) +
+ Debugging a Plugin + + If the fwupd daemon is started with --plugin-verbose=$plugin + then the environment variable FWUPD_$PLUGIN_VERBOSE is + set process-wide. + This allows plugins to detect when they should output detailed debugging + information that would normally be too verbose to keep in the journal. + For example, using --plugin-verbose=unifying would set + FWUPD_UNIFYING_VERBOSE=1. + +
+ diff --git a/plugins/dfu/dfu-target.c b/plugins/dfu/dfu-target.c index dc0f4eb40..2743fb2b2 100644 --- a/plugins/dfu/dfu-target.c +++ b/plugins/dfu/dfu-target.c @@ -700,7 +700,7 @@ dfu_target_download_chunk (DfuTarget *target, guint16 index, GBytes *bytes, gsize actual_length; /* low level packet debugging */ - if (g_getenv ("DFU_TRACE") != NULL) { + if (g_getenv ("FWUPD_DFU_VERBOSE") != NULL) { gsize sz = 0; const guint8 *data = g_bytes_get_data (bytes, &sz); for (gsize i = 0; i < sz; i++) @@ -793,7 +793,7 @@ dfu_target_upload_chunk (DfuTarget *target, guint16 index, gsize buf_sz, } /* low level packet debugging */ - if (g_getenv ("DFU_TRACE") != NULL) { + if (g_getenv ("FWUPD_DFU_VERBOSE") != NULL) { for (gsize i = 0; i < actual_length; i++) g_print ("Message: r[%" G_GSIZE_FORMAT "] = 0x%02x\n", i, (guint) buf[i]); } diff --git a/plugins/ebitdo/fu-ebitdo-device.c b/plugins/ebitdo/fu-ebitdo-device.c index 9b50de2d9..9aae3ef8d 100644 --- a/plugins/ebitdo/fu-ebitdo-device.c +++ b/plugins/ebitdo/fu-ebitdo-device.c @@ -184,7 +184,7 @@ fu_ebitdo_device_send (FuEbitdoDevice *device, } /* debug */ - if (g_getenv ("FU_EBITDO_DEBUG") != NULL) { + if (g_getenv ("FWUPD_EBITDO_VERBOSE") != NULL) { fu_ebitdo_dump_raw ("->DEVICE", packet, (gsize) hdr->pkt_len + 1); fu_ebitdo_dump_pkt (hdr); } @@ -246,7 +246,7 @@ fu_ebitdo_device_receive (FuEbitdoDevice *device, } /* debug */ - if (g_getenv ("FU_EBITDO_DEBUG") != NULL) { + if (g_getenv ("FWUPD_EBITDO_VERBOSE") != NULL) { fu_ebitdo_dump_raw ("<-DEVICE", packet, (gsize) hdr->pkt_len - 1); fu_ebitdo_dump_pkt (hdr); } @@ -560,7 +560,7 @@ fu_ebitdo_device_write_firmware (FuEbitdoDevice *device, GBytes *fw, payload_data = g_bytes_get_data (fw, NULL); payload_data += sizeof(FuEbitdoFirmwareHeader); for (guint32 offset = 0; offset < payload_len; offset += chunk_sz) { - if (g_getenv ("FU_EBITDO_DEBUG") != NULL) { + if (g_getenv ("FWUPD_EBITDO_VERBOSE") != NULL) { g_debug ("writing %u bytes to 0x%04x of 0x%04x", chunk_sz, offset, payload_len); } diff --git a/plugins/nitrokey/fu-nitrokey-device.c b/plugins/nitrokey/fu-nitrokey-device.c index b63116062..4ab96841d 100644 --- a/plugins/nitrokey/fu-nitrokey-device.c +++ b/plugins/nitrokey/fu-nitrokey-device.c @@ -117,7 +117,7 @@ fu_nitrokey_device_class_init (FuNitrokeyDeviceClass *klass) static void _dump_to_console (const gchar *title, const guint8 *buf, gsize buf_sz) { - if (g_getenv ("NITROKEY_DEBUG") == NULL) + if (g_getenv ("FWUPD_NITROKEY_VERBOSE") == NULL) return; g_debug ("%s", title); for (gsize i = 0; i < buf_sz; i++) diff --git a/plugins/unifying/lu-device.c b/plugins/unifying/lu-device.c index 749820418..df04671b1 100644 --- a/plugins/unifying/lu-device.c +++ b/plugins/unifying/lu-device.c @@ -379,7 +379,7 @@ lu_device_hidpp_send (LuDevice *device, lu_device_hidpp_dump (device, "host->device", (guint8 *) msg, len); /* detailed debugging */ - if (g_getenv ("UNIFYING_HW_DEBUG") != NULL) { + if (g_getenv ("FWUPD_UNIFYING_VERBOSE") != NULL) { g_autofree gchar *str = lu_device_hidpp_msg_to_string (device, msg); g_print ("%s", str); } @@ -473,7 +473,7 @@ lu_device_hidpp_receive (LuDevice *device, } /* detailed debugging */ - if (g_getenv ("UNIFYING_HW_DEBUG") != NULL) { + if (g_getenv ("FWUPD_UNIFYING_VERBOSE") != NULL) { g_autofree gchar *str = lu_device_hidpp_msg_to_string (device, msg); g_print ("%s", str); } diff --git a/src/fu-debug.c b/src/fu-debug.c index 0d8de9991..dcf2f5fde 100644 --- a/src/fu-debug.c +++ b/src/fu-debug.c @@ -1,6 +1,6 @@ /* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- * - * Copyright (C) 2010-2015 Richard Hughes + * Copyright (C) 2010-2017 Richard Hughes * * Licensed under the GNU General Public License Version 2 * @@ -26,23 +26,19 @@ #include -static gboolean _verbose = FALSE; -static gboolean _console = FALSE; +typedef struct { + gboolean verbose; + gboolean console; + gchar **plugin_verbose; +} FuDebug; -gboolean -fu_debug_is_verbose (void) +static void +fu_debug_free (FuDebug *self) { - /* local first */ - if (_verbose) - return TRUE; - - /* fall back to env variable */ - if (g_getenv ("VERBOSE") != NULL) - return TRUE; - return FALSE; + g_strfreev (self->plugin_verbose); + g_free (self); } - static void fu_debug_ignore_cb (const gchar *log_domain, GLogLevelFlags log_level, @@ -68,6 +64,7 @@ fu_debug_handler_cb (const gchar *log_domain, const gchar *message, gpointer user_data) { + FuDebug *self = (FuDebug *) user_data; g_autofree gchar *tmp = NULL; g_autoptr(GDateTime) dt = g_date_time_new_now_utc (); g_autoptr(GString) domain = NULL; @@ -89,7 +86,7 @@ fu_debug_handler_cb (const gchar *log_domain, g_string_append (domain, " "); /* to file */ - if (!_console) { + if (!self->console) { if (tmp != NULL) g_print ("%s ", tmp); g_print ("%s ", domain->str); @@ -124,10 +121,14 @@ fu_debug_pre_parse_hook (GOptionContext *context, gpointer data, GError **error) { + FuDebug *self = (FuDebug *) data; const GOptionEntry main_entries[] = { - { "verbose", 'v', 0, G_OPTION_ARG_NONE, &_verbose, + { "verbose", 'v', 0, G_OPTION_ARG_NONE, &self->verbose, /* TRANSLATORS: turn on all debugging */ N_("Show debugging information for all files"), NULL }, + { "plugin-verbose", '\0', 0, G_OPTION_ARG_STRING_ARRAY, &self->plugin_verbose, + /* TRANSLATORS: this is for plugin development */ + N_("Show plugin verbose information"), "PLUGIN-NAME" }, { NULL} }; @@ -136,38 +137,42 @@ fu_debug_pre_parse_hook (GOptionContext *context, return TRUE; } -void -fu_debug_destroy (void) -{ -} - -void -fu_debug_setup (gboolean enabled) -{ - if (enabled) { - g_log_set_fatal_mask (NULL, G_LOG_LEVEL_ERROR | - G_LOG_LEVEL_CRITICAL); - g_log_set_default_handler (fu_debug_handler_cb, NULL); - } else { - /* hide all debugging */ - g_log_set_handler (G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, - fu_debug_ignore_cb, NULL); - } - - /* are we on an actual TTY? */ - _console = (isatty (fileno (stdout)) == 1); -} - static gboolean fu_debug_post_parse_hook (GOptionContext *context, GOptionGroup *group, gpointer data, GError **error) { + FuDebug *self = (FuDebug *) data; + /* verbose? */ - fu_debug_setup (_verbose); + if (self->verbose) { + g_setenv ("FWUPD_VERBOSE", "1", TRUE); + g_log_set_fatal_mask (NULL, G_LOG_LEVEL_ERROR | + G_LOG_LEVEL_CRITICAL); + g_log_set_default_handler (fu_debug_handler_cb, self); + } else { + /* hide all debugging */ + g_log_set_handler (G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, + fu_debug_ignore_cb, self); + } + + /* are we on an actual TTY? */ + self->console = (isatty (fileno (stdout)) == 1); g_debug ("Verbose debugging %s (on console %i)", - _verbose ? "enabled" : "disabled", _console); + self->verbose ? "enabled" : "disabled", self->console); + + /* allow each plugin to be extra verbose */ + if (self->plugin_verbose != NULL) { + for (guint i = 0; self->plugin_verbose[i] != NULL; i++) { + g_autofree gchar *name_caps = NULL; + g_autofree gchar *varname = NULL; + name_caps = g_ascii_strup (self->plugin_verbose[i], -1); + varname = g_strdup_printf ("FWUPD_%s_VERBOSE", name_caps); + g_debug ("setting %s=1", varname); + g_setenv (varname, "1", TRUE); + } + } return TRUE; } @@ -175,12 +180,13 @@ GOptionGroup * fu_debug_get_option_group (void) { GOptionGroup *group; + FuDebug *self = g_new0 (FuDebug, 1); group = g_option_group_new ("debug", /* TRANSLATORS: for the --verbose arg */ _("Debugging Options"), /* TRANSLATORS: for the --verbose arg */ _("Show debugging options"), - NULL, NULL); + self, (GDestroyNotify) fu_debug_free); g_option_group_set_parse_hooks (group, fu_debug_pre_parse_hook, fu_debug_post_parse_hook); diff --git a/src/fu-debug.h b/src/fu-debug.h index 75e7ab002..1f8f9abea 100644 --- a/src/fu-debug.h +++ b/src/fu-debug.h @@ -24,9 +24,6 @@ #include -gboolean fu_debug_is_verbose (void); GOptionGroup *fu_debug_get_option_group (void); -void fu_debug_setup (gboolean enabled); -void fu_debug_destroy (void); #endif /* __FU_DEBUG_H__ */ diff --git a/src/fu-main.c b/src/fu-main.c index 79751aa7c..110288c63 100644 --- a/src/fu-main.c +++ b/src/fu-main.c @@ -853,7 +853,7 @@ fu_main_on_bus_acquired_cb (GDBusConnection *connection, } /* dump startup profile data */ - if (fu_debug_is_verbose ()) + if (g_getenv ("FWUPD_VERBOSE") != NULL) fu_engine_profile_dump (priv->engine); }