Fix SqlServer sibling reference handling by paulmedynski · Pull Request #4259 · dotnet/SqlClient

paulmedynski · 2026-05-04T18:27:52Z

Summary

Core fix

Align Microsoft.SqlServer.Server sibling references across affected projects to respect ReferenceType (Project vs Package)
Ensure package-mode version flow includes SqlServerPackageVersion wiring through build orchestration
Update test/sample project references for consistent restore/build behavior

Additional changes

Fix Microsoft.SqlServer.Server pack output path so artefacts land in the expected artifacts/ directory
Gate unsigned net462 UDT tests via new SqlServerStrongNameTestCondition helper to avoid FileLoadException (0x80131044) when the assembly is unsigned (project-ref or local-feed builds)
Fix package-mode local feed mirroring and eliminate duplicate Versions.props imports
Add missing SqlServerPackageVersion to doc/samples project
Add implicit usings to Microsoft.Data.SqlClient.TestCommon to simplify shared test helpers
Add BuildAll target to build.proj that builds all projects across all OS/TFM combinations; hook CodeQL to BuildAll so more code is compiled during analysis
Remove obsolete/spurious <Project> attributes (ToolsVersion, stray DefaultTargets) from props files
Reduce build console noise by suppressing duplicate repo-URL messages
Keep BUILDGUIDE.md aligned with current build/pack entrypoint guidance
Post-rebase conflict cleanup in Microsoft.Data.SqlClient.csproj
Tokenize Microsoft.SqlServer.Server dependency version in SqlClient nuspec (was hardcoded 1.0.0, now uses $SqlServerPackageVersion$ matching the Abstractions/Logging pattern)
Import SqlServer Versions.props during pack for dynamic version resolution; add validation and token expansion in PrepareSqlClientPackNuspec
Pass $(PackageVersionSqlServerArgument) through PackSqlClient target in build.proj
Fix BuildTests to not forward ReferenceType to UnitTests (project-only)
Quote all project path properties in BuildTests/BuildSamples/BuildTools Exec commands for paths-with-spaces safety
Fix comment wording ("Set up the list of supported OSes")
Add "AKV Provider has no tests" note in BuildTests

Validation

dotnet build build.proj -p:Configuration=Debug — 0 warnings, 0 errors
dotnet build build.proj -t:Build -p:Configuration=Debug -p:ReferenceType=Package — 0 warnings, 0 errors
dotnet build build.proj -t:BuildAll -p:Configuration=Debug — 0 warnings, 0 errors
dotnet build build.proj -t:PackSqlClient -p:Configuration=Debug — SqlServer.Server dependency now 1.0.0-dev (dynamic)
dotnet build src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj -f net8.0 -v minimal
dotnet build src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj -f net8.0 -v minimal
dotnet build build.proj -t:BuildSqlClientUnix -p:ReferenceType=Package -v minimal
dotnet build src/Microsoft.Data.SqlClient.slnx -v minimal

Copilot

Pull request overview

This PR updates how the repo’s sibling Microsoft.SqlServer.Server dependency is referenced under ReferenceType (Project vs Package), so package-mode builds/restores can resolve consistent versions and projects that require internals can explicitly enforce project-mode.

Changes:

Switched several test/sample projects from unconditional Microsoft.SqlServer.Server package references to ProjectReference/PackageReference conditioned on $(ReferenceType).
Added a unit-test guard to explicitly reject ReferenceType=Package.
Updated build.proj/Directory.Packages.props to flow SqlServerPackageVersion in package mode and to pack Microsoft.SqlServer.Server into the local packages feed.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj	Adds `Microsoft.SqlServer.Server` project reference and rejects `ReferenceType=Package`.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Utf8String/Utf8String.csproj	Conditional project/package reference for `Microsoft.SqlServer.Server`.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Shapes/Shapes.csproj	Conditional project/package reference for `Microsoft.SqlServer.Server`.
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Circle/Circle.csproj	Conditional project/package reference for `Microsoft.SqlServer.Server` (also normalizes file header).
src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/UdtTest/UDTs/Address/Address.csproj	Conditional project/package reference for `Microsoft.SqlServer.Server` (also normalizes file header).
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj	Centralizes conditional `Microsoft.SqlServer.Server` reference for manual tests.
src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj	Removes unused `Microsoft.SqlServer.Server` package reference from ref build project.
src/Microsoft.Data.SqlClient/notsupported/Microsoft.Data.SqlClient.csproj	Uses project reference for `Microsoft.SqlServer.Server` in project mode and package ref in package mode.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj	Formatting-only tweak to conditional reference items.
doc/samples/Microsoft.Data.SqlClient.Samples.csproj	Makes `Microsoft.SqlServer.Server` a conditional project/package reference based on `ReferenceType`.
Directory.Packages.props	Moves `Microsoft.SqlServer.Server` version declaration under `ReferenceType=Package` and uses `$(SqlServerPackageVersion)`.
build.proj	Imports `Versions.props` to set default package versions in package mode; fixes SqlServer version forwarding; packs `Microsoft.SqlServer.Server` to `$(PackagesDir)`; adds package-mode pack dependencies to build targets.

