Add support for .NET 9 by mdaigle · Pull Request #2946 · dotnet/SqlClient

mdaigle · 2024-10-31T19:18:48Z

Enhancements:

Adds .NET 9 to target frameworks

Fixes:

Adds missing DynamicallyAccessedMembers attribute to GetProviderSpecificFieldType and GetFieldType in netcore ref project.
- Removes suppressions due to missing annotations in net6.0
Switches X509Certificate construction to use X509CertificateLoader.
Switches PKCS decoding to use SignedCMS.
Adds internal ReadExactly extension to System.IO.Stream in netfx to provide API parity between netcore and netfx and to address CA2022.
Guards ServicePointManager usage to netfx only, deprecated in .NET 9
- https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/syslib0014
- https://learn.microsoft.com/en-us/dotnet/api/system.net.servicepointmanager?view=net-9.0

Package Versions:

Adds System.Security.Cryptography.Pkcs:9.0.0 to address SYSLIB0057
Adds Microsoft.Bcl.Cryptography:9.0.0, dependency of the pkcs package
Bumps Microsoft.Extensions.Caching.Memory to 9.0.0
Bumps System.Configuration.ConfigurationManager to 9.0.0

Test Dependencies:

Bumps Microsoft.DotNet.RemoteExecutor to 10.0.0-beta.24564.1
Bumps Xunit to 2.6.3 for all frameworks

…msbuild." This reverts commit c010bd3.

…A2022 and IL2093 errors.

roji

Am curious - is there any net9.0 API that's actually needed/used? If not, is there a specific reason to add the net9.0 TFM?

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Server/SqlNormalizer.cs

David-Engel · 2024-11-01T17:54:42Z

Am curious - is there any net9.0 API that's actually needed/used? If not, is there a specific reason to add the net9.0 TFM?

@roji JSON support in SqlDbType: dotnet/runtime#103925

roji · 2024-11-02T08:33:36Z

@David-Engel of course, I forgot about that, thanks :)

…stall step.

…net9-add-support

codecov · 2024-11-22T01:32:48Z

Codecov Report

Attention: Patch coverage is 35.71429% with 9 lines in your changes missing coverage. Please review.

Project coverage is 72.60%. Comparing base (1348c3c) to head (6ac682e).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...c/Microsoft/Data/SqlClient/Server/SqlNormalizer.cs	0.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2946      +/-   ##
==========================================
- Coverage   72.67%   72.60%   -0.08%     
==========================================
  Files         285      285              
  Lines       59160    59162       +2     
==========================================
- Hits        42997    42954      -43     
- Misses      16163    16208      +45

Flag	Coverage Δ
addons	`92.58% <ø> (ø)`
netcore	`75.40% <35.71%> (-0.04%)`	⬇️
netfx	`71.00% <30.76%> (-0.07%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs

edwardneal · 2024-11-22T04:51:27Z

src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/StreamExtensions.netfx.cs

+namespace System.IO
+{
+    // Helpers to read/write Span/Memory<byte> to Stream before netstandard 2.1
+    internal static class StreamExtensions


Is there a shared path we can put this which both the test projects and M.D.S can refer to? It's identical to src/Microsoft.Data.SqlClient/src/System/IO/StreamExtensions.netfx.cs.

Ideally, I would create a Common/Shared project that both test and src projects depend on, but this class is too small for that. If we have more cases like this, we can consider coming up with a common project.

@mdaigle Like I said in the previous PR, internals of the MDS projects should be visible to the test projects. If they aren't, then we should make them so.

@cheenamalhotra I personally don't think we need to add another project for this purpose, if we leverage internalsvisibleto. We can discuss more if we need to, tho.

tools/props/VersionsNet9OrLater.props

tools/props/Versions.props

eng/pipelines/common/templates/steps/ci-project-build-step.yml

src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/VirtualSecureModeEnclaveProvider.cs

src/Microsoft.Data.SqlClient/tests/Directory.Build.props

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/DataStreamTest.cs

…net9-add-support

… transitive dependency.

…king transitive dependency.

tools/props/VersionsNet8OrLater.props

...sts/tools/Microsoft.Data.SqlClient.ExtUtilities/Microsoft.Data.SqlClient.ExtUtilities.csproj

tools/props/Versions.props

cheenamalhotra · 2024-11-23T05:40:15Z

src/Microsoft.Data.SqlClient/tests/ManualTests/Extensions/StreamExtensions.netfx.cs

+namespace System.IO
+{
+    // Helpers to read/write Span/Memory<byte> to Stream before netstandard 2.1
+    internal static class StreamExtensions


Ideally, I would create a Common/Shared project that both test and src projects depend on, but this class is too small for that. If we have more cases like this, we can consider coming up with a common project.

cheenamalhotra · 2024-11-23T05:41:04Z

src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs

            AliasName = c.AliasName;
            IsJsonSupported = c.IsJsonSupported;

+#if NETFRAMEWORK


Can you elaborate on this change?

ServicePointManager is marked as obsolete in Net 9.
https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/syslib0014
I removed the call to no effect in the tests, so it didn't seem to be changing anything. But if there's any concern, I can add a suppression instead.

@mdaigle Are you sure it's the ServicePointManager? I had seen similar warnings in the code when using Tls12 security type.

…net9-add-support

benrr101

Looks good to me in general. I think we should make InternalsVisibleTo test projects (I think that change was only made in the clientx branch) so we can avoid duplication of the stream extensions. But I won't hold up approval on that.

ErikEJ and others added 9 commits July 15, 2024 12:51

Update to .NET 9 preview 6

1eeaea4

Testing Net9 SDK support on pipelines

5b05bd4

Revert NuGet.config change

2913009

Update pipeline

e54daf3

Suppressed error codes IL2093 and CA2022 when compiled using msbuild.

c010bd3

Revert "Suppressed error codes IL2093 and CA2022 when compiled using …

65710d6

…msbuild." This reverts commit c010bd3.

Derived from ErikEJ's issue-2669 branch and added fixes for msbuild C…

119d70b

…A2022 and IL2093 errors.

Updated packages for .net9.0 to rc-1 versions.

179fa81

Merge main

4ee429f

mdaigle changed the title ~~Add support for .NET 9~~ Not Ready for Review | Add support for .NET 9 Oct 31, 2024

mdaigle marked this pull request as ready for review October 31, 2024 19:27

roji reviewed Oct 31, 2024

View reviewed changes

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Server/SqlNormalizer.cs Show resolved Hide resolved

mdaigle added the 💡 Enhancement label Nov 1, 2024

mdaigle added 2 commits November 1, 2024 11:33

Merge changes from Aris's branch

b2a0c4b

merge main

8324368

mdaigle added 13 commits November 4, 2024 10:06

Add usedotnet task before build.

61da001

Fix formatting.

c32d351

dotnet location debug logs.

54e4850

List sdk and runtime versions and locations.

df7c8e8

Switch image for nuget restore job.

ceb4f19

Bump image for nuget restore.

174ae48

Temporarily remove test steps to reduce wait time. Remove dotnet 9 in…

257868b

…stall step.

Add global.json file to lock sdk version.

81756b3

Fix target frameworks for netcore ref.

fc60090

Try base image with dotnet installed according to global.json

5c7ab1c

Merge branch 'main' into net9-add-support

0710337

Install dotnet 8 sdk.

7197ae7

Merge branch 'net9-add-support' of github.com:mdaigle/SqlClient into …

17667e2

…net9-add-support

mdaigle requested a review from cheenamalhotra November 21, 2024 23:38

How did these get dropped?

fe363e2

edwardneal reviewed Nov 22, 2024

View reviewed changes