Add | Add net8 compatibility

[Wraith2]

closes #1930

In netcore builds SqlGuidCaster and the type workaround that used it are not required. The problem they solved has already been change to use the ReadOnlySpan ctor. The SqlGuid workarounds in netcore are removed. Netfx builds use a more recent approach to the original problem and do not need to change.

I have also added ifdeffed use of the new api's introduced to the ryntime in dotnet/runtime#72724 so if/when a net8 build is created the new methods will be used and the workarounds will be ifdeffed out entirely.

Testing on net was done manually with a small repro app that used reflection to force SqlGuidCaster to be compiled. Before these change the reported TypeLoadException was encountered, after them it is not.

/cc @Alerinos @MichalPetryka

[JRahnama]

I will do some changes to this PR, such as the csproj file to include SqlTypeWorkarounds to be included if it is net6 only and changing the Net 8 to net 7 for now.

[Wraith2]

You can't change the net8 to 7 because this is adding support for new apis that aren't in 7. For the rest, go ahead.

[codecov]

Codecov Report

Patch coverage: 45.45% and project coverage change: -0.16 ⚠️

Comparison is base (a4f18ca) 70.82% compared to head (37fb9a0) 70.67%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1934      +/-   ##
==========================================
- Coverage   70.82%   70.67%   -0.16%     
==========================================
  Files         292      305      +13     
  Lines       61777    61794      +17     
==========================================
- Hits        43752    43670      -82     
- Misses      18025    18124      +99     

Flag	Coverage Δ
addons	92.88% <ø> (+0.49%)	⬆️
netcore	73.43% <45.45%> (-1.01%)	⬇️
netfx	69.27% <ø> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...rosoft/Data/SqlTypes/SqlTypeWorkarounds.netcore.cs	94.73% <ø> (+12.91%)	⬆️
...c/Microsoft/Data/SqlClient/Server/ValueUtilsSmi.cs	64.16% <ø> (ø)
...qlClient/src/Microsoft/Data/SqlClient/SqlBuffer.cs	80.45% <ø> (ø)
.../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs	68.58% <45.45%> (-4.81%)	⬇️

... and 93 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[David-Engel]

You can't change the net8 to 7 because this is adding support for new apis that aren't in 7. For the rest, go ahead.

The new APIs made it into .net7 last year. I think we are good to go on this PR.
https://github.com/dotnet/runtime/commits/v7.0.0/src/libraries/System.Data.Common/src/System/Data/SQLTypes

[FS-FrankLakmann]

Hello, we just stumbled over #1930
One question concerning your release strategy: Why was this fix not added to the LTS 5.1.x branch? Now we have a situation where the only branch with half-decent (not enough but better than the other branches) support lifetime is not compatible to the current LTS version of the dotnet runtime itself.
Please remember: not everybody does inhouse development and can push releases every month. There are ISVs out there who release once a year and those have to run for 2 years with only minor bugfixes. We need stable and long-term support for the runtime and essential base libs like SqlClient.
Your LTS schedule should be aligned with the release of the framework LTS-versions and keep compatibility with that version.

[Wraith2]

Why is your code, or your vendor code, touching internal types in an assembly it does not own?

[FS-FrankLakmann]

I'm not sure what you mean by touching. We just used SqlClient with NHibernate in dotnet 8 for some months in an ordinary line-of-business-application. Everthinig was fine with version 5.1.6. Then we made some changes to entity definitions and data mappings and got the same runtime exception as in #1930. I have no idea what triggered the change but we were not doing anything fancy in the internals.
Edit: The repo example provided in #1930 looks super harmless.

[Wraith2]

The only way to touch SqlGuidCaster is through reflection accessing internal types inside SqlClient. That's why it wasn't found in testing, it wasn't used and the public surface area wasn't changed. If your dependency, NHibernate was using private reflection then it should not have been.

[ErikEJ]

@Wraith2 Looks like this repros:

assembly.GetTypes()

[FS-FrankLakmann]

Small Correction: There was no code change in the data access layer (Entiies, ,mappings). The new code that triggers the exception is a readonly reflection call
AppDomain.CurrentDomain.GetAssemblies().AsEnumerable() .SelectMany(x => x.GetTypes()) .Where(x => typeof(IAnyHandler).IsAssignableFrom(x) && !x.IsInterface && !x.IsAbstract) .... that iterates all types of all loaded assemblies. Almost identical to the code by Alerinos in the initial issue post of #1930.

If this looking into the types via reflection without changing anything is what you mean with "touching" then yes - we are touching the types. It's readonly - we are not changing anything!
And the main point of my criticism is the sqlclient roadmap. Why am I forced off the LTS branch of sqlclient to stay compatible with the LTS version of the framework?