Merge | Pack Mds Target, Wire PR/CI Pipelines to Build2.proj by benrr101 · Pull Request #4033 · dotnet/SqlClient

benrr101 · 2026-03-11T18:25:41Z

Description

This PR adds a PackMds target to Build2.proj that conveniently packages the MDS projects into a NuGet package. This PR also reworks the CI/PR pipelines to use the build2.proj targets to build, test, and package the MDS project.

Commentary has been added in-line.

Testing

Ran PR validation while still in draft mode and verifies that it generates good output. There are a lot of things to go through to verify that everything is behaving correctly, but it seems good.

Remove unnecessary dependencies from genapi project

Fix xml documentation file generation for implementation project

... it doesn't work

Copilot

Pull request overview

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

Copilot · 2026-03-12T19:12:28Z

build2.proj

+    <Error Text="PackageVersionAbstractions must be set!"
+           Condition="'$(PackageVersionAbstractions)' == ''" />
+    <Error Text="PackageVersionLogging must be set!"
+           Condition="'$(PackageVersionLogging)' == ''" />


PackMds currently fails unless the wrapper properties (PackageVersionAbstractions/Logging/Mds) are explicitly set. However, AbstractionsPackageVersion/LoggingPackageVersion can be resolved by the imported *Versions.props / *Versions.props files even when the wrapper properties are empty, and the nuspec substitution ultimately needs AbstractionsPackageVersion/LoggingPackageVersion. Consider validating the resolved AbstractionsPackageVersion/LoggingPackageVersion (and/or defaulting the wrapper properties to those resolved values) so PackMds works with default versioning and the behavior matches the parameter docs.

Suggested change

<Error Text="PackageVersionAbstractions must be set!"

Condition="'$(PackageVersionAbstractions)' == ''" />

<Error Text="PackageVersionLogging must be set!"

Condition="'$(PackageVersionLogging)' == ''" />



<PropertyGroup>

<PackageVersionAbstractions Condition="'$(PackageVersionAbstractions)' == '' and '$(AbstractionsPackageVersion)' != ''">$(AbstractionsPackageVersion)</PackageVersionAbstractions>

<PackageVersionLogging Condition="'$(PackageVersionLogging)' == '' and '$(LoggingPackageVersion)' != ''">$(LoggingPackageVersion)</PackageVersionLogging>

</PropertyGroup>



<Error Text="Abstractions package version could not be resolved. Set PackageVersionAbstractions or ensure AbstractionsPackageVersion is defined."

Condition="'$(PackageVersionAbstractions)' == '' and '$(AbstractionsPackageVersion)' == ''" />

<Error Text="Logging package version could not be resolved. Set PackageVersionLogging or ensure LoggingPackageVersion is defined."

Condition="'$(PackageVersionLogging)' == '' and '$(LoggingPackageVersion)' == ''" />

eng/pipelines/onebranch/steps/build-all-configurations-signed-dlls-step.yml

codecov · 2026-03-12T20:28:18Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.09%. Comparing base (14b96a2) to head (efbdbb2).
⚠️ Report is 17 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (14b96a2) and HEAD (efbdbb2). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (14b96a2) HEAD (efbdbb2)

CI-SqlClient 1 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4033      +/-   ##
==========================================
- Coverage   74.62%   68.09%   -6.54%     
==========================================
  Files         280      275       -5     
  Lines       43814    66924   +23110     
==========================================
+ Hits        32698    45570   +12872     
- Misses      11116    21354   +10238

Flag	Coverage Δ
CI-SqlClient	`?`
PR-SqlClient-Project	`68.09% <100.00%> (?)`

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

Looks great - keep truckin'! Just a couple of comments.

src/Microsoft.Data.SqlClient/MdsVersions.props

paulmedynski · 2026-03-12T20:24:02Z

tools/GenAPI/Microsoft.DotNet.GenAPI/Microsoft.DotNet.GenAPI.csproj

    <ProjectReference Include="..\Microsoft.Cci.Extensions\Microsoft.Cci.Extensions.csproj" />

    <PackageReference Include="McMaster.Extensions.CommandLineUtils" />
-    <PackageReference Include="System.Diagnostics.TextWriterTraceListener" />


This is the 3rd PR I've seen this diff in lately, so they're definitely not needed 😄

tools/props/Versions.props

build2.proj

eng/pipelines/common/templates/steps/run-all-tests-step.yml

eng/pipelines/onebranch/steps/code-analyze-step.yml

src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj

