On Thu, Mar 07, 2019 at 10:35:32AM +1000, Allan McRae wrote:

On 5/3/19 7:46 am, Florian Wehner wrote:

...

The time logged is currently given as localtime without any time-zone information. This is confusing in various scenarios.

Examples: * If one is travelling across time-zones and the timestamps in the log appear out of order.

Stop updating multiple times per hour! :D

...

* Comparing dates with `datediff` gives an offset by the time-zone

This patch would reformat the time-stamp to a full ISO-8601 version. It includes the 'T' separating date and time. This could be removed.

Old: [2019-03-04 16:15] New: [2019-03-04T16:15-05:00]

Can we switch from localtime() to gmtime() and just use UTC?

I wouldn't do this... it's not normal to have local machine logs in another timezone. It'll make correlation harder if your natural timezone isn't already UTC. And, if we magically change the timezone with a new release of pacman and don't include the TZ explicitly in the timestamp, it's just going to be a source of confusion for people. I'm +1 on keeping localtime and explicitly adding the timezone identifier to the logline.

...

Signed-off-by: Florian Wehner <florian@whnr.de> --- lib/libalpm/log.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c index e46ad3c3..cf869a08 100644 --- a/lib/libalpm/log.c +++ b/lib/libalpm/log.c @@ -20,6 +20,7 @@

#include <stdio.h> #include <stdarg.h> +#include <stdlib.h> #include <errno.h> #include <syslog.h>

@@ -37,12 +38,17 @@ static int _alpm_log_leader(FILE *f, const char *prefix) { time_t t = time(NULL); + int tz_h, tz_m; struct tm *tm = localtime(&t);

+ /* Calculate the timezone offset ±hh:mm */ + tz_h = tm->tm_gmtoff/3600; + tz_m = abs(tm->tm_gmtoff - (tz_h*3600))/60; + /* Use ISO-8601 date format */ - return fprintf(f, "[%04d-%02d-%02d %02d:%02d] [%s] ", + return fprintf(f, "[%04d-%02d-%02dT%02d:%02d%+03d:%02d] [%s] ", tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, - tm->tm_hour, tm->tm_min, prefix); + tm->tm_hour, tm->tm_min, tz_h, tz_m, prefix); }

/** A printf-like function for logging.

The tm_gmtoff field is derived from BSD and is a GNU library extension;

So we can't use that.

Why reinventing the wheel? Using `strftime` would do the work for us, right? --Florian The time logged is currently given as localtime without any timezone information. This is confusing in various scenarios. Examples: * If one is travelling across time-zones and the timestamps in the log appear out of order. * Comparing dates with `datediff` gives an offset by the time-zone This patch would reformat the time-stamp to a full ISO-8601 version. It includes the 'T' separating date and time including seconds. Old: [2019-03-04 16:15] New: [2019-03-04T16:15:45-05:00] Signed-off-by: Florian Wehner <florian@whnr.de> --- lib/libalpm/log.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c index e46ad3c3..d8842a55 100644 --- a/lib/libalpm/log.c +++ b/lib/libalpm/log.c @@ -22,6 +22,7 @@ #include <stdarg.h> #include <errno.h> #include <syslog.h> +#include <time.h> /* libalpm */ #include "log.h" @@ -38,11 +39,12 @@ static int _alpm_log_leader(FILE *f, const char *prefix) { time_t t = time(NULL); struct tm *tm = localtime(&t); + int length = 32; + char timestamp[length]; /* Use ISO-8601 date format */ - return fprintf(f, "[%04d-%02d-%02d %02d:%02d] [%s] ", - tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, - tm->tm_hour, tm->tm_min, prefix); + strftime(timestamp,length,"%FT%X%z", tm); + return fprintf(f, "[%s] [%s] ", timestamp, prefix); } /** A printf-like function for logging. -- 2.21.0