Conversation
* Drop buildConfiguration parameter * Introduce symbols parameters * Signing parameters with prefix
….yml to parameters.
Forward default values to parameters on sqlclient-non-official.yml and sqlclient-official.yml
Replace old libraries with new libraries Replace old library references with new library references
benrr101
added
the
Area\Engineering
There was a problem hiding this comment.
Pull request overview
Cleans up and modernizes the SqlClient OneBranch official/non-official pipeline definitions by centralizing version/artifact variables, threading parameters explicitly through templates, and reducing reliance on macro-expanded variables to improve compile-time validation and readability.
Changes:
- Centralize package versions/artifact names into a shared
package-variables.ymltemplate and thread them through build/release stages. - Refactor OneBranch build/release templates/jobs to use explicit parameters (signing, symbols, versions) and updated stage/job naming (SqlClient vs MDS).
- Simplify solution maintenance for
eng/pipelines/by adding a non-buildablePipelines.csprojthat glob-includes pipeline files.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient.slnx | Replaces explicit pipeline file entries with a single non-build project to surface eng/pipelines in the IDE. |
| eng/pipelines/Pipelines.csproj | Adds a “files-only” SDK project to include all eng/pipelines/** files in the solution without building. |
| eng/pipelines/onebranch/variables/sqlclient-validation-variables.yml | Removes legacy validation variable template (validation now parameterized within the job). |
| eng/pipelines/onebranch/variables/package-variables.yml | Introduces centralized package artifact/version/file-version variables (preview vs GA). |
| eng/pipelines/onebranch/variables/onebranch-variables.yml | Switches to modern variable groups for signing/symbols and keeps core OneBranch template variables. |
| eng/pipelines/onebranch/variables/common-variables.yml | Trims legacy variable groups/versions and adds well-known paths like PACK_INPUT/PACK_OUTPUT. |
| eng/pipelines/onebranch/steps/roslyn-analyzers-sqlclient-step.yml | Renames/retargets Roslyn analyzer step to SqlClient naming and parameters. |
| eng/pipelines/onebranch/steps/roslyn-analyzers-csproj-step.yml | Refactors Roslyn analyzer step to take a build target + arguments with clearer display names. |
| eng/pipelines/onebranch/steps/pack-sqlclient-step.yml | Renames/retargets pack step for SqlClient (Build2.proj Pack). |
| eng/pipelines/onebranch/steps/pack-csproj-step.yml | Simplifies pack step to always use Release configuration and modernizes quoting/args. |
| eng/pipelines/onebranch/steps/copy-apiscan-files-sqlclient-step.yml | Adds a SqlClient-specific APIScan file copy step for dll/pdb outputs. |
| eng/pipelines/onebranch/steps/build-sqlclient-step.yml | Renames/retargets Build2.proj build step to SqlClient parameter naming. |
| eng/pipelines/onebranch/steps/build-csproj-step.yml | Simplifies csproj build step to always use Release and updates output labeling. |
| eng/pipelines/onebranch/stages/release-stages.yml | Threads artifact names/versions as parameters and renames AKV parameter casing. |
| eng/pipelines/onebranch/stages/build-stages.yml | Threads signing/symbols/package parameters through all build stages; renames validation stage. |
| eng/pipelines/onebranch/sqlclient-official.yml | Updates official pipeline to consume new variable templates and parameterized stages/jobs. |
| eng/pipelines/onebranch/sqlclient-non-official.yml | Updates non-official pipeline similarly (including new version/signing/symbol groups). |
| eng/pipelines/onebranch/jobs/validate-signed-package-job.yml | Refactors package validation to be parameter-driven and adds additional checks/structure. |
| eng/pipelines/onebranch/jobs/publish-nuget-package-job.yml | Uses $(PACK_OUTPUT) for OneBranch artifact output directory. |
| eng/pipelines/onebranch/jobs/build-signed-sqlclient-package-job.yml | Renames job/template usage from MDS to SqlClient and wires new step/template names. |
| eng/pipelines/onebranch/jobs/build-signed-csproj-package-job.yml | Parameterizes signing/symbols inputs and standardizes build/pack behavior and artifact paths. |
Comments suppressed due to low confidence (1)
eng/pipelines/onebranch/stages/release-stages.yml:33
debugis declared as a parameter here but it is never referenced anywhere in this template. Either wire it into the release job/stage (e.g., to enable extra logging) or remove it to avoid a no-op queue-time option.
# ── General parameters ─────────────────────────────────────────────────
# True to enable debug information and steps.
- name: debug
type: boolean
| # $(Build.BuildNumber) has the format XXX.YY. Additionally, each version part must be a positive | ||
| # 16-bit integer less than 65535. Simply concatenating both parts of $(Build.BuildNumber) could | ||
| # produce values larger than 65534, so we must omit the second part entirely. Unfortunately, this | ||
| # may result in multiple subsequent pipline builds using the same C# assembly versions. The |
There was a problem hiding this comment.
Typo: "pipline" should be "pipeline".
| # may result in multiple subsequent pipline builds using the same C# assembly versions. The | |
| # may result in multiple subsequent pipeline builds using the same C# assembly versions. The |
| ################################################################################# | ||
|
|
||
| # This file is only included in MDS OneBranch Official pipelines. | ||
| # This file is only included in SqlClient OneBranch Official pipelines. |
There was a problem hiding this comment.
The file header says this template is only included in SqlClient OneBranch official pipelines, but it is also included by the non-official pipeline (sqlclient-non-official.yml). Please update the comment to avoid misleading future edits.
| # This file is only included in SqlClient OneBranch Official pipelines. | |
| # This file is included by SqlClient OneBranch pipelines (official and non-official). |
| # True if this is a preview build, which uses the preview version numbers from | ||
| # common-variables.yml. | ||
| - name: isPreview | ||
| type: boolean |
There was a problem hiding this comment.
The isPreview parameter comment still says preview versions come from common-variables.yml, but version selection now appears to be centralized in variables/package-variables.yml. Please update the comment so future maintainers look in the correct place.
There was a problem hiding this comment.
I'll do you one better - this parameter isn't used in build-stages at all. Let's just get rid of it.
| - name: artifactPath | ||
| value: '$(Pipeline.Workspace)\${{ parameters.artifactName }}' | ||
|
|
||
| - ${{ if parameters.isPreview }}: | ||
| - name: extractedNugetPath | ||
| value: $(extractedNugetRootPath).$(mdsPackagePreviewVersion) | ||
| - name: mdsPackageVersion | ||
| value: $(mdsPackagePreviewVersion) | ||
| # Path to the SqlClient NuGet package after installation. This path will only exist once the package | ||
| # been installed. | ||
| - name: nugetPackageInstallPath | ||
| value: '$(Pipeline.Workspace)\nugetPackageInstalls\Microsoft.Data.SqlClient\${{ parameters.expectedPackageVersion }}' | ||
|
|
||
| # Root folder where NuGet package will be installed locally | ||
| - name: nugetPackageInstallRoot | ||
| value: '$(Pipeline.Workspace)\nugetPackageInstalls' |
There was a problem hiding this comment.
nugetPackageInstallPath assumes the package will be installed under ...\Microsoft.Data.SqlClient\<version>, but Install-Package -Destination <root> typically creates a folder named Microsoft.Data.SqlClient.<version> directly under <root> (note the dot, not a nested directory). As written, all subsequent Get-ChildItem -Path $nugetPackageInstallPath ... calls are likely to point at a non-existent directory. Consider either deriving the install path from the actual installed folder (e.g., discover it after install) or aligning the expected path to the installer’s output; also consider pinning Install-Package to expectedPackageVersion to avoid accidentally installing a different version if multiple are present in the artifact folder.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #4123 +/- ##
==========================================
- Coverage 73.22% 66.44% -6.79%
==========================================
Files 280 274 -6
Lines 43000 65778 +22778
==========================================
+ Hits 31486 43703 +12217
- Misses 11514 22075 +10561
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:
|
paulmedynski
left a comment
paulmedynski
left a comment
There was a problem hiding this comment.
Excellent changes! We still need to land on a decision for YAML string quoting, but that can be a separate discussion and PR.
| # Verify strong name signing ##################################### | ||
| echo "> 1. Verifying strong name signing of DLLs ..." | ||
|
|
||
| # @TODO: This path seems brittle to VS upgrades, can we make it more flexible? |
There was a problem hiding this comment.
Yes - see my changes in #4073 that try to locate symchk.exe.
| - build_dependent | ||
| - mds_package_validation | ||
| - ${{ if parameters.releaseAKVProvider }}: | ||
| - sqlclient_package_validation |
There was a problem hiding this comment.
These changes will conflict with #4073. I will bring these changes into that PR once this one merges.
| recursive folder trees of non-compilable files, with the only solution to be manually adding | ||
| each file as a linked file in the solution. This is a terrible situation because it means each | ||
| time a file is added or removed from this folder, it must be manually updated in the solution. | ||
| It is also the case that most engineers forget to do this step, and it comes down to more |
There was a problem hiding this comment.
Some developers (me) have never found a use for the curated "solution" view, and instead just use the filesystem tree view. Maybe we could adopt that approach and avoid this hackery?
There was a problem hiding this comment.
Eh... I think that deviates from the norm too much. It's expected that dotnet projects have solution files. You're more than welcome to not use the solution, but we should provide a working solution for vs diehards.
Description
Since I'm taking the lead on pipeline ownership and have been tasked to be bold, I'm introducing a collection of cleanup work to keep our official/non-official pipelines following pipeline guidance.
Biggest changes:
🤖
Used to verify that pipeline changes were effective before submitting for commit. May have utilized in a few instances to do menial tasks.
Testing
Check the following builds to verify results:
https://sqlclientdrivers.visualstudio.com/ADO.Net/_build/results?buildId=145613&view=results