NotificationsYou must be signed in to change notification settings
Fork311
Star927

Add support for .NET 9#2946

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

mdaigle merged 83 commits intodotnet:mainfrommdaigle:net9-add-support

Nov 25, 2024

Merged

Add support for .NET 9#2946

mdaigle merged 83 commits intodotnet:mainfrommdaigle:net9-add-support

Nov 25, 2024

Conversation

Copy link

Contributor

mdaigle commentedOct 31, 2024•
edited
Loading

Enhancements:

Adds .NET 9 to target frameworks

Fixes:

Adds missing DynamicallyAccessedMembers attribute toGetProviderSpecificFieldTypeandGetFieldType 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 addressCA2022.
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 addressSYSLIB0057
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

ErikEJand others added9 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 commitc010bd3.

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

Copy link

Member

roji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.csShow resolvedHide resolved

Copy link

Contributor

David-Engel commentedNov 1, 2024

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

mdaigle added the Enhancement 💡Issues that are feature requests for the drivers we maintain. label

Nov 1, 2024

mdaigle added2 commits

November 1, 2024 11:33

Merge changes from Aris's branch

b2a0c4b

merge main

8324368

Copy link

Member

roji commentedNov 2, 2024

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

mdaigle added13 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 fromcheenamalhotra

November 21, 2024 23:38

Copy link

codecovbot commentedNov 22, 2024•
edited
Loading

Codecov Report

Attention: Patch coverage is35.71429% with9 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.

How did these get dropped?

fe363e2

edwardneal reviewed

Nov 22, 2024

View reviewed changes

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs OutdatedShow resolvedHide resolved

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

Copy link

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.

Copy link

Contributor

benrr101Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@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 OutdatedShow resolvedHide resolved

tools/props/Versions.props OutdatedShow resolvedHide resolved

MichelZ reviewed

Nov 22, 2024

View reviewed changes

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

MichelZ reviewed

Nov 22, 2024

View reviewed changes

src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj OutdatedShow resolvedHide resolved

MichelZ reviewed

Nov 22, 2024

View reviewed changes

src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj OutdatedShow resolvedHide resolved

MichelZ reviewed

Nov 22, 2024

View reviewed changes

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/VirtualSecureModeEnclaveProvider.cs OutdatedShow resolvedHide resolved

MichelZ reviewed

Nov 22, 2024

View reviewed changes

src/Microsoft.Data.SqlClient/tests/Directory.Build.props OutdatedShow resolvedHide resolved

MichelZ reviewed

Nov 22, 2024

View reviewed changes

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/DataStreamTest.cs OutdatedShow resolvedHide resolved

mdaigleand others added10 commits

November 22, 2024 10:07

Address review comments.

393b66e

Add System.ServiceProcess.ServiceController to manual tests in netfx.

43a709f

Update for package build

b2e5579

Reference System.Transactions

baea11f

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

a701b8a

…net9-add-support

Fix System.Transaction reference.

8936f45

Add pkcs to manual tests project to support package pipeline, lacking…

789a76a

… transitive dependency.

Add pkcs to functional tests project to support package pipeline, lac…

32df37c

…king transitive dependency.

Add Microsoft.Bcl.Cryptography

0a0f389

Merge branch 'main' into net9-add-support

d68d275

cheenamalhotra reviewed

Nov 23, 2024

View reviewed changes

tools/props/VersionsNet8OrLater.props OutdatedShow resolvedHide resolved

...sts/tools/Microsoft.Data.SqlClient.ExtUtilities/Microsoft.Data.SqlClient.ExtUtilities.csproj OutdatedShow resolvedHide resolved

tools/props/Versions.props OutdatedShow resolvedHide resolved

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

Copy link

Member

cheenamalhotraNov 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

		@@ -180,7 +180,9 @@ static DataTestUtility()
		AliasName = c.AliasName;
		IsJsonSupported = c.IsJsonSupported;

		#if NETFRAMEWORK

Copy link

Member

cheenamalhotraNov 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can you elaborate on this change?

Copy link

ContributorAuthor

mdaigleNov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.

Copy link

Contributor

benrr101Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

cheenamalhotra mentioned this pull request

Nov 23, 2024

Upgrade / align packages in test projects#2980

Closed

mdaigle added3 commits

November 24, 2024 21:13

Address review comments.

45d4497

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

d10cbad

…net9-add-support

Bump ExtUtilities to net9.

6ac682e

benrr101 approved these changes

Nov 25, 2024

View reviewed changes

Copy link

Contributor

benrr101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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.