Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for additional TFM's #26

Merged
merged 4 commits into from
Mar 17, 2022

Conversation

mchylek
Copy link
Contributor

@mchylek mchylek commented Mar 8, 2022

Add support for .netstandard2.0 and net5.0

Related to mbraceproject/FsPickler#123

mchylek added 2 commits March 8, 2022 11:18
Add support for .netstandard2.0 and net5.0
@dsyme
Copy link
Contributor

dsyme commented Mar 8, 2022

Great!

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Mar 8, 2022

The library intentionally doesn't target netstandard (since .NET framework is no longer supported). What would be the benefit of targeting net5.0? Isn't netcoreapp3.1 sufficient and compatible with new releases?

@mchylek
Copy link
Contributor Author

mchylek commented Mar 8, 2022

Hi @eiriktsarpalis, actually the main driver for this PR is to bring back support for .Net Framework (required for project I'm working on). Regarding TFM's I'm happy to remove explicit target for net5.0 - I was under impression it's recommended to use both netcoreapp* and net5.0+.

@eiriktsarpalis
Copy link
Member

Feel free to try, but as mentioned support was removed intentionally (can't recall the precise reason but I believe it might have something to do with AppDomains and assembly loading semantics). Have you verified that tests are passing for netfx? Seems to be failing in CI.

@mchylek
Copy link
Contributor Author

mchylek commented Mar 8, 2022

For netfx I had to polyfill AssemblyLoadContext with single AppDomain that was enough to make tests passing (also works fine in my internal project) but I'm aware of it's limitations.

CI is failing currently on netcoreapp3.1 tests (looks like ThunkServer didn't start)

@eiriktsarpalis
Copy link
Member

According to the CI logs only net5.0, net6.0 and netcoreapp3.1 are being tested, but no netfx targets. I'd argue that only one Core runtime is needed in testing (the latest, net6.0 should suffice).

@mchylek
Copy link
Contributor Author

mchylek commented Mar 9, 2022

NetFx tests are also executed https://ci.appveyor.com/project/dsyme/vagabond/builds/42827493#L384

Passed!  - Failed:     0, Passed:    56, Skipped:     1, Total:    57, Duration: 49 s - Vagabond.Tests.dll (net472)

Just to confirm - nuget packages should target netcoreapp3.1 + netstandard2.0, and test should be executed on latest runtime (net6.0) and netfx (net472)?

Also CI currently fails on generating docs. I've tried to update FSharp.Formatting but looks like Razor support was dropped. Any recommendations? 😊

C:\projects\vagabond> "dotnet.exe"  fsi --define:RELEASE docs/tools/generate.fsx (In: false, Out: false, Err: false)
error FS0193: internal error: Could not load type 'range' from assembly 'FSharp.Compiler.Service, Version=41.0.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.

@eiriktsarpalis
Copy link
Member

Just to confirm - nuget packages should target netcoreapp3.1 + netstandard2.0, and test should be executed on latest runtime (net6.0) and netfx (net472)?

Yes please 👍

Also CI currently fails on generating docs. I've tried to update FSharp.Formatting but looks like Razor support was dropped. Any recommendations? 😊

No recommendations unfortunately. Have you tried updating to the latest version of FSharp.Formatting?

@mchylek
Copy link
Contributor Author

mchylek commented Mar 9, 2022

For FSharp.Formatting 14.0.1 (latest) it just failing with

generate.fsx(12,24): error FS0039: The namespace 'Razor' is not defined.

And FSharp.Formatting.Razor 4.1.0 is returning the original error

error FS0193: internal error: Could not load type 'range' from assembly 'FSharp.Compiler.Service, Version=41.0.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.

Alternatively I wonder if there is a switch to run fsi from older sdk (3.1)

@eiriktsarpalis
Copy link
Member

I think it boils down to updating the local generate.fsx so that it works with the latest FSharp.Formatting. That might include removing deprecated namespaces.

@mchylek
Copy link
Contributor Author

mchylek commented Mar 9, 2022

Are you fine with migrating docs generation to fsdocs?

@eiriktsarpalis
Copy link
Member

Sure, whatever is bleeding edge I approve of.

@dsyme
Copy link
Contributor

dsyme commented Mar 9, 2022

Yes just move to latest fsdocs-tool (though it requires net5.0 to run, we must fix that)

@mchylek
Copy link
Contributor Author

mchylek commented Mar 17, 2022

Unfortunately I had problems with migrating to fsdocs-tool as it didn't work with net6.0, and when switching to net5.0 I've observed issues with FAKE.

CI is green now as I've made workaround to use SDK 3.1 purely for docs generation. I'm happy to migrate to fsdocs (separate PR) once issue with net6.0 will be resolved.

@eiriktsarpalis eiriktsarpalis merged commit ccec017 into mbraceproject:master Mar 17, 2022
@eiriktsarpalis
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants