Skip to content
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

Isolate VulkanHppGenerator and VideoHppGenerator in CMakeLists.txt #2037

Open
M2-TE opened this issue Jan 8, 2025 · 4 comments · May be fixed by #2042
Open

Isolate VulkanHppGenerator and VideoHppGenerator in CMakeLists.txt #2037

M2-TE opened this issue Jan 8, 2025 · 4 comments · May be fixed by #2042

Comments

@M2-TE
Copy link

M2-TE commented Jan 8, 2025

When adding the vulkan-hpp CMake project (e.g. through FetchContent), people may sometimes want to merely access the headers under vulkan/ and therefore only need access to the interface library providing the correct include directory:

FetchContent_Declare(vulkan-hpp
    GIT_REPOSITORY "https://github.com/KhronosGroup/Vulkan-Hpp.git"
    GIT_TAG "v1.4.303"
    GIT_SHALLOW ON
    GIT_SUBMODULES ""
    OVERRIDE_FIND_PACKAGE
    EXCLUDE_FROM_ALL
    SYSTEM)
FetchContent_MakeAvailable(vulkan-hpp)

As the submodules are not necessary for this, the VulkanHppGenerator and VideoHppGenerator targets will not be set up properly (even when not used), as the TINYXML2_SOURCES are missing, prompting CMake to complain about not knowing what language the target should be in:

Vulkan-Hpp/CMakeLists.txt

Lines 366 to 383 in 7527784

# The generator executable
add_executable( VulkanHppGenerator VulkanHppGenerator.cpp VulkanHppGenerator.hpp XMLHelper.hpp ${TINYXML2_SOURCES} ${TINYXML2_HEADERS} )
vulkan_hpp__setup_warning_level( NAME VulkanHppGenerator )
target_compile_definitions( VulkanHppGenerator PUBLIC BASE_PATH="${CMAKE_CURRENT_SOURCE_DIR}" VK_SPEC="${vk_spec}" )
target_include_directories( VulkanHppGenerator PRIVATE ${VULKAN_HPP_TINYXML2_SRC_DIR} )
set_target_properties( VulkanHppGenerator PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON )
if( UNIX )
target_link_libraries( VulkanHppGenerator PUBLIC pthread )
endif()
# The video generator executable
add_executable( VideoHppGenerator VideoHppGenerator.cpp VideoHppGenerator.hpp XMLHelper.hpp ${TINYXML2_SOURCES} ${TINYXML2_HEADERS} )
vulkan_hpp__setup_warning_level( NAME VideoHppGenerator )
file( TO_NATIVE_PATH ${VulkanRegistry_DIR}/video.xml video_spec )
string( REPLACE "\\" "\\\\" video_spec ${video_spec} )
target_compile_definitions( VideoHppGenerator PUBLIC BASE_PATH="${CMAKE_CURRENT_SOURCE_DIR}" VIDEO_SPEC="${video_spec}" )
target_include_directories( VideoHppGenerator PRIVATE ${VULKAN_HPP_TINYXML2_SRC_DIR} )
set_target_properties( VideoHppGenerator PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON )

Fetching as shown will result in the following error during configuration:

[cmake] CMake Error at build/_deps/vulkan-hpp-src/CMakeLists.txt:367 (add_executable):
[cmake]   Cannot find source file:
[cmake] 
[cmake]     build/_deps/vulkan-hpp-src/tinyxml2/tinyxml2.cpp
[cmake] 
[cmake]   Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .ixx .cppm
[cmake]   .ccm .cxxm .c++m .h .hh .h++ .hm .hpp .hxx .in .txx .f .F .for .f77 .f90
[cmake]   .f95 .f03 .hip .ispc
[cmake] 
[cmake] 
[cmake] CMake Error at build/_deps/vulkan-hpp-src/CMakeLists.txt:367 (add_executable):
[cmake]   No SOURCES given to target: VulkanHppGenerator
[cmake] 
[cmake] 
[cmake] CMake Error at build/_deps/vulkan-hpp-src/CMakeLists.txt:374 (add_executable):
[cmake]   No SOURCES given to target: VideoHppGenerator
[cmake] 
[cmake] 
[cmake] CMake Generate step failed.  Build files cannot be regenerated correctly.

It would be nice if these two targets could be hidden behind an if condition or some function, making this repo a convenient way to fetch a specific header version, rather than always generate the bindings.
The current solution is to set SOURCE_SUBDIR "disabled" to disable the cmake configuration for this project and adding vulkan-hpp_SOURCE_DIR as an include directory, which works but is not as nice as having access to an INTERFACE target, especially when wanting to use the c++20 module

@asuessenbach
Copy link
Contributor

@M2-TE You mean, something like

option( VULKAN_HPP_BUILD_GENERATOR "Build the HPP generator" ON )

and

if( VULKAN_HPP_BUILD_GENERATOR )
	# The generator executable
        ...

	# The video generator executable
        ...
endif()

Would you be so kind and test if that works with your situation?

@M2-TE
Copy link
Author

M2-TE commented Jan 9, 2025

That works perfectly well for hiding both targets.

As for testing linking against the header target, how is vulkan_hpp__setup_library supposed to be used? I only wish to have access to the interface target, for which setting a TARGET_FOLDER seems unnecessary. Or at least, I'm not exactly sure what the parameter will do when I am merely including the headers.
Apart from the target itself, it should probably also provide an ALIAS target, similar to how vulkan-headers does it with Vulkan::Headers (very good as CMake will catch missing aliased targets when linking against them).

I could open a PR about these if you want

@asuessenbach
Copy link
Contributor

I could open a PR about these if you want

Would be great!

@asuessenbach
Copy link
Contributor

how is vulkan_hpp__setup_library supposed to be used?

That helper function is used with the two libraries generated in this repo: utils and RAII_utils. It just sets up the common stuff for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants