Workaround about php bug 74274 #1071
Conversation
EOL forced to LF in .gitattributes for all text files
…about issue briannesbitt#88 The realisation of initial issue briannesbitt#88 is wrong. The problem does not relies to `local` / `not local` timezone. Php handles date changes similar for both cases. So in fact changes was made 'fixes' non-local dates but local dates works same as before. And it 'fixes' the problem and not actually fixes it because in fact the PHP handling of DST changing in non-UTC dates is quite right. If developer don't want to adjusting timestamp to DST change he should use UTC timezone. Next commits reverted by hand because conflicts in automatic revert. * Commit 2cc6988 Fix CS 07.12.2015 13:04 Lucas Michot * Commit 5394301 Fix issue 88 07.12.2015 12:55 Lucas Michot
There was an error on the win7: ``` > composer test > ./vendor/bin/phpunit; ./vendor/bin/php-cs-fixer fix -v --diff --dry-run; The system cannot find the path specified. Script ./vendor/bin/phpunit; ./vendor/bin/php-cs-fixer fix -v --diff --dry-run; handling the test event returned with error code 1 ``` In the composer documentstion [written](https://getcomposer.org/doc/articles/scripts.md#writing-custom-commands) that > Note: Before executing scripts, Composer's bin-dir is temporarily pushed on top of the PATH environment variable so that binaries of dependencies are easily accessible. In this example no matter if the phpunit binary is actually in vendor/bin/phpunit or bin/phpunit it will be found and executed. So it is should be safe to use just `phpunit` instead `./vendor/bin/phpunit`.
Just tests by now
Ovsyanka
force-pushed
the
pr-WaPhpBug74274
branch
from
cb19ca0 to
2922c65
Compare
|
Sadly, by now I don't see effective way to fix it without fixing PHP itself. |
|
@Ovsyanka: what is the status here? |
|
My last comment still actual. The problem is the DateTime can't represent local standard time in repeated hour while backward transition. Example: The only workaround I see is to add some property to save right timestamp and then use it in all methods instead internal timestamp. I am not sure it worth it. I will be glad to know your thoughts about that. I made the pull-request because I think my tests 1) shows the problems 2) someone can use it to try to solve the issue. |
|
This PR also need to replace |
|
Hmm. This PR have only failing tests, those indicates the bug. I may not merge failing tests. Did I misunderstand you? |
|
This one couldn't be merged (failing tests, compatibility and style) but the source modification is the same than in #1070, so if there is something in this PR you still want to address, it seems better to me to put it in your other PR. |
|
Ah, it is just the same commits from another pull-requests because this one needs them to work. They are not the part of this PR actually and will be removed after rebase to the master. This PR is about the failing tests. I already wrote why I made it and if you think that it is right to close it - then OK. |
|
I see, you created pr-WaPhpBug74274 from pr-revert-548-modify-errors. So this PR has nothing to be merged. Understood. If PHP decided to adjust timestamp with DST, there's nothing we should do. I recommend to work around with using GMT timestamp only. Changing tz for +00:00, set the timestamp, then go back to America/New_York tz and the same when getting. |
Yep, seems like you got it =)
In case if it doing it right - yes. But it has errors (look at the my first commentary #1071 (comment)). And I think that it would be great if we could make this working right in Carbon library like there is no error in PHP. Like in #900. But by now I don't see good way to do it. And this PR is the only place where I imagine to put tests, that describes the bug. Now someone could try to fix that using my tests as goal. Personally I not suffer about this bugs - I prefere to use UTC dates most the time instead of local, but in general the errors exists so I decided to share what I already did in this matter. |
By now it is just tests that I want to discuss before trying to fix it. Questions and suggestions are appreciated. If anyone could help me with proper documentation it will be very nice.
I made tests to check this situations:
Each modification should be reverted. If you add one hour and then sub one hour the date shouldn't be changd. There is testAddHoursReversable test.
When you create date from timestamp - it should have this initial timestamp. There is testTimestampNotChanged test.
When you crossing DST change in the local timezones PHP adjusting timestamp to looks like offset didn't changed. It works well. But if you change date into the skipped or repeated hour there is errors. There is testAddHours test for that.
If you creating Date from timestamp into repeated hour PHP always think that it is Standard Time and not the Daylight Time. There is getCreateFromTimestampTests for that.