GNOME Bugzilla – Bug 794285
glib-compile-resources should not noisily g_printerr() when xmllint is not found
Last modified: 2018-03-14 15:09:31 UTC
xmllint = g_strdup (g_getenv ("XMLLINT")); if (xmllint == NULL) xmllint = g_find_program_in_path ("xmllint"); if (xmllint == NULL) g_printerr ("XMLLINT not set and xmllint not found in path; skipping xml preprocessing.\n"); If I'm compiling an app with no XML resources and/or don't care about stripping blanks in them, I shouldn't be bombarded with this message while compiling. Shouldn't this only be printed if the user explicitly tried to use xmllint, by setting the relevant attribute on a resource component?
Ideally we could for xml-stripblanks et al. what we do for to-pixdata: only error if an attribute requests the preprocessor: if (gdk_pixbuf_pixdata == NULL) { g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, "to-pixbuf preprocessing requested but GDK_PIXBUF_PIXDATA " "not set and gdk-pixbuf-pixdata not found in path"); goto cleanup; }
Created attachment 369655 [details] [review] Conditionally error if pre-processing tools are not found There's no need to unconditionally print an error message if xmllint or json-glib-format are not found when running glib-compile-resources is called; we only need to error out if they are not available when we need them.
Attachment 369655 [details] depends on the patches for bug 794284.
Review of attachment 369655 [details] [review]: Sure. Are you planning on marking the strings as translatable in a separate commit?
Created attachment 369666 [details] [review] Conditionally error if pre-processing tools are not found There's no need to unconditionally print an error message if xmllint or json-glib-format are not found when running glib-compile-resources is called; we only need to error out if they are not available when we need them.
Ugh, forgot about that; patch updated.
Now that I'm looking at it, though, I see a snag. Using to-pixdata without gdkpixbuf-to-pixdata is a fatal error because the expected data will be different; not stripping blanks from XML or JSON is not really a fatal error. This means we should just warn, not error out.
Review of attachment 369666 [details] [review]: Will update with a non-fatal warning.
Review of attachment 369666 [details] [review]: ::: gio/glib-compile-resources.c @@ +307,3 @@ + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + _("xml-stripblanks preprocessing requested but XMLLINT " + "not set and xmllint not found in PATH")); To reduce the number of strings the translators have to translate, you could do this as: g_strdup_printf (_("%s preprocessing requested but %s not set and %s not found in PATH"), "xml-stripblanks", "XMLLINT", "xmllint")
Created attachment 369674 [details] [review] Conditionally warn if pre-processing tools are not found There's no need to unconditionally print an error message if xmllint or json-glib-format are not found when running glib-compile-resources is called; we only need to warn if they are not available when we need them. To avoid spamming the build logs, we can also warn once.
Created attachment 369675 [details] [review] Conditionally warn if pre-processing tools are not found There's no need to unconditionally print an error message if xmllint or json-glib-format are not found when running glib-compile-resources is called; we only need to warn if they are not available when we need them. To avoid spamming the build logs, we can also warn once.
Review of attachment 369675 [details] [review]: Looks good to commit with three minor changes. ::: gio/glib-compile-resources.c @@ +314,3 @@ + "XMLLINT", + "xmllint"); + g_printerr ("%s", warn); Missing \n. Add it here, rather than in the translated string. @@ +368,3 @@ + "JSON_GLIB_FORMAT", + "json-glib-format"); + g_printerr ("%s", warn); Same here. @@ +417,3 @@ + * %s is a command line tool + */ + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, Probably wise to add a comment here clarifying why this one is fatal and the other two aren’t.
Created attachment 369679 [details] [review] Conditionally warn if pre-processing tools are not found There's no need to unconditionally print an error message if xmllint or json-glib-format are not found when running glib-compile-resources is called; we only need to warn if they are not available when we need them. To avoid spamming the build logs, we can also warn once.
Review of attachment 369679 [details] [review]: Go go go.
Attachment 369679 [details] pushed as b4117b8 - Conditionally warn if pre-processing tools are not found
Thanks!