paulmedynski · 2026-05-04T18:40:59Z

  -->
-  <Target Name="BuildSqlClientNotSupported">
+  <PropertyGroup>
+    <BuildSqlClientNotSupportedDependsOn Condition="'$(ReferenceType.ToLower())' == 'package'">PackAbstractions</BuildSqlClientNotSupportedDependsOn>


@benrr101 - Do we want these Build* and Pack* targets to depend on the Pack targets of the siblings this target depends on?

I think I would like this to work without any previous targets run, and use default package versions:

dotnet build -t:BuildSqlClient -p:ReferenceType=Package

The latest commits achieve the above. Package mode now automatically ensures that the NuGet packages of sibling dependencies are built and put into packages/.

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

benrr101

Blocking on:

Package/project reference going in their own item group, as per existing pattern
Revisiting the intention of including the version props file in build.proj and confusing the recalculation of versions.

benrr101 · 2026-05-07T18:45:19Z

    <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
    <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
-    <PackageReference Include="Microsoft.SqlServer.Server" />
+    <PackageReference Include="Microsoft.SqlServer.Server" Condition="'$(ReferenceType)' == 'Package'" />


Please follow the above pattern above - put both package and project references (like this) in one item group.

benrr101 · 2026-05-07T18:45:44Z

    <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
    <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
-    <PackageReference Include="Microsoft.SqlServer.Server" />
+    <PackageReference Include="Microsoft.SqlServer.Server" Condition="'$(ReferenceType)' == 'Package'" />


Remove this in favor of the package/project reference in a separate item group above

benrr101 · 2026-05-07T18:51:57Z

  <!-- Imports ========================================================= -->
  <Import Project="src/Directory.Build.props" />

+  <!-- Import each project's Versions.props to apply default version numbers in Package mode. -->


I don't think this will work as well as you think it will ... The parameters passed to build.proj are PackageVersion* while the parameters passed to the csproj (and props files) are *PackageVersion. This was a compromise between wanting neat and orderly parameters in build2.proj, and not obliterating the existing csproj parameters.

Yep, this wasn't the correct approach. Added imports to Directory.Packages.props to ensure defaults are computed early enough in the build process to be applied correctly for Package builds.

benrr101 · 2026-05-07T18:58:33Z

      Example:        1.0.1-dev2345
    -->
-    <PackageVersionAbstractions Condition="'$(PackageVersionAbstractions)' == ''" />
+    <PackageVersionAbstractions Condition="'$(PackageVersionAbstractions)' == ''">$(AbstractionsPackageVersion)</PackageVersionAbstractions>


This more or less confound the whole versioning process I laid out...
The intention was that each project would know what its versions should be, and any projects that depend on another project just references that project and gets the sibling version. Further, the intention is that package reference goes away, so the remaining scenario where a package version is needed goes away.

Doing things this way means .... we calculate the versions with only some of the parameters provided, then call dotnet with this version, then recalculate the version with the package version provided. I think it's confusing and I'm not sure what scenario its trying to solve.

Correct - see above reply.

benrr101 · 2026-05-07T18:59:52Z

        $(ReferenceTypeArgument)
        $(PackageVersionAbstractionsArgument)
        $(PackageVersionLoggingArgument)
+        $(PackageVersionSqlServerArgument)


This isn't being added as a parameter at the top.

This property is computed as expected now.

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

build.proj:459

The dotnet build invocation uses the unquoted $(SqlClientRefProjectPath). This breaks when the repo is checked out under a path containing spaces; other targets in this file quote project paths. Quote the project path consistently here (and keep the rest of the command unchanged).

      <DotnetCommand>
        "$(DotnetPath)dotnet" build $(SqlClientRefProjectPath)
        -p:Configuration=$(Configuration)
        $(SigningKeyPathArgument)

build.proj:489

The dotnet build invocation uses the unquoted $(SqlClientProjectPath). If the repo path contains spaces, this command line will fail to parse correctly. Quote $(SqlClientProjectPath) (matching the quoting used in other build/pack commands in this file).

This issue also appears on line 515 of the same file.

    <PropertyGroup>
      <DotnetCommand>
        "$(DotnetPath)dotnet" build $(SqlClientProjectPath)
        -p:Configuration=$(Configuration)
        -p:TargetOs=Unix
        $(SigningKeyPathArgument)

build.proj:520

The dotnet build invocation uses the unquoted $(SqlClientProjectPath). This can fail when the working directory path contains spaces; please quote the project path consistently with other targets (e.g., PackSqlClient and ancillary Build* targets).

    <PropertyGroup>
      <DotnetCommand>
        "$(DotnetPath)dotnet" build $(SqlClientProjectPath)
        -p:Configuration=$(Configuration)
        -p:TargetOs=Windows_NT
        $(SigningKeyPathArgument)

…ports

…mbinations to help catch obvious problems pre-commit. - Updated CodeQL to use the new BuildAll target, since it compiles more code than using the solution. - Removed some obsolete and spurious <Project> attributes. - Reduced build console noise about repo URLs.

- Extract BuildNumber/FileVersionBuildNumber into src/Versioning.props so that FileVersionBuildNumber is defined before any Versions.props runs, regardless of Package vs Project mode. - Import Versioning.props from Directory.Packages.props (Package mode) and Directory.Build.props (Project mode, with guard). - Remove unnecessary blank line in Directory.Packages.props. - Fix 'Setup of list' comment wording in build.proj. - Add 'AKV Provider has no tests' note in build.proj BuildTests target.

Directory.Build.props evaluates before Directory.Packages.props, so import Versioning.props unconditionally from Directory.Build.props only. Remove the import from Directory.Packages.props and the now-unnecessary guard flag.

Only one import site exists (Directory.Build.props evaluates before Directory.Packages.props), so a separate file is unnecessary.

- 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).

Protect against spaces in checkout paths by quoting all project path properties in Exec commands, matching the pattern used by other targets.

The Azure test jobs restore SqlClient as a NuGet package, which has a transitive dependency on Microsoft.SqlServer.Server. In Package mode the CI-versioned SqlServer package (e.g. 1.0.0-ci13309) only exists as a pipeline artifact, so we must download it to packages/ before restore. Add sqlServerArtifactsName parameter and DownloadPipelineArtifact step to test-azure-package-ci-job.yml, pass it through the stage template, and add build_sqlserver_package_stage to the Azure stage dependencies in ci-core.

Add sqlServerPackageVersion and sqlServerArtifactsName parameters to every pipeline layer that already forwarded Abstractions/Logging versions but was missing SqlServer: Pipeline build/pack path: - ci-project-build-step.yml: forward to Build Driver and AKV Provider - ci-build-nugets-job.yml: add param, download artifact, forward to build steps and PackSqlClient - build-sqlclient-package-ci-stage.yml: add params, forward to job - dotnet-sqlclient-ci-core.yml: pass to MDS stage, add build_sqlserver_package_stage dependency Pipeline test path: - run-all-tests-step.yml: add param, pass -p:PackageVersionSqlServer to all 14 MSBuild test invocations - ci-run-tests-job.yml: add params, download SqlServer artifact, forward to run-all-tests-step - ci-run-tests-stage.yml: add params, forward to both job invocations - dotnet-sqlclient-ci-core.yml: pass to test stage, add build_sqlserver_package_stage dependency build.proj targets: - TestSqlClientFunctional, TestSqlClientManual, BuildAkvProvider, PackAkvProvider, TestAzure: add $(PackageVersionSqlServerArgument)

Copilot

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 2 comments.

+    /// which requires a strongly named <c>Microsoft.SqlServer.Server</c> assembly.  The .NET
+    /// runtime doesn't enforce this relationship, and the tests run without incident regardless of
+    /// the signed-ness of the SqlServer assembly.  However, .NET Framework _does_ enforce that the
+    /// SqlServer assembly us signed, and the tests fail to compile and/or run.


paulmedynski · 2026-05-15T11:00:13Z

Split into two separate PRs: #4290 and #4291.

Copilot AI review requested due to automatic review settings May 4, 2026 18:27

github-project-automation Bot added this to SqlClient Board May 4, 2026

github-project-automation Bot moved this to To triage in SqlClient Board May 4, 2026

Copilot started reviewing on behalf of paulmedynski May 4, 2026 18:28 View session

paulmedynski added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label May 4, 2026

paulmedynski moved this from To triage to In progress in SqlClient Board May 4, 2026

paulmedynski added this to the 7.1.0-preview2 milestone May 4, 2026

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj Outdated

Comment thread build.proj Outdated

paulmedynski commented May 4, 2026

View reviewed changes

paulmedynski mentioned this pull request May 4, 2026

Migrate SqlClient packaging to dotnet pack and centralize pack fallback #4257

Merged

