Skip to content

Consolidate and align SqlClient versioning - #4336

Open
paulmedynski wants to merge 6 commits into
mainfrom
dev/paul/canonical-versions
Open

Consolidate and align SqlClient versioning#4336
paulmedynski wants to merge 6 commits into
mainfrom
dev/paul/canonical-versions

Conversation

@paulmedynski

@paulmedynski paulmedynski commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Overall goal

Make package version specification and computation simple, centralized, and unambiguous:

  1. One source of truth per version family. Each version value (PackageVersion, FileVersion, AssemblyVersion) is specified/computed in exactly one place — once for the entire SqlClient family and once for Microsoft.SqlServer.Server.
  2. Treat the SqlClient family as a single shippable unit. Microsoft.Data.SqlClient, Internal.Logging, Extensions.Abstractions, Extensions.Azure, and the AlwaysEncrypted AzureKeyVaultProvider always share one version (package, file, and assembly) and release together. Microsoft.SqlServer.Server is versioned and released independently.
  3. Compute versions centrally in pipelines. Official (OneBranch), CI, and PR pipelines all extract versions up front from a single compute-versions stage and feed them downstream, instead of hardcoding or relying on undefined fallbacks.
  4. Remove version-naming ambiguity. Eliminate the confusing custom AssemblyFileVersion property and use only the unambiguous SDK terms FileVersion (Major.Minor.Patch.Revision) and AssemblyVersion (Major.0.0.0).

Why one PR with several large commits (not one PR per commit)

This is a single cross-cutting change to how versions flow through projects, build orchestration, and three pipeline families. The pieces share tight, bidirectional contracts, so splitting them into independently-mergeable PRs would mean writing throwaway scaffolding (temporary shims, dual code paths, and compatibility aliases) purely to keep main green between merges — strictly more total work that we would then have to unwind. The commits are instead structured so a reviewer can read one logical body of work at a time, while the PR merges atomically. Concrete examples of the interdependencies:

  • Pipelines call build targets that this PR creates. Both new compute-versions stages shell out to dotnet build build.proj -t:GetVersionsSqlClient / GetVersionsSqlServer and parse a PackageVersion: / FileVersion: stdout contract. Those targets and that stdout format are introduced in the build commit. A standalone "pipeline" PR would invoke targets that don't exist on main; a standalone "build.proj" PR would delete the old per-package GetVersions* targets that the current pipelines still call — breaking CI on main the moment it merged.
  • Cross-stage variable contracts span commits. The CI build/pack stages read stageDependencies.compute_versions_ci.compute_versions_job.outputs['versions.SqlClientPackageVersion'] and feed it into the PackageVersionSqlClient build argument. The producing stage (pipeline commit) and the consuming argument (build commit) are only meaningful together; neither half is correct alone.
  • Deleting the per-package version files and adding the central import are two halves of one switch. The build commit deletes the four per-package Versions.props files and adds the single Directory.Build.props import of the family Versions.props. Done separately, one ordering leaves family projects with no version defined, and the other leaves duplicate/conflicting version definitions — so they cannot be reviewed/merged as independently-correct units.
  • The constant rename must be atomic with its consumers. Renaming the generated ThisAssembly.InformationalVersion/NuGetPackageVersion constants without simultaneously updating the six consumers (AdapterUtil, SqlDiagnosticListener, UserAgent, the Azure auth provider, UserAgentTests, the stress runner) fails to compile. Even this "small" change is irreducible — which is exactly why it is one commit, not one-PR-per-file.

Commits (review order)

The branch began with five logical commits and now includes three focused follow-up commits. Each is a single coherent body of work; they are intended to be reviewed in order. Individual commits are not guaranteed to build/test in isolation (see above).

1. Rename ThisAssembly version constants to FileVersion and PackageVersion

A self-contained refactor that removes the ambiguous version naming. In tools/targets/GenerateThisAssemblyCs.targets the generated ThisAssembly constants are renamed: InformationalVersionFileVersion and NuGetPackageVersionPackageVersion. All consumers are updated: AdapterUtil, SqlDiagnosticListener, UserAgent, the Azure ActiveDirectoryAuthenticationProvider, UserAgentTests, and the reflection-based stress runner. (7 files)

2. Unify SqlClient family packages onto a single shared version

