Skip to content

[7.0] Merge | Pack Mds Target, Wire PR/CI Pipelines to Build2.proj#4110

Open
paulmedynski wants to merge 1 commit intorelease/7.0from
cherry-pick/7.0/4033
Open

[7.0] Merge | Pack Mds Target, Wire PR/CI Pipelines to Build2.proj#4110
paulmedynski wants to merge 1 commit intorelease/7.0from
cherry-pick/7.0/4033

Conversation

@paulmedynski
Copy link
Copy Markdown
Contributor

@paulmedynski paulmedynski commented Mar 31, 2026

Cherry-pick of #4033 to release/7.0


Original PR Description

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.

* In progress work for pack target ... kinda working

* Add config to genapi path.
Remove unnecessary dependencies from genapi project

* Fix generation of docs in ref project

* Dump reference type specific builds
Fix xml documentation file generation for implementation project

* Generating a package file works!!!

* Resync pipelines folder in solution

* Add an assembly build number argument?
... it doesn't work

* Wiring up build numbers through build2.proj

* Build number argument

* Maybe wiring it up?!?

* Reinstate the assemblybuildnumber property

* Build all of MDS, once

* PR comments from copilot

* Specify test results folder for CI builds

* TargetFramework => TestFramework

* Fixing a couple more comments

* Couple more comments from copilot

* downgrade back to 7.0.0 :)

* I dunno, fixing some stuff, I guess.

* Fix indenting

* Back out changes to official pipelines

* A couple more comments from CoPilot

* Generate AKV documentation file during CI build ... idk why this is only a problem *now*

* Comments from Copilot ... I'd love it if it could give the same comments across commits.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 PackMds target in build2.proj to package Microsoft.Data.SqlClient (MDS) from the new artifacts layout, and updates PR/CI pipeline steps and project files to build/test/package via build2.proj while centralizing MDS versioning.

Changes:

  • Centralize MDS versioning in src/Microsoft.Data.SqlClient/MdsVersions.props and import it from the impl/ref/notsupported projects.
  • Update MDS build output paths and the MDS .nuspec file list to align with the new artifacts/Microsoft.Data.SqlClient/<ReferenceType>-<Configuration>/<os>/<tfm>/ layout.
  • Rewire test/build pipeline steps to call build2.proj targets (e.g., TestMdsUnit, TestMdsFunctional, TestMdsManual, BuildMds) and ensure PR pipelines trigger on build2.proj changes.

Reviewed changes

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

Show a summary per file
File Description
tools/targets/GenerateThisAssemblyCs.targets Defaults AssemblyFileVersion to AssemblyVersion for generated ThisAssembly metadata.
tools/specs/Microsoft.Data.SqlClient.nuspec Updates file includes to the new artifacts folder structure and separates lib/ref/runtime binaries.
tools/props/Versions.props Removes inlined MDS version properties in favor of MdsVersions.props.
tools/intellisense/TrimDocs.ps1 Makes trimming fail-fast by throwing on missing input and stopping on errors.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.cs Fixes <include> doc paths and whitespace cleanup.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlJson.cs Fixes <include> doc paths and formatting cleanup.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlDbTypeExtensions.cs Fixes <include> doc paths for snippets.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs Fixes <include> doc paths and whitespace cleanup in comments.
src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj Imports MdsVersions.props, enables XML docs, and changes OutputPath to include ReferenceType-Configuration.
src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj Imports MdsVersions.props, enables XML docs, updates OutputPath, and refines TrimDocs target conditions/inputs.
src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.cs Fixes snippet include paths and minor whitespace formatting.
src/Microsoft.Data.SqlClient/notsupported/Microsoft.Data.SqlClient.csproj Imports MdsVersions.props and aligns output/ref artifact paths with new layout.
src/Microsoft.Data.SqlClient/MdsVersions.props New centralized MSBuild versioning logic for MDS assembly/package versions.
src/Microsoft.Data.SqlClient.sln Updates solution items (adds MdsVersions.props, adjusts pipeline solution items).
eng/pipelines/sqlclient-pr-project-ref-pipeline.yml Adds build2.proj to PR path filters.
eng/pipelines/sqlclient-pr-package-ref-pipeline.yml Adds build2.proj to PR path filters.
eng/pipelines/common/templates/steps/run-all-tests-step.yml Switches test execution from build.proj to build2.proj targets and updates property names.
eng/pipelines/common/templates/steps/ci-project-build-step.yml Switches MDS build step to build2.proj/BuildMds and updates arguments passed.
build2.proj Adds PackMds, expands build/test parameters, and aligns build/test targets with the new artifacts/versioning conventions.


<!-- Versioning arguments -->
$(BuildNumberArgument)
$(PacakgeVersionMdsArgument)
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

In BuildMdsNotSupported, the command uses $(PacakgeVersionMdsArgument) (typo). This property is undefined, so the PackageVersionMds override won’t be forwarded to the notsupported build, which can cause version mismatches between the notsupported/ref/impl assemblies when a package version override is supplied. Rename this to $(PackageVersionMdsArgument) so -p:MdsPackageVersion=... is passed correctly.

Suggested change
$(PacakgeVersionMdsArgument)
$(PackageVersionMdsArgument)

Copilot uses AI. Check for mistakes.

<!-- Versioning arguments -->
$(BuildNumberArgument)
$(PacakgeVersionMdsArgument)
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

In BuildMdsRef, the command uses $(PacakgeVersionMdsArgument) (typo). This means PackageVersionMds overrides won’t be forwarded to the ref build, so the ref assembly version may not match the implementation/package version when overrides are used. Rename this to $(PackageVersionMdsArgument) so -p:MdsPackageVersion=... is applied consistently.

Suggested change
$(PacakgeVersionMdsArgument)
$(PackageVersionMdsArgument)

Copilot uses AI. Check for mistakes.
Comment on lines +84 to +90
msbuildArguments:
-t:BuildMds
-p:ReferenceType=${{ parameters.referenceType }}
-p:AbstractionsPackageVersion=${{ parameters.abstractionsPackageVersion }}
-p:LoggingPackageVersion=${{ parameters.loggingPackageVersion }}
-p:MdsPackageVersion=${{ parameters.mdsPackageVersion }}
retryCountOnTaskFailure: 1

- ${{ if eq(parameters.build, 'allNoDocs') }}:
- task: MSBuild@1
displayName: 'Build Driver (no docs) [Win]'
condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))
inputs:
solution: build.proj
msbuildArchitecture: x64
platform: '${{ parameters.platform }}'
configuration: '${{ parameters.buildConfiguration }}'
msbuildArguments: >-
-t:BuildAllConfigurations
-p:ReferenceType=${{ parameters.referenceType }}
-p:GenerateNuget=false
-p:GenerateDocumentationFile=false
-p:BuildNumber=${{ parameters.buildNumber }}
-p:AssemblyBuildNumber=${{ parameters.assemblyBuildNumber }}
-p:AbstractionsPackageVersion=${{ parameters.abstractionsPackageVersion }}
-p:LoggingPackageVersion=${{ parameters.loggingPackageVersion }}
-p:MdsPackageVersion=${{ parameters.mdsPackageVersion }}

- ${{ if or(eq(parameters.build, 'MDS'), eq(parameters.build, 'all')) }}:
- task: MSBuild@1
displayName: 'Build Driver [Win]'
condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))
inputs:
solution: build.proj
msbuildArchitecture: x64
platform: '${{ parameters.platform }}'
configuration: '${{ parameters.buildConfiguration }}'
msbuildArguments:
-t:BuildAllConfigurations
-p:ReferenceType=${{ parameters.referenceType }}
-p:GenerateNuget=false
-p:BuildNumber=${{ parameters.buildNumber }}
-p:AssemblyBuildNumber=${{ parameters.assemblyBuildNumber }}
-p:AbstractionsPackageVersion=${{ parameters.abstractionsPackageVersion }}
-p:LoggingPackageVersion=${{ parameters.loggingPackageVersion }}
-p:MdsPackageVersion=${{ parameters.mdsPackageVersion }}
-p:BuildNumber=${{ parameters.buildNumber }}
-p:PackageVersionAbstractions=${{ parameters.abstractionsPackageVersion }}
-p:PackageVersionLogging=${{ parameters.loggingPackageVersion }}
-p:PackageVersionMds=${{ parameters.mdsPackageVersion }}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

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

msbuildArguments under the MSBuild@1 task is currently written as a YAML mapping (each line contains a :), not a scalar string. Azure Pipelines expects msbuildArguments to be a string, so this will either fail YAML parsing/type validation or pass the wrong value to the task. Use a folded scalar (>-) or quote the entire argument string so the task receives a single msbuild arguments string.

Copilot uses AI. Check for mistakes.
@paulmedynski paulmedynski added this to the 7.0.1 milestone Mar 31, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.74%. Comparing base (864f666) to head (558fa54).

❗ There is a different number of reports uploaded between BASE (864f666) and HEAD (558fa54). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (864f666) HEAD (558fa54)
CI-SqlClient 1 0
Additional details and impacted files
@@               Coverage Diff               @@
##           release/7.0    #4110      +/-   ##
===============================================
- Coverage        73.07%   65.74%   -7.34%     
===============================================
  Files              280      275       -5     
  Lines            42997    65822   +22825     
===============================================
+ Hits             31422    43273   +11851     
- Misses           11575    22549   +10974     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 65.74% <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.

@mdaigle mdaigle moved this from To triage to In review in SqlClient Board Apr 1, 2026
@paulmedynski paulmedynski marked this pull request as ready for review April 2, 2026 15:48
@paulmedynski paulmedynski requested a review from a team as a code owner April 2, 2026 15:48
@paulmedynski paulmedynski modified the milestones: 7.0.1, 7.0.2 Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

4 participants