Skip to content
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

Bug 54567 DateTimeZone serialize/unserialize #208

Closed
wants to merge 4 commits into from

Conversation

lonnylot
Copy link

Make DateTimeZone serializable and implement __set_state

@smalyshev
Copy link
Contributor

This code crashes for me:
php -r 'echo serialize(date_create("2012-10-21 12:30:00 PST")->getTimezone());'

The problem seems to be in this one:

case TIMELIB_ZONETYPE_ABBR:
            ZVAL_STRING(zv, tzobj->tzi.tz->timezone_abbr, 1);

tzobj->tzi.tz doesn't see to have anything useful here.
Please fix and add tests for different zone types, not only one.

@lonnylot
Copy link
Author

So you're absolutely right on that. Thank you for catching my mistake. Over the past month I've gotten the ABBR timezone to work correctly.

I am still working on the OFFSET timezone. Currently we do not have a way to instantiate a DateTimeZone using an offset. I am playing around with creating a way to do this. If that doesn't turn out to be reasonable I think we'll have to do it w/o implementing the OFFSET timezone.

@lstrojny
Copy link
Contributor

lstrojny commented Nov 4, 2012

Will this work with microseconds?
Example

<?php
$date = DateTime::createFromFormat('U.u', microtime(true));
var_dump(unserialize(serialize($date))->format('U.u'));

@lstrojny
Copy link
Contributor

lstrojny commented Nov 4, 2012

Ping @derickr

@lonnylot
Copy link
Author

lonnylot commented Nov 4, 2012

@lstrojny I am not sure, but this pull request has to do specifically w/ DateTimeZone objects, not DateTime objects.

@derickr
Copy link
Member

derickr commented Nov 4, 2012

Looks good, but misses tests for type 1 and 2 timezone types.

@lstrojny
Copy link
Contributor

lstrojny commented Nov 4, 2012

@lonnylot my bad :)

@lstrojny
Copy link
Contributor

lstrojny commented Jan 6, 2013

@lonnylot could you please rebase?

@lstrojny
Copy link
Contributor

@lonnylot ping

Lonny Kapelushnik added 4 commits January 14, 2013 21:26
Make DateTimeZone serializable and implement __set_state
Fixed handling of unserializing types 1 and 3
Implemented Dmitrys change from df97c3a
Moved the timelib_parse_tz_cor function to ext/date/lib/timelib.c
@lonnylot
Copy link
Author

@lstrojny Been a tad busy :)

I rebased and made a few other changes (all mentioned in the commit message). All tests passed.

@lonnylot
Copy link
Author

Any chance of this making it into 5.5?

@derickr
Copy link
Member

derickr commented Mar 30, 2013

Good for reminding me, I'll merge this bug fix after the
DateTimeImmutable fix that I've been working on.

@derickr
Copy link
Member

derickr commented Mar 31, 2013

I've merged it after adding the .c parser files too, but for some odd reason I can't close this PR.

@dsp dsp closed this Mar 31, 2013
@lonnylot
Copy link
Author

Can someone close the related bug: https://bugs.php.net/bug.php?id=54567

@nikic
Copy link
Member

nikic commented Apr 26, 2013

@lonnylot Done (and for the other PR as well) :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants