vcpkg icon indicating copy to clipboard operation
vcpkg copied to clipboard

Ensure that MSBuild integration bootstraps Vcpkg

Open mschofie opened this issue 1 year ago • 0 comments

Fixes #23366

The current MSBuild integration doesn't bootstrap VCPkg when running in 'manifest' mode - meaning that consumers have to manually run, say, bootstrap-vcpkg.bat before building, or the build fails with errors. #23366 tracks this and calls out the problems in running bootstrap-vcpkg.bat - just adding a target that runs bootstrap-vcpkg.bat can cause concurrency problems on parallel, multi-project builds. When building in parallel, multiple MSBuild 'engines' are invoked for the multiple projects, and multiple could attempt to run bootstrap-vcpkg.bat concurrently, and bootstrap-vcpkg.bat doesn't protect against concurrent execution, resulting in errors writing files. Under CMake this isn't a problem since the CMake integration runs during the CMake 'configuration' phase, which is executed sequentially. But MSBuild doesn't have a similar, single threaded equivalent.

MSBuild does provide a threading guarantee that we can use - it guarantees that an MSBuild task for a given project, and a given set of global properties will run once (called out in the documentation for the MSBuild task). So if we invoke an MSBuild task with a global property of the Vcpkg root to bootstrap then MSBuild will ensure that it is only run once, blocking all other MSBuild engines until the bootstrap is complete. And that's what this PR does.

There's a few pieces to this change:

  1. I introduce the VcpkgAutoBootstrap property and default it to true. This allows consumers to opt-out of the functionality. I'm fine with defaulting to false, and making it opt-in if a more cautious roll-out is needed.
  2. In scripts/buildsystems/msbuild/vcpkg.targets I conditionally import scripts/buildsystems/msbuild/support/Bootstrap.targets that contains the bulk of the implementation. By putting the logic in a separate 'targets' file, then it can be 'firewalled' a little. The conditionality requires:
    1. VcpkgEnableManifest to be true, to only run on manifest builds (a requirement called out in the tracking issue)
    2. VcpkgAutoBootstrap to be true, to allow folks to opt-out (a requirement called out in the tracking issue)
    3. MSBuild is 16.5 and higher - The implementation depends on IBuildEngine6.GetGlobalProperties, which was introduced in MSBuild 16.5. The MSBuild 16.5 release notes don't call this out, I binary searched the "Microsoft.Build.Framework" NuGet packages, and 16.5 is the first version to contain IBuildEngine6.
    4. Windows - I only need this on Windows, so I'm just scoping it for now. If non-Windows support is needed, it can be added separately.
  3. scripts/buildsystems/msbuild/support/Bootstrap.targets provides a VcpkgBootstrap target that marks itself as BeforeTargets="VcpkgInstallManifestDependencies" to ensure that Vcpkg is bootstrapped before it is used. The target also configures Inputs and Outputs appropriately, so that when bootstrapped the target won't be run, and the added cost is minimal. The implementation uses a custom task to get the current global property names, and then invokes MSBuild to run the VcpkgBootstrapImplementation removing all global properties and setting the _ZVcpkgRoot property. MSBuild will only run a single VcpkgBootstrapImplementation instance, and so the work that VcpkgBootstrapImplementation does will only run once.
  4. scripts\buildsystems\msbuild\support\GetGlobalProperties.task provides the implementation of the custom task to get the global properties. The task has a single output parameter - GlobalPropertyNames - which is a semi-colon delimited list of the global property names, that the VcpkgBootstrap target specifies as the RemoveProperties parameter to MSBuild.

I've been using an implementation of this fix in my consuming repo for a week or so, and I've buddy tested this fix by forcing a long delay in bootstrap-vcpkg.bat. On a build where vcpkg.exe isn't present, the VcpkgBootstrap target is 'built' by multiple projects (showing that there's contention by default), but the VcpkgBootstrapImplementation task is only run once (so MSBuild is protecting it). On an incremental build, the VcpkgBootstrap task is skipped entirely, thanks to the MSBuild up-to-date checks.

mschofie avatar Oct 16 '24 23:10 mschofie