From d5a67cb162d9328fcc49dbd3992763fc1bfa1a82 Mon Sep 17 00:00:00 2001 From: Jehan Date: Thu, 8 Feb 2018 20:48:16 +0100 Subject: [PATCH] app: make debugging preference finer-grained than a boolean. Replacing the boolean property "generate-backtrace" by an enum "debug-policy". This property allows one to choose whether to debug WARNING, CRITICAL and FATAL (crashes), or CRITICAL and FATAL only, or only FATAL, or finally nothing. By default, a stable release will debug CRITICAL and crashes, and unstable builds will start debugging at WARNINGs. The reason for the settings is that if you stumble upon a reccurring bug in your workflow (and this bug is not major enough for data corruption, and "you can live with it"), you still have to wait for a new release. At some point, you may want to disable getting a debug dialog, at least temporarily. Oppositely, even when using a stable build, you may want to obtain debug info for lesser issues, even WARNINGs, if you wish to help the GIMP project. It can be argued though whether the value GIMP_DEBUG_POLICY_NEVER is really useful. There is nothing to gain from refusing debugging info when the software crashed anyway. But I could still imagine that someone is not interested in helping at all. It's sad but not like we are going to force people to report. Let's just allow disabling the whole debugging system. --- app/config/gimpcoreconfig.c | 25 ++++----- app/config/gimpcoreconfig.h | 2 +- app/core/core-enums.c | 33 ++++++++++++ app/core/core-enums.h | 12 +++++ app/dialogs/preferences-dialog.c | 10 ++-- app/errors.c | 91 +++++++++++++++++++++----------- 6 files changed, 127 insertions(+), 46 deletions(-) diff --git a/app/config/gimpcoreconfig.c b/app/config/gimpcoreconfig.c index 87d31ccd60..e76c58f1ee 100644 --- a/app/config/gimpcoreconfig.c +++ b/app/config/gimpcoreconfig.c @@ -113,7 +113,7 @@ enum PROP_EXPORT_METADATA_EXIF, PROP_EXPORT_METADATA_XMP, PROP_EXPORT_METADATA_IPTC, - PROP_GENERATE_BACKTRACE, + PROP_DEBUG_POLICY, /* ignored, only for backward compatibility: */ PROP_INSTALL_COLORMAP, @@ -655,16 +655,17 @@ gimp_core_config_class_init (GimpCoreConfigClass *klass) FALSE, GIMP_PARAM_STATIC_STRINGS); - GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_GENERATE_BACKTRACE, - "generate-backtrace", - "Try generating backtrace upon errors", - GENERATE_BACKTRACE_BLURB, + GIMP_CONFIG_PROP_ENUM (object_class, PROP_DEBUG_POLICY, + "debug-policy", + "Try generating backtrace upon errors", + GENERATE_BACKTRACE_BLURB, + GIMP_TYPE_DEBUG_POLICY, #ifdef GIMP_UNSTABLE - TRUE, + GIMP_DEBUG_POLICY_WARNING, #else - FALSE, + GIMP_DEBUG_POLICY_CRITICAL, #endif - GIMP_PARAM_STATIC_STRINGS); + GIMP_PARAM_STATIC_STRINGS); /* only for backward compatibility: */ GIMP_CONFIG_PROP_BOOLEAN (object_class, PROP_INSTALL_COLORMAP, @@ -968,8 +969,8 @@ gimp_core_config_set_property (GObject *object, case PROP_EXPORT_METADATA_IPTC: core_config->export_metadata_iptc = g_value_get_boolean (value); break; - case PROP_GENERATE_BACKTRACE: - core_config->generate_backtrace = g_value_get_boolean (value); + case PROP_DEBUG_POLICY: + core_config->debug_policy = g_value_get_enum (value); break; case PROP_INSTALL_COLORMAP: @@ -1167,8 +1168,8 @@ gimp_core_config_get_property (GObject *object, case PROP_EXPORT_METADATA_IPTC: g_value_set_boolean (value, core_config->export_metadata_iptc); break; - case PROP_GENERATE_BACKTRACE: - g_value_set_boolean (value, core_config->generate_backtrace); + case PROP_DEBUG_POLICY: + g_value_set_enum (value, core_config->debug_policy); break; case PROP_INSTALL_COLORMAP: diff --git a/app/config/gimpcoreconfig.h b/app/config/gimpcoreconfig.h index 57ed447418..9822863382 100644 --- a/app/config/gimpcoreconfig.h +++ b/app/config/gimpcoreconfig.h @@ -98,7 +98,7 @@ struct _GimpCoreConfig gboolean export_metadata_exif; gboolean export_metadata_xmp; gboolean export_metadata_iptc; - gboolean generate_backtrace; + GimpDebugPolicy debug_policy; }; struct _GimpCoreConfigClass diff --git a/app/core/core-enums.c b/app/core/core-enums.c index 83076d5409..9a74f0a09c 100644 --- a/app/core/core-enums.c +++ b/app/core/core-enums.c @@ -654,6 +654,39 @@ gimp_thumbnail_size_get_type (void) return type; } +GType +gimp_debug_policy_get_type (void) +{ + static const GEnumValue values[] = + { + { GIMP_DEBUG_POLICY_WARNING, "GIMP_DEBUG_POLICY_WARNING", "warning" }, + { GIMP_DEBUG_POLICY_CRITICAL, "GIMP_DEBUG_POLICY_CRITICAL", "critical" }, + { GIMP_DEBUG_POLICY_FATAL, "GIMP_DEBUG_POLICY_FATAL", "fatal" }, + { GIMP_DEBUG_POLICY_NEVER, "GIMP_DEBUG_POLICY_NEVER", "never" }, + { 0, NULL, NULL } + }; + + static const GimpEnumDesc descs[] = + { + { GIMP_DEBUG_POLICY_WARNING, NC_("debug-policy", "Debug warnings, critical errors and crashes"), NULL }, + { GIMP_DEBUG_POLICY_CRITICAL, NC_("debug-policy", "Debug critical errors and crashes"), NULL }, + { GIMP_DEBUG_POLICY_FATAL, NC_("debug-policy", "Debug crashes only"), NULL }, + { GIMP_DEBUG_POLICY_NEVER, NC_("debug-policy", "Never debug GIMP"), NULL }, + { 0, NULL, NULL } + }; + + static GType type = 0; + + if (G_UNLIKELY (! type)) + { + type = g_enum_register_static ("GimpDebugPolicy", values); + gimp_type_set_translation_context (type, "debug-policy"); + gimp_enum_set_value_descriptions (type, descs); + } + + return type; +} + GType gimp_undo_mode_get_type (void) { diff --git a/app/core/core-enums.h b/app/core/core-enums.h index 9cd2cdb071..1a249aa967 100644 --- a/app/core/core-enums.h +++ b/app/core/core-enums.h @@ -300,6 +300,18 @@ typedef enum /*< pdb-skip >*/ GIMP_THUMBNAIL_SIZE_LARGE = 256 /*< desc="Large (256x256)" >*/ } GimpThumbnailSize; +#define GIMP_TYPE_DEBUG_POLICY (gimp_debug_policy_get_type ()) + +GType gimp_debug_policy_get_type (void) G_GNUC_CONST; + +typedef enum /*< pdb-skip >*/ +{ + GIMP_DEBUG_POLICY_WARNING, /*< desc="Debug warnings, critical errors and crashes" >*/ + GIMP_DEBUG_POLICY_CRITICAL, /*< desc="Debug critical errors and crashes" >*/ + GIMP_DEBUG_POLICY_FATAL, /*< desc="Debug crashes only" >*/ + GIMP_DEBUG_POLICY_NEVER /*< desc="Never debug GIMP" >*/ +} GimpDebugPolicy; + #define GIMP_TYPE_UNDO_MODE (gimp_undo_mode_get_type ()) diff --git a/app/dialogs/preferences-dialog.c b/app/dialogs/preferences-dialog.c index ccf28c7c0c..0d7d745564 100644 --- a/app/dialogs/preferences-dialog.c +++ b/app/dialogs/preferences-dialog.c @@ -1257,9 +1257,13 @@ prefs_dialog_new (Gimp *gimp, vbox2 = prefs_frame_new (_("Bug Reporting"), GTK_CONTAINER (vbox), FALSE); - button = prefs_check_button_add (object, "generate-backtrace", - _("Try generating debug data for bug reporting when appropriate"), - GTK_BOX (vbox2)); + size_group = gtk_size_group_new (GTK_SIZE_GROUP_HORIZONTAL); + table = prefs_table_new (1, GTK_CONTAINER (vbox2)); + + button = prefs_enum_combo_box_add (object, "debug-policy", 0, 0, + _("Try generating debug data for bug reporting when appropriate"), + GTK_TABLE (table), 0, size_group); + gtk_widget_set_sensitive (button, FALSE); hbox = prefs_hint_box_new (GIMP_ICON_DIALOG_WARNING, diff --git a/app/errors.c b/app/errors.c index 9fc9fd0c2a..22059ded01 100644 --- a/app/errors.c +++ b/app/errors.c @@ -143,10 +143,9 @@ errors_init (Gimp *gimp, for (i = 0; i < G_N_ELEMENTS (log_domains); i++) g_log_set_handler (log_domains[i], -#ifdef GIMP_UNSTABLE G_LOG_LEVEL_WARNING | -#endif - G_LOG_LEVEL_MESSAGE | G_LOG_LEVEL_CRITICAL, + G_LOG_LEVEL_MESSAGE | + G_LOG_LEVEL_CRITICAL, gimp_message_log_func, gimp); g_log_set_handler ("GEGL", @@ -212,33 +211,49 @@ gimp_message_log_func (const gchar *log_domain, { static gint n_traces; GimpMessageSeverity severity = GIMP_MESSAGE_WARNING; - Gimp *gimp = data; - gchar *trace = NULL; - GimpCoreConfig *config = gimp->config; - gboolean generate_backtrace = FALSE; + Gimp *gimp = data; + gchar *trace = NULL; + const gchar *reason; - g_object_get (G_OBJECT (config), - "generate-backtrace", &generate_backtrace, - NULL); - - if (generate_backtrace && - (flags & (G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_WARNING))) + if (flags & (G_LOG_LEVEL_CRITICAL | G_LOG_LEVEL_WARNING)) { - severity = (flags & G_LOG_LEVEL_CRITICAL) ? - GIMP_MESSAGE_ERROR : GIMP_MESSAGE_WARNING; + GimpCoreConfig *config = gimp->config; + GimpDebugPolicy debug_policy; - if (n_traces < MAX_TRACES) + g_object_get (G_OBJECT (config), + "debug-policy", &debug_policy, + NULL); + + if ((debug_policy == GIMP_DEBUG_POLICY_CRITICAL && + (flags & G_LOG_LEVEL_CRITICAL)) || + debug_policy == GIMP_DEBUG_POLICY_WARNING) { - /* Getting debug traces is time-expensive, and worse, some - * critical errors have the bad habit to create more errors - * (the first ones are therefore usually the most useful). - * This is why we keep track of how many times we made traces - * and stop doing them after a while. - * Hence when this happens, critical errors are simply processed as - * lower level errors. + severity = (flags & G_LOG_LEVEL_CRITICAL) ? + GIMP_MESSAGE_ERROR : GIMP_MESSAGE_WARNING; + + if (n_traces < MAX_TRACES) + { + /* Getting debug traces is time-expensive, and worse, some + * critical errors have the bad habit to create more errors + * (the first ones are therefore usually the most useful). + * This is why we keep track of how many times we made traces + * and stop doing them after a while. + * Hence when this happens, critical errors are simply processed as + * lower level errors. + */ + gimp_print_stack_trace (NULL, &trace); + n_traces++; + } + } + if (! trace) + { + /* Since we overrided glib default's WARNING and CRITICAL + * handler, if we decide not to handle this error in the end, + * let's just print it in terminal in a similar fashion as + * glib's default handler (though without the fancy terminal + * colors right now). */ - gimp_print_stack_trace (NULL, &trace); - n_traces++; + goto print_to_stderr; } } @@ -249,8 +264,23 @@ gimp_message_log_func (const gchar *log_domain, } else { - g_printerr ("%s: %s\n\n", - gimp_filename_to_utf8 (full_prog_name), message); +print_to_stderr: + switch (flags & G_LOG_LEVEL_MASK) + { + case G_LOG_LEVEL_WARNING: + reason = "WARNING"; + break; + case G_LOG_LEVEL_CRITICAL: + reason = "CRITICAL"; + break; + default: + reason = "MESSAGE"; + break; + } + + g_printerr ("%s: %s-%s: %s\n", + gimp_filename_to_utf8 (full_prog_name), + log_domain, reason, message); if (trace) g_printerr ("Back trace:\n%s\n\n", trace); } @@ -274,7 +304,7 @@ gimp_eek (const gchar *reason, gboolean use_handler) { GimpCoreConfig *config = the_errors_gimp->config; - gboolean generate_backtrace = FALSE; + GimpDebugPolicy debug_policy; gboolean eek_handled = FALSE; /* GIMP has 2 ways to handle termination signals and fatal errors: one @@ -286,7 +316,7 @@ gimp_eek (const gchar *reason, * The GUI backtrace has priority if it is set. */ g_object_get (G_OBJECT (config), - "generate-backtrace", &generate_backtrace, + "debug-policy", &debug_policy, NULL); /* Let's just always output on stdout at least so that there is a @@ -299,7 +329,8 @@ gimp_eek (const gchar *reason, if (use_handler) { #ifndef GIMP_CONSOLE_COMPILATION - if (generate_backtrace && ! the_errors_gimp->no_interface) + if (debug_policy != GIMP_DEBUG_POLICY_NEVER && + ! the_errors_gimp->no_interface) { FILE *fd; gboolean has_backtrace = TRUE;