Repository navigation
Time: follow daylight saving time on Windows - #19
Merged
Merged
Conversation
With TZ unset, RubyTimeZone.Machine() reads /etc/localtime, and on Windows,
which has none, fell back to a fixed offset taken from
TimeZoneInfo.Local.BaseUtcOffset. That is the zone's standard offset, so every
local Time was an hour off all summer: Time.now on 2026-10-12 in
Europe/Zurich ("W. Europe Standard Time") said 20:20:46 +0100 while
DateTime.Now said 21:20:46, and Time.at, Time.local/mktime/new, #localtime and
dst? were wrong the same way.
The machine zone is now backed by TimeZoneInfo.Local itself and asked for each
instant (GetUtcOffset, IsDaylightSavingTime), so it carries the zone's DST and
historical rules. Wall-clock readings go through the same GetOffsetForWallClock
as the zoneinfo-backed zones, so a reading in the spring gap or the autumn
overlap resolves as in MRI. Time#zone reports the zone's DaylightName during
daylight saving time and its StandardName otherwise.
Nothing changes where zoneinfo exists: /etc/localtime and the TZ ids and POSIX
rules still go through TzFile.
Util/time-local-matrix.rb prints utc_offset, dst? and the wall clock for the
machine's zone across a year, through both EU and US changes and for the gap
and overlap readings. On Windows 11 in W. Europe Standard Time, against CRuby
4.0.1 on the same machine: 102 of 299 lines differed before, none after; with
TZ=CET-1CEST,M3.5.0,M10.5.0/3 (the TzFile path) the output is identical too.
TZ=America/Los_Angeles (or Europe/Zurich, or a Windows id such as "W. Europe Standard Time") is looked up in the zoneinfo directories, which Windows does not have, then parsed as a POSIX rule, which it is not, so the Time came out in UTC. On Windows, an id with no zoneinfo file is now handed to TimeZoneInfo.FindSystemTimeZoneById, which knows the Windows ids and, through ICU, the Olson ones; POSIX strings such as "PST8:00:00" or "CET-1CEST,M3.5.0,M10.5.0/3" are not ids and still take the POSIX path. Linux and macOS are unchanged: there the lookup stays zoneinfo, then POSIX. spec/core/time on Windows: 639 examples, 1 failure (Time#dst? in America/Los_Angeles) before this and the previous commit; 0 failures after. Together they also fix File::Stat#atime/#ctime/#mtime/#birthtime there, whose specs compare with local Times.
_dump wrote the zone name with MutableString.CreateAscii, so on a Windows whose
zone names are localized ("Mitteleuropäische Zeit") Marshal.dump of any local
Time raised EncoderFallbackException. It now encodes the name as Time#zone
already does: ASCII when it is, the path encoding otherwise.
spec/core/marshal on a German Windows: 6 errors before (all "for a Time"),
none after.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding Windows daylight-saving-time support for Time handling. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Largo
added a commit
to khasinski/ironruby
that referenced
this pull request
Oct 13, 2026
CI builds libprism from prism's main branch, and since ruby/prism#4255 (attach_comments, merged 2026-10-12) every build job has failed in the smoke test with "Unknown prism node type: 0 (TypeError)" - master, Largo#17, Largo#18 and Largo#19 alike. That change altered prism's serialization in two places without moving the serialization version off 1.9, so the version check in PrismLoader could not catch it: - the options blob has a new attach_comments byte after freeze. The blob IronRuby wrote was one byte short, so prism read the scope count from the wrong offset; - the serialized header has a comment-attachment list after the comments (node id, count, comment indices). PrismLoader read its count as the magic comment count and walked off from there. PrismParser.BuildOptionsData writes the byte (0: IronRuby never asks for attachments) and PrismLoader.Load skips the list. PrismLex only reads the token stream, which ends before the header, so it needs nothing. The rest of the revision comes along so that every piece names the same prism commit, as Src/Prism/README.md asks: - Src/Prism/Generated is regenerated from cd76b0d1's config.yml: SymbolNode#value_loc became the non-optional content_loc (the bridge reads neither), and a WORDS_SEP_IMPLICIT token was added after WORDS_SEP. Ripper looks tokens up by name and skips the ones it has no event for, which is what prism's own lex_compat does with this one. - Src/StdLib/ruby/4.0/prism is prism's lib/ at cd76b0d1, generated files included; prism.rb keeps IronRuby's backend branch. parse_result/comments.rb is gone upstream - attaching comments moved into libprism. - Src/StdLib/ironruby/prism/ironruby.rb follows ffi.rb: attach_comments is a known option and is dumped after freeze, and the *_file APIs take a Pathname (to_path / to_str) the way PrismSource.with_file now does. - release.yml's PRISM_REF and the README build instructions move to cd76b0d1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Windows,
Time.nowignored daylight saving time. Reproduced on 2026-10-12 in Europe/Zurich ("W. Europe Standard Time",TZunset):It also showed up inside a .NET 10 host (a JobRouter service running as LocalSystem, using
irubyc --library).Cause
With
TZunset,RubyTimeZone.Machine()reads/etc/localtime. Windows has no such file, so it fell back to a fixed offset taken fromTimeZoneInfo.Local.BaseUtcOffset. That is the zone's standard offset, so every local Time was an hour off all summer.Changes
TimeZoneInfo.Localand looks up the offset for each instant (GetUtcOffset,IsDaylightSavingTime). This fixesTime.now,Time.at,Time.local/mktime/new,#localtimeanddst?. Times in the spring gap or the autumn overlap go through the existingGetOffsetForWallClock, so they resolve as in MRI.Time#zonereturns the zone'sDaylightNameduring DST.TZthrough TimeZoneInfo on Windows.TZ=America/Los_Angeles(orEurope/Zurich, or a Windows id) used to end up as UTC. On Windows only, an id with no zoneinfo file now goes toFindSystemTimeZoneById. POSIX strings likePST8:00:00still take the POSIX path.Time#_dumpkeeps a non-ASCII zone name. On a localized Windows ("Mitteleuropäische Zeit"),Marshal.dumpof a local Time raisedEncoderFallbackException. The name is now encoded the wayTime#zonealready does it.Linux and macOS take the same path as before:
/etc/localtime, zoneinfo ids and POSIX rules all still go throughTzFile.Testing
Run on Windows 11, W. Europe Standard Time, Release build, net10.0.
Util/time-local-matrix.rbvs CRuby 4.0.1 on the same machinespec/core/time(639 examples)Time#dst?in America/Los_Angeles)spec/core/marshalspec/core/file/statatime/ctime/mtime/birthtimespec/library/time,spec/library/dateandspec/library/datetimehave 0 failures.IronRuby.TestsC# tests pass.TZ=CET-1CEST,M3.5.0,M10.5.0/3, which goes through the sameTzFileengine Linux uses, the matrix output also matches CRuby.Not run locally: Linux (no WSL or Docker on the test machine) and the net8.0 target (only the .NET 10 runtime is installed). CI covers both.
Summary by CodeRabbit