Re: [libvirt] [PATCH 01/30] Don't mark log messages for translation

2010/4/5 Eric Blake <eblake(a)redhat.com&gt;: > On 04/04/2010 11:36 AM, Matthias Bolte wrote: >> Also remove manually added function names from log messages, the logging >> macros already record them and the logging framework outputs them. > > I see in [1] where the translation was first added, but no justification > there for why log messages are marked, nor in your patch why they should > not be marked.  I'm not opposed to the idea, but I think it would make a > lot more sense if the commit message explained exactly why we don't > think log messages deserve translations. > > [1] https://www.redhat.com/archives/libvir-list/2009-January/msg00005.html > >> +++ b/cfg.mk >> @@ -168,28 +168,16 @@ sc_prohibit_gethostby: >>  # |grep -vE '^(qsort|if|close|assert|fputc|free|N_|vir.*GetName|.*Unlock|virNodeListDevices|virHashRemoveEntry|freeaddrinfo|.*[fF]ree|xdrmem_create|xmlXPathFreeObject|virUUIDFormat|openvzSetProgramSentinal|polkit_action_unref)$' >> >>  msg_gen_function = >> -msg_gen_function += DEBUG0 > > For example, I can easily see why DEBUG0 does not need translation (it > is for developers, and should never be expanded into a log message in a > distro's installation, so why waste the translator's efforts on it)... > >> -msg_gen_function += DISABLE_fprintf >> -msg_gen_function += ERROR >> -msg_gen_function += ERROR0 >> -msg_gen_function += REMOTE_DEBUG >>  msg_gen_function += ReportError >> -msg_gen_function += VIR_FREE >> -msg_gen_function += VIR_INFO > > But the name VIR_INFO seems like it might be something the end user may > eventually want to see in their own language.  Maybe the reasoning that > I'm not seeing here is that grep-ability of system logs is more > important than translating system logs into the user's language? > >> @@ -236,7 +223,7 @@ func_re := ($(func_or)) >>  #    "%s", _("no storage vol w..." >>  sc_libvirt_unmarked_diagnostics: >>       @grep -nE                                                       \ >> -         '\<$(func_re) \([^"]*"[^"]*[a-z]{3}' $$($(VC_LIST_EXCEPT))  \ >> +         '\<$(func_re) *\([^"]*"[^"]*[a-z]{3}' $$($(VC_LIST_EXCEPT)) \ >>         | grep -v '_''(' &&                                           \ >>         { echo '$(ME): found unmarked diagnostic(s)' 1>&2;            \ >>           exit 1; } || : > > ACK to this hunk, independently of the overall issue on why we need this > patch. > >> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c >> index 208ffca..68fe95a 100644 >> --- a/daemon/libvirtd.c >> +++ b/daemon/libvirtd.c >> @@ -272,7 +272,7 @@ remoteCheckCertFile(const char *type, const char *file) >>      struct stat sb; >>      if (stat(file, &sb) < 0) { >>          char ebuf[1024]; >> -        VIR_ERROR(_("Cannot access %s '%s': %s"), >> +        VIR_ERROR("Cannot access %s '%s': %s", >>                    type, file, virStrerror(errno, ebuf, sizeof ebuf)); >>          return -1; >>      } > > Again, this seems like something the user would want to see in their own > language.  So while the rest of this patch looks mechanically correct, > I'm hesitant to give an ACK without more reasoning on which functions > deserve to bypass translation; it may requiring reducing the scope of > this patch. > DanPB's argument [1] against marking log messages for translation was mainly that they aren't meant for the end user, but for debugging purpose and to be included in bug reports.

On the other hand one can argue about VIR_ERROR on the libvirtd side where it's used to report critical errors like the one you quoted above.