The core project/build change. The four per-package family Versions.props files (AKV provider, Abstractions, Azure, Logging) are deleted. src/Microsoft.Data.SqlClient/Versions.props becomes the single source of truth for the family (SqlClientPackageVersion, SqlClientFileVersion, SqlClientAssemblyVersion); src/Microsoft.SqlServer.Server/Versions.props stays independent. src/Directory.Build.props imports the family Versions.props for every project, Directory.Packages.props pins family packages to the shared range, and all family .csproj files plus the nuspec consume the shared properties. build.proj collapses the per-package GetVersions* targets to GetVersionsSqlClient and GetVersionsSqlServer, with a single PackageVersionSqlClient parameter (old per-package arguments kept as aliases). (15 files)

3. Compute SqlClient family versions centrally in CI/PR pipelines

Adds eng/pipelines/stages/compute-versions-ci-stage.yml, which runs the two GetVersions* targets with a BuildSuffix and emits the family + SqlServer versions. All family build/test/pack stages, jobs, steps, and shared templates take a dependency on the new stage and map their version variables from the shared SqlClient* outputs. CI version parameters are consolidated onto a single packageVersion parameter (Microsoft.SqlServer.Server keeps its own); PR pipelines pass buildSuffix: pr, CI pipelines pass buildSuffix: ci. (25 files)

4. Compute SqlClient family versions centrally in OneBranch pipelines

Adds eng/pipelines/onebranch/stages/compute-versions-stage.yml, resolving one SqlClient family version and one SqlServer version up front. build-stages.yml, publish-symbols-stage.yml, and the build/pack/publish jobs and steps consume sqlClient{Package,File}Version for all family packages and sqlServer* for SqlServer. release-stages.yml collapses the five family release toggles to one releaseSqlClient (kept releaseSqlServerServer separate); package-variables.yml drops hardcoded version values. (13 files)

5. Document unified SqlClient family versioning

Adds .github/instructions/sqlclient-package-versions.instructions.md describing family/SqlServer version resolution across all scenarios, renames package-versions.instructions.md3rd-party-package-versions.instructions.md (now scoped to external deps), refreshes the OneBranch pipeline design notes, and updates BUILDGUIDE.md pack/build parameter tables and package-mode examples for the single family parameter. (4 files)

6. Address reviewer feedback

Applies reviewer-requested corrections across canonical version imports, build.proj, OneBranch version computation and argument flow, and the stress runner. It also replaces the compute-stage Bash with cross-platform PowerShell and tightens explicit package/file-version handling. (11 files)

7. Remove unused OneBranch isPreview parameter

Removes the dead isPreview parameter and forwarding from both OneBranch entry pipelines and package-variables.yml; canonical Versions.props values now determine preview or stable package versions directly. (3 files)

8. Add optional OneBranch package version revisions

Adds the opt-in addRevision parameter to official and non-official OneBranch pipelines. Pipeline runs use $(Rev:r) and can append the mapped run revision to packages built by that run, preserving prerelease suffixes, wrapping above the 65535 file-version limit with an Azure DevOps warning, and leaving an unbuilt published SqlServer dependency unchanged. Version computation is extracted into a documented, tested PowerShell script. (9 files)

Version source of truth

After this PR, versions are computed in just two files:

File Computes
src/Microsoft.Data.SqlClient/Versions.props SqlClientPackageVersion, SqlClientFileVersion, SqlClientAssemblyVersion — shared by the whole family
src/Microsoft.SqlServer.Server/Versions.props SqlServerPackageVersion, SqlServerFileVersion, SqlServerAssemblyVersion

Each Versions.props uses a 3-tier priority to resolve the version:

  1. Explicit *PackageVersion parameter - supersedes all (legacy PR/CI and OneBranch use this approach).
  2. BuildNumber (+ optional BuildSuffix) → prerelease tag; BuildNumber is the FileVersion revision component (modern PR/CI use this approach)
  3. Fallback -dev suffix for local developer builds.

AssemblyVersion is always derived as Major.0.0.0 for strong-name backward compatibility.

Resulting version differentiation

Pipeline Suffix Example Version
Developer (local) -dev 7.1.0-preview3-dev
PR validation -pr 7.1.0-preview3-pr20250602.1
CI (merge trigger) -ci 7.1.0-preview3-ci20250602.1
OneBranch official (none) 7.1.0-preview3

Verification

  • Local dotnet build still produces -dev versions:
    • Project ref mode
    • Package ref mode
  • New sqlclient-pr produces -pr suffixed versions:
  • Legacy PR pipeline produces -pr suffixed versions:
  • CI pipeline produces -ci suffixed versions:
  • OneBranch Non-Official pipeline produces correct family + SqlServer versions:
    • Use NuGet.org SqlServer package: 26194.2
    • Build SqlServer package: 26194.3

Package validation results

Every produced package was verified with the PackageValidator tool (package, file, and assembly versions; symbol packages; and — for the official run — the Microsoft.SqlServer.Server dependency range). All runs passed with no unexpected-version findings.

Pipeline Run Suffix Microsoft.SqlServer.Server Result
OneBranch Non-Official 26194.3 (none) built at 1.1.0-preview1
OneBranch Official (release) 26194.2 (none) not built; family depends on published [1.0.0, 2.0.0)
sqlclient-pr 19402 -pr19402 built at 1.1.0-preview1-pr19402
Legacy PR Project 19402 -pr19402 built at 1.1.0-preview1-pr19402
Legacy PR Package 19402 -pr19402 built at 1.1.0-preview1-pr19402
CI Project 19409 -ci19409 built at 1.1.0-preview1-ci19409
CI Package 19403 -ci19403 built at 1.1.0-preview1-ci19403

Confirmed for every run:

  • The SqlClient family (Microsoft.Data.SqlClient, Internal.Logging, Extensions.Abstractions, Extensions.Azure, and the AKV Provider) — all 5 packages and every per-target-framework assembly — carry an identical package version, file version (7.1.0.<build>), and assembly version (7.0.0.0).
  • Microsoft.SqlServer.Server is versioned independently (1.1.0-preview1 / assembly 1.0.0.0).
  • AssemblyVersion is always Major.0.0.0 for strong-name backward compatibility.
  • Each pipeline stamps the correct suffix (-dev / -pr / -ci / none).
  • Official runs build only the family and pin the SqlServer dependency to the last-published 1.0.0; non-official builds SqlServer, giving nightly coverage of that flow.

Future work

Deferred follow-ups, to be tracked/implemented separately:

  • Pre-compute versions in the new PR/CI pipelines. Bring the new PR and CI pipelines in line with the central version-computation model used by the legacy PR/CI and OneBranch pipelines: add a compute-versions stage that resolves the SqlClient family version and the SqlServer version once, up front, and pass them down explicitly to the pack and test stages instead of re-deriving them per step. (build.proj already exposes the needed GetVersions* targets and PackageVersionSqlClient/PackageVersionSqlServer arguments, so this is pipeline-only work.)
  • Add/enhance package validation across all pipelines using the new PackageValidator tool. Wire the PackageValidator tool into every pipeline (CI, PR, and OneBranch) to validate produced packages (contents, layout, dependency version ranges, and the unified family vs. independent SqlServer versioning) as a gating step.
  • **Align on PackageVersionXYZ naming convention for version properties across pipelines, build.proj, and C# projects/props files.

Copilot AI review requested due to automatic review settings June 3, 2026 13:56
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Jun 3, 2026
@paulmedynski paulmedynski added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label Jun 3, 2026
@paulmedynski paulmedynski moved this from To triage to In progress in SqlClient Board Jun 3, 2026
@paulmedynski paulmedynski added this to the 7.1.0-preview2 milestone Jun 3, 2026
@paulmedynski paulmedynski changed the title Add compute-versions stage for CI/PR pipelines Add compute-versions stage for CI/PR/Official pipelines Jun 3, 2026
@paulmedynski paulmedynski changed the title Add compute-versions stage for CI/PR/Official pipelines Consolidate package version specification and computation Jun 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a centralized “compute versions” stage for CI/PR pipelines (mirroring the OneBranch approach) and refactors version sources so pipelines can compute/package versions once and flow them to downstream stages/jobs.

Changes:

  • Added compute_versions_ci stage to extract package/file versions up-front via new GetVersions* targets in build.proj.
  • Refactored each package’s Versions.props to introduce *NextVersion and *PublishedVersion properties.
  • Began wiring CI/PR and OneBranch pipelines/templates to consume computed versions (but the current wiring is incomplete and will break pipeline execution).

Reviewed changes

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

