Malta BCL zone issue on Linux/Mono #743

Closed
jskeet opened this Issue Apr 4, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@jskeet
Member

jskeet commented Apr 4, 2017

From #97.

Test NodaTime.Test.TimeZones.BclDateTimeZoneSourceTest.ProviderReturnsBclDateTimeZoneForAllAdvertisedIds fails with this exception

System.InvalidOperationException : Requested transition for a ZoneYearOffset of February 29th in a non-leap year, not moving backwards to find a day-of-week
  at NodaTime.TimeZones.ZoneYearOffset.GetOccurrenceForYear (System.Int32 year) [0x0006d] in <c90fbbee5fa9426392c49904a3d9b7d0>:0 

The problem is in Europe/Malta, and I suspect it's due to this BCL adjustment rule:

01/01/1948 00:00:00 - 31/12/1948 00:00:00:
Transition start: Day: 29 / DayOfWeek: Sunday / Fixed: True / Month: 2 / Week: 1 / TimeOfDay: 01/01/0001 02:00:00
Transition end: Day: 3 / DayOfWeek: Sunday / Fixed: True / Month: 10 / Week: 1 / TimeOfDay: 01/01/0001 03:00:00

We're asking for the transition in 1949 for some reason, despite the rule only applying until 1948. That's odd to say the least.

@jskeet jskeet added the bug label Apr 4, 2017

@jskeet jskeet self-assigned this Apr 4, 2017

@jskeet

This comment has been minimized.

Show comment
Hide comment
@jskeet

jskeet Apr 4, 2017

Member

The problem is that we're trying to build the final ZoneInterval in that PartialZoneIntervalMap, which involves finding the next transition after 1948-12-31T22:59:59Z, which wouldn't exist.

Will need to think about this carefully. In the meantime, I can add a unit test to mimic this situation...

Member

jskeet commented Apr 4, 2017

The problem is that we're trying to build the final ZoneInterval in that PartialZoneIntervalMap, which involves finding the next transition after 1948-12-31T22:59:59Z, which wouldn't exist.

Will need to think about this carefully. In the meantime, I can add a unit test to mimic this situation...

jskeet added a commit to jskeet/nodatime that referenced this issue Apr 4, 2017

"Handle" Feb 29th rules more cleanly
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

Fixes #743.

jskeet added a commit to jskeet/nodatime that referenced this issue Apr 4, 2017

"Handle" Feb 29th rules more cleanly
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

Fixes #743.

jskeet added a commit to jskeet/nodatime that referenced this issue Apr 4, 2017

"Handle" Feb 29th rules more cleanly
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

Fixes #743.

jskeet added a commit to jskeet/nodatime that referenced this issue Apr 4, 2017

"Handle" Feb 29th rules more cleanly
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

Fixes #743.

@jskeet jskeet closed this in #744 Apr 4, 2017

jskeet added a commit that referenced this issue Apr 4, 2017

"Handle" Feb 29th rules more cleanly
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

Fixes #743.

@malcolmr malcolmr added this to the 2.0.1 milestone May 3, 2017

jskeet added a commit to jskeet/nodatime that referenced this issue May 3, 2017

"Handle" Feb 29th rules more cleanly
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

Fixes #743.

jskeet added a commit that referenced this issue May 4, 2017

"Handle" Feb 29th rules more cleanly
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

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