Skip to content

[vcpkg] Fix external include path#21544

Merged
BillyONeal merged 2 commits intomicrosoft:masterfrom
RT2Code:external-include-path
Nov 26, 2021
Merged

[vcpkg] Fix external include path#21544
BillyONeal merged 2 commits intomicrosoft:masterfrom
RT2Code:external-include-path

Conversation

@RT2Code
Copy link
Copy Markdown
Contributor

@RT2Code RT2Code commented Nov 19, 2021

My external include path change #18820 was broken by an unfortunate merge. The definition of the external include path relies on the _ZVcpkgCurrentInstalledDir property and therefore must be defined after it.

@Neumann-A
Copy link
Copy Markdown
Contributor

Still wondering if the integration in vs2015 is ok with this?

@jasjuang
Copy link
Copy Markdown
Contributor

After I apply the diff of this PR, is it supposed to just work? The vcpkg include directories, e.g. vcpkg\installed\x64-windows\include still shows up in "Additional Include Directories" instead of "External Include Directories" for me. The bare minimal example to reproduce this is

CMakeLists.txt

set(CMAKE_TOOLCHAIN_FILE $ENV{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake)

cmake_minimum_required(VERSION 3.20)

project(example)

find_package(CGAL REQUIRED)

add_executable(${PROJECT_NAME} main.cpp)

target_link_libraries(${PROJECT_NAME} PUBLIC CGAL::CGAL)

main.cpp

int main() { return 0; }
mkdir build && cd build
cmake -A x64 ..

Double click example.sln and right click to look at the properties.

I also ran bootstrap-vcpkg.bat but nothing changes, I do notice it is downloading tools from 11/15. Is this the reason why it's not working? Or am I totally misunderstanding the purpose of this PR? It will be phenomenal if vcpkg installed third-party libraries can be treated as /external. A -isystem equivalent for Windows is really sorely needed.

@Neumann-A
Copy link
Copy Markdown
Contributor

@jasjuang msbuild and cmake integration are two different things. This pr is about the msbuild integration. What you see is probably solved with cmake 3.22. It is a cmake issue.

@jasjuang
Copy link
Copy Markdown
Contributor

@Neumann-A thanks for the clarification, but I think it's not solved in cmake yet https://gitlab.kitware.com/cmake/cmake/-/issues/17904

@JackBoosY JackBoosY added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Nov 22, 2021
Comment thread scripts/buildsystems/msbuild/vcpkg.targets Outdated
@BillyONeal BillyONeal added requires:author-response and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Nov 22, 2021
Co-authored-by: Billy O'Neal <bion@microsoft.com>
@JackBoosY JackBoosY added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Nov 24, 2021
@BillyONeal BillyONeal merged commit 989f559 into microsoft:master Nov 26, 2021
@BillyONeal
Copy link
Copy Markdown
Member

Thanks for the fix and confirmation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants