Skip to content

[6.1] Dependency Cleanup #3843

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.

Sign up for GitHub

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

Open
paulmedynski wants to merge 5 commits into
base: release/6.1
Choose a base branch
from
+279 −158
Open

[6.1] Dependency Cleanup #3843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b1eb044
- Removed unused dependencies across all driver and test projects.
paulmedynski Dec 12, 2025
e942a0b
- Replaced Microsoft.SqlServer.Server project refs with package refs …
paulmedynski Dec 12, 2025
bfaef22
- Reverting the addition of Microsoft.SqlServer.Server to the MDS .NE…
paulmedynski Dec 12, 2025
b3dae93
- Removed Pkcs from .NET Framework.
paulmedynski Dec 12, 2025
1f7a52f
- Added several direct dependencies that were mistakenly removed earl…
paulmedynski Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@

<Target Name="BuildTools" Condition="'$(BuildTools)' == 'true'">
<PropertyGroup>
<DotnetBuildCmd>$(DotNetCmd) dotnet build -c Release -p:ReferenceType=$(ReferenceType)</DotnetBuildCmd>
<DotnetBuildCmd>$(DotnetPath)dotnet build -c Release -p:ReferenceType=$(ReferenceType)</DotnetBuildCmd>
</PropertyGroup>
<Exec Command="$(DotnetBuildCmd)" WorkingDirectory="$(GenAPISrcDir)Microsoft.DotNet.GenAPI\" />
</Target>
Expand Down Expand Up @@ -328,7 +328,7 @@
<TestCommand>$(TestCommand.Replace($([System.Environment]::NewLine), " "))</TestCommand>
</PropertyGroup>
<Message Text=">>> Running Manual test for Windows via command: $(TestCommand)" />
<Exec ConsoleToMsBuild="true" Command="$(TestCommand)" />

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient (Win22_Azure_ARM64_Sql net8_0_AnyCPU_NativeSNI_3)

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetcoreVersion=net8.0         -p:ReferenceType=Project         -p:TestSet=3         -p:TestTargetOS=Windowsnetcoreapp         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetcoreapptests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetcoreapp-3"       " exited with code 1.

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient-Package (Win22_Azure_Sql net462_AnyCPU_3)

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetfxVersion=net462         -p:ReferenceType=Package         -p:TestSet=3         -p:TestTargetOS=Windowsnetfx         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetfxtests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetfx-3"       " exited with code 1.

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient (win11_Azure_Sql net9_0_AnyCPU_NativeSNI_3)

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetcoreVersion=net9.0         -p:ReferenceType=Project         -p:TestSet=3         -p:TestTargetOS=Windowsnetcoreapp         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetcoreapptests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetcoreapp-3"       " exited with code 1.

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient (Win22_Azure_ARM64_Sql net8_0_AnyCPU_ManagedSNI_3)

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetcoreVersion=net8.0         -p:ReferenceType=Project         -p:TestSet=3         -p:TestTargetOS=Windowsnetcoreapp         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetcoreapptests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetcoreapp-3"       " exited with code 1.

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient-Package (win11_Azure_Sql net9_0_AnyCPU_NativeSNI_3)

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetcoreVersion=net9.0         -p:ReferenceType=Package         -p:TestSet=3         -p:TestTargetOS=Windowsnetcoreapp         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetcoreapptests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetcoreapp-3"       " exited with code 1.

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient-Package (win11_Azure_Sql net8_0_AnyCPU_NativeSNI_3)

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetcoreVersion=net8.0         -p:ReferenceType=Package         -p:TestSet=3         -p:TestTargetOS=Windowsnetcoreapp         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetcoreapptests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetcoreapp-3"       " exited with code 1.

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient-Package

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetfxVersion=net462         -p:ReferenceType=Package         -p:TestSet=3         -p:TestTargetOS=Windowsnetfx         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetfxtests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetfx-3"       " exited with code 1.

Check failure on line 331 in build.proj

View check run for this annotation

