Skip to content

Commit

Permalink
Ensure all header files shipped have the vk_ prefix
Browse files Browse the repository at this point in the history
CI will fail now if we ship a .h / .hpp file that doesn't use
the correct prefix.

I also documented the precedent for this prefix and the existing
integration testing.
  • Loading branch information
juan-lunarg committed Sep 8, 2023
1 parent a3235b3 commit 6710b67
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 53 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
cmake --build tests/find_package/build --config ${{matrix.config}} --verbose
- name: Test add_subdirectory support
run: |
cmake -S tests/add_subdirectory -B tests/add_subdirectory/build -D CMAKE_BUILD_TYPE=${{matrix.config}} -D GITHUB_VULKAN_HEADER_SOURCE_DIR=${{ github.workspace }}/external/${{matrix.config}}/Vulkan-Headers/
cmake -S tests/add_subdirectory -B tests/add_subdirectory/build -D CMAKE_BUILD_TYPE=${{matrix.config}} -D VULKAN_HEADER_SOURCE_DIR=${{ github.workspace }}/external/${{matrix.config}}/Vulkan-Headers/
cmake --build tests/add_subdirectory/build --config ${{matrix.config}} --verbose
android:
Expand Down
2 changes: 1 addition & 1 deletion BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ static_library("vulkan_layer_settings") {
"include/vulkan/layer/vk_layer_settings.h",
"include/vulkan/layer/vk_layer_settings.hpp",
"include/vulkan/layer/vk_layer_settings_ext.h",
"include/vulkan/utility/vul_dispatch_table.h",
"include/vulkan/utility/vk_dispatch_table.h",
"include/vulkan/vk_enum_string_helper.h",
"src/layer/layer_settings_manager.cpp",
"src/layer/layer_settings_manager.hpp",
Expand Down
2 changes: 1 addition & 1 deletion docs/generated_code.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ If only dealing with a single file, run `scripts/generate_source.py` with `--ta

```bash
# Example - only generates chassis.h
scripts/generate_source.py external/Vulkan-Headers/registry/ --target vul_dispatch_table.h
scripts/generate_source.py external/Vulkan-Headers/registry/ --target vk_dispatch_table.h
```

When making change to the `scripts/` folder, make sure to run `generate_source.py`
Expand Down
2 changes: 1 addition & 1 deletion include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ lunarg_target_compiler_configurations(VulkanUtilityHeaders VUL_WERROR)
# https://cmake.org/cmake/help/latest/release/3.19.html#other
if (CMAKE_VERSION VERSION_GREATER_EQUAL "3.19")
target_sources(VulkanUtilityHeaders PRIVATE
vulkan/utility/vul_dispatch_table.h
vulkan/utility/vk_dispatch_table.h
vulkan/vk_enum_string_helper.h
)
endif()
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion scripts/generate_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def RunGenerators(api: str, registry: str, targetFilter: str) -> None:

# Build up a list of all generators and custom options
generators = {
'vul_dispatch_table.h' : {
'vk_dispatch_table.h' : {
'generator' : DispatchTableOutputGenerator,
'directory' : 'include/vulkan/utility',
},
Expand Down
2 changes: 1 addition & 1 deletion tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
# SPDX-License-Identifier: Apache-2.0
add_subdirectory(layer)
add_subdirectory(generated)
add_subdirectory(vul_dispatch_table)
add_subdirectory(vk_dispatch_table)
41 changes: 41 additions & 0 deletions tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!--
Copyright 2023 The Khronos Group Inc.
Copyright 2023 Valve Corporation
Copyright 2023 LunarG, Inc.
SPDX-License-Identifier: Apache-2.0
-->

# Library integration testing

In order to avoid disruption of downstream users. It's important to test how this
repository is consumed.

1. Self contained headers

It's easy to write header files that aren't self contained. By compiling
a single source file that includes a single header we ensure a smooth experience for
downstream users.

2. Ensure C compatibility of C header files

It's VERY easy to write invalid C code. Especially for experience C++ programmers.

## tests/find_package

Test find_package support. The intent is to ensure we properly install files.

Used by system/language package managers and the Vulkan SDK packaging.

## tests/add_subdirectory

1. Test add_subdirectory support

While we don't have to support add_subdirectory it is a common feature request for CMake projects.

2. Ensure file name consistency of header files we install

All header files we ship will have the `vk_` prefix

This convention was originally established in VulkanHeaders for files created by LunarG.
- EX: `vk_icd.h`, `vk_layer.h`, `vk_platform.h`
53 changes: 40 additions & 13 deletions tests/add_subdirectory/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,55 @@ cmake_minimum_required(VERSION 3.17)

project(TEST_ADD_SUBDIRECTORY LANGUAGES C)

add_library(add_subdirectory_example STATIC)
if (NOT DEFINED VULKAN_HEADER_SOURCE_DIR)
message(FATAL_ERROR "VULKAN_HEADER_SOURCE_DIR not defined!")
endif()

# Test c99 support
target_compile_features(add_subdirectory_example PRIVATE c_std_99)
add_subdirectory(${VULKAN_HEADER_SOURCE_DIR} ${PROJECT_BINARY_DIR}/headers)

target_sources(add_subdirectory_example PRIVATE
client.c
vk_enum_string_helper.c
vul_dispatch_table.c
)
add_subdirectory(${PROJECT_SOURCE_DIR}/../../ ${PROJECT_BINARY_DIR}/vul)

# NOTE: Because VulkanHeaders is a PUBLIC dependency it needs to be found prior to VulkanUtilityLibraries
add_subdirectory(${GITHUB_VULKAN_HEADER_SOURCE_DIR} ${PROJECT_BINARY_DIR}/headers)
get_filename_component(VUL_INCLUDE_DIR "${PROJECT_SOURCE_DIR}/../../include/" ABSOLUTE)

add_subdirectory(${PROJECT_SOURCE_DIR}/../../ ${PROJECT_BINARY_DIR}/vul)
# Get all the header files we SHIP
file(GLOB_RECURSE public_headers
CONFIGURE_DEPENDS
"${VUL_INCLUDE_DIR}/*.h" "${VUL_INCLUDE_DIR}/*.hpp"
)

# Ensure each file has the same `vk_` prefix naming convention.
foreach(header IN LISTS public_headers)
get_filename_component(header ${header} "NAME")
message(DEBUG "Checking ${header}")
if (${header} MATCHES "^vk_")
message(DEBUG "Correct naming convention!")
else()
# EX: "Invalid file name! vk_dispatch_table.h"
message(FATAL_ERROR "Invalid file name! ${header}")
endif()
endforeach()

# The intent is ensuring we don't accidentally change the names of these
# targets. Since it would break downstream users.
if (NOT TARGET Vulkan::LayerSettings)
message(FATAL_ERROR "Vulkan::LayerSettings target not defined!")
endif()

if (NOT TARGET Vulkan::UtilityHeaders)
message(FATAL_ERROR "Vulkan::UtilityHeaders target not defined!")
endif()

target_link_libraries(add_subdirectory_example PRIVATE Vulkan::LayerSettings Vulkan::UtilityHeaders)
add_library(add_subdirectory_example STATIC)

# Test c99 support
target_compile_features(add_subdirectory_example PRIVATE c_std_99)

target_sources(add_subdirectory_example PRIVATE
vk_dispatch_table.c
vk_enum_string_helper.c
vk_layer_settings.c
)

target_link_libraries(add_subdirectory_example PRIVATE
Vulkan::LayerSettings
Vulkan::UtilityHeaders
)
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@
// Copyright 2023 LunarG, Inc.
//
// SPDX-License-Identifier: Apache-2.0
#include <vulkan/utility/vul_dispatch_table.h>
#include <vulkan/utility/vk_dispatch_table.h>

PFN_vkVoidFunction local_vkGetInstanceProcAddr(VkInstance instance, const char *pName) {
PFN_vkVoidFunction VKAPI_PTR local_vkGetInstanceProcAddr(VkInstance instance, const char *pName) {
(void)instance;
(void)pName;
return NULL;
}

PFN_vkVoidFunction local_vkGetDeviceProcAddr(VkDevice device, const char *pName) {
PFN_vkVoidFunction VKAPI_PTR local_vkGetDeviceProcAddr(VkDevice device, const char *pName) {
(void)device;
(void)pName;
return NULL;
}

void foobar() {
void vk_dispatch_table() {
VulDeviceDispatchTable device_dispatch_table;
VulInstanceDispatchTable instance_dispatch_table;

Expand Down
2 changes: 1 addition & 1 deletion tests/add_subdirectory/vk_enum_string_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <vulkan/vk_enum_string_helper.h>

// Ensure vk_enum_string_helper.h can be compiled with a C compiler
const char* foobar() { return string_VkResult(VK_SUCCESS); }
const char* string_VkResult_compiles() { return string_VkResult(VK_SUCCESS); }

// Ensure string_VkPipelineStageFlagBits2 is callable by C users
const char* vk_format_feature_2_sampled_image_bit() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// SPDX-License-Identifier: Apache-2.0
#include <vulkan/layer/vk_layer_settings.h>

VkBool32 foobar() {
VkBool32 vk_layer_settings() {
VlLayerSettingSet layerSettingSet = VK_NULL_HANDLE;
vlCreateLayerSettingSet("VK_LAYER_LUNARG_test", NULL, NULL, NULL, &layerSettingSet);

Expand Down
4 changes: 2 additions & 2 deletions tests/find_package/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ add_library(find_package_example STATIC)
target_compile_features(find_package_example PRIVATE c_std_99)

target_sources(find_package_example PRIVATE
${CMAKE_CURRENT_LIST_DIR}/../add_subdirectory/client.c
${CMAKE_CURRENT_LIST_DIR}/../add_subdirectory/vk_layer_settings.c
${CMAKE_CURRENT_LIST_DIR}/../add_subdirectory/vk_enum_string_helper.c
${CMAKE_CURRENT_LIST_DIR}/../add_subdirectory/vul_dispatch_table.c
${CMAKE_CURRENT_LIST_DIR}/../add_subdirectory/vk_dispatch_table.c
)

# NOTE: Because VulkanHeaders is a PUBLIC dependency it needs to be found prior to VulkanUtilityLibraries
Expand Down
24 changes: 24 additions & 0 deletions tests/vk_dispatch_table/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright 2023 The Khronos Group Inc.
# Copyright 2023 Valve Corporation
# Copyright 2023 LunarG, Inc.
#
# SPDX-License-Identifier: Apache-2.0
set(CMAKE_FOLDER "${CMAKE_FOLDER}/VulkanUtilityHeaders/tests")

find_package(GTest REQUIRED CONFIG)

include(GoogleTest)

add_executable(test_vk_dispatch_table test_interface.cpp)

lunarg_target_compiler_configurations(test_vk_dispatch_table VUL_WERROR)

target_link_libraries(test_vk_dispatch_table PRIVATE
GTest::gtest
GTest::gtest_main
Vulkan::UtilityHeaders
)

target_include_directories(test_vk_dispatch_table PRIVATE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>)

gtest_discover_tests(test_vk_dispatch_table)
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include <gtest/gtest.h>

#include <vulkan/utility/vul_dispatch_table.h>
#include <vulkan/utility/vk_dispatch_table.h>

// Only exists so that local_vkGetDeviceProcAddr can return a 'real' function pointer
inline void empty_func() {}
Expand All @@ -27,7 +27,7 @@ inline PFN_vkVoidFunction local_vkGetDeviceProcAddr(VkDevice device, const char
return reinterpret_cast<PFN_vkVoidFunction>(&empty_func);
}

TEST(test_vul_dispatch_table, cpp_interface) {
TEST(test_vk_dispatch_table, cpp_interface) {
VulDeviceDispatchTable device_dispatch_table{};
VulInstanceDispatchTable instance_dispatch_table{};

Expand Down
24 changes: 0 additions & 24 deletions tests/vul_dispatch_table/CMakeLists.txt

This file was deleted.

0 comments on commit 6710b67

Please sign in to comment.