[pacman-dev] [PATCH] Print callback messages to stderr
Fixes FS#25099. Signed-off-by: Allan McRae <allan@archlinux.org> --- src/pacman/callback.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 873e3fc..5ee4e5a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -690,7 +690,7 @@ void cb_log(alpm_loglevel_t level, const char *fmt, va_list args) output = alpm_list_add(output, string); } } else { - pm_vfprintf(stdout, level, fmt, args); + pm_vfprintf(stderr, level, fmt, args); } } -- 1.7.6
On 20/08/11 12:42, Allan McRae wrote:
Fixes FS#25099.
Signed-off-by: Allan McRae<allan@archlinux.org> --- src/pacman/callback.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 873e3fc..5ee4e5a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -690,7 +690,7 @@ void cb_log(alpm_loglevel_t level, const char *fmt, va_list args) output = alpm_list_add(output, string); } } else { - pm_vfprintf(stdout, level, fmt, args); + pm_vfprintf(stderr, level, fmt, args); } }
This breaks some pactests because stdout/stderr output is not being kept in sync so timestamps with --debug get printed all over the place. e.g.
./src/pacman/pacman -T glibc --debug <snip> debug: unregistering database 'local' debug: freeing package cache for repository 'local' debug: unregistering database 'allanbrokeit' debug: unregistering database 'kernel64' debug: unregistering database 'testing' debug: unregistering database 'core' debug: unregistering database 'extra' debug: unregistering database 'community-testing' debug: unregistering database 'community' [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34]
On Sat, Aug 20, 2011 at 03:15:34PM +1000, Allan McRae wrote:
On 20/08/11 12:42, Allan McRae wrote:
Fixes FS#25099.
Signed-off-by: Allan McRae<allan@archlinux.org> --- src/pacman/callback.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 873e3fc..5ee4e5a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -690,7 +690,7 @@ void cb_log(alpm_loglevel_t level, const char *fmt, va_list args) output = alpm_list_add(output, string); } } else { - pm_vfprintf(stdout, level, fmt, args); + pm_vfprintf(stderr, level, fmt, args); } }
This breaks some pactests because stdout/stderr output is not being kept in sync so timestamps with --debug get printed all over the place. e.g.
./src/pacman/pacman -T glibc --debug <snip> debug: unregistering database 'local' debug: freeing package cache for repository 'local' debug: unregistering database 'allanbrokeit' debug: unregistering database 'kernel64' debug: unregistering database 'testing' debug: unregistering database 'core' debug: unregistering database 'extra' debug: unregistering database 'community-testing' debug: unregistering database 'community' [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34]
Wouldn't it make more sense to write the timestamps to stderr along with the logging they belong to? If someone were to enable --debug and send stderr off to a file, you'd see some really weird output. d
On Sat, Aug 20, 2011 at 7:58 AM, Dave Reisner <d@falconindy.com> wrote:
On Sat, Aug 20, 2011 at 03:15:34PM +1000, Allan McRae wrote:
On 20/08/11 12:42, Allan McRae wrote:
Fixes FS#25099.
Signed-off-by: Allan McRae<allan@archlinux.org> --- src/pacman/callback.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 873e3fc..5ee4e5a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -690,7 +690,7 @@ void cb_log(alpm_loglevel_t level, const char *fmt, va_list args) output = alpm_list_add(output, string); } } else { - pm_vfprintf(stdout, level, fmt, args); + pm_vfprintf(stderr, level, fmt, args); } }
This breaks some pactests because stdout/stderr output is not being kept in sync so timestamps with --debug get printed all over the place. e.g.
./src/pacman/pacman -T glibc --debug <snip> debug: unregistering database 'local' debug: freeing package cache for repository 'local' debug: unregistering database 'allanbrokeit' debug: unregistering database 'kernel64' debug: unregistering database 'testing' debug: unregistering database 'core' debug: unregistering database 'extra' debug: unregistering database 'community-testing' debug: unregistering database 'community' [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34]
Wouldn't it make more sense to write the timestamps to stderr along with the logging they belong to? If someone were to enable --debug and send stderr off to a file, you'd see some really weird output.
Yes- we shouldn't be trying to multiplex stderr and stdout correctly on the same line. These need to both go to the same place, and why aren't they printed in the same printf() invocation? -Dan
On Sat, Aug 20, 2011 at 10:11:11AM -0500, Dan McGee wrote:
On Sat, Aug 20, 2011 at 7:58 AM, Dave Reisner <d@falconindy.com> wrote:
On Sat, Aug 20, 2011 at 03:15:34PM +1000, Allan McRae wrote:
On 20/08/11 12:42, Allan McRae wrote:
Fixes FS#25099.
Signed-off-by: Allan McRae<allan@archlinux.org> --- src/pacman/callback.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 873e3fc..5ee4e5a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -690,7 +690,7 @@ void cb_log(alpm_loglevel_t level, const char *fmt, va_list args) output = alpm_list_add(output, string); } } else { - pm_vfprintf(stdout, level, fmt, args); + pm_vfprintf(stderr, level, fmt, args); } }
This breaks some pactests because stdout/stderr output is not being kept in sync so timestamps with --debug get printed all over the place. e.g.
./src/pacman/pacman -T glibc --debug <snip> debug: unregistering database 'local' debug: freeing package cache for repository 'local' debug: unregistering database 'allanbrokeit' debug: unregistering database 'kernel64' debug: unregistering database 'testing' debug: unregistering database 'core' debug: unregistering database 'extra' debug: unregistering database 'community-testing' debug: unregistering database 'community' [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34]
Wouldn't it make more sense to write the timestamps to stderr along with the logging they belong to? If someone were to enable --debug and send stderr off to a file, you'd see some really weird output.
Yes- we shouldn't be trying to multiplex stderr and stdout correctly on the same line. These need to both go to the same place, and why aren't they printed in the same printf() invocation?
-Dan
Probably because we only timestamp when we build with --enable-debug so its #ifdef'd. That said, I don't think the timestamps are incredibly intrusive and they add value to debug output when we request it from users. We could kill two birds with one stone here and get rid of the #ifdef to just print the log line all at once. d
On Sat, Aug 20, 2011 at 10:16 AM, Dave Reisner <d@falconindy.com> wrote:
On Sat, Aug 20, 2011 at 10:11:11AM -0500, Dan McGee wrote:
On Sat, Aug 20, 2011 at 7:58 AM, Dave Reisner <d@falconindy.com> wrote:
On Sat, Aug 20, 2011 at 03:15:34PM +1000, Allan McRae wrote:
On 20/08/11 12:42, Allan McRae wrote:
Fixes FS#25099.
Signed-off-by: Allan McRae<allan@archlinux.org> --- src/pacman/callback.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 873e3fc..5ee4e5a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -690,7 +690,7 @@ void cb_log(alpm_loglevel_t level, const char *fmt, va_list args) output = alpm_list_add(output, string); } } else { - pm_vfprintf(stdout, level, fmt, args); + pm_vfprintf(stderr, level, fmt, args); } }
This breaks some pactests because stdout/stderr output is not being kept in sync so timestamps with --debug get printed all over the place. e.g.
./src/pacman/pacman -T glibc --debug <snip> debug: unregistering database 'local' debug: freeing package cache for repository 'local' debug: unregistering database 'allanbrokeit' debug: unregistering database 'kernel64' debug: unregistering database 'testing' debug: unregistering database 'core' debug: unregistering database 'extra' debug: unregistering database 'community-testing' debug: unregistering database 'community' [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34]
Wouldn't it make more sense to write the timestamps to stderr along with the logging they belong to? If someone were to enable --debug and send stderr off to a file, you'd see some really weird output.
Yes- we shouldn't be trying to multiplex stderr and stdout correctly on the same line. These need to both go to the same place, and why aren't they printed in the same printf() invocation?
-Dan
Probably because we only timestamp when we build with --enable-debug so its #ifdef'd. That said, I don't think the timestamps are incredibly intrusive and they add value to debug output when we request it from users. We could kill two birds with one stone here and get rid of the #ifdef to just print the log line all at once. The real fix is this simple though, right?
$ git diff | cat diff --git a/src/pacman/util.c b/src/pacman/util.c index 91625a1..9c1e646 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1416,7 +1416,7 @@ int pm_vfprintf(FILE *stream, alpm_loglevel_t level, const char *format, va_list strftime(timestr, 9, "%H:%M:%S", tmp); timestr[8] = '\0'; - printf("[%s] ", timestr); + fprintf(stream, "[%s] ", timestr); } #endif
On Sat, Aug 20, 2011 at 12:05:40PM -0500, Dan McGee wrote:
On Sat, Aug 20, 2011 at 10:16 AM, Dave Reisner <d@falconindy.com> wrote:
On Sat, Aug 20, 2011 at 10:11:11AM -0500, Dan McGee wrote:
On Sat, Aug 20, 2011 at 7:58 AM, Dave Reisner <d@falconindy.com> wrote:
On Sat, Aug 20, 2011 at 03:15:34PM +1000, Allan McRae wrote:
On 20/08/11 12:42, Allan McRae wrote:
Fixes FS#25099.
Signed-off-by: Allan McRae<allan@archlinux.org> --- src/pacman/callback.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 873e3fc..5ee4e5a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -690,7 +690,7 @@ void cb_log(alpm_loglevel_t level, const char *fmt, va_list args) output = alpm_list_add(output, string); } } else { - pm_vfprintf(stdout, level, fmt, args); + pm_vfprintf(stderr, level, fmt, args); } }
This breaks some pactests because stdout/stderr output is not being kept in sync so timestamps with --debug get printed all over the place. e.g.
./src/pacman/pacman -T glibc --debug <snip> debug: unregistering database 'local' debug: freeing package cache for repository 'local' debug: unregistering database 'allanbrokeit' debug: unregistering database 'kernel64' debug: unregistering database 'testing' debug: unregistering database 'core' debug: unregistering database 'extra' debug: unregistering database 'community-testing' debug: unregistering database 'community' [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34] [15:11:34]
Wouldn't it make more sense to write the timestamps to stderr along with the logging they belong to? If someone were to enable --debug and send stderr off to a file, you'd see some really weird output.
Yes- we shouldn't be trying to multiplex stderr and stdout correctly on the same line. These need to both go to the same place, and why aren't they printed in the same printf() invocation?
-Dan
Probably because we only timestamp when we build with --enable-debug so its #ifdef'd. That said, I don't think the timestamps are incredibly intrusive and they add value to debug output when we request it from users. We could kill two birds with one stone here and get rid of the #ifdef to just print the log line all at once. The real fix is this simple though, right?
$ git diff | cat diff --git a/src/pacman/util.c b/src/pacman/util.c index 91625a1..9c1e646 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1416,7 +1416,7 @@ int pm_vfprintf(FILE *stream, alpm_loglevel_t level, const char *format, va_list strftime(timestr, 9, "%H:%M:%S", tmp); timestr[8] = '\0';
- printf("[%s] ", timestr); + fprintf(stream, "[%s] ", timestr); } #endif
Yeah, assuming we don't want to get into the business of always printing the timestamps for --debug output. d
participants (3)
-
Allan McRae
-
Dan McGee
-
Dave Reisner