[DRAFT] Azure Split - Use MDS Common Project #3873
Conversation
…meworks, and choose better TargetOs defaults. - Updated Azure project tests to use the MDS common project.
paulmedynski
changed the base branch from
feat/azure-split
to
dev/paul/azure-split/move-tests
There was a problem hiding this comment.
Pull request overview
This pull request implements infrastructure changes to enable the Azure split work by using a common MDS project and eliminating the previous MDS AssemblyName workaround. The primary mechanism introduces conditional assembly name suffixes (.NetCore and .NetFx) for project-reference builds to resolve MSBuild conflicts when building downstream projects.
Key changes:
- Introduces
ApplyMdsAssemblyNameSuffixproperty to conditionally append assembly name suffixes - Creates new test infrastructure with
UsernamePasswordProviderandManagedIdentityProviderfor AAD authentication tests - Refactors test organization by migrating tests from MDS to Azure Extensions package
- Updates build targets, NuGet specs, and pipeline configurations to support the new assembly naming scheme
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Directory.Build.props | Adds assembly name suffix logic for project-reference builds |
| tools/targets/GenerateMdsPackage.targets | Passes assembly name suffix parameters to NuGet pack |
| tools/targets/CopySniDllsForNetFxProjectReferenceBuilds.targets | New target file to copy SNI DLLs for .NET Framework project-reference builds |
| tools/specs/Microsoft.Data.SqlClient.nuspec | Updates file paths to use assembly name suffix variables |
| src/Microsoft.Data.SqlClient/*/Microsoft.Data.SqlClient.csproj | Applies assembly name suffix based on ApplyMdsAssemblyNameSuffix property |
| src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/UsernamePasswordProvider.cs | New provider for username/password authentication in tests |
| src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/ManagedIdentityProvider.cs | New provider for managed identity authentication in tests |
| src/Microsoft.Data.SqlClient.Extensions/Azure/test/* | New test files migrated from MDS ManualTests |
| src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderInternal.cs | Updates to handle dynamic assembly name resolution |
| eng/pipelines/**/*.yml | Pipeline configuration updates for new build structure |
| <ProjectReference | ||
| Condition="'$(TargetFramework)' == 'net462'" | ||
| Include="$(NetFxSource)src\Microsoft.Data.SqlClient.csproj" /> | ||
| <ProjectReference Include="../../../Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj" /> |
There was a problem hiding this comment.
Hooray! No more target-framework-specific project references.
| using System.Text; | ||
| using System.Threading.Tasks; | ||
|
|
||
| #if _WINDOWS |
There was a problem hiding this comment.
I found a few places where #if NETFRAMEWORK was being used as equivalent to#if _WINDOWS, which isn't correct.
| } | ||
|
|
||
| #if NETFRAMEWORK | ||
| #if NETFRAMEWORK && _WINDOWS |
There was a problem hiding this comment.
@benrr101 - Should this one be just #if _WINDOWS ?
|
|
||
| <!-- Target Frameworks =============================================== --> | ||
| <PropertyGroup> | ||
| <TargetFrameworks>net462;net8.0;net9.0</TargetFrameworks> |
There was a problem hiding this comment.
I have flipped things around here to achieve the goal of allowing projects that reference MDS to build for all supported target frameworks at once without any special build arguments, regardless of host OS.
Instead of using the host OS to choose which target frameworks to build, we're now using the target framework to choose a default target OS.
For example, on Linux the old approach would not have compiled for net462 by default, which would prevent downstream projects that reference this project from compiling successfully if they also target net462 (i.e. Azure project tests).
With the new approach, on Linux now downstream projects can target all of the supported frameworks, and they are all compiled by default.
As before, you can't execute the assemblies generated for a different OS than the host, but now you can compile them without any special flags.
| <EmbeddedResource Include="Resources/ILLink.Substitutions.xml" | ||
| Condition="'$(NormalizedTargetOs)' == 'windows_nt' AND '$(TargetFramework)' != 'net462'" /> | ||
|
|
||
| <!-- Base strings in English --> |
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <!-- |
There was a problem hiding this comment.
We don't need any of this foo now that the fancy MDS combined project exists. We're no longer referencing 2 different MDS projects that generate assemblies with the same name.
Description
This PR aims to have the Azure split work start using the MDS common project, and eliminate wonky MDS AssemblyName hack introduced earlier.