Make prep_status() message translatable

Started by Kyotaro Horiguchi9 months ago4 messages

horikyota.ntt@gmail.com

9 months ago

1 attachment(s)

Hello,

The recent commit 173c97812ff made the following change:

-	prep_status("Adding \".old\" suffix to old global/pg_control");
+	prep_status("Adding \".old\" suffix to old " XLOG_CONTROL_FILE);

This change results in a message that is untranslatable, at least into
Japanese. In addition, the file name should be quoted.

The attached patch modifies the message to use %s for XLOG_CONTROL_FILE,
making it properly translatable.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

0001-Make-prep_status-message-translatable.patchtext/x-patch; charset=us-asciiDownload

From 793131e1a75821531f0aff61e0ef244229cb64b5 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Mon, 7 Apr 2025 15:48:43 +0900
Subject: [PATCH] Make prep_status() message translatable

Avoid string literal concatenation involving XLOG_CONTROL_FILE, which
was introduced by commit 73c97812ff and made the message
untranslatable. Use %s instead, and quote the file name for
consistency with similar messages.
---
 src/bin/pg_upgrade/controldata.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 8b7c349a875..21bf719f697 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -758,7 +758,8 @@ disable_old_cluster(transferMode transfer_mode)
 				new_path[MAXPGPATH];
 
 	/* rename pg_control so old server cannot be accidentally started */
-	prep_status("Adding \".old\" suffix to old " XLOG_CONTROL_FILE);
+	/* translator: %s is the file path of the control file. */
+	prep_status("Adding \".old\" suffix to old \"%s\"", XLOG_CONTROL_FILE);
 
 	snprintf(old_path, sizeof(old_path), "%s/%s", old_cluster.pgdata, XLOG_CONTROL_FILE);
 	snprintf(new_path, sizeof(new_path), "%s/%s.old", old_cluster.pgdata, XLOG_CONTROL_FILE);
-- 
2.43.5

Fujii Masao

masao.fujii@oss.nttdata.com

9 months ago

In reply to: Kyotaro Horiguchi (#1)

Re: Make prep_status() message translatable

On 2025/04/07 15:55, Kyotaro Horiguchi wrote:

Hello,

The recent commit 173c97812ff made the following change:
-	prep_status("Adding \".old\" suffix to old global/pg_control");
+	prep_status("Adding \".old\" suffix to old " XLOG_CONTROL_FILE);
This change results in a message that is untranslatable, at least into
Japanese. In addition, the file name should be quoted.

The attached patch modifies the message to use %s for XLOG_CONTROL_FILE,
making it properly translatable.

Thanks for the report and patch!

The fix looks good to me.

pg_log(PG_REPORT, "\n"
"If you want to start the old cluster, you will need to remove\n"
"the \".old\" suffix from %s/%s.old.\n"
"Because \"link\" mode was used, the old cluster cannot be safely\n"
"started once the new cluster has been started.",
old_cluster.pgdata, XLOG_CONTROL_FILE);

Commit 173c97812ff also updated the above part of disable_old_cluster()
and replaced the hardcoded "global/pg_control" with %s using XLOG_CONTROL_FILE.
Maybe we should also add a translator: comment and wrap %s/%s.old in
double quotes there?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Fujii Masao

masao.fujii@oss.nttdata.com

9 months ago

In reply to: Fujii Masao (#2)

1 attachment(s)

Re: Make prep_status() message translatable

On 2025/04/07 16:26, Fujii Masao wrote:

On 2025/04/07 15:55, Kyotaro Horiguchi wrote:
Hello,

The recent commit 173c97812ff made the following change:
-    prep_status("Adding \".old\" suffix to old global/pg_control");
+    prep_status("Adding \".old\" suffix to old " XLOG_CONTROL_FILE);
This change results in a message that is untranslatable, at least into
Japanese. In addition, the file name should be quoted.

The attached patch modifies the message to use %s for XLOG_CONTROL_FILE,
making it properly translatable.
Thanks for the report and patch!

The fix looks good to me.

        pg_log(PG_REPORT, "\n"
               "If you want to start the old cluster, you will need to remove\n"
               "the \".old\" suffix from %s/%s.old.\n"
               "Because \"link\" mode was used, the old cluster cannot be safely\n"
               "started once the new cluster has been started.",
               old_cluster.pgdata, XLOG_CONTROL_FILE);

Commit 173c97812ff also updated the above part of disable_old_cluster()
and replaced the hardcoded "global/pg_control" with %s using XLOG_CONTROL_FILE.
Maybe we should also add a translator: comment and wrap %s/%s.old in
double quotes there?

I've updated the patch to reflect this comment.
Barring any objections, I'm thinking to commit this patch.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Attachments:

v2-0001-Make-pg_upgrade-log-message-with-control-file-pat.patchtext/plain; charset=UTF-8; name=v2-0001-Make-pg_upgrade-log-message-with-control-file-pat.patchDownload

From 36c91f4ecf21cdaabbb49d46f3a967e89b4f6714 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Thu, 17 Apr 2025 12:44:15 +0900
Subject: [PATCH v2] Make pg_upgrade log message with control file path
 translatable.

Commit 173c97812ff replaced the hardcoded "global/pg_control" in pg_upgrade
log message with a string literal concatenation of XLOG_CONTROL_FILE macro.
However, this change made the message untranslatable.

This commit fixes the issue by using %s with XLOG_CONTROL_FILE instead of
that literal concatenation, allowing the message to be translated properly.
It also wraps the file path in double quotes for consistency with similar
log messages.

Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Masao Fujii <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/20250407.155546.2129693791769531891.horikyota.ntt@gmail.com
---
 src/bin/pg_upgrade/controldata.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 8b7c349a875..90cef0864de 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -758,7 +758,8 @@ disable_old_cluster(transferMode transfer_mode)
 				new_path[MAXPGPATH];
 
 	/* rename pg_control so old server cannot be accidentally started */
-	prep_status("Adding \".old\" suffix to old " XLOG_CONTROL_FILE);
+	/* translator: %s is the file path of the control file */
+	prep_status("Adding \".old\" suffix to old \"%s\"", XLOG_CONTROL_FILE);
 
 	snprintf(old_path, sizeof(old_path), "%s/%s", old_cluster.pgdata, XLOG_CONTROL_FILE);
 	snprintf(new_path, sizeof(new_path), "%s/%s.old", old_cluster.pgdata, XLOG_CONTROL_FILE);
@@ -768,9 +769,10 @@ disable_old_cluster(transferMode transfer_mode)
 	check_ok();
 
 	if (transfer_mode == TRANSFER_MODE_LINK)
+		/* translator: %s/%s is the file path of the control file */
 		pg_log(PG_REPORT, "\n"
 			   "If you want to start the old cluster, you will need to remove\n"
-			   "the \".old\" suffix from %s/%s.old.\n"
+			   "the \".old\" suffix from \"%s/%s.old\".\n"
 			   "Because \"link\" mode was used, the old cluster cannot be safely\n"
 			   "started once the new cluster has been started.",
 			   old_cluster.pgdata, XLOG_CONTROL_FILE);
-- 
2.49.0

Fujii Masao

masao.fujii@oss.nttdata.com

9 months ago

In reply to: Fujii Masao (#3)

Re: Make prep_status() message translatable

On 2025/04/17 13:12, Fujii Masao wrote:

I've updated the patch to reflect this comment.
Barring any objections, I'm thinking to commit this patch.

Pushed. Thanks!

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION