Remove unused PInvokes #2828
Open
Remove unused PInvokes #2828
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Changed the exception type thrown by SQL.SocketDidNotThrow from InternalException. * Excluded InternalException and NetEventSource from compilation in .NET 8.0 target.
JRahnama
added
the
➕ Code Health
JRahnama
reviewed
Sep 1, 2024
src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Interop/SNINativeMethodWrapper.cs
Outdated
Show resolved
Hide resolved
|
Please resolve conflicts to help us move forward with reviews. |
|
Thanks @cheenamalhotra - now resolved, and being CI'd. |
|
I'm working on dropping net6 support here #2927 |
Follows the merge of SqlFileStream.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2828 +/- ##
==========================================
+ Coverage 71.92% 72.30% +0.38%
==========================================
Files 294 288 -6
Lines 60342 59723 -619
==========================================
- Hits 43398 43182 -216
+ Misses 16944 16541 -403
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first step of a process to merge PInvokes between the .NET and .NET Framework projects. I've started by removing most of the unused PInvokes from both projects. There shouldn't be any surprises in CI.
I've also changed SqlUtil's
SocketDidNotThrowmethod to throw an Exception rather than an InternalException. This was the only reference to InternalException which for the .NET 8.0 target, so this change means that when .NET 6.0 support is dropped, everything in the netcore/src/Common/src/System folder except for NotImplemented.cs can be removed.The next step of the PInvoke merge process will be more involved. What's the coding standard for this? dotnet/runtime uses one folder per OS, one subfolder per DLL, one file per method, and there are quite a few files which fit this pattern. Other parts of the .NET project have a single file containing every PInvoke for a single DLL (the SNI DLL.)
Step 2 will be to merge the OS PInvokes which are needed for .NET Framework and .NET 8.0, and step 3 is currently to address the native SNI PInvokes. At the point of step 3, I'd also like to change the .NET Framework project to refer to Microsoft.Data.SqlClient.SNI.runtime, so it matches the .NET project. This might depend on #2671.