tools/targets/GenerateThisAssemblyCs.targets

# Conflicts: # build2.proj # eng/pipelines/onebranch/steps/code-analyze-step.yml

He's on vacation and said it's ok

Copilot

Pull request overview

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

tools/props/Versions.props

eng/pipelines/onebranch/steps/build-all-configurations-signed-dlls-step.yml

build2.proj

src/Microsoft.Data.SqlClient/MdsVersions.props

…nly a problem *now*

Copilot

Pull request overview

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

build2.proj

benrr101 · 2026-03-18T18:42:46Z

src/Microsoft.Data.SqlClient/MdsVersions.props

+      This is the version that is used as the version for the NuGet package. If it is not
+      overridden, the above default version will be used with the build number and a string suffix
+      appended (to indicate the version is pre-production).
+
+      NOTE/@TODO: Since the PackMds target in build2.proj directly calls nuget.exe, it will not
+          pick up this property.
+    -->
+    <MdsPackageVersion Condition="'$(MdsPackageVersion)' == ''">$(MdsVersionDefault).$(BuildNumber)-dev</MdsPackageVersion>
+
+    <!--
+      This is the version that is used as the AssemblyVersion and FileVersion fields in the
+      assembly's manifest. If it is not overridden, it will default to the above default version
+      with the build number appended.
+      As a convenience feature, if a package version but not an assembly version is supplied, the
+      package version will have any suffix trimmed to generate valid assembly version.
+
+      NOTE: It is possible that the build number can be in the format xxxx.y which *may* overflow
+      the 16-bit integer for the revision part of the assembly version. For that reason, we simply
+      discard everything after the first part of the build number for the assembly version.
+    -->
+    <MdsAssemblyVersion Condition="'$(MdsAssemblyVersion)' == '' AND '$(MdsPackageVersion)' == ''">$(MdsVersionDefault).$(BuildNumber.Split('.')[0])</MdsAssemblyVersion>
+    <MdsAssemblyVersion Condition="'$(MdsAssemblyVersion)' == '' AND '$(MdsPackageVersion)' != ''">$(MdsPackageVersion.Split('-')[0])</MdsAssemblyVersion>


Yeah I don't think all this is necessary. We just need to flip the order of assigning the assembly version and the package version defaults.

tools/specs/Microsoft.Data.SqlClient.nuspec

+    <!-- lib/netstandard2.0 - These are PlatformNotSupportedException assemblies -->
+    <file src="..\..\artifacts\Microsoft.Data.SqlClient.notsupported\$ReferenceType$-$Configuration$\netstandard2.0\Microsoft.Data.SqlClient.dll" target="lib\netstandard2.0\" exclude="" />
+    <file src="..\..\artifacts\Microsoft.Data.SqlClient.notsupported\$ReferenceType$-$Configuration$\netstandard2.0\Microsoft.Data.SqlClient.pdb" target="lib\netstandard2.0\" exclude="" />
+    <file src="..\..\artifacts\Microsoft.Data.SqlClient.ref\$ReferenceType$-$Configuration$\netstandard2.0\Microsoft.Data.SqlClient.xml" target="lib\netstandard2.0\" exclude="" />
+


build2.proj

+        <!-- Versioning arguments -->
+        $(BuildNumberArgument)
+        $(PacakgeVersionArgument)
+
+        <!-- Reference Type Arguments -->
+        $(ReferenceTypeArgument)
+        $(PackageVersionAbstractionsArgument)
+        $(PackageVersionLoggingArgument)


benrr101 added 10 commits March 9, 2026 12:35

In progress work for pack target ... kinda working

1feabab

Add config to genapi path.

c840aac

Remove unnecessary dependencies from genapi project

Fix generation of docs in ref project

7c24eaa

Dump reference type specific builds

7d0f219

Fix xml documentation file generation for implementation project

Generating a package file works!!!

e971112

Resync pipelines folder in solution

8e06806

Add an assembly build number argument?

64e3974

... it doesn't work

Wiring up build numbers through build2.proj

9e8d78d

Build number argument

b26e97a

Maybe wiring it up?!?

b93ffc1

benrr101 requested a review from a team as a code owner March 11, 2026 18:25

Copilot AI review requested due to automatic review settings March 11, 2026 18:25

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

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

benrr101 marked this pull request as draft March 11, 2026 18:25

Copilot started reviewing on behalf of benrr101 March 11, 2026 18:26 View session

This comment was marked as outdated.

Sign in to view

benrr101 added 2 commits March 11, 2026 14:02