Azure Pipelines / CI-SqlClient (win11_Azure_Sql net9_0_AnyCPU_ManagedSNI_3)

build.proj#L331 

build.proj(331,5): Error MSB3073: The command "         dotnet test "src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj"         --no-build         -v n         -p:Configuration=Release         -p:TargetnetcoreVersion=net9.0         -p:ReferenceType=Project         -p:TestSet=3         -p:TestTargetOS=Windowsnetcoreapp         --collect "Code coverage"         --results-directory TestResults         --filter "category!=nonnetcoreapptests&category!=failing&category!=nonwindowstests"         --logger:"trx;LogFilePrefix=Manual-Windowsnetcoreapp-3"       " exited with code 1.
</Target>

<!-- Run all Manual tests applicable to Unix. -->
Expand Down
2 changes: 1 addition & 1 deletion eng/pipelines/common/templates/steps/build-and-run-tests-netfx-step.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ steps:
displayName: 'MSBuild Build Tests for ${{parameters.TargetNetFxVersion }}'
inputs:
solution: build.proj
msbuildArguments: ' -t:BuildTestsNetFx -p:ReferenceType=${{parameters.referenceType }} -p:TestMicrosoftDataSqlClientVersion=${{parameters.NugetPackageVersion }} -p:TargetNetFxVersion=${{parameters.TargetNetFxVersion }} -p:Configuration=${{parameters.configuration }} -p:Platform=${{parameters.platform }}'
msbuildArguments: ' -t:BuildTestsNetFx -p:ReferenceType=${{parameters.referenceType }} -p:TestMicrosoftDataSqlClientVersion=${{parameters.NugetPackageVersion }} -p:TF=${{parameters.TargetNetFxVersion }} -p:Configuration=${{parameters.configuration }} -p:Platform=${{parameters.platform }}'
Copy link
Contributor Author

@paulmedynski paulmedynski Dec 17, 2025

Choose a reason for hiding this comment

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

$(TF) is the correct way to specify the target framework for add-ons and tests when using build.proj.


# Don't run unit tests using package reference. Unit tests are only run using project reference.

Expand Down
145 changes: 79 additions & 66 deletions src/Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,69 +1,82 @@
<Project>
<PropertyGroup>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
</PropertyGroup>
<!-- NetFx project dependencies -->
<ItemGroup>
<PackageVersion Include="Microsoft.Data.SqlClient.SNI" Version="6.0.2" />
<PackageVersion Include="System.Buffers" Version="4.5.1" />
<PackageVersion Include="System.Memory" Version="4.5.5" />
<PackageVersion Include="System.Data.Common" Version="4.3.0" />
<PackageVersion Include="System.Text.Encodings.Web" Version="8.0.0" />
</ItemGroup>
<!-- NetFx and NetCore project dependencies -->
<ItemGroup>
<PackageVersion Include="Azure.Identity" Version="1.14.2" />
<PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.7.1" />
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="7.7.1" />
<PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
</ItemGroup>
<!-- NetCore project dependencies -->
<ItemGroup>
<PackageVersion Include="Microsoft.Data.SqlClient.SNI.runtime" Version="6.0.2" />
<PackageVersion Include="Microsoft.SqlServer.Server" Version="1.0.0" />
</ItemGroup>
<!-- AKV Provider project dependencies -->
<ItemGroup>
<PackageVersion Include="Azure.Core" Version="[1.47.1,2.0.0)" />
<PackageVersion Include="Azure.Security.KeyVault.Keys" Version="[4.7.0,5.0.0)" />
</ItemGroup>
<!-- Test Project Dependencies -->
<ItemGroup>
<PackageVersion Include="BenchmarkDotNet" Version="0.14.0" />
<PackageVersion Include="Microsoft.DotNet.PlatformAbstractions" Version="3.1.6" />
<PackageVersion Include="Microsoft.DotNet.RemoteExecutor" Version="10.0.0-beta.25164.6" />
<PackageVersion Include="Microsoft.DotNet.XUnitExtensions" Version="10.0.0-beta.25164.6" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
<PackageVersion Include="Microsoft.SqlServer.SqlManagementObjects" Version="172.76.0" />
<PackageVersion Include="Microsoft.SqlServer.Types" Version="160.1000.6" />
<PackageVersion Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.19.5" />
<PackageVersion Include="Microsoft.Win32.Registry" Version="5.0.0" />
<PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
<PackageVersion Include="System.Data.Odbc" Version="8.0.1" />
<PackageVersion Include="System.Security.Principal.Windows" Version="5.0.0" />
<PackageVersion Include="System.ServiceProcess.ServiceController" Version="8.0.1" />
<PackageVersion Include="System.Text.Encoding.CodePages" Version="6.0.0" />
<PackageVersion Include="xunit" Version="2.9.2" />
<PackageVersion Include="xunit.runner.visualstudio" Version="2.8.2" />
<PackageVersion Include="xunit.runner.console" Version="2.9.2" />
</ItemGroup>
<PropertyGroup>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
</PropertyGroup>

<!-- Target framework specific dependencies -->
<ItemGroup Condition="'$(TargetFramework)' == 'net9.0'">
<PackageVersion Include="Microsoft.Bcl.Cryptography" Version="9.0.5" />
<PackageVersion Include="Microsoft.Extensions.Caching.Memory" Version="9.0.5" />
<PackageVersion Include="Microsoft.Extensions.Hosting" Version="9.0.5" />
<PackageVersion Include="System.Configuration.ConfigurationManager" Version="9.0.5" />
<PackageVersion Include="System.Security.Cryptography.Pkcs" Version="9.0.5" />
<PackageVersion Include="System.Text.Json" Version="9.0.5" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' != 'net9.0'">
<PackageVersion Include="Microsoft.Bcl.Cryptography" Version="8.0.0" />
<PackageVersion Include="Microsoft.Extensions.Caching.Memory" Version="8.0.1" />
<PackageVersion Include="Microsoft.Extensions.Hosting" Version="8.0.1" />
<PackageVersion Include="System.Configuration.ConfigurationManager" Version="8.0.1" />
<PackageVersion Include="System.Security.Cryptography.Pkcs" Version="8.0.1" />
<PackageVersion Include="System.Text.Json" Version="8.0.5" />
</ItemGroup>
<!-- Driver .NET Framework Dependencies -->
Copy link
Contributor Author

@paulmedynski paulmedynski Dec 17, 2025

Choose a reason for hiding this comment

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

Same changes here as the test Directory.Packages.props file above. We now have some duplicate entries, but each target framework has a clear section of dependencies that closely matches the .nuspec files and our release notes.

<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<PackageVersion Include="Azure.Core" Version="1.50.0" />
<PackageVersion Include="Azure.Identity" Version="1.17.0" />
<PackageVersion Include="Azure.Security.KeyVault.Keys" Version="4.8.0" />
<PackageVersion Include="Microsoft.Data.SqlClient.SNI" Version="6.0.2" />
<PackageVersion Include="Microsoft.Extensions.Caching.Memory" Version="8.0.1" />
<PackageVersion Include="Microsoft.Identity.Client" Version="4.76.0" />
<PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.7.1" />
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="7.7.1" />
<PackageVersion Include="System.Buffers" Version="4.6.1" />
<PackageVersion Include="System.Data.Common" Version="4.3.0" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="8.0.1" />
<PackageVersion Include="System.IdentityModel.Tokens.Jwt" Version="7.7.1" />
<PackageVersion Include="System.Memory" Version="4.6.3" />
<PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
<PackageVersion Include="System.Security.Cryptography.Pkcs" Version="8.0.1" />
<PackageVersion Include="System.Text.Json" Version="8.0.6" />
<PackageVersion Include="System.Text.RegularExpressions" Version="4.3.1" />
</ItemGroup>

<!-- Driver .NET Standard 2.0 Dependencies -->
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<PackageVersion Include="Azure.Core" Version="1.50.0" />
<PackageVersion Include="Azure.Identity" Version="1.17.0" />
<PackageVersion Include="Azure.Security.KeyVault.Keys" Version="4.8.0" />
<PackageVersion Include="Microsoft.Data.SqlClient.SNI.runtime" Version="6.0.2" />
<PackageVersion Include="Microsoft.Extensions.Caching.Memory" Version="8.0.1" />
<PackageVersion Include="Microsoft.Identity.Client" Version="4.76.0" />
<PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.7.1" />
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="7.7.1" />
<PackageVersion Include="Microsoft.SqlServer.Server" Version="1.0.0" />
<PackageVersion Include="System.Configuration.ConfigurationManager" Version="8.0.1" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="8.0.1" />
<PackageVersion Include="System.IdentityModel.Tokens.Jwt" Version="7.7.1" />
<PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
<PackageVersion Include="System.Security.Cryptography.Pkcs" Version="8.0.1" />
<PackageVersion Include="System.Text.Json" Version="8.0.6" />
</ItemGroup>

<!-- Driver .NET 8.0 Dependencies -->
<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
<PackageVersion Include="Azure.Core" Version="1.50.0" />
<PackageVersion Include="Azure.Identity" Version="1.17.0" />
<PackageVersion Include="Azure.Security.KeyVault.Keys" Version="4.8.0" />
<PackageVersion Include="Microsoft.Data.SqlClient.SNI.runtime" Version="6.0.2" />
<PackageVersion Include="Microsoft.Extensions.Caching.Memory" Version="8.0.1" />
<PackageVersion Include="Microsoft.Identity.Client" Version="4.76.0" />
<PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.7.1" />
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="7.7.1" />
<PackageVersion Include="Microsoft.SqlServer.Server" Version="1.0.0" />
<PackageVersion Include="System.Configuration.ConfigurationManager" Version="8.0.1" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="6.0.1" />
Copy link
Contributor Author

@paulmedynski paulmedynski Dec 17, 2025

Choose a reason for hiding this comment

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

We previously weren't explicitly depending on System.Diagnostics.DiagnosticSource even though we use it directly. .NET Framework and .NET Standard 2.0 were pulling in v8.0.1, and .NET 8 and 9 were pulling v6.0.1. Hence the explicit difference here.

However This is now breaking dotnet CLI project-based restores for the add-on and test projects, since they employ a bunch of properties to choose target frameworks:

C:\Users\paulmedynski\dev\SqlClient\dev\paul\release\6.1\dependencies\src\Microsoft.Data.SqlClient\tests\ManualTests\Microsoft.Data.SqlClient.ManualTesting.Tests.csproj : error NU1109:
      Detected package downgrade: System.Diagnostics.DiagnosticSource from 8.0.1 to centrally defined 6.0.1. Update the centrally managed packag
      e version to a higher version.
       ManualTests -> Microsoft.Data.SqlClient -> System.Diagnostics.DiagnosticSource (>= 8.0.1)
       ManualTests -> System.Diagnostics.DiagnosticSource (>= 6.0.1)

The above shows the failure. The restore of the MDS project is incorrectly choosing v8.0.1 when it tries to restore the ManualTests project for .NET 8 or 9. It should be choosing v6.0.1, but the <ProjectReference> gets confused and is assuming a different target framework for MDS.

Builds via build.proj are fine since we always set a bunch of properties that force either .NET Framework or .NET - never both.

I'm not sure what we want to do about this. If I upgrade to v8.0.1 for all targets, the problem goes away. I tried to modify the add-ons and test projects to fix things, but nothing was working for me.

<PackageVersion Include="System.IdentityModel.Tokens.Jwt" Version="7.7.1" />
<PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
<PackageVersion Include="System.Security.Cryptography.Pkcs" Version="8.0.1" />
</ItemGroup>

<!-- Driver .NET 9.0 Dependencies -->
<ItemGroup Condition="'$(TargetFramework)' == 'net9.0'">
<PackageVersion Include="Azure.Core" Version="1.50.0" />
<PackageVersion Include="Azure.Identity" Version="1.17.0" />
<PackageVersion Include="Azure.Security.KeyVault.Keys" Version="4.8.0" />
<PackageVersion Include="Microsoft.Data.SqlClient.SNI.runtime" Version="6.0.2" />
<PackageVersion Include="Microsoft.Extensions.Caching.Memory" Version="9.0.11" />
<PackageVersion Include="Microsoft.Identity.Client" Version="4.76.0" />
<PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.7.1" />
<PackageVersion Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="7.7.1" />
<PackageVersion Include="Microsoft.SqlServer.Server" Version="1.0.0" />
<PackageVersion Include="System.Configuration.ConfigurationManager" Version="9.0.11" />
<PackageVersion Include="System.Diagnostics.DiagnosticSource" Version="6.0.1" />
<PackageVersion Include="System.IdentityModel.Tokens.Jwt" Version="7.7.1" />
<PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
<PackageVersion Include="System.Security.Cryptography.Pkcs" Version="9.0.11" />
</ItemGroup>
</Project>
1 change: 0 additions & 1 deletion ...ureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Azure.Core" />
<PackageReference Include="System.Text.Encodings.Web" />
<PackageReference Include="Azure.Security.KeyVault.Keys" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
</ItemGroup>
Expand Down
11 changes: 9 additions & 2 deletions src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,23 @@
<ItemGroup>
<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<PackageReference Include="Microsoft.Bcl.Cryptography" />
<PackageReference Include="Microsoft.Data.SqlClient.SNI.runtime" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.Identity.Client" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
<PackageReference Include="Microsoft.SqlServer.Server" />
<PackageReference Include="System.Configuration.ConfigurationManager" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" />
<PackageReference Include="System.IdentityModel.Tokens.Jwt" />
<PackageReference Include="System.Security.Cryptography.Pkcs" />
</ItemGroup>

<!-- Transitive dependencies that would otherwise bring in older, vulnerable versions. -->
<!--
Explicitly include some packages for .NET Standard 2.0 that are part of
the other .NET target base class libraries.
-->
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<PackageReference Include="System.Text.Json" />
</ItemGroup>

Expand Down
7 changes: 3 additions & 4 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,17 +1033,16 @@
<ItemGroup>
<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<PackageReference Include="Microsoft.Bcl.Cryptography" />
<PackageReference Include="Microsoft.Data.SqlClient.SNI.runtime" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.Identity.Client" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
<PackageReference Include="Microsoft.SqlServer.Server" />
<PackageReference Include="System.Configuration.ConfigurationManager" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" />
<PackageReference Include="System.IdentityModel.Tokens.Jwt" />
<PackageReference Include="System.Security.Cryptography.Pkcs" />

<!-- Transitive dependencies that would otherwise bring in older, vulnerable versions. -->
<PackageReference Include="System.Text.Json" />
</ItemGroup>

<Import Project="$(ToolsDir)targets\GenerateThisAssemblyCs.targets" />
Expand Down
24 changes: 21 additions & 3 deletions src/Microsoft.Data.SqlClient/netfx/ref/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,43 @@
<Reference Include="System.Data" />
<Reference Include="System.Xml" />
<Reference Include="System.Runtime.Caching" />
<Reference Include="System.EnterpriseServices" />
<Reference Include="System.Transactions" />
<Reference Include="System.EnterpriseServices" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<PackageReference Include="Microsoft.Bcl.Cryptography" />
<PackageReference Include="Microsoft.Data.SqlClient.SNI">
<PrivateAssets>All</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.Identity.Client" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
<PackageReference Include="System.Buffers" />
<!--
We must reference System.Data.Common explicitly because we use its
IDbColumnSchemaGenerator type that isn't included in .NET Framework
4.6.2 or 4.7. This avoids downstream apps from having to include it
themselves if they happen to target those older .NET Framework versions.
-->
<PackageReference Include="System.Data.Common" />
Copy link
Contributor Author

@paulmedynski paulmedynski Dec 12, 2025

Choose a reason for hiding this comment

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

Not used directly, likely an old transient vulnerability fix.

Copy link
Member

@cheenamalhotra cheenamalhotra Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It was added with PR: https://github.com/dotnet/SqlClient/pull/2967/files#diff-d6cdac7c9f92790e995dce24e966c09119385fdc076757b8f70b08bd14dde23fR47 since in .NET Framework v4.6.2 the type referenced by us from this namespace (IDbColumnSchemaGenerator) is not available directly in BCL and downloading this package with NuGet is required for applications to work. I would not recommend removing this dependency due to that reason.

Copy link
Contributor Author

@paulmedynski paulmedynski Dec 16, 2025

Choose a reason for hiding this comment

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

Is this a case where testing using .NET Framework Runtime 4.6.2 would have produced an error? But testing with .NET Framework Runtime 4.8.1 (which is what we do) doesn't?

<PackageReference Include="System.Diagnostics.DiagnosticSource" />
<PackageReference Include="System.IdentityModel.Tokens.Jwt" />
<PackageReference Include="System.Memory" />
<!--
Although System.Security.Cryptography.Pkcs is included in all .NET
Framework versions we target, the latest implementations of the types we
use are found in the NuGet package, so we reference it explicitly.
-->
<PackageReference Include="System.Security.Cryptography.Pkcs" />
Copy link
Member

@cheenamalhotra cheenamalhotra Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This package is referenced in

SqlClient/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/VirtualSecureModeEnclaveProviderBase.cs

Line 9 in deb8c31

using System.Security.Cryptography.Pkcs;

I would recommend we keep this explicit reference included.

Copy link
Contributor Author

@paulmedynski paulmedynski Dec 16, 2025

Choose a reason for hiding this comment

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

We only use the SignedCms class from this namespace, and Malcolm had pointed out that it is included with all of the .NET Framework runtimes that we target:

https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.pkcs.signedcms?view=netframework-4.8.1

I'm not sure why we would need to explicitly depend on it for .NET Framework.

Copy link
Member

@cheenamalhotra cheenamalhotra Dec 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Whether we use only 1 type or more shouldn't discount the importance of package reference IMO.
  2. The documentation for 'SingleCms' says the library and type is 'Package-provided' so isn't it available with the package only for those runtimes?

We need to consider runtimes that our customers would target, and not those that we target to build the driver. Otherwise our DLLs will reference type from runtime (expecting them to be available in client env), whereas in future .NET Framework runtimes if its not available and is package provided, it would throw exception failing to resolve the type.

System.Data.Common, e.g. is a MUST have reference, cannot be skipped.

Copy link
Contributor Author

@paulmedynski paulmedynski Dec 17, 2025

Choose a reason for hiding this comment

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

I've added the System.Security.Cryptography.Pkcs package back for all MDS targets. System.Data.Common has been added for .NET Framework 4.6.2.

Copy link
Contributor

@mdaigle mdaigle Dec 17, 2025

Choose a reason for hiding this comment

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

I don't think I fully understand this argument. Couldn't any type be moved out to a standalone package at any time? Why would we treat this type differently from any other that can be referenced via package?

The SignedCms type is available via runtime or package for .NET Framework. It's only package exclusive for .NET Standard and .NET. Presumably they added package support for .NET Framework to help people who were multitargeting. But in general, doesn't it complicate our dependency graph to include packages we don't need? Especially if we're not pruning any dependencies. I'd have less of an issue if we did enable pruning.

Copy link
Contributor

@mdaigle mdaigle Dec 18, 2025

Choose a reason for hiding this comment

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

After some investigation, I've learned that the .NET Framework DLLs for Pkcs are metadata only and type-forward to the GAC. We take a negligible package size hit to get the metadata. With that in mind, it does feel simpler to unconditionally include the package reference.

