KhronosGroup / Vulkan-Samples

One stop solution for all Vulkan samples
Apache License 2.0
4.33k stars 647 forks source link

cmake_minimum_required insufficient for used features #1100

Closed MarkCallow closed 3 months ago

MarkCallow commented 4 months ago

The root CMakeLists.txt specifies cmake_minimum_required(VERSION 3.16) right before calling FindVulkan. However FindVulkan, when finding a framework (such as for iOS), sets IMPORTED_LOCATION to the name of the framework directory because that is what ${Vulkan_LIBRARIES} expands to.

if(Vulkan_FOUND AND NOT TARGET Vulkan::Vulkan)
    add_library(Vulkan::Vulkan UNKNOWN IMPORTED)
    set_target_properties(Vulkan::Vulkan PROPERTIES
            IMPORTED_LOCATION "${Vulkan_LIBRARIES}"
            INTERFACE_INCLUDE_DIRECTORIES "${Vulkan_INCLUDE_DIRS}")
endif()

This is only supported in CMake 3.28+. Prior to that IMPORTED_LOCATION had to be set to the library just inside the framework directory. When using a pre 3.28 version of CMake this leads to a totally obscure error from ld

ld: file cannot be mmap()ed, errno=22 path=<...>/VulkanSDK/1.3.283.0/iOS/lib/vulkan.framework

3.28 is a very recent version of CMake so I think the appropriate fix is to change FindVulkan to handle versions pre and post 3.28.

Furthermore app/CMakeLists.txt also sets the minimum version to 3.16 but it uses XCODE_EMBED_FRAMEWORKS_CODE_SIGN_ON_COPY which was only introduced in CMake 3.20.

I will submit a PR to fix the above that changes the minimum version to 3.20 in both and fixes FindVulkan in the way described.

Note that many other places in this repo set even earlier minimum versions than 3.16. I do not know if any of those should be bumped.

gpx1000 commented 3 months ago

Thanks for finding this. Your PR does indeed fix this issue.