review feedback (Copilot): pin runtime vcpkg clones, share install dir across configs, fail fast, fix stale comments

Addresses 6 of 7 review comments from #48039 (review)
(the 7th — global VcpkgEnabled=true — is a deliberate trade-off; see the
expanded Cpp.Build.props comment for why per-project opt-in via spdlog.props
proved unworkable on this codebase's import order).

What changes
------------
Cpp.Build.props
  - VcpkgInstalledDir: dropped $(Configuration) from the path
    (now `vcpkg_installed\$(Platform)\`). vcpkg already separates debug/
    release artifacts inside the same triplet directory ($triplet/lib vs
    $triplet/debug/lib), so a single install dir per platform is correct
    and avoids redundant restores on config switches.
  - Added a fail-fast <Target>: when VcpkgEnabled=true but vcpkg.props
    cannot be found at the resolved VcpkgRoot, abort with a clear error
    listing the three resolution paths (VS component, VCPKG_ROOT, or
    build-essentials.cmd). Previously the import was silently skipped and
    surfaced as opaque C1083/LNK2019 errors much later in the build.
  - Expanded the leading comment to call out the per-project-opt-in
    trade-off explicitly so future readers don't repeat the attempt.

.pipelines/v2/templates/steps-install-vcpkg.yml
  - When falling back to a fresh vcpkg clone, check out the commit listed
    in vcpkg.json's `builtin-baseline` after cloning, before bootstrapping.
    Previously the clone tracked microsoft/vcpkg HEAD, which makes CI
    non-reproducible (a new vcpkg HEAD can change tool behavior or break
    restores even when the manifest baseline is pinned). The baseline
    commit is read from vcpkg.json at runtime so the two cannot drift.

tools/build/build-essentials.ps1
  - Same baseline-pinning behavior as the CI template.
  - Handles three states correctly: (a) no clone yet -> clone+checkout
    baseline, (b) existing clone at the wrong revision -> fetch+checkout
    baseline, (c) existing clone at the right revision -> no-op. Avoids
    unnecessary network round-trips on subsequent runs.
  - Updated the leading comment: vcpkg integration is wired in
    Cpp.Build.props (with vcpkg.targets in Cpp.Build.targets), not
    deps/spdlog.props.

.pipelines/v2/templates/job-build-project.yml
  - Updated the leading comment for the same reason.

Verification
------------
Local Release|x64 build of src/modules/fancyzones/FancyZonesLib/FancyZonesLib.vcxproj
green (~87 s) with the new VcpkgInstalledDir layout. /preprocess output
confirms the fail-fast <Target> is properly merged into every .vcxproj.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: LegendaryBlair

.pipelines/v2/templates/job-build-project.yml

@@ -276,9 +276,11 @@ jobs:
   # Visual Studio (Microsoft.VisualStudio.Component.Vcpkg) and falls back to a
   # fresh clone of microsoft/vcpkg into deps/vcpkg if VS doesn't have it.
   # Either way it sets the VCPKG_ROOT pipeline variable; MSBuild integration
-  # picks it up from deps/spdlog.props (three-tier VcpkgRoot fallback).
+  # is wired globally from Cpp.Build.props (with vcpkg.targets in
+  # Cpp.Build.targets) using the three-tier VcpkgRoot fallback
+  # (env var > VS-shipped > deps/vcpkg runtime clone).
   #
-  # Vcpkg's MSBuild integration runs `vcpkg install` once per build, so the
+  # Vcpkg's MSBuild integration runs `vcpkg install` once per project, so the
   # binary cache below saves ~3-5 minutes per triplet on cache hits.
   - template: .\steps-install-vcpkg.yml
 

.pipelines/v2/templates/steps-install-vcpkg.yml

@@ -4,22 +4,34 @@
 #   1. The Visual Studio installation (Microsoft.VisualStudio.Component.Vcpkg).
 #   2. A local clone at deps/vcpkg, cloned and bootstrapped on demand.
 #
-# Sets the pipeline-scoped VCPKG_ROOT variable so the rest of the build (and
-# MSBuild's vcpkg integration imported from deps/spdlog.props) can find vcpkg
-# without the repo having to carry a submodule.
+# When falling back to a local clone, the clone is pinned to the same commit
+# as vcpkg.json's `builtin-baseline` so CI builds are reproducible (vcpkg
+# HEAD changes can otherwise break restores even though the baseline is
+# pinned). The baseline commit is read from vcpkg.json at runtime so the
+# two never drift.
+#
+# Sets the pipeline-scoped VCPKG_ROOT variable; the rest of the build
+# resolves vcpkg through it (see the three-tier VcpkgRoot fallback in
+# Cpp.Build.props). No repo-level vcpkg submodule required.
 steps:
   - pwsh: |-
       $VsInstallRoot = & 'C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe' -latest -prerelease -requires Microsoft.VisualStudio.Component.Vcpkg -property installationPath
       If ([String]::IsNullOrEmpty($VsInstallRoot)) {
+        $manifest = Get-Content "$(build.sourcesdirectory)\vcpkg.json" -Raw | ConvertFrom-Json
+        $baseline = $manifest.'builtin-baseline'
+        if (-not $baseline) { throw "vcpkg.json is missing 'builtin-baseline'." }
+        Write-Host "Cloning microsoft/vcpkg pinned to builtin-baseline $baseline"
         Remove-Item -Recurse -Force deps/vcpkg -ErrorAction:Ignore
         git clone https://github.com/microsoft/vcpkg deps/vcpkg
         if ($LASTEXITCODE -ne 0) { throw "git clone vcpkg failed (exit $LASTEXITCODE)" }
         Push-Location deps/vcpkg
+        git checkout --quiet $baseline
+        if ($LASTEXITCODE -ne 0) { Pop-Location; throw "git checkout $baseline in vcpkg failed (exit $LASTEXITCODE)" }
         & ./bootstrap-vcpkg.bat -disableMetrics
         if ($LASTEXITCODE -ne 0) { Pop-Location; throw "bootstrap-vcpkg failed (exit $LASTEXITCODE)" }
         $VcpkgRoot = $PWD
         Pop-Location
-        Write-Host "Using vcpkg from local checkout ($VcpkgRoot)"
+        Write-Host "Using vcpkg from local checkout ($VcpkgRoot) at $baseline"
       } Else {
         $VcpkgRoot = Join-Path $VsInstallRoot 'VC\vcpkg'
         Write-Host "Using vcpkg from Visual Studio installation ($VcpkgRoot)"

Cpp.Build.props

@@ -124,22 +124,29 @@
 
   <!--
     vcpkg integration. Set globally (not per-project) so that import order is
-    guaranteed to be BEFORE Microsoft.Cpp.targets — this matters because
-    vcpkg's MSBuild integration registers a target (VcpkgInstallManifestDependencies)
-    that ClCompile must depend on, and that hook only takes effect when
-    vcpkg.props is loaded before the C++ targets. PowerToys has projects
-    that import deps/spdlog.props after Microsoft.Cpp.targets, so putting
-    vcpkg setup in spdlog.props loaded that way failed to wire include paths.
+    guaranteed to be BEFORE Microsoft.Cpp.targets — vcpkg.props registers a
+    target (VcpkgInstallManifestDependencies) that ClCompile must depend on,
+    and that hook only takes effect when vcpkg.props is loaded before the
+    C++ targets. PowerToys has projects that import deps/spdlog.props after
+    Microsoft.Cpp.targets, so a per-project opt-in via spdlog.props (the
+    earlier attempt) silently broke include-path wiring for those projects.
+
+    Trade-off: every C++ project now invokes `vcpkg install` once at build
+    time, even if it doesn't consume spdlog. With binary cache enabled this
+    is ~0.5 s/project on cache hits. The manifest declares only spdlog, so
+    the cost is fixed and bounded; non-spdlog projects just carry an unused
+    include path.
 
     Three-tier VcpkgRoot fallback matching microsoft/terminal's pattern:
       1. $(VCPKG_ROOT) env var (CI sets via .pipelines/v2/templates/steps-install-vcpkg.yml;
          local dev sets via tools/build/build-essentials.ps1).
       2. $(VsInstallRoot)\VC\vcpkg (vcpkg shipped with Visual Studio).
       3. $(RepoRoot)deps\vcpkg (gitignored runtime-cloned fallback).
 
-    The vcpkg manifest at the repo root declares only spdlog, so non-spdlog
-    C++ projects pay only the vcpkg install latency (~0.5 s on cache hits)
-    and an unused include path. With binary cache enabled this is negligible.
+    VcpkgInstalledDir is platform-scoped (not configuration-scoped); vcpkg
+    already separates debug/release artifacts inside the same triplet
+    directory ($triplet/lib vs $triplet/debug/lib). Splitting per-platform
+    keeps parallel x64/arm64 builds from racing on the same install lock.
   -->
   <PropertyGroup Label="vcpkg">
     <VcpkgEnabled>true</VcpkgEnabled>
@@ -151,13 +158,30 @@
     <!-- vcpkg validates triplets case-sensitively; PowerToys uses ARM64 capital-case. -->
     <VcpkgPlatformTarget Condition="'$(Platform)' == 'ARM64'">arm64</VcpkgPlatformTarget>
     <VcpkgApplocalDeps>false</VcpkgApplocalDeps>
-    <VcpkgInstalledDir>$(MSBuildThisFileDirectory)$(Platform)\$(Configuration)\vcpkg_installed\</VcpkgInstalledDir>
+    <VcpkgInstalledDir>$(MSBuildThisFileDirectory)vcpkg_installed\$(Platform)\</VcpkgInstalledDir>
     <VcpkgRoot Condition="'$(VcpkgRoot)' == ''">$(VCPKG_ROOT)</VcpkgRoot>
     <VcpkgRoot Condition="'$(VcpkgRoot)' == '' and '$(VsInstallRoot)' != ''">$(VsInstallRoot)\VC\vcpkg</VcpkgRoot>
     <VcpkgRoot Condition="'$(VcpkgRoot)' == '' or !Exists('$(VcpkgRoot)\vcpkg.exe')">$(MSBuildThisFileDirectory)deps\vcpkg</VcpkgRoot>
     <CAExcludePath>$(CAExcludePath);$(VcpkgInstalledDir)</CAExcludePath>
     <VCPkgLocalAppDataDisabled>true</VCPkgLocalAppDataDisabled>
   </PropertyGroup>
+  <!--
+    Fail fast with a clear message if vcpkg is enabled but vcpkg.props
+    cannot be found at the resolved VcpkgRoot. Without this the build
+    silently skips vcpkg integration and surfaces later as missing
+    <spdlog/*> includes or unresolved spdlog::* link errors, which are
+    much harder to root-cause.
+  -->
+  <Target Name="PowerToysEnsureVcpkgAvailable"
+          BeforeTargets="PrepareForBuild"
+          Condition="'$(MSBuildProjectExtension)' == '.vcxproj' and '$(VcpkgEnabled)' == 'true' and !Exists('$(VcpkgRoot)\scripts\buildsystems\msbuild\vcpkg.props')">
+    <Error Text="vcpkg is required to build this project but vcpkg.props was not found at '$(VcpkgRoot)\scripts\buildsystems\msbuild\vcpkg.props'.
+
+Resolve via one of:
+  1) Install the Visual Studio component 'Microsoft.VisualStudio.Component.Vcpkg' (in the VS Installer, under Desktop development with C++), or
+  2) Set the VCPKG_ROOT environment variable to an existing bootstrapped vcpkg checkout, or
+  3) Run tools\build\build-essentials.cmd at least once to clone+bootstrap a local vcpkg into deps\vcpkg." />
+  </Target>
   <Import Project="$(VcpkgRoot)\scripts\buildsystems\msbuild\vcpkg.props"
           Condition="'$(MSBuildProjectExtension)' == '.vcxproj' and Exists('$(VcpkgRoot)\scripts\buildsystems\msbuild\vcpkg.props')" />
 

tools/build/build-essentials.ps1

@@ -54,7 +54,14 @@ if (-not (Ensure-VsDevEnvironment)) { exit 1 }
 # Detect vcpkg using the Windows Terminal pattern: prefer VS-shipped vcpkg
 # (Microsoft.VisualStudio.Component.Vcpkg), fall back to a runtime clone at
 # deps/vcpkg if VS doesn't have it. Either way, set VCPKG_ROOT so MSBuild
-# picks it up via deps/spdlog.props' three-tier fallback. Idempotent.
+# picks it up via the three-tier VcpkgRoot fallback in Cpp.Build.props
+# (env var > VS-shipped > local clone). Idempotent.
+#
+# The fallback clone is pinned to vcpkg.json's `builtin-baseline` so local
+# builds match what CI uses and don't drift as vcpkg HEAD changes. If an
+# existing deps/vcpkg clone is at a different commit, it's checked out to
+# the baseline before bootstrap (the manifest baseline is the source of
+# truth; the clone is just a delivery mechanism).
 if (-not $env:VCPKG_ROOT) {
     $vswhere = 'C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe'
     $vsInstallRoot = $null
@@ -65,20 +72,39 @@ if (-not $env:VCPKG_ROOT) {
         $env:VCPKG_ROOT = Join-Path $vsInstallRoot 'VC\vcpkg'
         Write-Host "[BUILD-ESSENTIALS] Using vcpkg from Visual Studio installation ($env:VCPKG_ROOT)"
     } else {
+        $manifest = Get-Content (Join-Path $repoRoot 'vcpkg.json') -Raw | ConvertFrom-Json
+        $baseline = $manifest.'builtin-baseline'
+        if (-not $baseline) { Write-Error "vcpkg.json is missing 'builtin-baseline'."; exit 1 }
         $localVcpkg = Join-Path $repoRoot 'deps\vcpkg'
-        if (-not (Test-Path (Join-Path $localVcpkg 'vcpkg.exe'))) {
-            Write-Host "[BUILD-ESSENTIALS] VS-shipped vcpkg not found; cloning microsoft/vcpkg to $localVcpkg"
+        $needClone = -not (Test-Path (Join-Path $localVcpkg '.git'))
+        if (-not $needClone) {
+            $currentHead = (& git -C $localVcpkg rev-parse HEAD 2>$null).Trim()
+            if ($currentHead -ne $baseline) {
+                Write-Host "[BUILD-ESSENTIALS] Existing deps/vcpkg at $currentHead; checking out baseline $baseline"
+                & git -C $localVcpkg fetch --quiet origin $baseline
+                if ($LASTEXITCODE -ne 0) { Write-Warning "[BUILD-ESSENTIALS] fetch failed, re-cloning"; $needClone = $true } else {
+                    & git -C $localVcpkg checkout --quiet $baseline
+                    if ($LASTEXITCODE -ne 0) { Write-Error "git checkout $baseline in vcpkg failed (exit $LASTEXITCODE)"; exit 1 }
+                }
+            }
+        }
+        if ($needClone) {
+            Write-Host "[BUILD-ESSENTIALS] Cloning microsoft/vcpkg to $localVcpkg pinned to baseline $baseline"
             if (Test-Path $localVcpkg) { Remove-Item -Recurse -Force $localVcpkg }
             & git clone https://github.com/microsoft/vcpkg $localVcpkg
             if ($LASTEXITCODE -ne 0) { Write-Error "git clone vcpkg failed (exit $LASTEXITCODE)"; exit 1 }
+            & git -C $localVcpkg checkout --quiet $baseline
+            if ($LASTEXITCODE -ne 0) { Write-Error "git checkout $baseline in vcpkg failed (exit $LASTEXITCODE)"; exit 1 }
+        }
+        if (-not (Test-Path (Join-Path $localVcpkg 'vcpkg.exe'))) {
             Push-Location $localVcpkg
             try {
                 & .\bootstrap-vcpkg.bat -disableMetrics
                 if ($LASTEXITCODE -ne 0) { Write-Error "bootstrap-vcpkg failed (exit $LASTEXITCODE)"; exit 1 }
             } finally { Pop-Location }
         }
         $env:VCPKG_ROOT = $localVcpkg
-        Write-Host "[BUILD-ESSENTIALS] Using vcpkg from local checkout ($env:VCPKG_ROOT)"
+        Write-Host "[BUILD-ESSENTIALS] Using vcpkg from local checkout ($env:VCPKG_ROOT) at $baseline"
     }
 }