[pacman-dev] [PATCH 1/3] Fix pacman's log messages not being translated
Log messages should be translated as well. (Those from ALPM already are.) Signed-off-by: Olivier Brunel <i.am.jack.mail@gmail.com> --- src/pacman/callback.c | 6 +++--- src/pacman/pacman.c | 2 +- src/pacman/sync.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 01c6b61..c56c96a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -176,7 +176,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_ADD_DONE: - alpm_logaction(config->handle, "installed %s (%s)\n", + alpm_logaction(config->handle, _("installed %s (%s)\n"), alpm_pkg_get_name(data1), alpm_pkg_get_version(data1)); display_optdepends(data1); @@ -187,7 +187,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_REMOVE_DONE: - alpm_logaction(config->handle, "removed %s (%s)\n", + alpm_logaction(config->handle, _("removed %s (%s)\n"), alpm_pkg_get_name(data1), alpm_pkg_get_version(data1)); break; @@ -197,7 +197,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_UPGRADE_DONE: - alpm_logaction(config->handle, "upgraded %s (%s -> %s)\n", + alpm_logaction(config->handle, _("upgraded %s (%s -> %s)\n"), alpm_pkg_get_name(data1), alpm_pkg_get_version(data2), alpm_pkg_get_version(data1)); diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 1ca746d..7db46d7 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -743,7 +743,7 @@ static void cl_to_log(int argc, char *argv[]) *p++ = ' '; } strcpy(p, argv[i]); - alpm_logaction(config->handle, "Running '%s'\n", cl_text); + alpm_logaction(config->handle, _("Running '%s'\n"), cl_text); free(cl_text); } diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 532a667..938f9d6 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -787,7 +787,7 @@ static int sync_trans(alpm_list_t *targets) if(config->op_s_upgrade) { printf(_(":: Starting full system upgrade...\n")); - alpm_logaction(config->handle, "starting full system upgrade\n"); + alpm_logaction(config->handle, _("starting full system upgrade\n")); if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >= 2) == -1) { pm_printf(ALPM_LOG_ERROR, "%s\n", alpm_strerror(alpm_errno(config->handle))); trans_release(); @@ -954,7 +954,7 @@ int pacman_sync(alpm_list_t *targets) if(config->op_s_sync) { /* grab a fresh package list */ printf(_(":: Synchronizing package databases...\n")); - alpm_logaction(config->handle, "synchronizing package lists\n"); + alpm_logaction(config->handle, _("synchronizing package lists\n")); if(!sync_synctree(config->op_s_sync, sync_dbs)) { return 1; } -- 1.8.0.2
Instead of always logging "synchronizing package databases" we only log when (and which) db was synchronized. Signed-off-by: Olivier Brunel <i.am.jack.mail@gmail.com> --- src/pacman/sync.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 938f9d6..6c8923d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -338,6 +338,8 @@ static int sync_synctree(int level, alpm_list_t *syncs) success++; } else { success++; + alpm_logaction(config->handle, _("synchronized database %s\n"), + alpm_db_get_name(db)); } } @@ -954,7 +956,6 @@ int pacman_sync(alpm_list_t *targets) if(config->op_s_sync) { /* grab a fresh package list */ printf(_(":: Synchronizing package databases...\n")); - alpm_logaction(config->handle, _("synchronizing package lists\n")); if(!sync_synctree(config->op_s_sync, sync_dbs)) { return 1; } -- 1.8.0.2
On 13/12/12 21:38, Olivier Brunel wrote:
Instead of always logging "synchronizing package databases" we only log when (and which) db was synchronized.
This gains us very little because we do not know if a database was not updated due to being up-to-date or due to a failure. So if this is to be included, I think it would need logging for all situations.
Signed-off-by: Olivier Brunel <i.am.jack.mail@gmail.com> --- src/pacman/sync.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 938f9d6..6c8923d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -338,6 +338,8 @@ static int sync_synctree(int level, alpm_list_t *syncs) success++; } else { success++; + alpm_logaction(config->handle, _("synchronized database %s\n"), + alpm_db_get_name(db)); } }
@@ -954,7 +956,6 @@ int pacman_sync(alpm_list_t *targets) if(config->op_s_sync) { /* grab a fresh package list */ printf(_(":: Synchronizing package databases...\n")); - alpm_logaction(config->handle, _("synchronizing package lists\n")); if(!sync_synctree(config->op_s_sync, sync_dbs)) { return 1; }
The "starting sysupgrade" message was logged quite soon, making it be added even when nothing was actually done, because there was nothing to do, the user didn't confirm, or asked to only download packages. The log message is now added only when (before) committing the transaction. And we also log a message at the end (in case of success or failure). Signed-off-by: Olivier Brunel <i.am.jack.mail@gmail.com> --- src/pacman/sync.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 6c8923d..da014a7 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -789,7 +789,6 @@ static int sync_trans(alpm_list_t *targets) if(config->op_s_upgrade) { printf(_(":: Starting full system upgrade...\n")); - alpm_logaction(config->handle, _("starting full system upgrade\n")); if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >= 2) == -1) { pm_printf(ALPM_LOG_ERROR, "%s\n", alpm_strerror(alpm_errno(config->handle))); trans_release(); @@ -875,6 +874,7 @@ int sync_prepare_execute(void) goto cleanup; } + alpm_logaction(config->handle, _("starting full system upgrade\n")); if(alpm_trans_commit(config->handle, &data) == -1) { alpm_errno_t err = alpm_errno(config->handle); pm_printf(ALPM_LOG_ERROR, _("failed to commit transaction (%s)\n"), @@ -907,11 +907,14 @@ int sync_prepare_execute(void) default: break; } + alpm_logaction(config->handle, _("failed to commit transaction (%s)\n"), + alpm_strerror(err)); /* TODO: stderr? */ printf(_("Errors occurred, no packages were upgraded.\n")); retval = 1; goto cleanup; } + alpm_logaction(config->handle, _("full system upgrade completed\n")); /* Step 4: release transaction resources */ cleanup: -- 1.8.0.2
On Dec 13, 2012 6:39 AM, "Olivier Brunel" <i.am.jack.mail@gmail.com> wrote:
The "starting sysupgrade" message was logged quite soon, making it be
even when nothing was actually done, because there was nothing to do, the user didn't confirm, or asked to only download packages.
The log message is now added only when (before) committing the
added transaction. And
we also log a message at the end (in case of success or failure).
Signed-off-by: Olivier Brunel <i.am.jack.mail@gmail.com> --- src/pacman/sync.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 6c8923d..da014a7 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -789,7 +789,6 @@ static int sync_trans(alpm_list_t *targets)
if(config->op_s_upgrade) { printf(_(":: Starting full system upgrade...\n")); - alpm_logaction(config->handle, _("starting full system upgrade\n")); if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >= 2) == -1) { pm_printf(ALPM_LOG_ERROR, "%s\n", alpm_strerror(alpm_errno(config->handle))); trans_release(); @@ -875,6 +874,7 @@ int sync_prepare_execute(void) goto cleanup; }
+ alpm_logaction(config->handle, _("starting full system upgrade\n"));
But you don't know that you're running a full upgrade here... I suspect you'd log this on any -S operation.
if(alpm_trans_commit(config->handle, &data) == -1) { alpm_errno_t err = alpm_errno(config->handle); pm_printf(ALPM_LOG_ERROR, _("failed to commit transaction
@@ -907,11 +907,14 @@ int sync_prepare_execute(void) default: break; } + alpm_logaction(config->handle, _("failed to commit
(%s)\n"), transaction (%s)\n"),
+ alpm_strerror(err)); /* TODO: stderr? */ printf(_("Errors occurred, no packages were upgraded.\n")); retval = 1; goto cleanup; } + alpm_logaction(config->handle, _("full system upgrade completed\n"));
/* Step 4: release transaction resources */ cleanup: -- 1.8.0.2
On 12/13/12 12:41, Dave Reisner wrote:
On Dec 13, 2012 6:39 AM, "Olivier Brunel" <i.am.jack.mail@gmail.com> wrote:
The "starting sysupgrade" message was logged quite soon, making it be
even when nothing was actually done, because there was nothing to do, the user didn't confirm, or asked to only download packages.
The log message is now added only when (before) committing the
added transaction. And
we also log a message at the end (in case of success or failure).
Signed-off-by: Olivier Brunel <i.am.jack.mail@gmail.com> --- src/pacman/sync.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 6c8923d..da014a7 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -789,7 +789,6 @@ static int sync_trans(alpm_list_t *targets)
if(config->op_s_upgrade) { printf(_(":: Starting full system upgrade...\n")); - alpm_logaction(config->handle, _("starting full system upgrade\n")); if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >= 2) == -1) { pm_printf(ALPM_LOG_ERROR, "%s\n", alpm_strerror(alpm_errno(config->handle))); trans_release(); @@ -875,6 +874,7 @@ int sync_prepare_execute(void) goto cleanup; }
+ alpm_logaction(config->handle, _("starting full system upgrade\n"));
But you don't know that you're running a full upgrade here... I suspect you'd log this on any -S operation.
Oh, you're right. My bad, I'll send a revised version.
if(alpm_trans_commit(config->handle, &data) == -1) { alpm_errno_t err = alpm_errno(config->handle); pm_printf(ALPM_LOG_ERROR, _("failed to commit transaction
@@ -907,11 +907,14 @@ int sync_prepare_execute(void) default: break; } + alpm_logaction(config->handle, _("failed to commit
(%s)\n"), transaction (%s)\n"),
+ alpm_strerror(err)); /* TODO: stderr? */ printf(_("Errors occurred, no packages were upgraded.\n")); retval = 1; goto cleanup; } + alpm_logaction(config->handle, _("full system upgrade completed\n"));
/* Step 4: release transaction resources */ cleanup: -- 1.8.0.2
The "starting sysupgrade" message was logged quite soon, making it be added even when nothing was actually done, because there was nothing to do, the user didn't confirm, or asked to only download packages. The log message is now added only when (before) committing the transaction. And we also log a message at the end (in case of success or failure). Signed-off-by: Olivier Brunel <i.am.jack.mail@gmail.com> --- src/pacman/sync.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 6c8923d..2dca6ea 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -789,7 +789,6 @@ static int sync_trans(alpm_list_t *targets) if(config->op_s_upgrade) { printf(_(":: Starting full system upgrade...\n")); - alpm_logaction(config->handle, _("starting full system upgrade\n")); if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >= 2) == -1) { pm_printf(ALPM_LOG_ERROR, "%s\n", alpm_strerror(alpm_errno(config->handle))); trans_release(); @@ -875,6 +874,9 @@ int sync_prepare_execute(void) goto cleanup; } + if(config->op_s_upgrade) { + alpm_logaction(config->handle, _("starting full system upgrade\n")); + } if(alpm_trans_commit(config->handle, &data) == -1) { alpm_errno_t err = alpm_errno(config->handle); pm_printf(ALPM_LOG_ERROR, _("failed to commit transaction (%s)\n"), @@ -907,11 +909,16 @@ int sync_prepare_execute(void) default: break; } + alpm_logaction(config->handle, _("failed to commit transaction (%s)\n"), + alpm_strerror(err)); /* TODO: stderr? */ printf(_("Errors occurred, no packages were upgraded.\n")); retval = 1; goto cleanup; } + alpm_logaction(config->handle, (config->op_s_upgrade) + ? _("full system upgrade completed\n") + : _("installation completed\n")); /* Step 4: release transaction resources */ cleanup: -- 1.8.0.2
On 13/12/12 21:38, Olivier Brunel wrote:
Log messages should be translated as well. (Those from ALPM already are.)
I can not find a single alpm_logaction line in libalpm with a translated string...
On 12/13/12 12:48, Allan McRae wrote:
On 13/12/12 21:38, Olivier Brunel wrote:
Log messages should be translated as well. (Those from ALPM already are.)
I can not find a single alpm_logaction line in libalpm with a translated string...
Right, not sure what I did, but I got that all wrong indeed, apologies. Should I resend the other two patches without the translation in then? Quick question also: while not a native English speaker, I don't like/use translations myself; Is it how things usually work, translations are for user interface only, and don't apply to logs? Or is this especially for "system" softwares?
On 13/12/12 22:12, jjacky wrote:
On 12/13/12 12:48, Allan McRae wrote:
On 13/12/12 21:38, Olivier Brunel wrote:
Log messages should be translated as well. (Those from ALPM already are.)
I can not find a single alpm_logaction line in libalpm with a translated string...
Right, not sure what I did, but I got that all wrong indeed, apologies. Should I resend the other two patches without the translation in then?
Not yet... I suggest waiting for comments
Quick question also: while not a native English speaker, I don't like/use translations myself; Is it how things usually work, translations are for user interface only, and don't apply to logs? Or is this especially for "system" softwares?
We are debating this in IRC... The --debug output should never be translated, but whether the log file should be is debatable. One thing to note is that translation will break tools that deal with the log files like paclog-pkglist from pacman-contrib. I'd like to hear others opinions on this. Allan
On Dec 13, 2012 7:20 AM, "Allan McRae" <allan@archlinux.org> wrote:
On 13/12/12 22:12, jjacky wrote:
On 12/13/12 12:48, Allan McRae wrote:
On 13/12/12 21:38, Olivier Brunel wrote:
Log messages should be translated as well. (Those from ALPM already
I can not find a single alpm_logaction line in libalpm with a
are.) translated
string...
Right, not sure what I did, but I got that all wrong indeed, apologies. Should I resend the other two patches without the translation in then?
Not yet... I suggest waiting for comments
Quick question also: while not a native English speaker, I don't like/use translations myself; Is it how things usually work, translations are for user interface only, and don't apply to logs? Or is this especially for "system" softwares?
We are debating this in IRC... The --debug output should never be translated, but whether the log file should be is debatable.
One thing to note is that translation will break tools that deal with the log files like paclog-pkglist from pacman-contrib.
I'd like to hear others opinions on this.
Allan
I think this could/should be treated like any other proposed change in log format. Is it worth it in this case? I tend to think not. We've had display issues in the past and even errors in scripts because of bad translations. Obviously the "attack" surface is small here given the limited strings being translated, but I'm generally -1 on this.
On Thu, Dec 13, 2012 at 6:25 AM, Dave Reisner <d@falconindy.com> wrote:
On Dec 13, 2012 7:20 AM, "Allan McRae" <allan@archlinux.org> wrote:
On 13/12/12 22:12, jjacky wrote:
On 12/13/12 12:48, Allan McRae wrote:
On 13/12/12 21:38, Olivier Brunel wrote:
Log messages should be translated as well. (Those from ALPM already
I can not find a single alpm_logaction line in libalpm with a
are.) translated
string...
Right, not sure what I did, but I got that all wrong indeed, apologies. Should I resend the other two patches without the translation in then?
Not yet... I suggest waiting for comments
Quick question also: while not a native English speaker, I don't like/use translations myself; Is it how things usually work, translations are for user interface only, and don't apply to logs? Or is this especially for "system" softwares?
We are debating this in IRC... The --debug output should never be translated, but whether the log file should be is debatable.
One thing to note is that translation will break tools that deal with the log files like paclog-pkglist from pacman-contrib.
I'd like to hear others opinions on this.
Allan
I think this could/should be treated like any other proposed change in log format. Is it worth it in this case? I tend to think not. We've had display issues in the past and even errors in scripts because of bad translations. Obviously the "attack" surface is small here given the limited strings being translated, but I'm generally -1 on this.
Definite NACK. commit 4906e15d0d31bf0442c25af44bcb637b87a7b027 Author: Dan McGee <dan@archlinux.org> Date: Mon Jul 9 00:46:29 2007 -0400 Remove gettext from any alpm_logaction calls We shouldn't translate log messages to pacman.log so it is consistant and can be parsed by other tools. Remove all gettext _() around these strings. Signed-off-by: Dan McGee <dan@archlinux.org> https://projects.archlinux.org/pacman.git/commit/?id=4906e15d0d31bf0442c25af... -Dan
On 13.12.2012 12:38, Olivier Brunel wrote:
Log messages should be translated as well. (Those from ALPM already are.)
NACK This would break paclog-pkglist which can be pretty useful in case of local db corruption. Another example: You'd also run into problems if there are mutliple admins on a system and each one has a different locale != English. If they use sudo the LANG variable will not be changed so pacman would log different languages in the same file. -- Florian Pritz
participants (6)
-
Allan McRae
-
Dan McGee
-
Dave Reisner
-
Florian Pritz
-
jjacky
-
Olivier Brunel