Re: [HACKERS] add more NLS to bin

[Michael Paquier]

On Tue, Nov 8, 2016 at 12:13 AM, Peter Eisentraut
 wrote:
> On 11/5/16 8:03 AM, Michael Paquier wrote:
>> On Sat, Nov 5, 2016 at 9:17 AM, Peter Eisentraut
>>  wrote:
>>> > On 11/3/16 7:17 PM, Michael Paquier wrote:
 >> This patch not being complicated, so I would vote for those being
 >> addressed now so as they are not forgotten even if there is a FIXME
 >> flag added. Perhaps you don't think so, and as that's a minor issue
 >> I'll be fine with your judgement as well.
>>> >
>>> > OK, I just wrapped it in translation markers as is, which should work
>>> > well enough.
>> Agreed. I don't see anything wrong with that.
>
> committed and closed the patch

Thanks for adjusting pg_rewind as well.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Peter Eisentraut]

On 11/5/16 8:03 AM, Michael Paquier wrote:
> On Sat, Nov 5, 2016 at 9:17 AM, Peter Eisentraut
>  wrote:
>> > On 11/3/16 7:17 PM, Michael Paquier wrote:
>>> >> This patch not being complicated, so I would vote for those being
>>> >> addressed now so as they are not forgotten even if there is a FIXME
>>> >> flag added. Perhaps you don't think so, and as that's a minor issue
>>> >> I'll be fine with your judgement as well.
>> >
>> > OK, I just wrapped it in translation markers as is, which should work
>> > well enough.
> Agreed. I don't see anything wrong with that.

committed and closed the patch

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Peter Eisentraut]

On 11/5/16 8:03 AM, Michael Paquier wrote:
>> > Yeah that was wrong anyway.  The previously existing translation markers
>> > were wrong.  We want to translate the fmt, not the formatted message.
> Does using one way or the other actually change something? Because
> pg_rewind/logging.c is not marking fmt but the message, the opposite
> of your v2 patch. One way or the other, having consistency for both
> would be nice.

Consider the following for example:

pg_log(PG_DEBUG, "fetched file \"%s\", length %d\n", filename, len);

If you pass fmt to gettext(), then it will look up the string that you
see in the code.  If you pass the formatted message to gettext(), then
it will look up something like

"fetched file \"foo\", length 12\n"

which it will not find, unless you provide translations for all
combinations of file names and lengths.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Michael Paquier]

On Sat, Nov 5, 2016 at 9:17 AM, Peter Eisentraut
 wrote:
> On 11/3/16 7:17 PM, Michael Paquier wrote:
>> This patch not being complicated, so I would vote for those being
>> addressed now so as they are not forgotten even if there is a FIXME
>> flag added. Perhaps you don't think so, and as that's a minor issue
>> I'll be fine with your judgement as well.
>
> OK, I just wrapped it in translation markers as is, which should work
> well enough.

Agreed. I don't see anything wrong with that.

 In util.c, doesn't pg_log_v() need to handle strings used in fprintf?
>>>
>>> Which specific lines do you have in mind?
>>
>> The verbose logs at the top. In pg_rewind for example those logs are
>> getting translated via the pg_log() calls used with PG_DEBUG.
>
> Yeah that was wrong anyway.  The previously existing translation markers
> were wrong.  We want to translate the fmt, not the formatted message.

Does using one way or the other actually change something? Because
pg_rewind/logging.c is not marking fmt but the message, the opposite
of your v2 patch. One way or the other, having consistency for both
would be nice.

The patch looks good to me, the instructions for the script could be
changed... But you are right let's not mess up with non-ASCII
characters in it, like from some Asian language translation.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Peter Eisentraut]

On 11/3/16 7:17 PM, Michael Paquier wrote:
> This patch not being complicated, so I would vote for those being
> addressed now so as they are not forgotten even if there is a FIXME
> flag added. Perhaps you don't think so, and as that's a minor issue
> I'll be fine with your judgement as well.

OK, I just wrapped it in translation markers as is, which should work
well enough.

>>> In util.c, doesn't pg_log_v() need to handle strings used in fprintf?
>>
>> Which specific lines do you have in mind?
> 
> The verbose logs at the top. In pg_rewind for example those logs are
> getting translated via the pg_log() calls used with PG_DEBUG.

Yeah that was wrong anyway.  The previously existing translation markers
were wrong.  We want to translate the fmt, not the formatted message.

New patch attached.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 91a5d23f1d8a188d11de2f2dfc6d455ebf04cfc5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Fri, 14 Oct 2016 12:00:00 -0400
Subject: [PATCH v2 1/3] pg_upgrade: Add NLS

