Merge | Official Builds From Common Project by benrr101 · Pull Request #4068 · dotnet/SqlClient

benrr101 · 2026-03-19T21:51:00Z

Description

This PR is the last functional PR for common project completion. It updates the official/non-official builds to use the targets from build2.proj, and effectively build the common project for official releases. I had plans for a larger overhaul of the "new" official pipeline, but I lost control of the direction, so I took a step back, pulled out the minimal changes to make this happen and am submitting this now.

Since the *csproj* steps/jobs are based on build.proj, I've created separate steps for MDS build.

🤖

Dropped "compound-" from steps files and fixed links. I thought I was onto something when I adopted that naming convention a while back, but ... no.

Issues

N/A

Testing

Currently running test build can he found:
~~https://sqlclientdrivers.visualstudio.com/ADO.Net/_build/results?buildId=144204&view=results~~
~~https://sqlclientdrivers.visualstudio.com/ADO.Net/_build/results?buildId=144572&view=results~~
~~https://sqlclientdrivers.visualstudio.com/ADO.Net/_build/results?buildId=144579&view=results~~
https://sqlclientdrivers.visualstudio.com/ADO.Net/_build/results?buildId=144586&view=results

…blish-symbols-step.

…hanges)

…ackage-job.yml

Copilot

Pull request overview

Updates OneBranch official/non-official pipeline templates to build Microsoft.Data.SqlClient (MDS) from build2.proj (common project) and refactors pipeline steps by removing the prior “compound-*” templates in favor of more granular build/sign/pack/analyze steps.

Changes:

Replace the old SqlClient official build job with a new build-signed-mds-package-job.yml that drives MDS build/pack via build2.proj.
Add new OneBranch step templates for Roslyn analyzers, MDS build/pack, ESRP signing, APIScan file extraction, and symbols publishing parameterization.
Remove deprecated “compound-*” step templates and the prior SqlClient job template.

Reviewed changes

Copilot reviewed 17 out of 20 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient.sln	Updates solution items to reference new OneBranch job/step templates.
eng/pipelines/onebranch/variables/common-variables.yml	Updates comments around template aliases.
eng/pipelines/onebranch/stages/build-stages.yml	Switches SqlClient build job to new MDS/build2-based job and wires new parameters.
eng/pipelines/onebranch/jobs/build-signed-mds-package-job.yml	New job orchestrating build2-based MDS build/analyze/sign/pack/publish-symbols.
eng/pipelines/onebranch/jobs/build-signed-csproj-package-job.yml	Migrates to new Roslyn analyzers/sign/pack/symbol templates.
eng/pipelines/onebranch/steps/build-mds-step.yml	New step template to run `build2.proj -t:BuildMds`.
eng/pipelines/onebranch/steps/pack-mds-step.yml	New step template to run `build2.proj -t:PackMds`.
eng/pipelines/onebranch/steps/extract-apiscan-files-mds-step.yml	New step template to copy MDS DLL/PDB outputs to APIScan folders.
eng/pipelines/onebranch/steps/roslyn-analyzers-mds-step.yml	New step template to run Roslyn analyzers for build2-based MDS build.
eng/pipelines/onebranch/steps/roslyn-analyzers-csproj-step.yml	New generic Roslyn analyzers step for build.proj-driven builds.
eng/pipelines/onebranch/steps/publish-symbols-step.yml	Refactors the symbols publish template parameter model and implementation.
eng/pipelines/onebranch/steps/esrp-dll-signing-step.yml	Updates ESRP DLL signing inputs/paths and parameter quoting.
eng/pipelines/onebranch/steps/esrp-nuget-signing-step.yml	New dedicated ESRP NuGet signing step template.
eng/pipelines/onebranch/steps/pack-csproj-step.yml	New dedicated pack step for csproj-based extension packages.
eng/pipelines/onebranch/steps/build-csproj-step.yml	Comment update to reference new pack step template name.
eng/pipelines/onebranch/steps/esrp-code-signing-step.yml	Removed legacy combined DLL/pkg signing template.
eng/pipelines/onebranch/steps/compound-publish-symbols-step.yml	Removed deprecated compound symbols template.
eng/pipelines/onebranch/steps/compound-nuget-pack-step.yml	Removed deprecated compound NuGet pack template.
eng/pipelines/onebranch/steps/build-all-configurations-signed-dlls-step.yml	Removed legacy MDS build-all-configurations step driven by build.proj.
eng/pipelines/onebranch/jobs/build-signed-sqlclient-package-job.yml	Removed prior SqlClient official build job template.

