Repository navigation
Fix POSIX TZ week-of-month calculation near DST transitions - #760
Closed
abhinav-phi wants to merge 2 commits into
Closed
abhinav-phi wants to merge 2 commits into
abhinav-phi wants to merge 2 commits into
Conversation
MwdForTime::from_seconds divided the day of month by 7 before subtracting one, so day 7 and day 8 both fell in the second week. Comparing a date in that range with a transition date inverted the order for some months, and the offset resolution then picked the daylight saving offset a day early. Use (day - 1) / 7 so the weekly blocks start on day 1.
Manishearth
approved these changes
Oct 31, 2026
Manishearth
left a comment
Manishearth
left a comment
Contributor
There was a problem hiding this comment.
This looks about right.
| #[test] | ||
| fn new_york_standard_time_before_dst_start() { | ||
| use crate::{options::Disambiguation, tzdb::CompiledTzdbProvider, PlainDateTime, TimeZone}; | ||
| let provider = &CompiledTzdbProvider::default(); |
Contributor
There was a problem hiding this comment.
Please use the test_all_providers macro
If necessary, move it into a test_utils.rs file.
Contributor
This doesn't fix the Europe/London issue |
Contributor
|
Made #761 |
Member
|
Superseeded by #761 |
This was referenced Nov 2, 2026
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
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.
Fixes #759
MwdForTime::from_secondscomputedweek_of_monthasday_of_month / 7 + 1,so day 7 (and day 14, 21, 28) landed in the following week block. When the
queried date and the POSIX transition date fall on opposite sides of that
boundary, the weekday comparison in
cmp_seconds_to_transitionsorders themwrongly and the DST offset is picked a day early.
Concretely, for America/New_York in 2026, March 1 is a Sunday. March 7
(Saturday) and March 8 (Sunday, the DST start) were both normalized to week 3,
so March 7 compared as after the transition:
Use
(day_of_month - 1) / 7 + 1so the seven day blocks start on day 1, whichis what
normalize_to_week_numberassumes. One line changes.Tests:
new_york_standard_time_before_dst_startincrates/temporal/src/builtins/compiled/plain_date_time.rs(fails before thechange with
-04:00, passes after).cargo test -p temporal: 191 passed, 55 doc tests passed.cargo test -p timezone_provider --features tzif: 24 passed.