search

unoplatform / uno

Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported.
https://platform.uno
Apache License 2.0
8.48k stars 690 forks source link

[WASM] DateTime.AddDays/AddSeconds/AddMinutes not fully working if project configured for Uno.SkiaSharp #1890

Closed peternary closed 4 years ago

peternary commented 4 years ago

Current behavior

If you have 'release-dynamic' set in the .csproj file for a wasm project, as specified here, then using a negative number in DateTime.AddDays returns the DateTime object's current value, not an updated value. DateTime.AddDays with a positive number works as expected. Similar behavior exhibited by AddMinutes, AddHours, and AddSeconds. Behavior does not appear to be affected by the actual presence of Uno.SkiaSharp nuget packages.

Note: the AOT build does work properly.

Expected behavior

DateTime.AddDays (and related functions) should properly calculate negative values.

How to reproduce it (as minimally and precisely as possible)

In the attached sample, load the Uwp build and observe that the dates are yesterday, today, and tomorrow. Load WASM and observe that the dates are today, today, and tomorrow. In TestMonoWasmConfiguration.Wasm.csproj remove the line '\release-dynamic\' and rebuild WASM - observe that the dates are yesterday, today, and tomorrow.

TestMonoWasmConfiguration.zip

Environment

Nuget Package: Uno.UI

Package Version(s): 1.0.0-dev.302

Affected platform(s):

Visual Studio:

Relevant plugins:

Anything else we need to know?

jeromelaban commented 4 years ago

Thanks Peter for the detailed report!

This looks like a mono-wasm issue, and at this point, the only option we have is to update the runtime to a later version to fix it.

At present time, the update is blocked on https://github.com/unoplatform/Uno.Wasm.Bootstrap/issues/114, which also blocks #1451. This issue is definitely a different one, so we'll track it as a different one, as at this time, the mono-wasm latest release-dynamic runtime crashes a simple Console.WriteLine.

DierkDroth commented 4 years ago

@jeromelaban would you have any idea on when the next mono-wasm update would be due?

jeromelaban commented 4 years ago

@DierkDroth multiple issues need to be fixed first:

The first one I fast tracked by fixing it, it's waiting on validation, the second one is pending, but I do not know when I'll be available. This current issue will be a third one that needs fixing.

So to answer your question, there's no due time at the moment, unfortunately.

DierkDroth commented 4 years ago

@jeromelaban thanks for sharing that info.

DierkDroth commented 4 years ago

@jeromelaban the issues on links you provided above appear to be closed (pls correct me if I'm wrong). Does this mean work on this very issue could be resumed?

jeromelaban commented 4 years ago

@DierkDroth there is another emscripten issue, related to https://github.com/unoplatform/Uno.Wasm.Bootstrap/issues/128, which is the exact issue this bug is about. The current runtime fails to perform the computation (as you've seen), yet silently, and the latest emscripten fails explicitly.

Once this issue is fixed, which should be soon, as there's activity there in the past few days, the work will resume.

DierkDroth commented 4 years ago

Thanks for the update @jeromelaban

jeromelaban commented 4 years ago

Update: Another issue needs to be fixed https://github.com/emscripten-core/emscripten/issues/9850

DierkDroth commented 4 years ago

Shoot ... thanks for the update @jeromelaban

jeromelaban commented 4 years ago

It's almost fixed, though, should be quick :)

DierkDroth commented 4 years ago

Appears emscripten-core/emscripten#9850 is fixed now. Any chance that the original issue could be addressed?

jeromelaban commented 4 years ago

@DierkDroth it is fixed, but not published yet unfortunately... There's not release date but the activity suggests there may be one soon.

DierkDroth commented 4 years ago

Thanks for the update @jeromelaban

jeromelaban commented 4 years ago

Emscripten has been updated, an update to mono has been submitted : https://github.com/mono/mono/pull/18016

jeromelaban commented 4 years ago

There's another update that needs to happen in emscripten (https://github.com/emscripten-core/emscripten/issues/9950), we'll update mono in parallel. This emscripten issue does not touch Uno directly, but prevents the compilation of Skia.

DierkDroth commented 4 years ago

Thanks for the update @jeromelaban. Looks like a never ending story ...

DierkDroth commented 4 years ago

@jeromelaban would it make sense to poke the right people in order to see some progress on the pending issues so we had a chance to get this very issue resolved?

jeromelaban commented 4 years ago

@DierkDroth we're still waiting for an update on the binaryen side, so we do not have control over the priority of that issue, unfortunately. It looks that a few other people are waiting on that specific issue as well, so it may move faster.

DierkDroth commented 4 years ago

Thanks for your feedback @jeromelaban

I know it's not your/nventive's responsibility nevertheless I wanted to share the feeling that things are moving way slower than I'd hoped on the mono-wasm support side of tings.

jeromelaban commented 4 years ago

@DierkDroth On this particular topic, indeed it's slower than we would have hoped. In general, we can influence issues by contributing but in this case, this is a very deep issue and it's the third try to fix it from the original author. Upvoting the issue on emscripten may help as well for visibility.

DierkDroth commented 4 years ago

Upvoting the issue on emscripten may help as well for visibility.

I'm sorry, I'm too far off the tech side of things here: how/where could I upvote?

jeromelaban commented 4 years ago

@DierkDroth Adding a pair of eyes or a thumbs up: image

DierkDroth commented 4 years ago

@jeromelaban looks like the relevant merges took place. Any chance to get the original issue addressed?

jeromelaban commented 4 years ago

@DierkDroth the merges have been made in Binaryen, but are not yet in an emscripten release. Once it is available, we'll be able to move forward.

DierkDroth commented 4 years ago

I see. Where could that be tracked/viewed?

jeromelaban commented 4 years ago

Releases are available there: https://github.com/emscripten-core/emsdk but there's no timetable for individual releases that I could find.

GitHub
emscripten-core/emsdk
Emscripten SDK. Contribute to emscripten-core/emsdk development by creating an account on GitHub.
DierkDroth commented 4 years ago

Hmm, I saw no releases there but saw releases here https://github.com/emscripten-core/emscripten/releases

Is that what I needed to be looking for?

GitHub
emscripten-core/emscripten
Emscripten: An LLVM-to-Web Compiler. Contribute to emscripten-core/emscripten development by creating an account on GitHub.
jeromelaban commented 4 years ago

Yes, those are the past releases, we'll most likely take 1.39.7 if the binaryen fixes stick.

DierkDroth commented 4 years ago

Got it. Thx

DierkDroth commented 4 years ago

@jeromelaban 1.39.7 appears to be out now. No idea if it would hold the relevant fix(es) though...

jeromelaban commented 4 years ago

Thanks, the update to 1.39.7 has passed in mono already https://github.com/mono/mono/pull/18687, we should be able to go further in the investigation.

DierkDroth commented 4 years ago

Excellent, thanks. Looking forward to see this issue closed.

jeromelaban commented 4 years ago

I've been testing the latest release of emscripten (1.39.7), and unfortunately there are other issues that have been introduced when using the dynamic linking mode.

I'm going to be closing this issue as it is fixed in the latest mono (as part of the 1.1.0-dev.409 bootstrapper version), but the emscripten updated does not fix the issues related to libSkia (https://github.com/unoplatform/uno/issues/1451). I'll provide updates for the bootstrapper changes there.

peternary commented 4 years ago

Issue tests as fixed for me.