nblockchain / fsx

FSX is the ideal tool for people that use F# for their scripting needs.
MIT License
15 stars 5 forks source link

Fsdk: fix build #40

Closed knocte closed 8 months ago

knocte commented 8 months ago

Somehow, the macOS--dotnet6-and-mono and linux-*-github--dotnet-* CI lanes started failing with this compiler error:

/home/runner/work/fsx/fsx/Fsdk/Process.fs(22,31): error FS0041: No overloads match for method 'Read'.

Known type of argument: int ref

Available overloads:
 - Volatile.Read(location: inref<bool>) : bool // Argument 'location' doesn't match
 - Volatile.Read(location: inref<byte>) : byte // Argument 'location' doesn't match
 - Volatile.Read(location: inref<float32>) : float32 // Argument 'location' doesn't match
 - Volatile.Read(location: inref<float>) : float // Argument 'location' doesn't match
 - Volatile.Read(location: inref<int16>) : int16 // Argument 'location' doesn't match
 - Volatile.Read(location: inref<int64>) : int64 // Argument 'location' doesn't match
 - Volatile.Read(location: inref<int>) : int // Argument 'location' doesn't match
 - Volatile.Read(location: inref<nativeint>) : nativeint // Argument 'location' doesn't match
 - Volatile.Read(location: inref<sbyte>) : sbyte // Argument 'location' doesn't match
 - Volatile.Read(location: inref<uint16>) : uint16 // Argument 'location' doesn't match
 - Volatile.Read(location: inref<uint32>) : uint32 // Argument 'location' doesn't match
 - Volatile.Read(location: inref<uint64>) : uint64 // Argument 'location' doesn't match
 - Volatile.Read(location: inref<unativeint>) : unativeint // Argument 'location' doesn't match
 - Volatile.Read<'T when 'T: not struct>(location: inref<'T>) : 'T // Argument 'location' doesn't match

Which makes me think that they are upgrading to newer .NET versions instead of being pinned...

The build fix works but not sure it's correct at runtime TBH. We will see when we start using the new version of Fsdk nuget package soon...

knocte commented 8 months ago

Upstream bug is 16524 in dotnet/fsharp; and looking at a referenced PR from it, it looks like this PR as it is currently might not be the best fix.