Show a summary per file
File Description
src/Microsoft.SqlServer.Server/Versions.props Adds Published/Next version properties; updates computed package/file version logic to use SqlServerNextVersion.
src/Microsoft.Data.SqlClient/Versions.props Adds Published/Next version properties; updates computed package/file version logic to use SqlClientNextVersion.
src/Microsoft.Data.SqlClient.Internal/Logging/src/Versions.props Adds Published/Next version properties; updates computed package/file version logic to use LoggingNextVersion.
src/Microsoft.Data.SqlClient.Extensions/Azure/src/Versions.props Adds Published/Next version properties; updates computed package/file version logic to use AzureNextVersion.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Versions.props Adds Published/Next version properties; updates computed package/file version logic to use AbstractionsNextVersion.
src/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider/src/Versions.props Adds Published/Next version properties; updates computed package/file version logic to use AkvProviderNextVersion.
eng/pipelines/stages/compute-versions-ci-stage.yml New CI/PR stage that runs GetVersions* targets and exports versions as stage/job outputs.
eng/pipelines/stages/build-sqlserver-package-ci-stage.yml Adds dependency on compute stage and maps its outputs to stage variables (but needs additional wiring updates).
eng/pipelines/stages/build-sqlclient-package-ci-stage.yml Adds dependency on compute stage and maps its outputs to stage variables; attempts to use those versions downstream.
eng/pipelines/stages/build-logging-package-ci-stage.yml Adds dependency on compute stage and maps its outputs to stage variables (but needs downstream job wiring updates).
eng/pipelines/stages/build-azure-package-ci-stage.yml Adds dependency on compute stage and maps its outputs to stage variables (but downstream job calls still use template parameters).
eng/pipelines/stages/build-abstractions-package-ci-stage.yml Adds dependency on compute stage and maps its outputs to stage variables (but downstream job calls still use template parameters).
eng/pipelines/sqlclient-pr-project-ref-pipeline.yml Passes buildSuffix: 'pr' into the CI core template.
eng/pipelines/sqlclient-pr-package-ref-pipeline.yml Passes buildSuffix: 'pr' into the CI core template.
eng/pipelines/dotnet-sqlclient-ci-project-reference-pipeline.yml Passes buildSuffix: 'ci' into the CI core template.
eng/pipelines/dotnet-sqlclient-ci-package-reference-pipeline.yml Passes buildSuffix: 'ci' into the CI core template.
eng/pipelines/dotnet-sqlclient-ci-core.yml Adds buildSuffix parameter and injects the compute-versions stage before other stages.
eng/pipelines/libraries/ci-build-variables.yml Removes previously hardcoded per-package version variables in favor of computed versions.
build.proj Adds GetVersions* targets that emit labeled version lines for pipeline parsing.
eng/pipelines/onebranch/variables/package-variables.yml Removes OneBranch per-package version variables; keeps artifact naming variables.
eng/pipelines/onebranch/stages/compute-versions-stage.yml New OneBranch stage to compute “next/published/effective” versions and emit output variables.
eng/pipelines/onebranch/stages/build-stages.yml Refactors build template to require pre-computed versions; comments out SqlClient package validation stage.
eng/pipelines/onebranch/stages/publish-symbols-stage.yml Refactors parameters to require pre-computed package versions for symbol publishing.
eng/pipelines/onebranch/stages/release-stages.yml Removes explicit per-version packagePath in favor of default/wildcarded publish behavior.
eng/pipelines/onebranch/jobs/build-buildproj-job.yml Threads “pre-computed version” concept through build/pack steps; tweaks APIScan versionNumber handling.
eng/pipelines/onebranch/jobs/publish-symbols-job.yml Refactors job variables to list form; clarifies version requirement.
eng/pipelines/onebranch/steps/roslyn-analyzers-buildproj-step.yml Updates comments to reflect version must be pre-computed.
eng/pipelines/onebranch/steps/pack-buildproj-step.yml Updates comments to reflect version must be pre-computed.
eng/pipelines/onebranch/steps/build-buildproj-step.yml Updates comments to reflect version must be pre-computed.
eng/pipelines/onebranch/sqlclient-official.yml Removes passing version vars to templates (but does not yet provide new required version parameters).
eng/pipelines/onebranch/sqlclient-non-official.yml Removes passing version vars to templates (but does not yet provide new required version parameters).
.github/instructions/sqlclient-package-versions.instructions.md New documentation describing version resolution flow (examples need to match actual pipeline build-number format).
.github/instructions/3rd-party-package-versions.instructions.md Renames/re-scopes instructions to explicitly cover third-party dependency versions.

