Fix #80974: Wrong diff between 2 dates in different timezones #6896
Conversation
Wrong diff between 2 dates in different timezones
6399f23 to
53627f9
Compare
ext/date/lib/interval.c
Outdated
| rt->h -= dst_h_corr + (two->dst - one->dst); | ||
| rt->i -= dst_m_corr; | ||
|
|
||
| timelib_do_rel_normalize(rt->invert ? one : two, rt); |
There was a problem hiding this comment.
This is the missing normalization would solve the test case (without breaking other ones :D )
Let me just refactor this without duplicate code and it's good to review.
|
Thank you for the PR, but changes to date/lib should go to https://github.com/derickr/timelib. Please submit a respective PR there. |
|
Hello @cmb69 what about the unit test? Does not seem to be a place for them in derick/timelib. |
|
timelib has its own unit tests written in cpputest framework, so if possible you should add respective tests there. Having an additional PHPT to verify the correct behavior is certainly not a bad idea, but I'm not sure whether adding XFAIL tests is a good idea. @derickr, what do you think? |
|
Related timelib PR: derickr/timelib#110 |
|
I added the test in timelib. Still I think this test should be part of the php/php-src suites too to avoid any regression of this case in next versions. Don't you think so? |
|
Yes, but the test fails without the patch. Anyhow, going to reopen. |
|
Hello @cmb69 and @derickr is there something wrong in the fix derickr/timelib#110? I'm ready to help/improve, but very please, don't release current state as is. 🙏 |
|
@kylekatarnls, no worries. @derickr just told me that he needs some time to work on type 2 and comparisons, and there is plenty of time till PHP 8.1.0. :) |
|
It looks like a fix for this issue landed with the last timelib update, and a test was added as well, so I'm closing this PR. |
Here is first the test case to show the failure. Note that this passed with PHP < 8.1
See: https://bugs.php.net/bug.php?id=80974