fsprojects / FSharp.Data.SqlClient

A set of F# Type Providers for statically typed access to MS SQL database
http://fsprojects.github.io/FSharp.Data.SqlClient/
Other
205 stars 71 forks source link

Add support for netcore/netstandard based tooling (for example FAKE) #337

Open matthid opened 5 years ago

matthid commented 5 years ago

See conversation in https://github.com/fsharp/FAKE/issues/2272 Basically, we should ship a netstandard based design-time assembly.

smoothdeveloper commented 5 years ago

@matthid, am I understanding right that this is blocking usage of the library in FAKE 5+ scripts?

I don't know as of yet what are the changes necessary, but it seems the design assembly depends on assemblies that aren't available on netstandard:

https://www.nuget.org/packages/Microsoft.SqlServer.TransactSql.ScriptDom/ https://www.nuget.org/packages/Microsoft.SqlServer.Types/

I haven't checked what impact it has on the code if we singled out those dependencies for netstandard target, maybe this is a viable first step.

smoothdeveloper commented 5 years ago

@matthid, I've looked a bit, it may be easier to achieve this if we'd drop net40 support for SqlClient.DesignTime assembly.

I don't know if it makes sense to keep net40 support in the design time, I think mono support works with net461 target already, it would be supporting VS2013 scenario.

The concern is that each target has a mix of dependencies which needs to be defined explicitly in the fsproj, I don't know if there is a way to improve on that because it will hurt maintenance when adding / updating targets & dependencies.

Right now it is tedious to get all targets to build.

I'm defining USE_SQL_SERVER_TYPES_ASSEMBLY and looking at the impact of removing the Microsoft.SqlServer.* assemblies, but it may be blocking early; I've just contacted the owner of those packages on nuget.org to hear if they have any plans for netstandard20 target.

thinkbeforecoding commented 5 years ago

There seems to be a official Microsoft preview of those assemblies for net46 and netcoreapp2.2:

https://www.nuget.org/packages/Microsoft.SqlServer.DacFx/150.4384.2-preview

It probably may help !

samhanes commented 5 years ago

It looks like a version was just released that provides actual netstandard versions (rather than netcoreapp2.1) of the key dlls: https://www.nuget.org/packages/Microsoft.SqlServer.DACFx/150.4451.1-preview

This should allow us to have a true netstandard version of the DTC, rather than the hybrid approach we are using now, which would solve a whole bunch of problems.

I can take a crack at a version that uses that package for the Microsoft.SqlServer.Types and Microsoft.SqlServer.TransactSql.ScriptDom assemblies...

smoothdeveloper commented 5 years ago

Thanks for the heads-up, last release that @vasily-kirichenko pushed includes my changes from #338, I'll look again at updating those dependencies but I'm really busy for few weeks ahead, if someone makes a PR with updated deps in meantime, I'll be happy to review/test it.

samhanes commented 5 years ago

@smoothdeveloper I did some poking around with this last night, and I noticed that you added some safety around referencing System.Data.Common.DbProviderFactories with #338, since it is not available in netstandard2.0. However, it is available in netcoreapp2.1/netstandard2.1.

That means in theory we should be able to support enum types (in the DTC) for netfx >= 4.6.1 and netcore > 2.1. One way to accomplish that would be to provide separate DTC assemblies for net461, netstandard2.0, and netstandard2.1 - of the three, only netstandard2.0 would fail to handle the SqlEnumProvider types.

I'm concerned this would lead to strange behavior for, say, net472 apps which I presume would attempt to use the netstandard2.0 DTC? That's not any worse than what happens in 2.0.5, but it's not great. Another alternative would be to abandon netstandard altogether and target the library at net461/netcoreapp2.0/netcoreapp2.1, which is counter to guidance but might provide the best experience for everyone.

Thoughts?

samhanes commented 5 years ago

Ok wait, Nuget would always choose net461 over netstandard2.0 for a net472 app because it's the same framework, so never mind... ☝️

I do think I will add a netstandard2.1 version of the DTC, though, so we can support netcoreapp2.1+ apps using SqlEnumProvider.

matthid commented 5 years ago

But FAKE chooses netstandard 2.0 at this time

charlesroddie commented 5 years ago

Does it make sense to switch to Microsoft.Data.SqlClient as part of this? https://devblogs.microsoft.com/dotnet/introducing-the-new-microsoftdatasqlclient/ https://github.com/dotnet/SqlClient/wiki/Frequently-Asked-Questions

smoothdeveloper commented 5 years ago

@charlesroddie I've been miffed by that anouncement, talking about forking the eco-system, MSFT couldn't do better.

I assume switching has implications on:

For now, I don't see urgency to support that new client but there are several things that should be discussed, and probably we should wait for actual feedback on usage of the new client library.

@charlesroddie if this is something you have been looking forward already, and you used the new client, do you see any advantages in using it yet (features that could be added to the TP)? please feel free to open a separate issue on that specific Microsoft.Data.SqlClient support.

charlesroddie commented 5 years ago

OK just thought that this might be part of "standardization". Giving the expected dependency of behaviour on the package version (without relying on what happens to be installed on Windows), and in future feature parity between Core and Framework. If this doesn't help to create a .net standard verion of FSharp.Data.SqlClient then we can forget about it until it's released.

thinkbeforecoding commented 4 years ago

The Microsoft.SqlServer.DACFx has been released and is not in preview anymore:

https://www.nuget.org/packages/Microsoft.SqlServer.DACFx/150.4573.2

thinkbeforecoding commented 4 years ago

I don't think it's a problem to require dotnet 3.0 or higher for the design time assemblies. Sometime it's touchy to require latest version on runtime, but it's usually far easier for development tooling and CI. Especially for building on linux which was not possible until now anyway..

matthid commented 4 years ago

Like I said in the OP this means it will not work in FAKE which this issue is about.

thinkbeforecoding commented 4 years ago

Yes but you don't have to load the type provider design time assembly inside Fake, Fake will just launch a dotnet build, right ?

matthid commented 4 years ago

@thinkbeforecoding We are talking about using type providers within the build script itself.