Comment thread eng/pipelines/stages/build-abstractions-package-ci-stage.yml Outdated
Comment thread eng/pipelines/stages/build-sqlserver-package-ci-stage.yml
Comment thread eng/pipelines/stages/build-sqlclient-package-ci-stage.yml Outdated
Comment thread eng/pipelines/stages/build-azure-package-ci-stage.yml Outdated
Comment thread eng/pipelines/onebranch/sqlclient-official.yml
Comment thread .github/instructions/sqlclient-package-versions.instructions.md
Comment thread .github/instructions/sqlclient-package-versions.instructions.md Outdated
Comment thread eng/pipelines/stages/build-logging-package-ci-stage.yml Outdated
Comment thread eng/pipelines/dotnet-sqlclient-ci-core.yml
Comment thread eng/pipelines/libraries/ci-build-variables.yml
Copilot AI review requested due to automatic review settings June 22, 2026 16:40
@paulmedynski
paulmedynski force-pushed the dev/paul/canonical-versions branch from 756dc7a to e63eafa Compare June 22, 2026 16:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread eng/pipelines/stages/compute-versions-ci-stage.yml
Comment thread eng/pipelines/stages/compute-versions-ci-stage.yml
Comment thread eng/pipelines/onebranch/stages/compute-versions-stage.yml Outdated
Copilot AI review requested due to automatic review settings June 23, 2026 20:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread Directory.Packages.props
Comment thread eng/pipelines/stages/compute-versions-ci-stage.yml
Comment thread eng/pipelines/onebranch/stages/compute-versions-stage.yml Outdated
Comment thread eng/pipelines/onebranch/stages/compute-versions-stage.yml Outdated
Comment thread eng/pipelines/onebranch/jobs/build-buildproj-job.yml Outdated
Comment thread eng/pipelines/onebranch/stages/release-stages.yml
Comment thread eng/pipelines/onebranch/stages/release-stages.yml
Comment thread eng/pipelines/onebranch/stages/release-stages.yml
Comment thread eng/pipelines/onebranch/stages/release-stages.yml
Comment thread eng/pipelines/onebranch/stages/release-stages.yml
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.81%. Comparing base (deabcc2) to head (dc7cb54).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ata/SqlClient/Diagnostics/SqlDiagnosticListener.cs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4336      +/-   ##
==========================================
- Coverage   64.71%   62.81%   -1.91%     
==========================================
  Files         288      283       -5     
  Lines       44088    67041   +22953     
==========================================
+ Hits        28532    42112   +13580     
- Misses      15556    24929    +9373     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 62.81% <40.00%> (?)

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

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI review requested due to automatic review settings June 25, 2026 12:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread eng/pipelines/onebranch/stages/compute-versions-stage.yml Outdated
Comment thread eng/pipelines/onebranch/jobs/build-buildproj-job.yml Outdated
Comment thread src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Program.cs Outdated
Comment thread .github/instructions/sqlclient-package-versions.instructions.md
paulmedynski added a commit that referenced this pull request Jun 25, 2026
- compute-versions (CI + OneBranch): install the global.json-pinned .NET SDK and
  restore dotnet tools before 'dotnet build'; fail fast if version extraction
  yields an empty value; add 'set -euo pipefail' to the resolve step.
  (review: #8, #9, #10, #12, #13, #14)
- build-buildproj-job: use an explicit non-empty check for package.artifactName in
  the compile-time 'if'; drop the no-op coalesce for the APIScan file version
  (OneBranch always supplies one). (review: #15, #25)
- compute-versions: document that a boolean ${{ }} parameter renders the
  PascalCase 'True'/'False', so comparing to 'True' is correct. (review: #24)
- publish-nuget-package-job: correct the stale packagePath comment
  (packages/ -> Package-Release/) and note coalesce() skips the empty default.
  (review: #16-#21)
- docs: sqlclient-package-versions.instructions.md uses buildSqlServer, not
  releaseSqlServer. (review: #27)
Copilot AI review requested due to automatic review settings June 25, 2026 18:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread Directory.Packages.props Outdated
Comment thread src/Microsoft.Data.SqlClient/Versions.props Outdated
Comment thread eng/pipelines/onebranch/steps/build-buildproj-step.yml Outdated
Comment thread eng/pipelines/onebranch/steps/pack-buildproj-step.yml Outdated
paulmedynski added a commit that referenced this pull request Jun 25, 2026
…eline cleanup

- Directory.Packages.props: clarify only the SqlServer Versions.props is imported here; the SqlClient family file comes from src/Directory.Build.props

- src/Microsoft.Data.SqlClient/Versions.props: drop stale claim that Directory.Packages.props imports this file

- onebranch build/pack-buildproj-step.yml: remove obsolete comment block (also fixes 'againast' typo)

- onebranch compute-versions-stage.yml: avoid literal empty ${{ }} in a script-block comment that ADO evaluated as a template expression
Copilot AI review requested due to automatic review settings June 26, 2026 13:07
@paulmedynski
paulmedynski force-pushed the dev/paul/canonical-versions branch from 17a911d to c78ee0f Compare June 26, 2026 13:07
@paulmedynski paulmedynski removed this from the 7.1.0-preview2 milestone Jun 26, 2026

@benrr101 benrr101 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think this is doing exactly what I envisioned. My only requests are 1) see if you agree with any of my points, 2) see if we can reduce the amount of special casing required in the pipelines.

// Look for the PackageVersion field, falling back to FileVersion.
var field = type.GetField(
"NuGetPackageVersion",
"PackageVersion",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

var field = type.GetField("PackageVersion", BindingFlags.NonPublic | BindingFlags.Static)
    ?? type.GetField("FileVersion", BindingFlags.NonPublic | BindingFlags.Static);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this to the compact null-coalescing lookup. The assignment wraps after = to respect the existing line length. Validated with a net8.0 Stress Runner build. Local commit: 9252526ba.

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj
Directory.Packages.props imports all Versions.props early in Package mode; the guard
prevents each csproj's own conditional import from re-importing and re-evaluating
version properties.
src/Directory.Build.props imports this file for every project; the guard prevents re-importing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What harm is caused by re-importing and re-evalating the properties?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this by forcing the import guard off. MSBuild emits warning MSB4011, describes the duplicate as a likely build-authoring error, and ignores the subsequent import, so it does not re-evaluate the properties. I kept the guards to avoid that warning and updated the comments to describe the behavior accurately. Local commit: 9252526ba.

<!--
This is the *next* version to release of SqlServer. Update this when the next version is
decided.
@TODO: Determine how versions should be updated in release branches vs main

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note can probably be deleted. I think you figured it out :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the TODO and replaced it with the concrete instruction to update SqlServerNextVersion after each release. Local commit: 9252526ba.

Comment thread build.proj Outdated
Applies to: TestSqlClient, TestSqlClientFunctional, TestSqlClientManual
BuildAkvProvider, PackAkvProvider
TestAzure
Applies to: BuildSqlClient*, PackSqlClient, BuildLogging, PackLogging, BuildAbstractions,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something here doesn't sit right with me. Maybe if we could have them grouped by Build / Pack / Test? I dunno.

I'm also wondering how much of these do actually apply regardless of reference type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grouped the applicability lists into Build, Pack, and Test, and clarified the reference-mode distinction: Build and Pack use the family version in both modes, while Test consumes it only in Package mode. I also corrected the SqlServer target list. Local commit: 9252526ba.


variables:
- name: sqlClientPackageVersion
value: $[ stageDependencies.compute_versions.compute_versions_job.outputs['versions.SqlClientPackageVersion'] ]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping we could avoid using the $[ ] syntax. It's hard enough to understand what ${{ }} and $( ) do 😅

dependencies:
- artifactName: '${{ parameters.abstractionsArtifactsName }}'
shortName: 'Abstractions'
version: '$(sqlClientPackageVersion)'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can, I think we should be using ${{ variables.sqlClientPackageVersion }} so it's clear where it's coming from

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value comes from a $[ stageDependencies... ] runtime output. ${{ variables.sqlClientPackageVersion }} is evaluated during template expansion before that output exists, so it would not carry the computed version. I also audited the other macros in this stage; they are runtime stage outputs or predefined runtime paths, so there are no safe promotions here.

# Extract versions (package, published, file) for the SqlClient family
# (GetVersionsSqlClient) and Microsoft.SqlServer.Server (GetVersionsSqlServer).
# Each target emits labeled messages to stdout.
- script: |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor concern here that using script implicitly means bash, which makes this stage non-portable. While above, we use pwsh which is portable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted both version extraction and effective-version resolution blocks from implicit Bash script steps to portable pwsh steps. I validated the PowerShell parsing against both real GetVersions targets and both next/published values. Local commit: 9252526ba.

# Provider) and PackageVersionSqlServer (Microsoft.SqlServer.Server). Provided by the caller.
# Examples: 'SqlClient' -> -p:PackageVersionSqlClient=7.1.0-preview3
# 'SqlServer' -> -p:PackageVersionSqlServer=1.0.0
- name: versionPropertySuffix

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see this derived from the packageShortName if possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A direct derivation only works for SqlServer and SqlClient: short names such as Logging, Abstractions, and Azure all use the shared PackageVersionSqlClient property rather than matching properties of their own. I moved that two-family mapping to explicit caller metadata, so this step and the shared job remain generic. Local commit: 9252526ba.

-p:PackageVersion${{ parameters.packageShortName }}="${{ parameters.packageVersion }}"
-p:BuildNumber=$(Build.BuildNumber)
${{ parameters.dependencyArguments }}
-p:PackageVersion${{ parameters.versionPropertySuffix }}=${{ parameters.packageVersion }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ... uh ... what? Nit: can this be kept next to the BuildNumber parameter? I tried to keep the versioning properties next to each other since they version the package being built rather than the dependencies.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the package-version argument directly beside BuildNumber. I applied the same ordering to the analyzer, build, and pack commands for consistency. Local commit: 9252526ba.

Replace the generated InformationalVersion/NuGetPackageVersion constants with FileVersion/PackageVersion and update all consumers (diagnostics, user agent, Azure extension, stress runner, and unit tests).
Replace the per-project Versions.props files for the AKV provider, Abstractions, Azure, and Logging packages with a single shared family version. Wire the consuming csproj files, nuspec, central package props, and build.proj version arguments to the unified version.
Add a compute-versions CI stage and thread the unified family version through the legacy Azure DevOps CI and PR pipelines (build/pack/test stages, jobs, steps, and shared templates).
Copilot AI review requested due to automatic review settings August 6, 2026 18:45
@paulmedynski
paulmedynski force-pushed the dev/paul/canonical-versions branch from 48e55ef to 3af74e1 Compare August 6, 2026 18:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 64 out of 64 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 7, 2026 00:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 67 out of 67 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/instructions/onebranch-pipeline-design.instructions.md:94

  • The OneBranch pipeline design doc still lists the removed isPreview build parameter and does not mention the new addRevision parameter introduced by this PR. It also leaves the subsequent paragraph and bullets (below this changed region) describing the old isPreview/effective*Version variable model, which no longer matches the new Versions.props + compute-versions-stage flow.
Build parameters:
- `debug` — enable debug output (default `false`)
- `isPreview` — use preview version numbers (default `false`)
- `publishSymbols` — publish symbols to servers (default `false`)
- `buildSqlServer` — build the Microsoft.SqlServer.Server package (default `true` in the non-official/nightly pipeline, `false` in the official pipeline). The SqlClient family is always built, so this is the only build toggle. It also drives the SqlServer dependency version the family uses (built/next vs published). Requesting `releaseSqlServer` without `buildSqlServer` fails template expansion.

build.proj:97

  • This comment describing which packages are in the SqlClient family omits Microsoft.Data.SqlClient itself. Since the family version applies to SqlClient too, the list should include it for clarity/accuracy.
      The Internal.Logging, Extensions.Abstractions, Extensions.Azure, and
      AlwaysEncrypted.AzureKeyVaultProvider packages are all part of the SqlClient family and share
      the SqlClient version.  There are no longer separate per-package version parameters for these
      packages — use PackageVersionSqlClient (below) to set the version for the entire family.
    -->

Copilot AI review requested due to automatic review settings August 7, 2026 11:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 67 out of 67 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Program.cs:262

  • The stress runner previously supported multiple generations of ThisAssembly field names (NuGetPackageVersion / InformationalVersion) to remain compatible when the referenced Microsoft.Data.SqlClient assembly differed. With the rename to PackageVersion/FileVersion, this now only probes the new field names, so running the updated runner against an older SqlClient assembly will return "" instead of a version.

Copilot AI review requested due to automatic review settings August 7, 2026 12:35
@paulmedynski
paulmedynski force-pushed the dev/paul/canonical-versions branch from 78b3f74 to dc7cb54 Compare August 7, 2026 12:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 67 out of 67 changed files in this pull request and generated no new comments.

Suppressed comments (3)

.github/instructions/onebranch-pipeline-design.instructions.md:101

  • This file still contains a later paragraph describing how isPreview affects effective*Version variables, but the OneBranch pipelines in this PR removed isPreview and compute preview/GA purely from Versions.props. That paragraph should be updated to avoid misleading guidance.
Official-only parameter:
- `releaseToProduction` — controls both NuGet target feed and symbol server destination (default `false`):

.github/instructions/onebranch-pipeline-design.instructions.md:93

  • The OneBranch pipeline design doc still references the removed isPreview parameter (and its related preview/GA resolution text). This makes the instructions inaccurate for contributors and risks misconfiguring pipeline runs.

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

Build parameters:
- `debug` — enable debug output (default `false`)
- `isPreview` — use preview version numbers (default `false`)
- `publishSymbols` — publish symbols to servers (default `false`)

eng/pipelines/onebranch/scripts/tests/compute-versions.Tests.ps1:40

  • The local dotnet mock output includes a PublishedVersion: line for the SqlClient family, but GetVersionsSqlClient only emits PackageVersion: and FileVersion: (PublishedVersion is SqlServer-only). Keeping the extra label in the mock can hide parser/test regressions and confuses the contract being validated.
                return @(
                    '  PackageVersion: 7.1.0-preview3'
                    '  PublishedVersion: 7.0.0'
                    "  FileVersion: 7.1.0.$global:computeVersionsFileRevision"
                )

@TODO: Determine how versions should be updated in release branches vs main
-->
<SqlServerVersionDefault>1.0.0</SqlServerVersionDefault>
<SqlServerNextVersion>1.1.0-preview1</SqlServerNextVersion>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this cause the next release to rely on an unpublished sqlserver version?

Add a compute-versions stage that resolves a single SqlClient family version and
a single Microsoft.SqlServer.Server version up front, and thread them through the
OneBranch official and non-official pipelines (build/publish/release stages, jobs,
steps, and package variables).

Each package is built, analyzed, and packed with the correct build.proj
package-version property -- PackageVersionSqlClient for the whole SqlClient family
and PackageVersionSqlServer for Microsoft.SqlServer.Server. Family packages that
depend on Microsoft.SqlServer.Server pin it to the computed SqlServer version
(freshly built this run, or the last published version otherwise); shared family
dependencies need no explicit pin, since PackageVersionSqlClient covers them.
Add the sqlclient-package-versions instructions, rename the package-versions instructions to 3rd-party-package-versions, refresh the OneBranch pipeline design notes, and update BUILDGUIDE for the unified version model.
Copilot AI review requested due to automatic review settings August 7, 2026 16:57
@paulmedynski
paulmedynski force-pushed the dev/paul/canonical-versions branch from dc7cb54 to dac28e8 Compare August 7, 2026 16:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 67 out of 67 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/instructions/onebranch-pipeline-design.instructions.md:94

  • onebranch-pipeline-design.instructions.md still documents an isPreview pipeline parameter and preview/GA version-selection behavior, but the OneBranch entry pipelines in this PR remove isPreview and compute versions via the compute-versions stage / canonical Versions.props. The doc should be updated so it matches the pipeline surface area and the new versioning flow.
Build parameters:
- `debug` — enable debug output (default `false`)
- `isPreview` — use preview version numbers (default `false`)
- `publishSymbols` — publish symbols to servers (default `false`)
- `buildSqlServer` — build the Microsoft.SqlServer.Server package (default `true` in the non-official/nightly pipeline, `false` in the official pipeline). The SqlClient family is always built, so this is the only build toggle. It also drives the SqlServer dependency version the family uses (built/next vs published). Requesting `releaseSqlServer` without `buildSqlServer` fails template expansion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems.

Projects

Status: Waiting for customer

Development

Successfully merging this pull request may close these issues.

5 participants