Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manifests should install dependencies earlier in build process, before the CustomBuild target #20830

Closed
aggieNick02 opened this issue Oct 18, 2021 · 4 comments
Assignees
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:manifests This PR or Issue pertains to the Manifests feature

Comments

@aggieNick02
Copy link
Contributor

aggieNick02 commented Oct 18, 2021

Describe the bug
Manifests should install dependencies before the CustomBuild target because their failure to do so means custom build tool items in a project cannot reliably use vcpkg installed dependencies.

This bug/issue is motivated by tools such as grpc and protobuf, which provide code generation tools to generate .h,.cpp.,.py files and more. A typical way to use them is to add a .proto file to your project and provide a custom build action that invokes protoc.exe or one of the grpc tools, generating files that are also part of the project.

grpc and protobuf are both ports in vcpkg, so vcpkg is a natural way to obtain them. Then the custom build action can just point to their tools in the vcpkg installed directory, e.g., "$(_ZVcpkgCurrentInstalledDir)tools\protobuf\protoc.exe"

The problem comes when using manifest mode. The targets in vcpkg.targets are specified with BeforeTargets=ClCompile, which means that the manifest dependencies are installed after any files with custom build tools specified are processed. That is, in a clean setup where vcpkg has not installed anything, a .proto file will fail to be processed by the protobuf or grpc tools that are specified, because those tools do not have a chance to be installed until after the .proto file is compiled

Simply changing VcpkgInstallManifestDependencies and VcpkgTripletSelection in vcpkg.targets to have BeforeTargets="CustomBuild" BeforeTargets="CustomBuild;ClCompile"addresses the issue. Note: I updated from the strikethrough to include both ClCompile and CustomBuild, since not specifying ClCompile breaks the most common usage :-P . But even this isn't great, as it doesn't cover other build actions, e.g., CudaBuild as hit by @dahubley in #20884 .

Is there any reason to not make this change in vcpkg.targets? I'm happy to submit the PR, but I'm not an expert in msbuild and the targets, so I'm not 100% sure that won't have any side effects.

Environment

  • OS: Windows
  • Compiler: Visual Studio 2019

To Reproduce
Steps to reproduce the behavior:

  1. .Get vcpkg and run .\bootstrap_vcpkg.bat, and then .\vcpkg.exe integrate install
  2. Create a new console app project and solution.
  3. Set the new project to use manifests in the vcpkg project properties.
  4. Create a vcpkg.json manifest next to the solution with the contents:
{
    "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json",
    "name": "sample-manifest",
    "version": "0.1",
    "dependencies": [
        {
            "name": "presentmon",
            "features": ["tools"]
        }
    ]
}
  1. Add a text file to your project. Right click on the text file and choose properties. Change the item type to "Custom Build Tool." Hit ok.
  2. Right click on the text file and choose to compile. When it completes, notice that there is not yet a vcpkg_installed\x86-windows\... hierarchy next to vcpkg.json or any mention in the compile output about vcpkg installing dependencies. This is the bug/issue - because custom build tool items don't cause vcpkg dependencies to install, they can't reliably use them.
  3. Right click on the console application cpp file and choose to compile. Notice the vcpkg manifest dependencies are now installed.

Expected behavior
Install manifest dependencies before any custom build items run

Failure logs
N/A

Additional context
N/A

@PhoebeHui PhoebeHui added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Oct 19, 2021
@PhoebeHui
Copy link
Contributor

@vicroms, could you help double confirm?

@PhoebeHui PhoebeHui added the info:manifests This PR or Issue pertains to the Manifests feature label Oct 19, 2021
@aggieNick02
Copy link
Contributor Author

A little bit related if you have thoughts on this as well: #20846 (It would be really nice if bootstrap_vcpkg.bat was lightweight enough that it did nothing when vcpkg.exe was up to date, so that it could always be called, either manually or by the vcpkg.targets manifest installation step)

@ghost
Copy link

ghost commented Oct 21, 2021

I think this is the same issue as #20884.

@aggieNick02
Copy link
Contributor Author

Investigating the bug filed by @dahubley made me realize my initial proposed fix is no good. I'm updating my report above with more details.

Even after the issue with my original fix is corrected, using BeforeTargets to do the vcpkg targets seems problematic because it has to be updated for each kind of build action, e.g., ClCompile, CustomBuild, CudaBuild. We can add CustomBuild and CudaBuild, but this doesn't cover everything.

@microsoft microsoft locked and limited conversation to collaborators Nov 22, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:manifests This PR or Issue pertains to the Manifests feature
Projects
None yet
Development

No branches or pull requests

3 participants