paulmedynski force-pushed the dev/paul/dotnet-pack-sqlclient branch from 2851620 to 775fa11 Compare May 5, 2026 12:54

Copilot AI review requested due to automatic review settings May 5, 2026 13:14

paulmedynski force-pushed the dev/paul/reference-type branch from f4fcba7 to d0492fe Compare May 5, 2026 13:14

Copilot started reviewing on behalf of paulmedynski May 5, 2026 13:15 View session

paulmedynski changed the title ~~Fix SqlServer sibling reference handling~~ Workstream 1.5: SqlClient dotnet pack migration with sibling reference alignment May 5, 2026

paulmedynski changed the title ~~Workstream 1.5: SqlClient dotnet pack migration with sibling reference alignment~~ Sibling prokect reference alignment May 5, 2026

paulmedynski changed the title ~~Sibling prokect reference alignment~~ Sibling project reference alignment May 5, 2026

paulmedynski changed the title ~~Sibling project reference alignment~~ Fix SqlServer sibling reference handling May 5, 2026

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj Outdated

Comment thread BUILDGUIDE.md Outdated

Comment thread Directory.Packages.props Outdated

benrr101 requested changes May 7, 2026

View reviewed changes

paulmedynski force-pushed the dev/paul/reference-type branch from d0492fe to 88d1198 Compare May 8, 2026 13:32

Base automatically changed from dev/paul/dotnet-pack-sqlclient to main May 8, 2026 19:10

Copilot AI review requested due to automatic review settings May 8, 2026 20:03

paulmedynski force-pushed the dev/paul/reference-type branch from 88d1198 to 8200659 Compare May 8, 2026 20:03

Copilot started reviewing on behalf of paulmedynski May 8, 2026 20:03 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj Outdated

Comment thread build.proj

Comment thread build.proj Outdated

Comment thread build.proj

Copilot AI review requested due to automatic review settings May 11, 2026 16:05

Copilot started reviewing on behalf of paulmedynski May 11, 2026 16:06 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj Outdated

Comment thread build.proj

Comment thread src/Microsoft.Data.SqlClient/notsupported/Microsoft.Data.SqlClient.csproj Outdated

Copilot AI review requested due to automatic review settings May 11, 2026 18:22

Copilot started reviewing on behalf of paulmedynski May 13, 2026 17:15 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/Common/SqlServerStrongNameTestCondition.cs

Copilot AI review requested due to automatic review settings May 13, 2026 18:52

Copilot started reviewing on behalf of paulmedynski May 13, 2026 18:53 View session

paulmedynski added 18 commits May 13, 2026 15:54

Fix SqlServer sibling reference handling

765dfc7

Address Copilot review comments in PR 4259

a0a26da

Resolve merge conflicts after rebase

f154c9b

Fix package-mode local feed mirroring and Versions.props duplicate im…

f9a54cd

…ports

Address remaining PR 4259 review feedback

6eb5b7a

Added missing SqlServer package version to Samples.

d8e02fc

Address remaining Copilot PR 4259 feedback

95e87f6

Fix SqlServer pack output path and gate unsigned net462 UDT tests

606a660

Added implicit usings to the Test.Common project.

45bc085

Fix MSB4011: remove duplicate Versioning.props import

de84654

Directory.Build.props evaluates before Directory.Packages.props, so import Versioning.props unconditionally from Directory.Build.props only. Remove the import from Directory.Packages.props and the now-unnecessary guard flag.

Fold Versioning.props back into Directory.Build.props

4503d8e

Only one import site exists (Directory.Build.props evaluates before Directory.Packages.props), so a separate file is unnecessary.

Quote project paths in BuildTests/BuildSamples/BuildTools

bccd9c5

Protect against spaces in checkout paths by quoting all project path properties in Exec commands, matching the pattern used by other targets.

Improved docs for the SqlServer assembly checker in the tests.

d9d6217

Copilot AI reviewed May 13, 2026

View reviewed changes

Plumb SqlServerPackageVersion to AKV pack and Azure test pipelines

2e1fc83

paulmedynski force-pushed the dev/paul/reference-type branch from 57dffbf to 2e1fc83 Compare May 14, 2026 11:03

This was referenced May 14, 2026

Add BuildAll target, CodeQL update, and build cleanup #4290

Open

Add SqlServer.Server as a sibling project reference #4291

Open

paulmedynski closed this May 15, 2026

github-project-automation Bot moved this from In progress to Done in SqlClient Board May 15, 2026

Conversation

paulmedynski commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Core fix

Additional changes

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulmedynski May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benrr101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

paulmedynski commented May 4, 2026 •

edited

Loading

paulmedynski May 4, 2026 •

edited

Loading