stevemk14ebr / PolyHook_2_0

C++20, x86/x64 Hooking Libary v2.0
MIT License
1.6k stars 227 forks source link

Including Polyhook 2 in CMake via vcpkg results in "`PolyHookOs.hpp` not found" errors #112

Closed acidicoala closed 2 years ago

acidicoala commented 2 years ago

I tried including this library in a CMake project via vcpkg in the same manner as I do other libraries:

find_package(PolyHook_2 CONFIG REQUIRED)

target_link_libraries(
        ${CMAKE_PROJECT_NAME} PRIVATE
        PolyHook_2::PolyHook_2
)

vcpkg is added a project submodule, so it is not global. The library is included statically via x64-windows-static triplet.

However, this results in a weird setup. All the headers include #include "polyhook2/PolyHookOs.hpp" line, which unfortunately breaks the build since PolyHookOs.hpp is not present in the include directory. It would seem that the PolyHookOs.hpp header somehow got left behind. I would appreciate any help with this issue.

I should also add that this issue occurs only if the vcpkg includes the most recent PolyHook update. Checking out older vcpkg commit ( < 3 Feb) does not produce this issue.

stevemk14ebr commented 2 years ago

The last update merged support for linux, that added PolyhookOs.hpp and PolyhookOsIncludes.hpp and their sources. Looks like they're missing here: https://github.com/stevemk14ebr/PolyHook_2_0/blob/951ab526b403c3d023ce265ff156a1c26c9cb7e4/CMakeLists.txt#L208 which might be the cause.

EDIT: please update the vcpkg portfile you have locally to use this commit (update the hash too by trying to install and copying expected hash) and retry the install https://github.com/stevemk14ebr/PolyHook_2_0/commit/5b3281e65a1377868ce76254c02a7dae2f36d1bf

acidicoala commented 2 years ago

To be honest, I never edited local vcpkg port files, so I don't know how. I am just using vcpkg.json manifest and let CMake configure it all automatically. The build also needs to be reproducible on CI, so I would have to automate those local fixes there as well. For now I am using older vcpkg commit hash that doesn't include the breaking changes. But I am looking forward to seeing those fixes merged in the main vcpkg branch. Thank you very much for looking into this.

stevemk14ebr commented 2 years ago

Ok no worries I will push a new vcpkg and CI wont be an issue, if you can though I need you to verify this fix works in your setup before I push the new vcpkg build to everyone. The portfile is easy to fix locally, please go to vcpkg/ports/polyhook2/portfile.cmake and replace the start of the file with this:

vcpkg_from_github(
    OUT_SOURCE_PATH SOURCE_PATH
    REPO stevemk14ebr/PolyHook_2_0
    REF  830213d6bfec62f14d31db92ad86adc6d03a4f81
    SHA512 44a9dcba0be0c81d83d9982de9cd020d86ab54389ae431c74631f054e5bdd269a5056cf25abfaf8b445a2184205fd18c441d3ad67900eb3e7651ec1efd2c7639
    HEAD_REF master
)

Then uninstall and install polyhook using vcpkg. This will use the latest commit of this repo and hopefully build. If this is successful for you I will update vcpkg.

acidicoala commented 2 years ago

Thank you for providing instructions. Applying this fix has fixed the build error with latest vcpkg.

There is only a warning left, which occurs only when I include polyhook2: LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library I suppose it is related to mismatched static & dynamic libraries. I typically configure the project to use static linking in the following manner:

set(VCPKG_TARGET_TRIPLET x64-windows-static CACHE STRING "VCPKG Target Triplet to use")
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")

Is there some option/variable that needs to be configured to resolve the warning? I know that polyhook2 includes dependencies of its own, so my guess is that one of them end up being misconfigured.

stevemk14ebr commented 2 years ago

Not sure which package that is from TBH and it may be related to you specifying the settings in that way this isn't a configuration I use - if you know a fix I welcome a PR otherwise I'm ignoring that. The main fix is pending: https://github.com/microsoft/vcpkg/pull/23044