Sqlclient: Build and project queries

[Wraith2]

I've cloned and followed the build instructions. It's quite chatty at the moment which probably helps with debugging and it almost works. What stops it is that the netcore project has xml comments enabled and is complaining that a lot of items are missing xml comments, 800+ of them. Some of these are new types from netfx and some are old that have been around a while. Is it a project goal to have the documentation complete? If not can we change the build to suppress the xml doc warnings/errors?

The project is split into two totally different branches for netfx and netcore. This is quite different to the corefx version which was built for two targets but had one set of source. Having two complete sets of source will make maintenance and changes more time consuming and error prone. Is this the final configuration for the repository? If so can you give me some detail about why this approach was chosen?

I've checked on a couple of my PR's that were open in corefx for bug fixes and those haven't been pulled into the 1.0 version. Do I need to get back over my PR history in corefx and diff those changes into this project?

Are there plans (however rough) for a release cadence that you can comment on? How often will official builds be made available on nuget?

[cheenamalhotra]

Yes we are working on that, our plan is to update both (NetFx and NetCore) API references and onboard to .NET API reference documentation with the next preview release. Efforts are in place, thanks for gathering that!

I keep this issue for 1.1 Milestone :v:

[cheenamalhotra]

For porting activities, yes the remaining PRs were flagged for porting post GA, feel free to port them over here, if you have availability!

[ErikEJ]

Is this the final configuration for the repository? If so can you give me some detail about why this approach was chosen?

and

Are there plans (however rough) for a release cadence that you can comment on? How often will official builds be made available on nuget?

[Wraith2]

For porting activities, yes the remaining PRs were flagged for porting post GA, feel free to port them over here, if you have availability!

It's not just open PRs. For example https://github.com/dotnet/corefx/pull/35549 was merged in march and those changes aren't in this codebase. Will I need to back through all my previous changes and port them over again? How far back do I go?

I've got 10+ branches for performance improvements on the corefx version that I've been holding back to try not to complicate the porting process. Some of those are very complex and I'll now have to reimplement them twice. This additional complexity seems unnecessary at the moment given that the original corefx source build from a single location.

[David-Engel]

Is this the final configuration for the repository? If so can you give me some detail about why this approach was chosen?

The two code sets are there because one set came from NetFx and one came from NetCore. NetCore forked from NetFx when NetCore was introduced and the code was heavily modified to pull out features that could not be made to support NetCore, cross-plat in the time frame required. From there, the code bases continued to diverge more as new SQL features were mostly added to NetFx only and NetCore was updated by other contributors. Merging the code bases while keeping things working for existing NetFx applications and keeping the driver stable was not possible given the time and resources we had. The long term goal is to have the code bases come back together again. In the meantime, we have the overhead of implementing changes in both code bases.

Are there plans (however rough) for a release cadence that you can comment on? How often will official builds be made available on nuget?

We don't have an official release cadence, yet. But the plan is for it to be relatively frequent. At least twice a year. 1.1 is targeting November and we would like to have a preview or two between now and then. Release plans beyond that are not firm.

Will I need to back through all my previous changes and port them over again? How far back do I go?

We made a list of all merged PRs to S.D.SqlClient and tried to port as many as we could to M.D.SqlClient (including to the NetFx code, when possible). Looking at the list, I wish we had gotten to 37270 for GA but our timeline was shortened while we were working through the list and we put the remainder of these on hold (there were originally ~40 to go through). Here are the ones that did not get into M.D.SqlClient yet:

CoreFx SqlClient PR | Comments
-- | --
37270: Fix bad usage of ArrayPool in TdsParserStateObject |  High priority fix
35549: SqlClient shrink SqlParameter and fix 34414 |
35363: SqlClient managed networking improvements |  
35304: Fix SqlClient udtTest |
34049: Optimize SqlClient rpc parameter usage | Conflicts heavily with AE changes. Needs more analysis.

I've got 10+ branches for performance improvements on the corefx version that I've been holding back to try not to complicate the porting process. Some of those are very complex and I'll now have to reimplement them twice. This additional complexity seems unnecessary at the moment given that the original corefx source build from a single location.

Sorry for the extra work, but I hope you understand where we were coming from and why we made the choices we did. We welcome discussions on strategies for merging the code bases back together.

We do have a lot of work to do between now and 1.1 including finishing up documentation, getting the AKV provider to support M.D.SqlClient, and bringing the AE Enclave provider into M.D.SqlClient, fixing issues with AE tests from NetFx, fixing other test issues (search the tests for ActiveIssue), and lots of other cleanup tasks.

[Wraith2]

I thought the intention for this library was to be a single package which contained a unified SQL ADO driver for netfx core and netstandard. Have I misunderstood? is this not the intent?

It looks and reads like we've just got the code for the two totally separate libraries with no attempt to merge them, if that's the case why did it take so long to publish the code?

[David-Engel]

