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 netcore/netstandard based tooling (for example FAKE) #337

Open
matthid opened this issue Apr 14, 2019 · 16 comments
Open

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

matthid opened this issue Apr 14, 2019 · 16 comments

Comments

@matthid
Copy link
Member

matthid commented Apr 14, 2019

See conversation in fsprojects/FAKE#2272
Basically, we should ship a netstandard based design-time assembly.

@smoothdeveloper
Copy link
Collaborator

@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
Copy link
Collaborator

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Member Author

matthid commented Jun 18, 2019

But FAKE chooses netstandard 2.0 at this time

@charlesroddie
Copy link

charlesroddie commented Jul 1, 2019

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
Copy link
Collaborator

smoothdeveloper commented Jul 1, 2019

@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:

  • all signatures exposing System.Data.SqlClient types
  • the internal implementation, especially in places we rely on sqlclient specific types rather than the common ado.net interfaces
  • forcing user to switch to new client nuget packages / adjust their code

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
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member Author

matthid commented Nov 9, 2019

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

@thinkbeforecoding
Copy link
Contributor

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
Copy link
Member Author

matthid commented Dec 12, 2019

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

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

No branches or pull requests

5 participants