Skip to content

Fix GH-24735: Resolve repeated local times consistently - #24738

Open
kairosci wants to merge 3 commits into
php:PHP-8.5from
kairosci:fix/gh-24735-datetime-settime
Open

kairosci wants to merge 3 commits into
php:PHP-8.5from
kairosci:fix/gh-24735-datetime-settime

Conversation

@kairosci

Copy link
Copy Markdown
Contributor

Fixes #24735.

@anun333

anun333 commented Oct 27, 2026

Copy link
Copy Markdown

Quoted below: written with Claude, an LLM, as CONTRIBUTING.md asks us to mark. The builds and test runs behind it are on my machine; the commands to repeat them are under the fold.

This fixes the issue's case, but a time in the second occurrence of the repeated hour now goes back to the first, in London as well as Dublin. On today's PHP-8.5 (6a020d606e4) with this applied, debug builds, 2026-10-25:

Start then PHP-8.5 this PR
Dublin 01:10:30 IST (first) setTime(1, 10, 59) 01:10:59 GMT (+3629s) 01:10:59 IST (+29s)
Dublin 01:10:30 GMT (second) setTime(1, 10, 59) 01:10:59 GMT (+29s) 01:10:59 IST (−3571s)
London 01:10:30 BST (first) setTime(1, 10, 59) 01:10:59 BST (+29s) 01:10:59 BST (+29s)
London 01:10:30 GMT (second) setTime(1, 10, 59) 01:10:59 GMT (+29s) 01:10:59 BST (−3571s)

modify('+29 seconds') gives the same result as setTime() in every row. A time in the spring-forward gap (01:30 on 2026-03-29) comes out the same on both builds, and ext/date/tests passes on both (694 tests; 695 with this one's).

So it seems to prefer the first occurrence even when the time started in the second; keeping the offset the time already had, whenever that offset is valid for the new wall time, would cover both. A test with a start in the second occurrence would catch it.

Also, since this changes ext/date/lib: that's timelib (https://github.com/derickr/timelib), and its logic changes have come into php-src through timelib releases (d83851dae86, "Update timelib to 2022.17", for one). Should this go to timelib first?

How this was tested

Linux x86_64, gcc 13.3, debug builds of origin/PHP-8.5 and of it with this PR's commit cherry-picked:

./buildconf --force
./configure --enable-debug --disable-cgi --disable-phpdbg --disable-fileinfo --enable-pcntl --enable-zend-test --enable-fpm
make -j4

Run with sapi/cli/php -n:

foreach (['Europe/Dublin', 'Europe/London'] as $tz) {
    foreach (['2026-10-25 00:10:30 UTC' => 'first', '2026-10-25 01:10:30 UTC' => 'second'] as $utc => $which) {
        $d = (new DateTimeImmutable($utc))->setTimezone(new DateTimeZone($tz));
        $e = $d->setTime((int) $d->format('H'), 10, 59);
        $m = $d->modify('+29 seconds');
        printf("%-14s %-6s %s -> setTime %s (%+ds), modify %s (%+ds)\n", $tz, $which, $d->format('H:i:s T'),
            $e->format('H:i:s T'), $e->getTimestamp() - $d->getTimestamp(), $m->format('H:i:s T'), $m->getTimestamp() - $d->getTimestamp());
    }
}
foreach (['Europe/Dublin', 'Europe/London'] as $tz) {
    echo (new DateTimeImmutable('2026-03-29 01:30:00', new DateTimeZone($tz)))->format('H:i:s T'), "\n";
}

Suites: TEST_PHP_EXECUTABLE=$PWD/sapi/cli/php sapi/cli/php run-tests.php -n -q -j4 ext/date/tests on both.

@kairosci

Copy link
Copy Markdown
Contributor Author

Overlap resolution now preserves the starting UTC offset when it remains valid, falling back to the earlier occurrence otherwise. Added regression coverage for setTime() and modify() starting in the second occurrence.

@anun333

anun333 commented Oct 29, 2026

Copy link
Copy Markdown

Quoted below: written with Claude, an LLM, as CONTRIBUTING.md asks us to mark. The builds and test runs behind it are on my machine; the commands to repeat them are under the fold.

Retested at 0def4f7b76 on today's PHP-8.5 (6de713d23b2): both occurrences now come out right, with setTime() and modify('+29 seconds'), in Dublin and London (+29s in all four rows of my earlier table), the spring-forward gap is unchanged, and the rest of ext/date/tests passes.

The one CI failure is the new test's eighth expected line: the London modify('+29 seconds') case expects 01:10:59 IST +29, but London is GMT there, and the build prints 01:10:59 GMT +29, which is right. Changing that line to GMT should make CI green.

How this was tested

Debug builds of origin/PHP-8.5 and of it with this PR's three commits cherry-picked, configured as before. My earlier script, the issue's script, and TEST_PHP_EXECUTABLE=$PWD/sapi/cli/php sapi/cli/php run-tests.php -n -q -j4 ext/date/tests on both builds; DateTime_setTime_dst_overlap.phpt fails only on its line 8, with the diff - 01:10:59 IST +29 / + 01:10:59 GMT +29.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants