-
Notifications
You must be signed in to change notification settings - Fork 317
[5.1] Dependency Cleanup #3838
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/5.1
Are you sure you want to change the base?
[5.1] Dependency Cleanup #3838
Conversation
- Updated some dependencies to avoid transitive vulnerabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes unused dependencies and updates some dependency versions across the Microsoft.Data.SqlClient driver and test projects to eliminate transitive vulnerabilities without introducing breaking changes.
Key Changes:
- Removed unused dependencies (
System.Text.Encodings.Web,System.Text.Json,System.Diagnostics.DiagnosticSource,System.Private.Uri,Microsoft.Win32.Registry) - Updated test and common dependency versions (e.g.,
Microsoft.NET.Test.Sdk,Newtonsoft.Json,System.Buffers,Microsoft.Extensions.Hosting) - Reorganized and improved comments in
Versions.propsfor better clarity
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/specs/add-ons/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.nuspec | Removed System.Text.Encodings.Web dependency from all target frameworks |
| tools/specs/Microsoft.Data.SqlClient.nuspec | Removed multiple unused dependencies across all target frameworks |
| tools/props/Versions.props | Updated dependency versions, removed obsolete version properties, and reorganized comments for clarity |
| src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj | Removed unused package references |
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj | Removed unused package references |
| src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj | Removed unused package references |
| src/Microsoft.Data.SqlClient/netfx/ref/Microsoft.Data.SqlClient.csproj | Removed unused package references |
| src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj | Removed unused package references |
| src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.csproj | Removed unused package references and conditional ItemGroup |
| src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj | Removed unused package reference |
paulmedynski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commentary for reviewers.
| <SystemTextEncodingsWebVersion>6.0.1</SystemTextEncodingsWebVersion> | ||
| <SystemTextJsonVersion>6.0.11</SystemTextJsonVersion> | ||
| <MicrosoftIdentityModelProtocolsOpenIdConnectVersion>6.35.0</MicrosoftIdentityModelProtocolsOpenIdConnectVersion> | ||
| <SystemBuffersVersion>4.6.1</SystemBuffersVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated from 4.5.1 -> 4.6.1
|
|
||
| <!-- MDS NetFx project dependencies --> | ||
| <PropertyGroup> | ||
| <MicrosoftDataSqlClientSniVersion>5.1.2</MicrosoftDataSqlClientSniVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from line 26 unchanged.
| <SystemBuffersVersion>4.5.1</SystemBuffersVersion> | ||
| <SystemTextEncodingsWebVersion>6.0.1</SystemTextEncodingsWebVersion> | ||
| <SystemTextJsonVersion>6.0.11</SystemTextJsonVersion> | ||
| <MicrosoftIdentityModelProtocolsOpenIdConnectVersion>6.35.0</MicrosoftIdentityModelProtocolsOpenIdConnectVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from line 32 unchanged.
|
|
||
| <!-- MDS NetStandard project dependencies --> | ||
| <PropertyGroup> | ||
| <MicrosoftWin32RegistryVersion>5.0.0</MicrosoftWin32RegistryVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from line 40 unchanged.
| <PropertyGroup> | ||
| <MicrosoftWin32RegistryVersion>5.0.0</MicrosoftWin32RegistryVersion> | ||
| <SystemRuntimeLoaderVersion>4.3.0</SystemRuntimeLoaderVersion> | ||
| <SystemSecurityCryptographyCngVersion>5.0.0</SystemSecurityCryptographyCngVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from line 47 unchanged.
| <MicrosoftSqlServerServerVersion>1.0.0</MicrosoftSqlServerServerVersion> | ||
| <SystemDiagnosticsDiagnosticSourceVersion>6.0.1</SystemDiagnosticsDiagnosticSourceVersion> | ||
| <SystemDiagnosticsPerformanceCounterVersion>6.0.1</SystemDiagnosticsPerformanceCounterVersion> | ||
| <SystemConfigurationConfigurationManagerVersion>6.0.1</SystemConfigurationConfigurationManagerVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from line 42 unchanged.
|
|
||
| <!-- Common Dependencies - Shared by multiple driver or test projects--> | ||
| <PropertyGroup> | ||
| <AzureCoreVersion>1.41.0</AzureCoreVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved from line 58 unchanged.
…sions don't work with xUnit.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/5.1 #3838 +/- ##
===============================================
- Coverage 71.51% 70.23% -1.29%
===============================================
Files 293 293
Lines 61928 61931 +3
===============================================
- Hits 44289 43498 -791
- Misses 17639 18433 +794
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
mdaigle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like System.Text.Json and System.Text.Encodings.Web were included to override vulnerable versions that were coming in transitively. If we make that type of change in the future, I feel we should add a comment and make the reference TF specific so that it's not accidentally carried forward to other targets. That's most applicable for .NET Framework because most things we need are now included in .NET.
- Removed unnecessary Asn1 package dependency.
- Fixed incorrectly cased filenames. - Added CodeQL GitHub workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 1 comment.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I brought this over from the newer branches to satisfy our GitHub ruleset config.
| </Compile> | ||
| <Compile Include="..\..\src\Microsoft\Data\SqlClient\SqlMetadataFactory.cs"> | ||
| <Link>Microsoft\Data\SqlClient\SqlMetadataFactory.cs</Link> | ||
| <Compile Include="..\..\src\Microsoft\Data\SqlClient\SqlMetaDataFactory.cs"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few case-sensitivity changes so the CodeQL workflow can build and scan on Ubuntu.
| <Compile Include="$(IntermediateOutputPath)$(GeneratedSourceFileName)" /> | ||
| </ItemGroup> | ||
| <PropertyGroup> | ||
| <PowerShellExe>pwsh</PowerShellExe> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More changes for CodeQL on Ubuntu. The GitHub Actions runners have pwsh installed by default.
| @@ -0,0 +1,5 @@ | |||
| { | |||
| "sdk": { | |||
| "version": "8.0.416" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .NET 6 SDK is EOL and difficult to install locally for devs, so I chose the nearest supported one. Note that some parts of the CI pipeline are already using .NET 10 SDK to build, so this is actually closer to .NET 6 than some of our existing jobs.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 23 out of 26 changed files in this pull request and generated 3 comments.
| System.Data.IDataReader System.Data.IDataRecord.GetData(int i) { throw null; } | ||
| } | ||
| /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SQLDebugging.xml' path='docs/members[@name="SQLDebugging"]/SQLDebugging/*'/> | ||
| /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDebugging.xml' path='docs/members[@name="SqlDebugging"]/SqlDebugging/*'/> |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XPath reference uses 'SqlDebugging' but the XML documentation file defines the members with name='SQLDebugging' (all caps SQL). The path should be docs/members[@name="SQLDebugging"]/SQLDebugging/* to match the actual element name in the XML file.
| public sealed partial class SQLDebugging | ||
| { | ||
| /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SQLDebugging.xml' path='docs/members[@name="SQLDebugging"]/ctor/*'/> | ||
| /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDebugging.xml' path='docs/members[@name="SqlDebugging"]/ctor/*'/> |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XPath reference uses 'SqlDebugging' but the XML documentation file defines the members with name='SQLDebugging' (all caps SQL). The path should be docs/members[@name="SQLDebugging"]/ctor/* to match the actual element name in the XML file.
| // or the iid for the ISQLDebug interface | ||
| // | ||
| /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDebugging.xml' path='docs/members[@name="SQLDebugging"]/SQLDebugging/*'/> | ||
| /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDebugging.xml' path='docs/members[@name="SqlDebugging"]/SqlDebugging/*'/> |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XPath reference uses 'SqlDebugging' but the XML documentation file defines the members with name='SQLDebugging' (all caps SQL). The path should be docs/members[@name="SQLDebugging"]/SQLDebugging/* to match the actual element name in the XML file.
Description
NOTE: Some packages have DOWNGRADED major versions due to migrating from Direct dependencies to Transitive dependencies. This will have no effect on downstream apps, since the intermediate packages were already compatible with the previous Direct dependency versions. If apps were directly using those packages at-or-above the previous versions, NuGet will automatically resolve the transitive dependencies as it was doing before.
Details
MDS
AKV
Issues
Resolves #3809.
Testing