Add SqlServer.Server as a sibling project reference#4291
Open
paulmedynski wants to merge 11 commits into
Open
Conversation
Wire Microsoft.SqlServer.Server as a project reference (ReferenceType=Project) or package reference (ReferenceType=Package) across all consuming projects: SqlClient src/ref/notsupported, Abstractions, Azure, AKV Provider, UDT test projects, samples, and doc projects. - Add Versions.props to each sibling project for package version management - Add PackageVersionSqlServer build argument to build.proj - Update Directory.Packages.props with SqlServer version properties - Add local feed mirroring for Package mode builds - Update versioning comment documentation in Directory.Build.props
- Add SqlServerStrongNameTestCondition to detect unsigned assemblies - Skip net462 UDT tests when SqlServer.Server is not strong-name signed - Fix SqlServer pack output path in build.proj
- Replace hardcoded SqlServer.Server 1.0.0 dependency in nuspec with $SqlServerPackageVersion$ token, matching Abstractions/Logging pattern. - Import SqlServer Versions.props in csproj for version resolution during pack. - Add SqlServerPackageVersion validation and token expansion in PrepareSqlClientPackNuspec target. - Pass PackageVersionSqlServer through build.proj PackSqlClient target. - Fix BuildTests to not pass ReferenceType to UnitTests (project-only).
Pass the locally-built SqlServer.Server package version through all build and test pipeline templates: - ci-build-nugets-job: accept and forward SqlServerPackageVersion - ci-run-tests-job/stage: download SqlServer artifacts, forward version - ci-project-build-step: pass version to dotnet build - run-all-tests-step: pass version to all MSBuild test invocations - dotnet-sqlclient-ci-core: wire version to all stages - build-sqlclient-package-ci-stage: forward to build job - build-azure-package-ci-stage: forward to build and test jobs - test-azure-package-ci-job: download SqlServer artifacts
Contributor
There was a problem hiding this comment.
Pull request overview
This PR wires Microsoft.SqlServer.Server as a first-class sibling dependency across the repo, supporting both ReferenceType=Project (project references) and ReferenceType=Package (package references) end-to-end, including build orchestration and CI template plumbing. It also adds test gating to avoid .NET Framework UDT failures when the SqlServer assembly is produced unsigned in local/PR builds.
Changes:
- Add
Microsoft.SqlServer.Serverproject/package references across SqlClient projects and UDT test projects; centralize version import behavior to avoid duplicateVersions.propsimports. - Tokenize
Microsoft.SqlServer.Serverdependency version inMicrosoft.Data.SqlClient.nuspecand ensure pack-time token expansion can resolveSqlServerPackageVersion. - Plumb
sqlServerPackageVersion/ artifact download through CI templates and add a sharedSqlServerStrongNameTestConditionfor UDT test gating.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.SqlServer.Server/Versions.props | Adds an import-guard property to prevent double-import when pulled in via central props. |
| src/Microsoft.SqlServer.Server/Microsoft.SqlServer.Server.csproj | Makes Versions.props import conditional on the new guard property. |
| src/Microsoft.Data.SqlClient/Versions.props | Adds an import-guard property for central version imports. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj | Switches SqlServer.Server to a project reference and adds a ReferenceType validation target. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Utf8String/Utf8String.csproj | Adds conditional ProjectReference/PackageReference for SqlServer.Server based on ReferenceType. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Shapes/Shapes.csproj | Adds conditional ProjectReference/PackageReference for SqlServer.Server based on ReferenceType. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Circle/Circle.csproj | Adds conditional ProjectReference/PackageReference for SqlServer.Server based on ReferenceType. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Address/Address.csproj | Adds conditional ProjectReference/PackageReference for SqlServer.Server based on ReferenceType. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/SqlServerTypesTest.cs | Consolidates UDT test gating into a single condition (incl. strong-name usability check). |
| src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj | Adds conditional SqlServer.Server reference and removes unconditional package reference. |
| src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlDataRecordTest.cs | Gates UDT-related theory under the strong-name usability condition. |
| src/Microsoft.Data.SqlClient/tests/Common/SqlServerStrongNameTestCondition.cs | Introduces shared condition to skip .NET Framework UDT tests when SqlServer.Server isn’t strong-name signed. |
| src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.nuspec | Replaces hardcoded SqlServer.Server dependency version with $SqlServerPackageVersion$ token. |
| src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj | Imports SqlServer Versions.props for pack token expansion and validates SqlServerPackageVersion is non-empty. |
| src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj | Adds conditional SqlServer.Server project/package reference for TFMs that need it; removes unconditional package references. |
| src/Microsoft.Data.SqlClient/notsupported/Microsoft.Data.SqlClient.csproj | Adds conditional SqlServer.Server reference and updates comments to reflect new dependency handling. |
| src/Microsoft.Data.SqlClient.Internal/Logging/src/Versions.props | Adds an import-guard property for central version imports. |
| src/Microsoft.Data.SqlClient.Internal/Logging/src/Logging.csproj | Makes Versions.props import conditional on the new guard property. |
| src/Microsoft.Data.SqlClient.Extensions/Azure/src/Versions.props | Adds an import-guard property for central version imports. |
| src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj | Makes Versions.props import conditional on the new guard property. |
| src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Versions.props | Adds an import-guard property for central version imports. |
| src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj | Makes Versions.props import conditional on the new guard property; minor whitespace cleanup. |
| src/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider/src/Versions.props | Adds an import-guard property for central version imports. |
| src/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider/src/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj | Makes Versions.props import conditional on the new guard property. |
| src/Directory.Build.props | Clarifies evaluation order rationale for versioning properties (Directory.Build.props vs Directory.Packages.props). |
| eng/pipelines/stages/build-sqlclient-package-ci-stage.yml | Threads SqlServer artifact name/version parameters into SqlClient build stage. |
| eng/pipelines/stages/build-azure-package-ci-stage.yml | Threads SqlServer artifact name/version parameters into Azure build stage. |
| eng/pipelines/jobs/test-azure-package-ci-job.yml | Downloads SqlServer package artifacts and passes PackageVersionSqlServer for package-mode test restores. |
| eng/pipelines/dotnet-sqlclient-ci-core.yml | Adds SqlServer stage dependencies and passes SqlServer artifact/version variables through core stage wiring. |
| eng/pipelines/common/templates/steps/run-all-tests-step.yml | Passes PackageVersionSqlServer into build.proj test invocations. |
| eng/pipelines/common/templates/steps/ci-project-build-step.yml | Threads PackageVersionSqlServer into build steps in package mode. |
| eng/pipelines/common/templates/stages/ci-run-tests-stage.yml | Threads SqlServer artifact/version parameters into test stage jobs. |
| eng/pipelines/common/templates/jobs/ci-run-tests-job.yml | Downloads SqlServer artifacts for package-mode test runs and threads version into test steps. |
| eng/pipelines/common/templates/jobs/ci-build-nugets-job.yml | Downloads SqlServer artifacts for package-mode builds and threads version into pack commands. |
| doc/samples/Microsoft.Data.SqlClient.Samples.csproj | Groups SqlClient-suite package references and ensures SqlServer.Server is explicitly referenced. |
| doc/Directory.Packages.props | Pins Microsoft.SqlServer.Server version for doc projects. |
| Directory.Packages.props | Imports sibling Versions.props in package mode and pins SqlServer.Server version via $(SqlServerPackageVersion). |
| build.proj | Adds PackageVersionSqlServer argument support, introduces package-mode dependency ordering, and mirrors packed artifacts into the local feed. |
paulmedynski
added
the
Area\Engineering
- Add ImplicitUsings=enable to TestCommon.csproj so System.Type resolves on net462 (fixes CS0246 in SqlServerStrongNameTestCondition.cs) - Fix typo: 'assembly us signed' -> 'assembly is signed'
paulmedynski
commented
May 14, 2026
| - name: akvPackageVersion | ||
| type: string | ||
|
|
||
| # The version of the SqlServer package to depend on when referenceType is 'Package'. |
Contributor
Author
There was a problem hiding this comment.
These YAML changes are temporary while @benrr101 and I setup the new PR and CI pipelines, but they will make it easier to use the existing YAML as a baseline.
|
|
||
| <!-- Versioning ====================================================== --> | ||
| <Import Project="Versions.props" /> | ||
| <Import Project="Versions.props" Condition="'$(AkvProviderVersionsImported)' != 'true'" /> |
Contributor
Author
There was a problem hiding this comment.
See the changes to Directory.Packages.props for an explanation of these conditional imports.
| <NuspecProperties>$(NuspecProperties);COMMITID=$(CommitId)</NuspecProperties> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- Check for empty sibling package versions. --> |
Contributor
Author
There was a problem hiding this comment.
@benrr101 - Do you think we need these checks? AI suggested them, but then why not for the other driver projects that depend on siblings?
| namespace Microsoft.Data.SqlClient.TestCommon; | ||
|
|
||
| /// <summary> | ||
| /// Provides a shared xUnit conditional check for tests that depend on the SqlServer assembly. |
Contributor
Author
There was a problem hiding this comment.
This is a side-effect of promoting the SqlServer project to a first-class citizen in the build system. Now that we have proper Project vs Package refs for it throughout, we end up sometimes wanting to run tests with an unsigned SqlServer assembly, which we never did before. A few of our tests require the unrelated Microsoft.SqlServer.Types package, which in turn depends on SqlServer, and on .NET Framework will require a signed SqlServer assembly.
We now avoid running these few tests when SqlServer isn't signed and we're using .NET Framework. These tests still run fine on all .NET runtimes. Our ADO.Net CI pipelines will sign everything, and these tests will run for .NET Framework there.
Alternatively, we could isolate these tests into their own project that always references a known published SqlServer package that is signed, and then they could run all the time. Thoughts?
| <Target Name="ValidateReferenceType" | ||
| BeforeTargets="Restore;PrepareForBuild" | ||
| Condition="'$(ReferenceType)' != 'Project'"> | ||
| <Error Text="Microsoft.Data.SqlClient.UnitTests.csproj only supports ReferenceType=Project." /> |
Contributor
Author
There was a problem hiding this comment.
Now ReferenceType=Package is an error rather than being silently ignored.
| --> | ||
| <Target Name="BuildSqlClientNotSupported"> | ||
| <PropertyGroup> | ||
| <BuildSqlClientNotSupportedDependsOn Condition="'$(ReferenceType.ToLower())' == 'package'">PackAbstractions;PackSqlServer</BuildSqlClientNotSupportedDependsOn> |
Contributor
Author
There was a problem hiding this comment.
In Package mode, we need the sibling packages we depend on to have been packed before we build.
| <SqlClientProjectPath>$(SqlClientSrcRoot)src/Microsoft.Data.SqlClient.csproj</SqlClientProjectPath> | ||
| <SqlClientRefProjectPath>$(SqlClientSrcRoot)ref/Microsoft.Data.SqlClient.csproj</SqlClientRefProjectPath> | ||
| <SqlClientNotSupportedProjectPath>$(SqlClientSrcRoot)notsupported/Microsoft.Data.SqlClient.csproj</SqlClientNotSupportedProjectPath> | ||
| <SqlClientPackageArtifactRoot>$(SqlClientArtifactRoot)$(ReferenceType)-$(Configuration)/</SqlClientPackageArtifactRoot> |
Contributor
Author
There was a problem hiding this comment.
@benrr101 - Do we want to apply a similar artifact directory structure to all of the other projects? Right now SqlClient is the only one like this.
Document the two-path import strategy (Directory.Packages.props for Package mode vs. csproj-level conditional imports for Project mode) and how guard properties prevent double-evaluation.
…ork builds
- Add [Trait("Category", "signed")] under #if NETFRAMEWORK to tests that
load Microsoft.SqlServer.Types (depends on strongly-named SqlServer.Server)
- Append category!=signed to TestFilters in build.proj when SigningKeyPath is
empty (unsigned build)
- Remove SqlServerStrongNameTestCondition runtime probe (no longer needed)
- Revert ConditionalFact wrappers to original form in SqlServerTypesTest
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
build.proj:455
- In Package mode, the dependency order here runs
PackAbstractionsbeforePackLogging, but Abstractions restores against the Logging package whenReferenceType=Package. This ordering can break builds unless Logging is already available on an external feed. Reorder these dependencies (and/or makePackAbstractionsdepend onPackLogging) so Logging is packed first.
<PropertyGroup>
<BuildSqlClientRefDependsOn Condition="'$(ReferenceType.ToLower())' == 'package'">PackAbstractions;PackSqlServer</BuildSqlClientRefDependsOn>
</PropertyGroup>
<Target Name="BuildSqlClientRef" DependsOnTargets="$(BuildSqlClientRefDependsOn)">
build.proj:484
- In Package mode,
BuildSqlClientUnixDependsOnrunsPackAbstractionsbeforePackLogging, but Abstractions restores against the Logging package whenReferenceType=Package. This ordering can cause restore/pack failures when using locally-produced versions. Pack Logging before Abstractions (or add an explicit PackAbstractions->PackLogging dependency).
<PropertyGroup>
<BuildSqlClientUnixDependsOn Condition="'$(ReferenceType.ToLower())' == 'package'">PackAbstractions;PackLogging;PackSqlServer</BuildSqlClientUnixDependsOn>
</PropertyGroup>
<Target Name="BuildSqlClientUnix" DependsOnTargets="$(BuildSqlClientUnixDependsOn)">
build.proj:516
- In Package mode,
BuildSqlClientWindowsDependsOnrunsPackAbstractionsbeforePackLogging, but Abstractions restores against the Logging package whenReferenceType=Package. This ordering can cause restore/pack failures when using locally-produced versions. Pack Logging before Abstractions (or add an explicit PackAbstractions->PackLogging dependency).
<PropertyGroup>
<BuildSqlClientWindowsDependsOn Condition="'$(ReferenceType.ToLower())' == 'package'">PackAbstractions;PackLogging;PackSqlServer</BuildSqlClientWindowsDependsOn>
</PropertyGroup>
<Target Name="BuildSqlClientWindows" DependsOnTargets="$(BuildSqlClientWindowsDependsOn)">
<PropertyGroup>
paulmedynski
commented
May 14, 2026
|
|
||
| [Theory] | ||
| #if NETFRAMEWORK | ||
| [Trait("Category", "signed")] // Requires strong-name signed Microsoft.SqlServer.Server |
Contributor
Author
There was a problem hiding this comment.
These tests fail on .NET Framework when we didn't sign the SqlServer assembly. I added a test category so we can filter these out in those situations. The new sqlclient-test pipeline will sign assemblies when it runs in the ADO.Net org, which will enable these tests.
paulmedynski
force-pushed
the
dev/paul/sql-server-project-refs
branch
from
f55c0d4 to
7306a00
Compare
Version bumps (next preview): - SqlServer: 1.0.0 -> 1.1.0-preview1 - Abstractions: 1.0.0 -> 1.1.0-preview1 - Logging: 1.0.0 -> 1.1.0-preview1 - Azure: 1.0.0 -> 1.1.0-preview1 - AKV Provider: 7.0.0 -> 7.1.0-preview1 This fixes NU1605 (package downgrade) errors in Package-mode CI builds where Microsoft.SqlServer.Types >= 1.0.0 conflicts with the CI-produced prerelease 1.0.0-ci* (prerelease sorts below stable in SemVer). build.proj fixes: - Reorder Package-mode DependsOn so PackLogging runs before PackAbstractions (Abstractions has a PackageReference on Logging) - Fix 'Encrpyted' typos in AKV section comments and messages - Fix AkvProviderProjectpath casing
paulmedynski
commented
May 14, 2026
| @TODO: Determine how versions should be updated in release branches vs main | ||
| --> | ||
| <AkvProviderVersionDefault>7.0.0</AkvProviderVersionDefault> | ||
| <AkvProviderVersionDefault>7.1.0-preview1</AkvProviderVersionDefault> |
Contributor
Author
There was a problem hiding this comment.
I bumped the default package versions to their correct "next" value. This was necessary for SqlServer to avoid transitive downgrades via Mircosoft.SqlServer.Types now that we reference SqlServer properly. I decided we should do it for all of our packages.
paulmedynski
commented
May 15, 2026
| # Abstractions library assembly file version | ||
| - name: abstractionsAssemblyFileVersion | ||
| value: 1.0.0.$(assemblyBuildNumber) | ||
| value: 1.1.0.$(assemblyBuildNumber) |
Contributor
Author
There was a problem hiding this comment.
Forgot that CI pipelines still use these version values, so I made them match the defaults in the Versions.props files.
| **/notsupported/*.cs | ||
|
|
||
| # C# language server cache | ||
| *.lscache |
Contributor
Author
There was a problem hiding this comment.
Cache files created by the latest C# Dev Kit apparently.
Change PackageVersionSqlServer to SqlServerPackageVersion in test-azure-package-ci-job.yml. This job runs dotnet build directly against Azure.Test.csproj (not build.proj), so the build.proj-level PackageVersionSqlServer property was silently ignored. The csproj consumes SqlServerPackageVersion via Directory.Packages.props and Versions.props.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 40 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
build.proj:839
- The local-feed mirroring logic copies from
$(AbstractionsArtifactRoot), but PackAbstractions doesn’t direct dotnet pack output to that folder (no PackageOutputPath/-o). This will likely result in zero files copied into packages/ in Package mode. Pass -p:PackageOutputPath=$ (AbstractionsArtifactRoot) (or copy from the actual pack output path, e.g., bin//).
<!-- In Package mode, mirror produced packages into local feed (packages/) for downstream restore. -->
<ItemGroup Condition="'$(ReferenceType.ToLower())' == 'package'">
<AbstractionsPackedArtifacts Include="$(AbstractionsArtifactRoot)*.nupkg" />
<AbstractionsPackedArtifacts Include="$(AbstractionsArtifactRoot)*.snupkg" />
</ItemGroup>
build.proj:941
- The Package-mode local-feed mirroring copies from
$(AzureArtifactRoot), but PackAzure doesn’t set PackageOutputPath/-o to that folder. Unless Azure.csproj configures its pack output elsewhere, this copy will miss the produced packages and packages/ won’t be populated. Consider setting -p:PackageOutputPath=$ (AzureArtifactRoot) for pack, or updating the copy source to the real pack output directory.
<!-- In Package mode, mirror produced packages into local feed (packages/) for downstream restore. -->
<ItemGroup Condition="'$(ReferenceType.ToLower())' == 'package'">
<AzurePackedArtifacts Include="$(AzureArtifactRoot)*.nupkg" />
<AzurePackedArtifacts Include="$(AzureArtifactRoot)*.snupkg" />
</ItemGroup>
build.proj:1034
- The local-feed mirroring for PackLogging copies from
$(LoggingArtifactRoot), but this target doesn’t pass PackageOutputPath/-o to dotnet pack. That means the produced .nupkg/.snupkg likely land under bin// and won’t be mirrored into packages/ as intended for Package mode downstream restores. Consider setting -p:PackageOutputPath=$ (LoggingArtifactRoot) or copying from the actual pack output folder.
<!-- In Package mode, mirror produced packages into local feed (packages/) for downstream restore. -->
<ItemGroup Condition="'$(ReferenceType.ToLower())' == 'package'">
<LoggingPackedArtifacts Include="$(LoggingArtifactRoot)*.nupkg" />
<LoggingPackedArtifacts Include="$(LoggingArtifactRoot)*.snupkg" />
</ItemGroup>
build.proj:1096
- The Package-mode mirroring in PackSqlServer copies from
$(SqlServerArtifactRoot), but the pack command doesn’t set PackageOutputPath/-o to that folder. As a result, the Copy may not find any .nupkg/.snupkg to mirror into packages/. Consider passing -p:PackageOutputPath=$ (SqlServerArtifactRoot) (or adjusting the copy source to the pack output directory).
<!-- In Package mode, mirror produced packages into local feed (packages/) for downstream restore. -->
<ItemGroup Condition="'$(ReferenceType.ToLower())' == 'package'">
<SqlServerPackedArtifacts Include="$(SqlServerArtifactRoot)*.nupkg" />
<SqlServerPackedArtifacts Include="$(SqlServerArtifactRoot)*.snupkg" />
</ItemGroup>
In Package mode, Azure.csproj depends on Microsoft.Data.SqlClient.Internal.Logging via PackageReference, pinned by LoggingPackageVersion in Directory.Packages.props. The Azure test job downloaded Logging artifacts but never passed the version property, so NuGet restore could pick a mismatched default version. Add loggingPackageVersion parameter to test-azure-package-ci-job.yml and thread it through build-azure-package-ci-stage.yml and dotnet-sqlclient-ci-core.yml so Package-mode restores pin the correct Logging version.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4291 +/- ##
==========================================
- Coverage 66.04% 65.91% -0.14%
==========================================
Files 275 278 +3
Lines 42976 66301 +23325
==========================================
+ Hits 28383 43701 +15318
- Misses 14593 22600 +8007
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:
|
This file contains hidden or 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
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.
Wire Microsoft.SqlServer.Server as a project reference (ReferenceType=Project) or package reference (ReferenceType=Package) across all consuming projects, with CI pipeline plumbing.
Changes
Core reference handling
Versions.propsto each sibling project for package version managementPackageVersionSqlServerbuild argument tobuild.projDirectory.Packages.propswith SqlServer version propertiesDefault versions
UDT test gating
SqlServerStrongNameTestConditionto detect unsigned assembliesNuspec tokenization
CI pipeline plumbing
SqlServerPackageVersionthrough all build and test pipeline templatesSplit from #4259.