Reinstate the assemblybuildnumber property

c183862

Build all of MDS, once

891b2ed

Copilot AI review requested due to automatic review settings March 11, 2026 21:18

Copilot started reviewing on behalf of benrr101 March 11, 2026 21:18 View session

This comment was marked as outdated.

Sign in to view

benrr101 added 2 commits March 11, 2026 16:28

PR comments from copilot

a23cb73

Specify test results folder for CI builds

73f0bf8

Copilot AI review requested due to automatic review settings March 11, 2026 22:32

Copilot started reviewing on behalf of benrr101 March 11, 2026 22:32 View session

This comment was marked as outdated.

Sign in to view

benrr101 added 2 commits March 11, 2026 17:56

TargetFramework => TestFramework

d0aa7b8

Fixing a couple more comments

ef05fcb

Copilot AI review requested due to automatic review settings March 12, 2026 04:30

Copilot AI review requested due to automatic review settings March 12, 2026 19:05

Copilot started reviewing on behalf of benrr101 March 12, 2026 19:05 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

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

paulmedynski previously requested changes Mar 12, 2026

View reviewed changes

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

mdaigle requested changes Mar 12, 2026

View reviewed changes

paulmedynski added this to the 7.0.0 milestone Mar 13, 2026

paulmedynski self-assigned this Mar 13, 2026

Merge branch 'main' into dev/russellben/common/pack2

ce8443f

mdaigle previously approved these changes Mar 13, 2026

View reviewed changes

Merge branch 'main' into dev/russellben/common/pack2

e50713c

# Conflicts: # build2.proj # eng/pipelines/onebranch/steps/code-analyze-step.yml

benrr101 dismissed mdaigle’s stale review via e50713c March 13, 2026 20:34

benrr101 requested a review from paulmedynski March 13, 2026 21:14

cheenamalhotra modified the milestones: 7.0.0, 7.1.0-preview1 Mar 16, 2026

Fix indenting

efbdbb2

Copilot AI review requested due to automatic review settings March 16, 2026 17:27

Copilot started reviewing on behalf of benrr101 March 16, 2026 17:27 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

Back out changes to official pipelines

cd744bf

benrr101 changed the title ~~Merge | Pack Mds Target, Wire Up Pipelines to Build2.proj~~ Merge | Pack Mds Target, Wire PR/CI Pipelines to Build2.proj Mar 16, 2026

benrr101 added 3 commits March 16, 2026 17:59

A couple more comments from CoPilot

34f53e2

Merge branch 'main' into dev/russellben/common/pack2

4050a33

Generate AKV documentation file during CI build ... idk why this is o…

cc41334

…nly a problem *now*

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

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

Copilot AI reviewed Mar 18, 2026

View reviewed changes

-    <Error Text="PackageVersionAbstractions must be set!"
-           Condition="'$(PackageVersionAbstractions)' == ''" />
-    <Error Text="PackageVersionLogging must be set!"
-           Condition="'$(PackageVersionLogging)' == ''" />
+    <!-- Default wrapper properties from resolved versions when not explicitly provided -->
+    <PropertyGroup>
+      <PackageVersionAbstractions Condition="'$(PackageVersionAbstractions)' == '' and '$(AbstractionsPackageVersion)' != ''">$(AbstractionsPackageVersion)</PackageVersionAbstractions>
+      <PackageVersionLogging Condition="'$(PackageVersionLogging)' == '' and '$(LoggingPackageVersion)' != ''">$(LoggingPackageVersion)</PackageVersionLogging>
+    </PropertyGroup>
+    <!-- Validate that a version is available either via wrapper or resolved property -->
+    <Error Text="Abstractions package version could not be resolved. Set PackageVersionAbstractions or ensure AbstractionsPackageVersion is defined."
+           Condition="'$(PackageVersionAbstractions)' == '' and '$(AbstractionsPackageVersion)' == ''" />
+    <Error Text="Logging package version could not be resolved. Set PackageVersionLogging or ensure LoggingPackageVersion is defined."
+           Condition="'$(PackageVersionLogging)' == '' and '$(LoggingPackageVersion)' == ''" />

Conversation

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

Uh oh!

Description

Testing

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

paulmedynski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulmedynski Mar 12, 2026

Choose a reason for hiding this comment

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

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

Pull request overview

Uh oh!

Uh oh!

benrr101 Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

benrr101 commented Mar 11, 2026 •

edited

Loading

codecov bot commented Mar 12, 2026 •

edited

Loading