---
 src/bin/pg_upgrade/function.c|   2 +-
 src/bin/pg_upgrade/info.c|   8 ++--
 src/bin/pg_upgrade/nls.mk|  12 +
 src/bin/pg_upgrade/option.c  | 101 +++
 src/bin/pg_upgrade/pg_upgrade.c  |   1 +
 src/bin/pg_upgrade/relfilenode.c |   6 ++-
 src/bin/pg_upgrade/server.c  |   4 +-
 src/bin/pg_upgrade/util.c|  14 +++---
 8 files changed, 80 insertions(+), 68 deletions(-)
 create mode 100644 src/bin/pg_upgrade/nls.mk

diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index 3009340..5b60f9f 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -252,7 +252,7 @@ check_loadable_libraries(void)
 			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
 pg_fatal("could not open file \"%s\": %s\n",
 		 output_path, strerror(errno));
-			fprintf(script, "could not load library \"%s\":\n%s\n",
+			fprintf(script, _("could not load library \"%s\":\n%s\n"),
 	lib,
 	PQerrorMessage(conn));
 		}
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 1200c7f..8af9eac 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -238,7 +238,7 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
 			{
 snprintf(reldesc + strlen(reldesc),
 		 sizeof(reldesc) - strlen(reldesc),
-		 " which is an index on \"%s.%s\"",
+		 _(" which is an index on \"%s.%s\""),
 		 hrel->nspname, hrel->relname);
 /* Shift attention to index's table for toast check */
 rel = hrel;
@@ -248,7 +248,7 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
 		if (i >= db->rel_arr.nrels)
 			snprintf(reldesc + strlen(reldesc),
 	 sizeof(reldesc) - strlen(reldesc),
-	 " which is an index on OID %u", rel->indtable);
+	 _(" which is an index on OID %u"), rel->indtable);
 	}
 	if (rel->toastheap)
 	{
@@ -260,7 +260,7 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
 			{
 snprintf(reldesc + strlen(reldesc),
 		 sizeof(reldesc) - strlen(reldesc),
-		 " which is the TOAST table for \"%s.%s\"",
+		 _(" which is the TOAST table for \"%s.%s\""),
 		 brel->nspname, brel->relname);
 break;
 			}
@@ -268,7 +268,7 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
 		if (i >= db->rel_arr.nrels)
 			snprintf(reldesc + strlen(reldesc),
 	 sizeof(reldesc) - strlen(reldesc),
-	 " which is the TOAST table for OID %u", rel->toastheap);
+	 _(" which is the TOAST table for OID %u"), rel->toastheap);
 	}
 
 	if (is_new_db)
diff --git a/src/bin/pg_upgrade/nls.mk b/src/bin/pg_upgrade/nls.mk
new file mode 100644
index 000..a0c846d
--- /dev/null
+++ b/src/bin/pg_upgrade/nls.mk
@@ -0,0 +1,12 @@
+# src/bin/pg_upgrade/nls.mk
+CATALOG_NAME = pg_upgrade
+AVAIL_LANGUAGES  =
+GETTEXT_FILES= check.c controldata.c dump.c exec.c file.c function.c \
+   info.c option.c parallel.c pg_upgrade.c relfilenode.c \
+   server.c tablespace.c util.c version.c
+GETTEXT_TRIGGERS = pg_fatal pg_log:2 prep_status report_status:2
+GETTEXT_FLAGS= \
+pg_fatal:1:c-format \
+pg_log:2:c-format \
+prep_status:1:c-format \
+report_status:2:c-format
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 2e9a40c..12a49ff 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -240,13 +240,13 @@ parseCommandLine(int argc, char *argv[])
 
 	/* Get values from env if not already set */
 	check_required_directory(_cluster.bindir, NULL, "PGBINOLD", "-b",
-			 "old cluster binaries reside");
+			 _("old cluster binaries reside"));
 	check_required_directory(_cluster.bindir, NULL, "PGBINNEW", "-B",
-			 "new 

Re: [HACKERS] add more NLS to bin

[Michael Paquier]

On Fri, Nov 4, 2016 at 1:44 AM, Peter Eisentraut
 wrote:
> On 10/31/16 1:58 AM, Michael Paquier wrote:
>> In info.c, missing some entries in report_unmatched_relation() when
>> reporting unmatching relations?
>
> Yeah, that will need a bit of a rewrite, so FIXME later?

This patch not being complicated, so I would vote for those being
addressed now so as they are not forgotten even if there is a FIXME
flag added. Perhaps you don't think so, and as that's a minor issue
I'll be fine with your judgement as well.

>> In util.c, doesn't pg_log_v() need to handle strings used in fprintf?
>
> Which specific lines do you have in mind?

The verbose logs at the top. In pg_rewind for example those logs are
getting translated via the pg_log() calls used with PG_DEBUG.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Peter Eisentraut]

