Bug 628937 - gracefully handle broken schemas

Implement the first of two features requested in the bug: when
encountering a broken .xml schema file, back out the changes in that
file and continue to parse other files.

This prevents a single broken .xml file from messing up GSettings for
everyone else.

Add a --strict option to get the old behaviour.  Use this from the test
cases.

2 files changed, 72 insertions, 18 deletions

diff --git a/gio/glib-compile-schemas.c b/gio/glib-compile-schemas.c
index 486747421..95770a747 100644
--- a/gio/glib-compile-schemas.c
+++ b/gio/glib-compile-schemas.c
@@ -992,6 +992,10 @@ typedef struct
   GHashTable  *flags_table;             /* string -> EnumState */
   GHashTable  *enum_table;              /* string -> EnumState */
 
+  GSList      *this_file_schemas;       /* strings: <schema>s in this file */
+  GSList      *this_file_flagss;        /* strings: <flags>s in this file */
+  GSList      *this_file_enums;         /* strings: <enum>s in this file */
+
   gchar       *schemalist_domain;       /* the <schemalist> gettext domain */
 
   SchemaState *schema_state;            /* non-NULL when inside <schema> */
@@ -1028,6 +1032,7 @@ parse_state_start_schema (ParseState  *state,
                           GError      **error)
 {
   SchemaState *extends;
+  gchar *my_id;
 
   if (g_hash_table_lookup (state->schema_table, id))
     {
@@ -1129,8 +1134,10 @@ parse_state_start_schema (ParseState  *state,
 
   state->schema_state = schema_state_new (path, gettext_domain,
                                           extends, extends_name, list_of);
-  g_hash_table_insert (state->schema_table, g_strdup (id),
-                       state->schema_state);
+
+  my_id = g_strdup (id);
+  state->this_file_schemas = g_slist_prepend (state->this_file_schemas, my_id);
+  g_hash_table_insert (state->schema_table, my_id, state->schema_state);
 }
 
 static void
@@ -1139,7 +1146,9 @@ parse_state_start_enum (ParseState   *state,
                         gboolean      is_flags,
                         GError      **error)
 {
+  GSList **list = is_flags ? &state->this_file_flagss : &state->this_file_enums;
   GHashTable *table = is_flags ? state->flags_table : state->enum_table;
+  gchar *my_id;
 
   if (g_hash_table_lookup (table, id))
     {
@@ -1151,7 +1160,10 @@ parse_state_start_enum (ParseState   *state,
     }
 
   state->enum_state = enum_state_new (is_flags);
-  g_hash_table_insert (table, g_strdup (id), state->enum_state);
+
+  my_id = g_strdup (id);
+  *list = g_slist_prepend (*list, my_id);
+  g_hash_table_insert (table, my_id, state->enum_state);
 }
 
 /* GMarkup Parser Functions {{{1 */
@@ -1569,12 +1581,13 @@ write_to_file (GHashTable   *schema_table,
 
 /* Parser driver {{{1 */
 static GHashTable *
-parse_gschema_files (gchar  **files,
-                     GError **error)
+parse_gschema_files (gchar    **files,
+                     gboolean   strict)
 {
   GMarkupParser parser = { start_element, end_element, text };
   ParseState state = { 0, };
   const gchar *filename;
+  GError *error = NULL;
 
   state.enum_table = g_hash_table_new_full (g_str_hash, g_str_equal,
                                             g_free, enum_state_free);
@@ -1591,28 +1604,61 @@ parse_gschema_files (gchar  **files,
       gchar *contents;
       gsize size;
 
+      if (!g_file_get_contents (filename, &contents, &size, &error))
+        {
+          fprintf (stderr, "%s\n", error->message);
+          g_clear_error (&error);
+          continue;
+        }
+
       context = g_markup_parse_context_new (&parser,
                                             G_MARKUP_PREFIX_ERROR_POSITION,
                                             &state, NULL);
 
-      if (!g_file_get_contents (filename, &contents, &size, error))
-        return NULL;
 
-      if (!g_markup_parse_context_parse (context, contents, size, error))
+      if (!g_markup_parse_context_parse (context, contents, size, &error) ||
+          !g_markup_parse_context_end_parse (context, &error))
         {
-          g_prefix_error (error, "%s: ", filename);
-          return NULL;
-        }
+          GSList *item;
 
-      if (!g_markup_parse_context_end_parse (context, error))
-        {
-          g_prefix_error (error, "%s: ", filename);
-          return NULL;
+          /* back out any changes from this file */
+          for (item = state.this_file_schemas; item; item = item->next)
+            g_hash_table_remove (state.schema_table, item->data);
+
+          for (item = state.this_file_flagss; item; item = item->next)
+            g_hash_table_remove (state.flags_table, item->data);
+
+          for (item = state.this_file_enums; item; item = item->next)
+            g_hash_table_remove (state.enum_table, item->data);
+
+          /* let them know */
+          fprintf (stderr, "%s: %s.  ", filename, error->message);
+
+          if (strict)
+            {
+              /* Translators: Do not translate "--strict". */
+              fprintf (stderr, _("--strict was specified; exiting.\n"));
+              g_hash_table_unref (state.schema_table);
+              g_hash_table_unref (state.flags_table);
+              g_hash_table_unref (state.enum_table);
+
+              return NULL;
+            }
+          else
+            fprintf (stderr, _("This entire file has been ignored.\n"));
         }
 
+      /* cleanup */
       g_markup_parse_context_free (context);
+      g_slist_free (state.this_file_schemas);
+      g_slist_free (state.this_file_flagss);
+      g_slist_free (state.this_file_enums);
+      state.this_file_schemas = NULL;
+      state.this_file_flagss = NULL;
+      state.this_file_enums = NULL;
     }
 
+  g_hash_table_unref (state.flags_table);
   g_hash_table_unref (state.enum_table);
 
   return state.schema_table;
@@ -1790,11 +1836,13 @@ main (int argc, char **argv)
   gchar *target;
   gboolean uninstall = FALSE;
   gboolean dry_run = FALSE;
+  gboolean strict = FALSE;
   gchar **schema_files = NULL;
   gchar **override_files = NULL;
   GOptionContext *context;
   GOptionEntry entries[] = {
     { "targetdir", 0, 0, G_OPTION_ARG_FILENAME, &targetdir, N_("where to store the gschemas.compiled file"), N_("DIRECTORY") },
+    { "strict", 0, 0, G_OPTION_ARG_NONE, &strict, N_("Abort on any errors in schemas"), NULL },
     { "dry-run", 0, 0, G_OPTION_ARG_NONE, &dry_run, N_("Do not write the gschema.compiled file"), NULL },
     { "uninstall", 0, 0, G_OPTION_ARG_NONE, &uninstall, N_("This option will be removed soon.") },
     { "allow-any-name", 0, 0, G_OPTION_ARG_NONE, &allow_any_name, N_("Do not enforce key name restrictions") },
@@ -1817,7 +1865,7 @@ main (int argc, char **argv)
   error = NULL;
   if (!g_option_context_parse (context, &argc, &argv, &error))
     {
-      fprintf (stderr, "%s", error->message);
+      fprintf (stderr, "%s\n", error->message);
       return 1;
     }
 
@@ -1884,12 +1932,17 @@ main (int argc, char **argv)
       override_files = (gchar **) g_ptr_array_free (overrides, FALSE);
     }
 
+  if ((table = parse_gschema_files (schema_files, strict)) == NULL)
+    {
+      g_free (target);
+      return 1;
+    }
 
-  if (!(table = parse_gschema_files (schema_files, &error)) ||
-      (override_files != NULL && !set_overrides (table, override_files, &error)) ||
+  if ((override_files != NULL && !set_overrides (table, override_files, &error)) ||
       (!dry_run && !write_to_file (table, target, &error)))
     {
       fprintf (stderr, "%s\n", error->message);
+      g_free (target);
       return 1;
     }
 
diff --git a/gio/tests/gschema-compile.c b/gio/tests/gschema-compile.c
index 9f1123c8e..1d261eb2b 100644
--- a/gio/tests/gschema-compile.c
+++ b/gio/tests/gschema-compile.c
@@ -22,6 +22,7 @@ test_schema (gpointer data)
       gchar *path = g_build_filename (SRCDIR, "schema-tests", filename, NULL);
       gchar *argv[] = {
         "../glib-compile-schemas",
+        "--strict",
         "--dry-run",
         "--schema-file", path,
         (gchar *)test->opt,