Обсуждение: Few untranslated error messages in OAuth
Hi, During testing of libpq, I noticed several error messages not processed by libpq_gettext, resulting in their exclusion from *.pot files. The attached patch wraps these messages. Best Regards, Hou zj
Вложения
On Thu, Nov 13, 2025 at 06:01:29AM +0000, Zhijie Hou (Fujitsu) wrote: > During testing of libpq, I noticed several error messages not processed by > libpq_gettext, resulting in their exclusion from *.pot files. The attached patch > wraps these messages. (Added Jacob and Daniel in CC.) async_ctx says that errcxt "will be translated for you.", but these are missing from the translation files. So yes, it looks like you are right. -- Michael
Вложения
On 2025-Nov-13, Zhijie Hou (Fujitsu) wrote: > Hi, > > During testing of libpq, I noticed several error messages not processed by > libpq_gettext, resulting in their exclusion from *.pot files. The attached patch > wraps these messages. Yeah, this roughly makes sense. I think the documentation for errctx in async_ctx needs to be updated though, because currently it says "it'll be translated for you" but this patch makes it the other way around (it must come translated). Alternatively, we could mark the strings with gettext_noop(), which only adds the messages to the catalog without translating them; so the responsibility of translating them would remain where it is today. This might be easier. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "No tengo por qué estar de acuerdo con lo que pienso" (Carlos Caszeli)
> On 13 Nov 2025, at 09:13, Álvaro Herrera <alvherre@kurilemu.de> wrote: > On 2025-Nov-13, Zhijie Hou (Fujitsu) wrote: >> During testing of libpq, I noticed several error messages not processed by >> libpq_gettext, resulting in their exclusion from *.pot files. The attached patch >> wraps these messages. Thanks for the report! > Yeah, this roughly makes sense. I think the documentation for errctx in > async_ctx needs to be updated though, because currently it says "it'll > be translated for you" but this patch makes it the other way around (it > must come translated). +1 > Alternatively, we could mark the strings with gettext_noop(), which only > adds the messages to the catalog without translating them; so the > responsibility of translating them would remain where it is today. This > might be easier. I was pondering that since auth error messages in backend libpq does this, but since we don't use gettext_noop anywhere else in frontend libpq today it makes sense to go with libpq_gettext and update the docs instead. Unless you, who has more translation/nls insights than me, feel strongly about gettext_noop I'll go ahead with the current patch. -- Daniel Gustafsson
On 2025-Nov-13, Daniel Gustafsson wrote: > I was pondering that since auth error messages in backend libpq does this, but > since we don't use gettext_noop anywhere else in frontend libpq today it makes > sense to go with libpq_gettext and update the docs instead. > > Unless you, who has more translation/nls insights than me, feel strongly about > gettext_noop I'll go ahead with the current patch. gettext_noop() does something completely different from libpq_gettext(), and the more I think about this, the more I think doing libpq_gettext() is the wrong thing. I can give it a shot if you want. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "Update: super-fast reaction on the Postgres bugs mailing list. The report was acknowledged [...], and a fix is under discussion. The wonders of open-source !" https://twitter.com/gunnarmorling/status/1596080409259003906
> On 13 Nov 2025, at 13:32, Álvaro Herrera <alvherre@kurilemu.de> wrote: > > On 2025-Nov-13, Daniel Gustafsson wrote: > >> I was pondering that since auth error messages in backend libpq does this, but >> since we don't use gettext_noop anywhere else in frontend libpq today it makes >> sense to go with libpq_gettext and update the docs instead. >> >> Unless you, who has more translation/nls insights than me, feel strongly about >> gettext_noop I'll go ahead with the current patch. > > gettext_noop() does something completely different from libpq_gettext(), > and the more I think about this, the more I think doing libpq_gettext() > is the wrong thing. If so, should that be extended to oauth_json_set_error as well? (and possibly others?) > I can give it a shot if you want. If you have time, that would be great. -- Daniel Gustafsson
On Thu, Nov 13, 2025 at 4:49 AM Daniel Gustafsson <daniel@yesql.se> wrote: > If so, should that be extended to oauth_json_set_error as well? (and possibly others?) That should be covered already by nls.mk, unless I messed that up too... I'll do a deeper dive. Another way to fix this would be to wrap the assignment to errctx in a macro that nls.mk can key off of, like oauth_json_set_error() does. But I'm curious what Alvaro has in mind? --Jacob
On Thu, Nov 13, 2025 at 8:23 AM Jacob Champion <jacob.champion@enterprisedb.com> wrote: > On Thu, Nov 13, 2025 at 4:49 AM Daniel Gustafsson <daniel@yesql.se> wrote: > > If so, should that be extended to oauth_json_set_error as well? (and possibly others?) > > That should be covered already by nls.mk, unless I messed that up > too... I'll do a deeper dive. I misread your email, sorry. We trigger on oauth_parse_set_error(), but not oauth_json_set_error(). I'll see how easy that is to change. There's more to fix here, after talking with Alvaro offlist. I got gettext to tell me [1] what else was missing and found that - fe-auth-oauth.c is not in GETTEXT_FILES at all - I didn't give jsonapi.c the same handling as in 3ddbac368 when I ported it to libpq, so it's not translated either I'm writing up a patch that combines all of that. Thanks, --Jacob [1] https://www.gnu.org/software/gettext/manual/html_node/Prioritizing-messages.html
On Thu, Nov 13, 2025 at 3:08 PM Jacob Champion <jacob.champion@enterprisedb.com> wrote: > I'm writing up a patch that combines all of that. And here's what I have so far. I could use help double-checking the .po result; I'm not entirely sure my usage of update-po is correct. I took this opportunity to address a complaint from Alvaro and Peter E a little while back, that some of the more internal-facing error messages are very difficult to translate (and wouldn't help users even if translation were easy). I can pull that (v2-0002) into its own thread if necessary. - v2-0001: combines Zhijie Hou's patch with the gettext_noop() suggestion from Alvaro and fixes the nls.mk omission - v2-0002: removes translation of multiplexer failures by adding an _internal macro - v2-0003: aligns oauth_json_set_error() with the prior commits - v2-0004: tries to get jsonapi.c translated too Unfortunately v2-0004 doesn't work. It puts the messages into the translation files, but we use the _() macro throughout jsonapi.c, which isn't helpful for libpq because libpq uses its own text domain. This was an oversight in the work done for 0785d1b8b, I think, and it may need its own patchset unless someone has a really quick fix. Please let me know if I forgot to address something already said above. I assume translation changes such as these are generally backportable? Thanks, --Jacob