Skip to content

Add BuildAll target#4290

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

Add BuildAll target#4290
paulmedynski wants to merge 9 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 63.40%. Comparing base (47f4e52) to head (5f4cf61).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4290      +/-   ##
==========================================
- Coverage   65.39%   63.40%   -2.00%     
==========================================
  Files         285      280       -5     
  Lines       43331    66193   +22862     
==========================================
+ Hits        28337    41968   +13631     
- Misses      14994    24225    +9231     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 63.40% <ø> (?)

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
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 thread build.proj
Comment thread build.proj
Comment thread build.proj
benrr101
benrr101 previously approved these changes Jun 11, 2026
…est build targets

BuildSqlClientTestsUnitUnix/Windows now forward $(ReferenceTypeArgument),
$(PackageVersionSqlClientArgument), and $(PackageVersionSqlServerArgument)
to align with the TestSqlClientUnit target.

BuildSqlClientTestsManualUnix/Windows now also forward
$(PackageVersionSqlServerArgument) for consistent Package-mode restore.

Addresses PR #4290 review feedback.

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 2 comments.

Comment thread build.proj
Comment thread build.proj
BuildSqlClientTestsUnitUnix/Windows now forward $(TestSigningKeyPathArgument)
so that signed-driver builds pass the correct InternalsVisibleTo key to the
unit test project, matching the TestSqlClientUnit target.

Addresses PR #4290 review feedback.
apoorvdeshmukh
apoorvdeshmukh previously approved these changes Jun 16, 2026

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

Looks Good. Can you also update BUILDGUIDE.md to reflect this new target?

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

Comment thread BUILDGUIDE.md Outdated
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.

6 participants