For this reason and the reasons you mentioned above, @cheenamalhotra, I think our standard should be to always unconditionally include package references for namespaces that are available via package for .NET Framework.

<PackageReference Include="System.Text.Encodings.Web" />
<PackageReference Include="System.Text.Json" />
<!--
We must reference System.Text.RegularExpressions to avoid a transitive
vulnerability from System.Data.Common 4.3.0.
-->
<PackageReference Include="System.Text.RegularExpressions" />
</ItemGroup>
<Import Project="$(ToolsDir)targets\TrimDocsForIntelliSense.targets" />
</Project>
45 changes: 32 additions & 13 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,6 @@
<PropertyGroup>
<DefineConstants>$(DefineConstants);NETFRAMEWORK;</DefineConstants>
</PropertyGroup>
<ItemGroup>
<Reference Include="System" />
<Reference Include="System.Configuration" />
<Reference Include="System.Runtime.Serialization" />
<Reference Include="System.Security" />
<Reference Include="System.Data" />
<Reference Include="System.Xml" />
<Reference Include="System.Runtime.Caching" />
<Reference Include="System.Transactions" />
<Reference Include="System.EnterpriseServices" />
</ItemGroup>
<!-- Contains common items shared between NetFx and NetCore -->
<ItemGroup>
<Compile Include="$(CommonSourceRoot)Interop\Common\Sni\SniErrors.cs">
Expand Down Expand Up @@ -954,21 +943,51 @@
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</EmbeddedResource>
</ItemGroup>
<ItemGroup>
<Reference Include="System" />
<Reference Include="System.Configuration" />
<Reference Include="System.Runtime.Serialization" />
<Reference Include="System.Security" />
<Reference Include="System.Data" />
<Reference Include="System.Xml" />
<Reference Include="System.Runtime.Caching" />
<Reference Include="System.Transactions" />
<Reference Include="System.EnterpriseServices" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<PackageReference Include="Microsoft.Bcl.Cryptography" />
<PackageReference Include="Microsoft.Data.SqlClient.SNI">
<PrivateAssets>All</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.Identity.Client" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
<PackageReference Include="System.Buffers" />
<!--
We must reference System.Data.Common explicitly because we use its
IDbColumnSchemaGenerator type that isn't included in .NET Framework
4.6.2 or 4.7. This avoids downstream apps from having to include it
themselves if they happen to target those older .NET Framework versions.
-->
<PackageReference Include="System.Data.Common" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" />
<PackageReference Include="System.IdentityModel.Tokens.Jwt" />
<PackageReference Include="System.Memory" />
<!--
Although System.Security.Cryptography.Pkcs is included in all .NET
Framework versions we target, the latest implementations of the types we
use are found in the NuGet package, so we reference it explicitly.
-->
<PackageReference Include="System.Security.Cryptography.Pkcs" />
<PackageReference Include="System.Text.Encodings.Web" />
Copy link
Member

@cheenamalhotra cheenamalhotra Dec 16, 2025

Choose a reason for hiding this comment

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

Removing System.Text.Encodings.Web seems fine, however we should be including "System.IdentityModel.Tokens.Jwt" NuGet package as we have a direct usage of it here:

SqlClient/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/AzureAttestationBasedEnclaveProvider.cs

Line 8 in deb8c31

using System.IdentityModel.Tokens.Jwt;

Copy link
Contributor Author

@paulmedynski paulmedynski Dec 17, 2025

Choose a reason for hiding this comment

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

I have added System.IdentityModel.Tokens.Jwt for all MDS targets.

<PackageReference Include="System.Text.Json" />
<!--
We must reference System.Text.RegularExpressions to avoid a transitive
vulnerability from System.Data.Common 4.3.0.
-->
<PackageReference Include="System.Text.RegularExpressions" />
</ItemGroup>
<Import Project="$(CommonSourceRoot)tools\targets\GenerateResourceStringsSource.targets" />
<Import Project="$(NetFxSource)tools\targets\GenerateThisAssemblyCs.targets" />
Expand Down
Loading
Loading