Skip to content

Add BuildAll target#4290

Open
paulmedynski wants to merge 4 commits into
mainfrom
dev/paul/build-all
Open

Add BuildAll target#4290
paulmedynski wants to merge 4 commits into
mainfrom
dev/paul/build-all

Conversation

@paulmedynski

@paulmedynski paulmedynski commented May 14, 2026

Copy link
Copy Markdown
Contributor

Description

Add BuildAll target that builds all projects for all OS+TFM combinations. This makes it easy for devs to compile local changes and avoid surprises for the code that otherwise wouldn't be compiled due to their host OS.

Changes

  • Added BuildAll target in build.proj that builds all projects (tests, samples, tools) across OS+TFM combos.
  • Updated CodeQL workflow to use BuildAll for broader code coverage.
  • Improved fidelity of build.proj messaging.

Split from #4259.

Copilot AI review requested due to automatic review settings May 14, 2026 13:15
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board May 14, 2026
@paulmedynski paulmedynski added this to the 7.1.0-preview2 milestone May 14, 2026
@paulmedynski paulmedynski added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label May 14, 2026
@paulmedynski paulmedynski moved this from To triage to In progress in SqlClient Board May 14, 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

Adds a new BuildAll orchestration target in build.proj and updates the CodeQL workflow to build more of the repository during analysis, alongside a handful of MSBuild/props cleanup and minor project tweaks.

Changes:

  • Introduces BuildAll (and supporting BuildTests/BuildSamples/BuildTools) and switches CodeQL to call it.
  • Adds a TrimDocs opt-out for ref-doc trimming to better support cross-build scenarios.
  • Cleans up legacy MSBuild ToolsVersion / DefaultTargets usage and reduces build log noise.

Reviewed changes

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

Show a summary per file
File Description
tools/targets/RepositoryInfo.targets Lowers message verbosity for translated repo URL logging.
tools/props/AssemblyRef.props Removes legacy <Project> attributes.
tools/props/AssemblyInfo.props Removes legacy <Project> attributes.
src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.ruleset Removes obsolete ToolsVersion and normalizes file ending.
src/Microsoft.Data.SqlClient/tests/Common/Microsoft.Data.SqlClient.TestCommon.csproj Enables implicit usings for the shared test helper project.
src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj Adds TrimDocs switch to optionally skip IntelliSense XML trimming.
src/Directory.Build.props Removes legacy <Project> attributes; clarifies versioning evaluation order.
build.proj Adds BuildAll + ancillary build targets and shared arg handling.
.github/workflows/codeql.yml Switches manual CodeQL build step to BuildAll.

Comment thread build.proj Outdated
Comment thread .github/workflows/codeql.yml
Comment thread src/Directory.Build.props Outdated
Comment thread src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj Outdated
Comment thread build.proj Outdated
Comment thread build.proj Outdated
Comment thread build.proj Outdated
Copilot AI review requested due to automatic review settings May 14, 2026 13:51

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 17 out of 17 changed files in this pull request and generated 1 comment.

Comment thread build.proj Outdated
Comment thread build.proj
@codecov

codecov Bot commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.10%. Comparing base (5ac26c9) to head (357e48c).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4290      +/-   ##
==========================================
- Coverage   66.50%   64.10%   -2.40%     
==========================================
  Files         285      280       -5     
  Lines       43311    66160   +22849     
==========================================
+ Hits        28806    42415   +13609     
- Misses      14505    23745    +9240     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 64.10% <ø> (?)

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 May 15, 2026 11:28
Comment thread .gitattributes
Comment thread .gitignore

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 18 out of 19 changed files in this pull request and generated 2 comments.

Comment thread build.proj Outdated
Comment thread build.proj Outdated
@paulmedynski paulmedynski marked this pull request as ready for review May 15, 2026 16:35
@paulmedynski paulmedynski requested a review from a team as a code owner May 15, 2026 16:35
Copilot AI review requested due to automatic review settings May 15, 2026 16:35
@paulmedynski paulmedynski enabled auto-merge (squash) May 15, 2026 16: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 18 out of 19 changed files in this pull request and generated 1 comment.

Comment thread build.proj

@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.

Few issues here.

1

wouldn't be compiled due to their host OS.

I think there's still a fundamental misunderstanding of the build(2).proj behavior. You can build any version on any OS. BuildSqlClient is dependent on BuildSqlClientWindows and BuildSqlClientUnix. The old build.proj would skip Windows on non-windows by default but that's not the case anymore.

2

There's already a "Build" target that builds all projects.

3

There's a lot of microscopic cleanup changes here. The important changes are getting lost in them. Can these be omitted from this PR?

@github-project-automation github-project-automation Bot moved this from In progress to Waiting for customer in SqlClient Board May 20, 2026
@benrr101 benrr101 added the Author attention needed PRs that require author to respond or make updates to PR. label May 20, 2026
@paulmedynski paulmedynski marked this pull request as draft May 21, 2026 10:52
auto-merge was automatically disabled May 21, 2026 10:52

Pull request was converted to draft

@paulmedynski paulmedynski moved this from Waiting for customer to In progress in SqlClient Board May 21, 2026
@paulmedynski paulmedynski removed the Author attention needed PRs that require author to respond or make updates to PR. label May 21, 2026
Copilot AI review requested due to automatic review settings June 10, 2026 13:04
@paulmedynski paulmedynski changed the title Add BuildAll target, CodeQL update, and build cleanup Add BuildAll target Jun 10, 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

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

Comment thread build.proj
Comment thread build.proj
Comment thread build.proj
Comment thread build.proj
Comment thread build.proj
Test projects previously used the built-in $(OS) property to gate
net462 inclusion. This meant passing -p:TargetOs=Windows_NT from
build.proj had no effect on test TFM selection.

Migrate all test csproj files and Directory.Build.props to define
TargetOs (defaulting to $(OS)) and condition on it, matching the
pattern already used by the driver project. This allows BuildAll to
build all TFMs regardless of host OS.
@paulmedynski paulmedynski marked this pull request as ready for review June 11, 2026 14:17
@paulmedynski paulmedynski requested review from a team and Copilot June 11, 2026 14:17
@paulmedynski paulmedynski enabled auto-merge (squash) June 11, 2026 14:17
@paulmedynski paulmedynski moved this from In progress to In review in SqlClient Board Jun 11, 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

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

Comment thread build.proj
Comment on lines +1200 to +1204
<DotnetCommand>
"$(DotnetPath)dotnet" build "$(SqlClientUnitTestProjectPath)"
-p:Configuration=$(Configuration)
-p:TargetOs=Unix
</DotnetCommand>
Comment thread build.proj
Comment on lines +1280 to +1284
$(ReferenceTypeArgument)
$(PackageVersionAbstractionsArgument)
$(PackageVersionLoggingArgument)
$(PackageVersionSqlClientArgument)
</DotnetCommand>
Comment thread build.proj
Comment on lines +1214 to +1218
<DotnetCommand>
"$(DotnetPath)dotnet" build "$(SqlClientUnitTestProjectPath)"
-p:Configuration=$(Configuration)
-p:TargetOs=Windows_NT
</DotnetCommand>
Comment thread build.proj
Comment on lines +1300 to +1304
$(ReferenceTypeArgument)
$(PackageVersionAbstractionsArgument)
$(PackageVersionLoggingArgument)
$(PackageVersionSqlClientArgument)
</DotnetCommand>
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: In review

Development

Successfully merging this pull request may close these issues.

5 participants