[pacman-dev] [PATCH 1/3] Fix pacman's log messages not being translated

newer
[pacman-dev] [PATCH 2/2] doc: Fix...

older
[pacman-dev] [GIT] The official...

Olivier Brunel

13 Dec 2012 13 Dec '12

11:38 a.m.

Log messages should be translated as well. (Those from ALPM already are.) Signed-off-by: Olivier Brunel --- 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

Show replies by date

Olivier Brunel

13 Dec 13 Dec

11:38 a.m.

New subject: [pacman-dev] [PATCH 2/3] Log only when, and which, database was synchronized

Instead of always logging "synchronizing package databases" we only log when (and which) db was synchronized. Signed-off-by: Olivier Brunel --- 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

Allan McRae

12:27 p.m.

New subject: [pacman-dev] [PATCH 2/3] Log only when, and which, database was synchronized

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 --- 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; }

Olivier Brunel

11:38 a.m.

New subject: [pacman-dev] [PATCH 3/3] Move logging of sysupgrade start, add log when done

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 --- 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

Dave Reisner

11:41 a.m.

New subject: [pacman-dev] [PATCH 3/3] Move logging of sysupgrade start, add log when done

On Dec 13, 2012 6:39 AM, "Olivier Brunel" 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 --- 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

jjacky

11:45 a.m.

New subject: [pacman-dev] [PATCH 3/3] Move logging of sysupgrade start, add log when done

On 12/13/12 12:41, Dave Reisner wrote:

...

On Dec 13, 2012 6:39 AM, "Olivier Brunel" 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 --- 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

Olivier Brunel

12:01 p.m.

New subject: [pacman-dev] [PATCH] Move logging of sysupgrade start, add log when done

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 --- 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

Allan McRae

11:48 a.m.

New subject: [pacman-dev] [PATCH 1/3] Fix pacman's log messages not being translated

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...

jjacky

12:12 p.m.

New subject: [pacman-dev] [PATCH 1/3] Fix pacman's log messages not being translated

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?

Allan McRae

12:20 p.m.

New subject: [pacman-dev] [PATCH 1/3] Fix pacman's log messages not being translated

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

Dave Reisner

12:25 p.m.

On Dec 13, 2012 7:20 AM, "Allan McRae" 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.

Dan McGee

15 Dec 15 Dec

6:52 p.m.

On Thu, Dec 13, 2012 at 6:25 AM, Dave Reisner wrote:

...

On Dec 13, 2012 7:20 AM, "Allan McRae" 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 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 https://projects.archlinux.org/pacman.git/commit/?id=4906e15d0d31bf0442c25af... -Dan

Florian Pritz

13 Dec 13 Dec

12:24 p.m.

New subject: [pacman-dev] [PATCH 1/3] Fix pacman's log messages not being translated

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

4157

Age (days ago)

4159

Last active (days ago)

List overview

Download

12 comments

6 participants

participants (6)

Allan McRae
Dan McGee
Dave Reisner
Florian Pritz
jjacky
Olivier Brunel