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.

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