eng/pipelines/onebranch/steps/pack-mds-step.yml

eng/pipelines/onebranch/stages/build-stages.yml

eng/pipelines/onebranch/jobs/build-signed-mds-package-job.yml

eng/pipelines/onebranch/jobs/build-signed-csproj-package-job.yml

eng/pipelines/onebranch/variables/common-variables.yml

eng/pipelines/onebranch/steps/copy-apiscan-files-mds-step.yml

eng/pipelines/onebranch/jobs/build-signed-mds-package-job.yml

eng/pipelines/onebranch/steps/roslyn-analyzers-mds-step.yml

eng/pipelines/onebranch/steps/publish-symbols-step.yml

codecov · 2026-03-19T23:28:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.62%. Comparing base (07a9280) to head (4fa9bd6).

❗ There is a different number of reports uploaded between BASE (07a9280) and HEAD (4fa9bd6). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (07a9280) HEAD (4fa9bd6)

CI-SqlClient 1 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4068      +/-   ##
==========================================
- Coverage   75.15%   65.62%   -9.54%     
==========================================
  Files         280      275       -5     
  Lines       43830    65825   +21995     
==========================================
+ Hits        32940    43197   +10257     
- Misses      10890    22628   +11738

Flag	Coverage Δ
CI-SqlClient	`?`
PR-SqlClient-Project	`65.62% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

paulmedynski · 2026-03-20T12:03:01Z

eng/pipelines/onebranch/jobs/build-signed-csproj-package-job.yml

+              artifactName: '${{ parameters.packageFullName }}_symbols_$(System.TeamProject)_$(Build.Repository.Name)_$(Build.SourceBranchName)_${{ parameters.packageVersion }}_$(System.TimelineId)'
+              azureSubscription: 'Symbols publishing Workload Identity federation service-ADO.Net'
+              packageName: '${{ parameters.packageFullName }}'
+              publishProjectName: 'Microsoft.Data.SqlClient.SNI'


That doesn't seem like the correct project name, but it is what the old code was using. Are we sure about this?

Yes, I've asked about this in the past and it seems to be correct. I in fact argued about this for quite a while about how we should have different symbols publishing libraries per project, but I was told that this will never be the case.

It may make more sense then to leave it hard coded in the task. If not, we should at least add a comment explaining this. I could see myself easily changing this thinking it was set mistakenly.

eng/pipelines/onebranch/stages/build-stages.yml

paulmedynski · 2026-03-20T12:10:17Z

eng/pipelines/onebranch/steps/build-mds-step.yml

This seems like an unnecessary level of abstraction. This file is only used in one place so its steps could be inlined in the calling job.

I see other existing steps like this as well, so let's re-visit after all of the build2.proj work is done.

Oh, maybe. But the csproj version does the same thing, and I think I was following a pattern from earlier when I built the AKV pipeline that I modeled this one after.

eng/pipelines/onebranch/steps/copy-apiscan-files-mds-step.yml

paulmedynski · 2026-03-20T12:16:21Z

eng/pipelines/onebranch/steps/copy-apiscan-files-mds-step.yml

We're copying, not extracting. Maybe copy-files-for-apiscan ?

I saw it as "extract them from the complete build output" not like uncompress "extract". Copy works too.

mdaigle

Really just the one comment, otherwise looks good. I would also like to review the pipeline run when we get one that's green.

mdaigle · 2026-03-20T17:52:29Z

eng/pipelines/onebranch/jobs/build-signed-csproj-package-job.yml

+              artifactName: '${{ parameters.packageFullName }}_symbols_$(System.TeamProject)_$(Build.Repository.Name)_$(Build.SourceBranchName)_${{ parameters.packageVersion }}_$(System.TimelineId)'
+              azureSubscription: 'Symbols publishing Workload Identity federation service-ADO.Net'
+              packageName: '${{ parameters.packageFullName }}'
+              publishProjectName: 'Microsoft.Data.SqlClient.SNI'


It may make more sense then to leave it hard coded in the task. If not, we should at least add a comment explaining this. I could see myself easily changing this thinking it was set mistakenly.

mdaigle

Really just the one comment, otherwise looks good. I would also like to review the pipeline run when we get one that's green.

mdaigle

🤖

Code Review Summary

Thanks for moving the MDS build to build2.proj — the cleanup of the "compound-" prefix and the separation of MDS into its own job/step templates is a nice improvement.

I found 3 critical issues and 1 major issue that will likely cause the official pipeline to produce incorrectly signed or broken packages:

Critical

#	Issue	File
C1	Typo: `PackageVersionAbstraction` → `PackageVersionAbstractions` (missing trailing 's') — pack step will fail	`pack-mds-step.yml:27`
C2	ESRP NuGet signing scans wrong directory: Signing searches `$(PACK_OUTPUT)` (`output/`) but `PackMds` emits `.nupkg` to `$(BUILD_OUTPUT)/Microsoft.Data.SqlClient/Package-Release/` — packages ship unsigned	`build-signed-mds-package-job.yml:142`
C3	`PackMds` re-builds over signed DLLs: `DependsOnTargets="BuildMds"` re-triggers the build in a fresh MSBuild invocation, overwriting ESRP-signed DLLs. `-p:PackBuild=false` has no effect (not implemented in `build2.proj`)	`pack-mds-step.yml:25`

Major

#	Issue	File
M1	`$(PACKAGE_NAME)` is undefined: Symbols publish with empty product name	`build-signed-mds-package-job.yml:157`

Minor (not commented inline)

Missing @self on pack-mds-step.yml template reference (line 139 of the job file)
ob_outputDirectory: '$(BUILD_OUTPUT)' publishes the entire artifacts/ tree instead of just nupkg — compare with csproj jobs which use $(PACK_OUTPUT)

Question

Has the test build (pipeline 144204) successfully produced and validated a signed MDS package? The issues above should cause failures in the signing/validation stages.

mdaigle · 2026-03-20T18:10:18Z

eng/pipelines/onebranch/steps/pack-mds-step.yml

+      msbuildArguments: >-
+        -t:PackMds
+        -p:PackBuild=false
+        -p:PackageVersionAbstractions="${{ parameters.abstractionsPackageVersion }}"


Critical: Typo — PackageVersionAbstraction should be PackageVersionAbstractions

The MSBuild property in build2.proj is PackageVersionAbstractions (with trailing s). This typo means the PackMds target will see PackageVersionAbstractions as empty and will fail with:

Error: PackageVersionAbstractions must be set!

The build step (build-mds-step.yml) uses the correct spelling (PackageVersionAbstractions), so only the pack step is affected.

Fix: Change -p:PackageVersionAbstraction= to -p:PackageVersionAbstractions=

mdaigle · 2026-03-20T18:10:29Z

eng/pipelines/onebranch/jobs/build-signed-mds-package-job.yml

+          abstractionsPackageVersion: '${{ parameters.abstractionsPackageVersion }}'
+          loggingPackageVersion: '${{ parameters.loggingPackageVersion }}'
+          mdsPackageVersion: '${{ parameters.mdsPackageVersion }}'
+


Critical: ESRP NuGet signing will not find any packages

The esrp-nuget-signing-step.yml scans $(PACK_OUTPUT) (= $(REPO_ROOT)/output) for *.*nupkg. However, the PackMds target in build2.proj outputs packages to:

$(MdsArtifactRoot)/$(ReferenceType)-$(Configuration) = artifacts/Microsoft.Data.SqlClient/Package-Release/

This is under $(BUILD_OUTPUT) (artifacts/), not $(PACK_OUTPUT) (output/). The ESRP NuGet signing step will find zero packages and exit silently, resulting in unsigned NuGet packages being published as the pipeline artifact.

Fix options:

Add a step before NuGet signing to copy the .nupkg/.snupkg from artifacts/Microsoft.Data.SqlClient/Package-Release/ to $(PACK_OUTPUT), or

Modify the PackMds target to output directly to $(PACK_OUTPUT) by passing -p:MdsArtifactRoot=$(PACK_OUTPUT) or an equivalent override

mdaigle · 2026-03-20T18:10:41Z

eng/pipelines/onebranch/steps/pack-mds-step.yml

+      solution: '$(REPO_ROOT)/build2.proj'
+      configuration: 'Release'
+      msbuildArguments: >-
+        -t:PackMds


Critical: PackMds will re-trigger BuildMds, overwriting ESRP-signed DLLs

The pipeline flow is: Build → ESRP DLL Sign → Pack → ESRP NuGet Sign.

However, PackMds in build2.proj has DependsOnTargets="BuildMds". Since the build step and pack step are separate MSBuild@1 task invocations, MSBuild's "run each target once" guarantee does not carry across invocations. When PackMds runs, it triggers BuildMds again, which overwrites the ESRP-signed DLLs with freshly-compiled unsigned ones. The unsigned DLLs then get packaged.

The -p:PackBuild=false property is passed here but is never checked anywhere in build2.proj — it has no effect.

Fix options:

Add a condition to build2.proj so that PackMds skips its DependsOnTargets when PackBuild=false:
<Target Name="PackMdsOnly" Condition="'$(PackBuild)' == 'false'"> ... </Target> <Target Name="PackMds" DependsOnTargets="BuildMds" Condition="'$(PackBuild)' != 'false'"> ... </Target>
Or more simply, make DependsOnTargets conditional.

Create a separate PackMdsNoBuild target that only contains the packaging logic.

mdaigle · 2026-03-20T18:10:50Z

eng/pipelines/onebranch/jobs/build-signed-mds-package-job.yml

+          - template: /eng/pipelines/onebranch/steps/publish-symbols-step.yml@self
+            parameters:
+              artifactName: 'Microsoft.Data.SqlClient_symbols_$(System.TeamProject)_$(Build.Repository.Name)_$(Build.SourceBranchName)_${{ parameters.mdsPackageVersion }}_$(System.TimelineId)'
+              azureSubscription: '${{ parameters.symbolsAzureSubscription }}'


Major: $(PACKAGE_NAME) is undefined — symbols will publish with an empty product name

The packageName parameter receives $(PACKAGE_NAME), but this variable is not defined in the job's variables block, in common-variables.yml, or in onebranch-variables.yml. It will resolve to an empty string at runtime, causing SymbolsProduct to be blank in the PublishSymbols@2 task.

Fix: Replace $(PACKAGE_NAME) with a literal value:

packageName: 'Microsoft.Data.SqlClient'

Not sure how the PackBuild target got lost, but I brought it back

Copilot

Pull request overview

Copilot reviewed 20 out of 22 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

eng/pipelines/onebranch/steps/esrp-nuget-signing-step.yml:36

searchPath is a new required parameter with no default. Existing callers that haven’t been updated (e.g., the csproj package job) will fail template expansion. Consider defaulting searchPath to $(PACK_OUTPUT) to keep the template backward-compatible and reduce call-site boilerplate.

eng/pipelines/onebranch/jobs/build-signed-csproj-package-job.yml

eng/pipelines/onebranch/jobs/build-signed-mds-package-job.yml

eng/pipelines/onebranch/jobs/build-signed-csproj-package-job.yml

eng/pipelines/onebranch/variables/onebranch-variables.yml

eng/pipelines/onebranch/jobs/build-signed-mds-package-job.yml

eng/pipelines/onebranch/steps/copy-apiscan-files-mds-step.yml

Copilot

Pull request overview

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

eng/pipelines/onebranch/steps/copy-apiscan-files-mds-step.yml

benrr101 added 7 commits March 19, 2026 16:42

Drop compound-* from onebranch step files for my own sanity

73e22d9

Consolidate compound-publish-symbols and publish-symbols-step into pu…

aa0e5b9

…blish-symbols-step.

Rename code-analyze-step to roslyn-analyzers-csproj-step (for later c…

b7ca977

…hanges)

Add mds-specific job/steps

a045113

Swap in build-signed-mds-package-job.yml for build-signed-sqlclient-p…

aa62816

…ackage-job.yml

Remove dead job/steps

69be1f3

Fix folder path for DLL signing steps.

4fa9bd6

benrr101 added this to the 7.0.1 milestone Mar 19, 2026

benrr101 requested a review from a team as a code owner March 19, 2026 21:51

benrr101 added the Common Project 🚮 Things that relate to the common project project label Mar 19, 2026

Copilot AI review requested due to automatic review settings March 19, 2026 21:51

github-project-automation bot added this to SqlClient Board Mar 19, 2026

github-project-automation bot moved this to To triage in SqlClient Board Mar 19, 2026

Copilot started reviewing on behalf of benrr101 March 19, 2026 21:51 View session

Copilot AI reviewed Mar 19, 2026

View reviewed changes

benrr101 commented Mar 19, 2026

View reviewed changes

eng/pipelines/onebranch/steps/publish-symbols-step.yml Show resolved Hide resolved

paulmedynski self-assigned this Mar 20, 2026

paulmedynski moved this from To triage to In review in SqlClient Board Mar 20, 2026

paulmedynski requested changes Mar 20, 2026

View reviewed changes

github-project-automation bot moved this from In review to In progress in SqlClient Board Mar 20, 2026

Addressing comments, round 1

4e00a55

mdaigle requested changes Mar 20, 2026

View reviewed changes

paulmedynski mentioned this pull request Mar 23, 2026

Add symbol verification to OneBranch pipelines #4073

Open

cheenamalhotra modified the milestones: 7.0.1, 7.1.0-preview1 Mar 23, 2026

Addressing comments, round 2

a87ee16

Not sure how the PackBuild target got lost, but I brought it back

Copilot AI review requested due to automatic review settings March 23, 2026 17:54

Copilot started reviewing on behalf of benrr101 March 23, 2026 17:54 View session

Add the new parameters to the csproj build job

e02d9c1

Copilot AI reviewed Mar 23, 2026

View reviewed changes

benrr101 added 2 commits March 23, 2026 13:30

Use variables artifact names in release stage

0f08f4e

Two more quick fixes as per comments

d1e4978

Copilot AI review requested due to automatic review settings March 23, 2026 18:33

Copilot started reviewing on behalf of benrr101 March 23, 2026 18:33 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

eng/pipelines/onebranch/steps/copy-apiscan-files-mds-step.yml Show resolved Hide resolved

Merging main

05355ed

Conversation

benrr101 commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

🤖

Issues

Testing

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 19, 2026

Codecov Report

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdaigle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdaigle left a comment

Choose a reason for hiding this comment

Uh oh!

mdaigle left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Code Review Summary

Critical

Major

Minor (not commented inline)

Question

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

benrr101 commented Mar 19, 2026 •

edited

Loading

mdaigle left a comment •

edited

Loading