On 10/31/16 1:58 AM, Michael Paquier wrote:
> 2) For 0002 and pg_test_fsync, I am seeing a missing entry:
> printf(NA_FORMAT, "n/a*\n");

ok

> 4) 0004 and pg_upgrade... In check.c, three places like that:
> if (!db_used)
> {
> fprintf(script, "Database: %s\n", active_db->db_name);
> db_used = true;
> }
> 
> In exec.c:
> #endif
> fprintf(log, "command: %s\n", cmd);
> #ifdef WIN32

These and several of the other places write into log or script files.  I
have chosen not to do anything about those for now.  That might be a
future change, or we just leave them.

> +GETTEXT_FLAGS= \
> +pg_fatal:1:c-format \
> +pg_log:2:c-format \
> +prep_status:1:c-format \
> +report_stats:2:c-forma
> s/report_stats/report_status/

ok

> In info.c, missing some entries in report_unmatched_relation() when
> reporting unmatching relations?

Yeah, that will need a bit of a rewrite, so FIXME later?

> In util.c, doesn't pg_log_v() need to handle strings used in fprintf?

Which specific lines do you have in mind?

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Michael Paquier]

On Thu, Oct 27, 2016 at 10:02 AM, Peter Eisentraut
 wrote:
> Here is a series of patches to add NLS to the remaining bin programs,
> which were moved from contrib a while ago.  (If you're missing pgbench,
> I'm skipping that for now because it's more complicated.)  I'll add this
> to the commit fest.

I have been looking at this patch set, and that's definitely a good
idea to do this change.
1) 0001 for pg_archivecleanup is missing nothing.

2) For 0002 and pg_test_fsync, I am seeing a missing entry:
printf(NA_FORMAT, "n/a*\n");

3) For pg_test_timing, the doc changes could be into a separate
change, but that's fine to group them as well. I am seeing no missing
strings for translations.

4) 0004 and pg_upgrade... In check.c, three places like that:
if (!db_used)
{
fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}

In exec.c:
#endif
fprintf(log, "command: %s\n", cmd);
#ifdef WIN32

+GETTEXT_FLAGS= \
+pg_fatal:1:c-format \
+pg_log:2:c-format \
+prep_status:1:c-format \
+report_stats:2:c-forma
s/report_stats/report_status/

In info.c, missing some entries in report_unmatched_relation() when
reporting unmatching relations?

In parseCommandLine() of option.c, the "pg_upgrade run on" string
needs to be handled.

In util.c, doesn't pg_log_v() need to handle strings used in fprintf?

In version.c, this one:
if (!db_used)
{
fprintf(script, "Database: %s\n", active_db->db_name);
db_used = true;
}


5) 0005 and pg_xlogdump, I am not seeing a missing entry.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Michael Paquier]

On Thu, Oct 27, 2016 at 10:27 AM, Peter Eisentraut
 wrote:
> On 10/26/16 9:18 PM, Michael Paquier wrote:
>> Side question just for curiosity: when is AVAIL_LANGUAGES filled? Once
>> the translations are added to the tree?
>
> Yes, that happens when the translations are copied in before a release.
>
> See here for the script that is normally used:
> https://git.postgresql.org/gitweb/?p=pgtranslation/admin.git;a=blob;f=cp-po;h=5b569a43e39a7c1fd5e28ba00a6708995df1b445;hb=57f0390ed14ecb974d2bde6242ce7b268e8261d2#l216

Ah, thanks. That explains the whole thing!
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Peter Eisentraut]

On 10/26/16 9:18 PM, Michael Paquier wrote:
> Side question just for curiosity: when is AVAIL_LANGUAGES filled? Once
> the translations are added to the tree?

Yes, that happens when the translations are copied in before a release.

See here for the script that is normally used:
https://git.postgresql.org/gitweb/?p=pgtranslation/admin.git;a=blob;f=cp-po;h=5b569a43e39a7c1fd5e28ba00a6708995df1b445;hb=57f0390ed14ecb974d2bde6242ce7b268e8261d2#l216

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] add more NLS to bin

[Michael Paquier]

On Thu, Oct 27, 2016 at 10:02 AM, Peter Eisentraut
 wrote:
> Here is a series of patches to add NLS to the remaining bin programs,
> which were moved from contrib a while ago.  (If you're missing pgbench,
> I'm skipping that for now because it's more complicated.)  I'll add this
> to the commit fest.

+1.

Side question just for curiosity: when is AVAIL_LANGUAGES filled? Once
the translations are added to the tree?
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers