Fix codeql run

[Evangelink]

No description provided.

[Copilot]

Pull request overview

Updates the Azure Pipelines CodeQL job to install the repo’s pinned .NET SDK (via global.json) and switches the build invocation to the Arcade eng/common/CIBuild.cmd entrypoint.

Changes:

• Add UseDotNet@2 to install the .NET SDK version from global.json.
• Replace the build step command with eng\common\cibuild.cmd and pass /p:Test=false.

Show a summary per file

File	Description
azure-pipelines-codeql.yml	Adds a .NET install step and changes the CodeQL build invocation to use Arcade CIBuild.cmd.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review Summary

The root-cause fix (UseDotNet@2 with useGlobalJson: true) is sound: pre-installing the .NET SDK from global.json means Arcade's InstallDotNetCore task finds it already present and skips the problematic step that was triggering MSB4216 on VS x86 MSBuild. This correctly makes -msbuildEngine dotnet unnecessary, which in turn makes VS MSBuild available again — and VS MSBuild handles UWP/uap10.0.16299 targets that dotnet MSBuild cannot.

Two concerns are worth discussing before merge (inline comments posted above):

1. /p:Test=false silently depends on @properties ordering in build.ps1 — it works today because @properties is expanded after /p:Test=$test in the Build function (MSBuild last-wins). A one-line comment would prevent a future regression.

2. cibuild.cmd hardcodes -pack -sign -publish — the original "CodeQL only needs source compilation" rationale was removed along with the comment. Pack/sign/publish are likely no-ops in the CodeQL pipeline (sign gates on $(DotNetSignType), etc.), but this is an implicit assumption. Also, CIBuild.cmd calls eng/common/build.ps1, not the repo-specific eng/build.ps1 that Build.cmd used; please verify the two produce equivalent compilation scope for CodeQL.

---

#	Dimension	Verdict	Notes
1	Algorithmic Correctness	⚠️	/p:Test=false works via non-obvious @properties-last ordering; pack/sign/publish now run implicitly
2	Threading & Concurrency	N/A	Pipeline YAML
3	Security & IPC Contract Safety	✅	No concerns
4	Public API & Binary Compatibility	N/A	Pipeline YAML
5	Performance & Allocations	⚠️	pack/sign/publish add unnecessary build time to the CodeQL scan
6	Cross-TFM Compatibility	✅	VS MSBuild (default with new approach) handles UWP/uap10.0.16299
7	Resource & IDisposable Management	N/A	Pipeline YAML
8	Defensive Coding at Boundaries	N/A	Pipeline YAML
9	Localization & Resources	N/A	Pipeline YAML
10	Test Isolation	N/A	Pipeline YAML
11	Assertion Quality	N/A	Pipeline YAML
12	Error Handling	✅	Build failures propagate correctly; -prepareMachine retained
13	Build & CI Infrastructure	⚠️	cibuild.cmd→eng/common/build.ps1 vs old Build.cmd→eng/build.ps1; pack/sign/publish now active
14	Naming Conventions	✅	Step names are clear
15	Code Comments & Documentation	⚠️	Removed 9-line comment block explaining MSB4216/UWP history with no replacement context
16	Null Safety / Defensive Coding	N/A	Pipeline YAML
17	Scope Discipline	✅	Single-file, focused change
18	Overarching Principles	⚠️	Drops "CodeQL only needs source compilation" principle silently; no explanation of why full CI build is now acceptable
19	Dependency Management	✅	useGlobalJson: true correctly pins the SDK version
20	Configuration Correctness	✅	-prepareMachine kept; -ci correctly dropped (CIBuild.cmd adds it internally)
21	Observability / Diagnostics	✅	Step display names unchanged; binary log still produced via -ci mode
22	Maintainability	⚠️	/p:Test=false mechanism and pack/sign/publish assumption need documentation

Overall: The fix is likely correct and will unblock the CodeQL run. The inline concerns are non-blocking documentation issues rather than correctness bugs — sign off once the cibuild.cmd scope (pack/sign/publish) is confirmed safe in this pipeline context.

Generated by Expert Code Review (on open) for issue #8935 · sonnet46 3.3M

[Evangelink]

/p:Test=false works, but the mechanism is entirely non-obvious and depends on implementation details of eng/common/build.ps1.

Why it works: eng/common/CIBuild.cmd hardcodes -test when calling build.ps1, which causes build.ps1 to emit /p:Test=True to MSBuild. However, build.ps1's Build function appends @properties (the catch-all remaining arguments) after the hardcoded /p:Test=... flag. MSBuild last-write-wins for duplicate global properties, so /p:Test=false arriving via @properties correctly overrides the earlier /p:Test=True.

This is fragile: if build.ps1 ever moves @properties before the hardcoded flags (a perfectly reasonable refactor), tests will silently start running. A one-line comment would help future maintainers:

# /p:Test=false overrides the -test flag hardcoded in CIBuild.cmd (last /p: wins in MSBuild)
/p:Test=false

[Evangelink]

eng\common\CIBuild.cmd hardcodes -pack -sign -publish -test in its call to build.ps1 (see eng/common/CIBuild.cmd). The removed comment block explicitly warned: "cibuild.cmd implicitly enables -pack/-sign/-publish, and the dotnet MSBuild engine cannot build the UWP/uap10.0.16299 targets used by some MSTest packages."

The new approach:

• Fixes the UWP issue by dropping -msbuildEngine dotnet (VS MSBuild handles UWP) — made safe by UseDotNet@2 resolving the MSB4216 root cause. ✅
• Still runs -pack -sign -publish. These are presumably no-ops here (pack would produce packages; sign/publish typically gate on variables like $(DotNetSignType) that aren't set in the CodeQL pipeline), but this is implicit assumption, not explicit.

The original rationale — "CodeQL only needs source compilation" — is valid and worth preserving as context. Also note that CIBuild.cmd drives eng/common/build.ps1 while the old Build.cmd drove eng/build.ps1 (the repo-specific script with -installWindowsSdk and other local customisations). The two scripts are not identical; please verify that the common one produces equivalent compilation output for CodeQL's purposes.