Simple changes to make it compatible with WASM #8722
Conversation
In Wasm `Assembly.Location` do not have directory path since assemblies reside in memory.
In Wasm `tryAppConfig` throws and env var `FSHARP_COMPILER_BIN` never gets tested, so do it first
|
|
realvictorprm
left a comment
realvictorprm
left a comment
There was a problem hiding this comment.
See comments, at least in my opinion they could be answered.
| // We let you set FSHARP_COMPILER_BIN. I've rarely seen this used and its not documented in the install instructions. | ||
| match Environment.GetEnvironmentVariable("FSHARP_COMPILER_BIN") with | ||
| | result when not (String.IsNullOrWhiteSpace result) -> Some result | ||
| |_-> |
There was a problem hiding this comment.
I would prefer if you intend the following code block to prevent any confusion.
There was a problem hiding this comment.
I was following the style already in use there which followed the pattern
match ... with
| Some .. -> ....
| _ ->
...
match ... with
| Some .. -> ....
| _ ->
Although admittedly in the end it broke it by indenting the final if then else
If you still prefer multiple indents I will change that section so its consistent.
| let getDefaultFSharpCoreLocation = Path.Combine(fSharpCompilerLocation, getFSharpCoreLibraryName + ".dll") | ||
| let getDefaultFsiLibraryLocation = Path.Combine(fSharpCompilerLocation, getFsiLibraryName + ".dll") | ||
| let implementationAssemblyDir = Path.GetDirectoryName(typeof<obj>.Assembly.Location) | ||
| let implementationAssemblyDir = Path.GetDirectoryName(typeof<obj>.Assembly.Location) |> ifEmptyUse fSharpCompilerLocation |
There was a problem hiding this comment.
Could you add additional explanation for that change?
There was a problem hiding this comment.
in Mono-Wasm typeof<obj>.Assembly.Location = "mscorlib.dll" with no path and so Path.GetDirectoryName(typeof<obj>.Assembly.Location) = "" which causes an exception later on. In this case I am using fSharpCompilerLocation which was resolved in the prior point.
There was a problem hiding this comment.
@amieres can you put a repro of something that fails on a GitHub repo somewhere. I certainly want to think about this.
The notion that assemblies are loaded from streams, plays havoc with the a couple of our commonly held invariants.
I think it is a good scenario for us to handle, but I would rather we think about what the solution looks like rather than a few targeted bug fixes. Although in the end it may just turn out to be soluble that way.
I think probably we might want to look at what fable does too since they may encounter and have solved similar issues.
There was a problem hiding this comment.
@KevinRansom
I created a website where different Assemblies for FCS can be tested:
https://amieres.github.io/WASM.html
After the initial load there is an option to select the set of Assemblies to use
with WASM:
- Interpreted FCS 25.0 modified
- Interpreted FCS 33.0
- Interpreted FCS 34.1
- Interpreted FCS 34.1 fmwk
The first one is the only that works. It is actually a fork of FCS 25.0 created for this purpose:
https://github.com/fsbolero/FSharp.Compiler.Service
Version 33.0 would work if the 2 issues in this PR were solved (plus a third one, that I don't quite recall right now).
Version 34.0 introduced a new issue by using AssemblyLoadContext which is not supported by Mono WASM because according to the documentation it is not supposed to be NetStandard, but only NetCore. OTOH, the package System.Runtime.Loader is published in Nuget.org as .NETStandard 1.5. I do not know who is right (the documentation or the nuget package), but the end result is that version 34 wont work with WASM until WASM goes to NetCore.
Let me know if this helps you in any way.
When you try to compile the snippets you will see the exceptions produced in each case.
|
@amieres sorry for the delay in handling this. Thanks for this. |
* ifEmptyUse fSharpCompilerLocation
In Wasm `Assembly.Location` do not have directory path since assemblies reside in memory.
* Check GetEnvironmentVariable("FSHARP_COMPILER_BIN") before tryAppConfig
In Wasm `tryAppConfig` throws and env var `FSHARP_COMPILER_BIN` never gets tested, so do it first
* ifEmptyUse with IsNullOrWhiteSpace
4 participants
In WASM assemblies reside in memory, therefore
Assembly.Locationreturns no directory path. That creates some problems. These 2 changes are to fix that.