Ensure that MSBuild integration bootstraps Vcpkg

[mschofie]

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.

[BillyONeal]

We discussed this and confirm that we would be happy to make this work in MSBuild if a solution to the concurrency problem can be found and the perf impacts aren't too significant.

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.

We still support back to VS2015 so this ends up being kind of a problem. On older MSBuild does this 'explode' or just fail to bootstrap?

I am somewhat concerned with the dependency on RoslynCodeTaskFactory, as not all MSBuild distributions are going to have that.

[mschofie]

Thanks for taking a look.

The reason I structured the change the way I did was to 'firewall' the new stuff. Everything in the edited files (vcpkg.props and vcpkg.targets) is as 'portable' MSBuild as I can make it. I couldn't find an explicit declaration of how far back VCPkg's support of MSBuild goes, but in vcpkg.target there's already usage of MSBuild property functions, so I used those in the version check. All 'modern' constructs are in Bootstrap.targets, and that won't be imported if the checks don't pass - that means that older MSBuild's won't even look for Bootstrap.targets and they'll get the existing behavior.

Bootstrap.targets uses a couple of 'modern' MSBuild features:

1. IBuildEngine6.GetGlobalProperties - This is what motivated the MSBuild 16.5 check blocking the import of Bootstrap.targets. It appears to have first shipped in MSBuild 16.5.
2. RoslynCodeTaskFactory - The documentation for RoslynCodeTaskFactory calls out that it first shipped in MSBuild 15.8. They give an example to show to use support down-level MSBuild (using CodeTaskFactory instead of RoslynCodeTaskFactory), but since I need MSBuild 16.5 and higher for the IBuildEngine6.GetGlobalProperties call, then I just left RoslynCodeTaskFactory in.

[LilyWangLL]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[BillyONeal]

All 'modern' constructs are in Bootstrap.targets, and that won't be imported if the checks don't pass - that means that older MSBuild's won't even look for Bootstrap.targets and they'll get the existing behavior.

OK, this means no blocker to merging it.

I still have reservations about using CodeTaskFactory or RoslynCodeTaskFactory; this pattern has exploded on me on a number of occasions. But I think if that ends up being a serious issue we could just revert this.

I asked for one more confirmation from the other maintainers before landing this, probably Tuesday of next week

[BillyONeal]

We need to make sure this file is in neither the VS version or the "one-liner" version. (No change requested in this PR)

Or export

[BillyONeal]

Note to self: investigate interaction with https://github.com/microsoft/vcpkg-tool/blob/d0d4200c1824a3df2d5344696f50bbadee25398f/src/vcpkg/commands.export.cpp#L151

[BillyONeal]

I need to check: We really only want this to run in the "git clone" vcpkg deployment paradigm

[mschofie]

I need to check: We really only want this to run in the "git clone" vcpkg deployment paradigm

Ah, interesting. I just looked at the VS redistribution of 'vcpkg', and it doesn't have bootstrap-vcpkg.bat at all. Which means that this PR needs to accommodate that - at the minute it assumes that bootstrap-vcpkg.bat is present. Would checking for the existence of bootstrap-vcpkg.bat be sufficient to limit this functionality to the "git clone" vcpkg deployment paradigms?

[mschofie]

I'll convert to a draft to prevent this from being completed (is there a better way? I'm more familiar with ADO than GitHub), until I've handled the bootstrap-vcpkg.bat-isn't-present problem.