I thought the intention for this library was to be a single package which contained a unified SQL ADO driver for netfx core and netstandard. Have I misunderstood? is this not the intent?

It looks and reads like we've just got the code for the two totally separate libraries with no attempt to merge them, if that's the case why did it take so long to publish the code?

SDS shipped with .NET Framework and we are not able to add features there anymore. So the main intention is to have a package we can implement new features in, supporting both .NET Framework and .NET Core (the .NET Core implementation also supports .NET Standard). There is still a huge customer base on .NET Framework who would like to get the latest SQL features.

As for why it took so long, there are many reasons. A few of them are the many tentacles SDS had deep into .NET, building up new build and test pipelines, porting features and tests from Framework to MDS, keeping up with PRs from Core, going through iterations of trying to support back-compat features in unique but ultimately discarded ways, changing the namespace, separating the native and managed code on the Framework side, removing closed source code, building new packages supporting the unique needs of SqlClient, migrating from ADAL to MSAL, packaging and indexing symbols, migrating documentation, setting up localization, working out licensing issues, etc. We originally thought we could get it done by May but it turned out to be a lot more work.

[Wraith2]

Ok. So on to what next.

Looking at the codebases there's a lot in there that's similar and I think that working towards a unified codebase for the drivers is possible but a long job with a lot of compatibility testing required. Is this something you'd support doing and if so how would you prefer it to be implemented? The goal would be to have all tfm's present all features and for changes in one to be quickly easily and automatically (where possible) be reflected in the others.

I think start by identifying shared files and moving them to a common location so both projects share them. Once done we'll have files with implementation specific behaviour neatly segregated and we can investigate how to align those implementations or keep them split in the leanest way possible. I usually like the corefx method of doing this by using project conditions and avoid #defines where possible.

[Wraith2]

Looking through the codebases some more a few more questions of policy and how we can contribute occur to me.

1. Should everthing be using the current corefx style of formatting (netfx code is using java style bracing.) and if so would you agree to taking a copy of the corefx editorconfig file and putting it hight enough up in the repo to affect everything? Then we could to fairly straight forward automatic format updates which would resolve a lot of differences in trivial files.
2. there are places where sql version specific checks are made and i've found at least one difference so far between core and fx in an isYukon (2005) check. This leads to two questions. Can we get a list of the supported SQL server targets so we can harmoize these checks, and can we rename these symbols from internal codenames to external marketting identifiers?.
3. I (and perhaps others) are looking to see how we contribute to this project in a co-operative way rather than just running off and doing my own thing which might cause conflicts. There's still a lot of performance work i've got mapped out that i'd like to bring into the project but i'd like to know if i can do so for all users and not just the netcore users.

[ErikEJ]

re 1: Yes, that would be a good step towards enabling code sharing between the two code bases

re 3: Would improving code sharing not help with this?

[Wraith2]

On 1. Yup, just need to know if that's an agreed way to go. and on 3. yes to some extent.
The managed implementation doesn't exist in desktop at all in desktop and lags in performance but it's the only true open implementation which is an important consideration or some consumers, all other implementations use a closed source native component.

[ErikEJ]

Maybe potential contributors could benefit from a wiki post with all this vital information?

[Wraith2]

Oh, and no Always Encrypted support on netstandard?

[ErikEJ]

Netstandard ?? Assume you mean .NET Core? And is it not supported now? Or do you mean native versus open source implementation?

[Wraith2]

I decided to sync the ref projects so the differences were clearer. the AE classes and members are conditionally included in the netcore build but not in the netstandard build. So if you add M.D.SqlClient to a netstandard TFM project you won't see the AE members and thus be unable to use it. Presumably since you can use AE on netcore unix the implementation is entirely portable so it may just be a mistake.

[David-Engel]

1. We have a task to configure coding style and apply it to all the existing code. @cheenamalhotra is working on it.
2. It will be in the documentation that we are working on, but SQL Server version support follows what SQL Server versions are still in support at the time of release. So SQL Server 2012 and up for 1.0. I am not against renaming those symbols. It may be possible to just remove some/most of them.
3. I know you like perf work. The managed SNI layer is the obvious target for improvement and doesn't require working in both netfx and netcore. If it can be brought to perf and stability parity to the native SNI layer, that would be awesome. Anything that can move us towards a fully managed stack across platforms.
   My understanding is that netstandard is missing certain encryption dependencies required by AE:
   https://github.com/dotnet/SqlClient/blob/master/release-notes/1.0/1.0.19249.1.md#always-encrypted
   I have a question out to the AE team about what exactly those dependencies are.

[cheenamalhotra]

Listing items here so we can resolve this issue:

• [x] Configure Coding Style with .editorconfig - Done
• [x] Roadmap transparency in Repository Docs and GitHub Projects - Done
• [x] .NET API Reference documentation for Microsoft.Data.SqlClient - Done (Docs)
• [x] Porting PRs from CoreFx - Done