-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Time::Piece reversibility #32
Comments
Hmm looks like there are no tests with strptime and '%z' currently. Looking at how the source handles the %z offset: https://github.com/Dual-Life/Time-Piece/blob/1.31_04/Piece.xs#L853 it simply adds the offset to the tm struct minutes and hours, sets the "got_gmt" flag (which it then does nothing with). So that explains why the time is wrong (it converts it to gmt essentially). Seems like this should be fixable, but the issue is I don't think strptime was ever really made to convert time zones from one locale to another. I don't know of an OS portable way to take a broken down tm struct in gmt, and then convert that into something else. What I can try is if '%z' is passed, I'll know that the tm parts that come back will have been offset into gmt, then perhaps I can do (internally) timegm(@tm_parts) to get epoch, then CORE::localtime($epoch) to return the right parts for what ever the current timezone is... But open to suggestions :) |
Oh and for a quick and dirty fix, if I take your test script and remove the time zone offset from the date string and remove the %z from the format string, I think I get the right behavior. So perhaps try parsing out the offset from your date strings. |
Hey, thanks for the quick response. I posted this on perlmonks too and got some helpful responses: http://www.perlmonks.org/?node_id=1193021 I think what cguevara showed may be the same as what you're suggesting above? He parsed the timestamp with timezone using Time::Piece->strptime. And then passed its epoch seconds to localtime(). So I guess strptime always returns a gmtime, even when it parses a timezone. I guess that makes sense. And if you want to display with a different offset, you need to explicitly create a localtime() instance from it. |
It would maybe be nice though if something like what I originally tried did work. e.g., localtime->strptime($datestr, '%Y-%m-%d %H:%M:%S%z'); would indicate that, since it's parsing within the context of localtime, we want a localtime instance back. And if there is no %z specified, it would assume the localtime zone. Not sure how difficult that last bit is. |
Not quite. An "epoch" is the seconds from 1970. It is the same everywhere. But strptime returns actual "time parts", year, month, hour, minute, etc. Those can be local or gmt, the difference being gmt will never have a daylight savings period. Time::Piece->strptime() looks to default to returning a Time::Piece object with gmt set to true. localtime->strptime() will of course have gmt set to false, unless you have gmt set as your current system TZ. But there is obviously a bug with how strptime handles %z at the moment. |
Thanks. Attached are some tests that I think exhibit the issue with localtime->strptime and time zones. I'm not sure how portable the tests are. I'm setting TZ to PST8PDT. Not sure if Windows can handle that? |
I will look into adding the tests. For my own reference this seems to be also related: https://rt.cpan.org/Ticket/Display.html?id=93095 |
I was just skimming the code, and I suspect the got_GMT flag from _strptime is the key. If got_GMT is true, then _mktime should use timegm() regardless of whether the caller $islocal. e.g., something like: $time->[c_epoch] = $got_gmt || !$islocal ? timegm(@tm_parts): timelocal(@tm_parts); And then $got_gmt needs to be a third, optional param to _mktime. |
Yea something like that I was thinking too. You can see how a more modern version of libc handles it at the bottom of the page here:https://opensource.apple.com/source/Libc/Libc-583/stdtime/strptime-fbsd.c.auto.html See on the Strangely, on an even newer libc: https://opensource.apple.com/source/Libc/Libc-1044.40.1/stdtime/FreeBSD/strptime.c.auto.html the process is different and only the Either way, in C, I don't think we have a cross platform variant of But we do have timelocal and timegm, but those are in perl. So from the perl strptime sub, we need to flag that the call to the C _strptime did not return as local... And seems we are in luck. Within |
Thanks for maintaining this module.
I'm trying to parse a timestamp that includes the timezone, and then output it as a localtime date string.
IOW, I want to do the following, except using Time::Piece instead of Time::Local:
That, for me, produces:
So the input was 10 o'clock, and the output is 10 o'clock.
However, if I try parsing that with strptime and outputting the result, I'm always getting GMT, no matter how I do it:
That produces:
The hour is always 17. Even when I successfully get tzoffset set, it's still displaying as 17.
I would think that if tzoffset is set, the object should be a "localtime" instance and anything generated should be in localtime. If you want to convert it to a gmtime instance, then call gmtime($localtime_instance) (just as you would with the the CORE:: localtime and gmtime functions).
Am I missing something?
Thanks.
The text was updated successfully, but these errors were encountered: