LocalDateTime to Instant conversion incorrect for mingwX64 target after daylight savings in America/New_York on 0.6.0

[kevincianfarini]

In 2023, the conversion from EDT to EST happened at 2023-11-05T06:00:00Z in America/New_York. This was a shift backwards of one hour in local time. On JVM, Linux, and Apple targets this conversion is calculated properly. However, on the mingwX64 target, this time difference is improperly calculated. The following test fails on the Windows target, but passes for all other available targets.

@Test fun calculated_instant_is_correct() {
    val tz = TimeZone.of("America/New_York")
    val expected = Instant.parse("2023-11-05T07:00:00Z")
    val actual = LocalDateTime(year = 2023, monthNumber = 11, dayOfMonth = 5, hour = 2, minute = 0).toInstant(tz)
    assertEquals(expected, actual)
}

With the following error message

 io.github.kevincianfarini.cardiologist.SuspendingTests.calculated_instant_is_correct[mingwX64] FAILED
    kotlin.AssertionError: Expected <2023-11-05T07:00:00Z>, actual <2023-11-05T06:00:00Z>.

In case it's useful, here's a reference clock aligning UTC and America/New_York during the instant in question.

[dkhalanskyjb]

Does the issue reproduce for you with code from the branch #390 ?

[kevincianfarini]

I can check tomorrow, though if you're able to get the above test failing that might be faster than me cloning and setting up the kotlinx-datetime library.

Also, I failed to mention this in the original post, but this seems to be a regression in 0.6.0. Prior versions did not exhibit this issue. Perhaps this regression was introduced in #286?

[kevincianfarini]

Unfortunately I'm unable to easily check which commit introduced this regression, and whether #390 resolves it. In my library I only noticed this issue because it's a multiplatform projects that offers a Windows binary, and CI started failing when the PR to upgrade this library to 0.6.0 was made. I do not have a personal Windows machine to debug on, and this repository doesn't use something like Github Actions which would allow me to easily test hypotheses in my own fork.

If anyone with a Windows machine wants to pull this repository and copy/paste the above test to see which commit introduced the regression, that would be helpful.

[kevincianfarini]

I can now confirm in my own fork that even #390 fails here. https://github.com/kevincianfarini/kotlinx-datetime/actions/runs/9292913736/job/25574876223?pr=2#step:4:182

[dkhalanskyjb]

Oh, this is actually a known bug, I just forgot to file it; filed it now: #403. Thanks for raising the problem!

[kevincianfarini]

Is this issue new as of #286, or has this existed elsewhere prior? I haven't upgraded to 0.6.0 yet because it's failing a CI check for me.

[dkhalanskyjb]

I can't say this conclusively without looking deeply into it, but IIRC, it didn't exist on Windows but has always existed on the JVM and JS, and then got ported to Native as well (including Linux). The exact datetimes where the issue is observed are just different across platforms. So, this isn't really a regression but an evolution of a bug.

[kevincianfarini]

I see. Is this something that will perhaps be resolved in 0.6.1? Or would this require a more disruptive change?

[dkhalanskyjb]

I used to think that this issue required some design discussions, but now that I'm looking at it again, it seems fairly straightforward. I'll try to fix it for 0.6.1.

[kevincianfarini]

Amazing, thank you so much.

[dkhalanskyjb]

Looks like I was mistaken and this is a regression, plain and simple. I couldn't reproduce this class of issues on the JVM and JS, so the fix only touches Native and was very straightforward to make: #404