From 8d23122189e14574715eca85e7ffa8981c173286 Mon Sep 17 00:00:00 2001 From: Juan Ramos <juan@lunarg.com> Date: Tue, 20 Jun 2023 14:32:16 -0600 Subject: [PATCH] cmake: Cleanup CMake Pulling in latest changes from VVL Use BUILD_TESTS instead of VUL_TESTS. While it's good practice to prefix variable names it makes updating this code more difficult. Also since tests can only ever be enabled if the project is top level, it doesn't affect add_subdirectory users. Other misc. CMake cleanup --- .github/workflows/vul.yml | 6 +-- BUILD.md | 2 +- CMakeLists.txt | 4 +- scripts/CMakeLists.txt | 99 ++++++++++++++++++++++++-------------- scripts/known_good.json | 2 +- scripts/update_deps.py | 36 ++++++++++---- tests/layer/CMakeLists.txt | 10 +--- 7 files changed, 99 insertions(+), 60 deletions(-) diff --git a/.github/workflows/vul.yml b/.github/workflows/vul.yml index f549528..700012c 100644 --- a/.github/workflows/vul.yml +++ b/.github/workflows/vul.yml @@ -35,7 +35,7 @@ jobs: with: python-version: '3.8' - name: Configure - run: cmake -S. -B build -D VUL_WERROR=ON -D VUL_TESTS=ON -D CMAKE_BUILD_TYPE=${{matrix.config}} -D UPDATE_DEPS=ON + run: cmake -S. -B build -D VUL_WERROR=ON -D BUILD_TESTS=ON -D CMAKE_BUILD_TYPE=${{matrix.config}} -D UPDATE_DEPS=ON - name: Build run: cmake --build build --config ${{matrix.config}} --verbose - name: Tests @@ -45,9 +45,9 @@ jobs: run: cmake --install build --prefix ${{ github.workspace }}/install --config ${{matrix.config}} - name: Test find_package support run: | - cmake -S tests/find_package -B tests/find_package/build -D CMAKE_PREFIX_PATH="${{ github.workspace }}/install;${{ github.workspace }}/external/Vulkan-Headers/build/install" -D CMAKE_BUILD_TYPE=${{matrix.config}} + cmake -S tests/find_package -B tests/find_package/build -D CMAKE_PREFIX_PATH="${{ github.workspace }}/install;${{ github.workspace }}/external/${{matrix.config}}/Vulkan-Headers/build/install" -D CMAKE_BUILD_TYPE=${{matrix.config}} 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/Vulkan-Headers/ + 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 --build tests/add_subdirectory/build --config ${{matrix.config}} --verbose diff --git a/BUILD.md b/BUILD.md index 76ceb6a..b72ee69 100644 --- a/BUILD.md +++ b/BUILD.md @@ -27,7 +27,7 @@ cmake --build build --config Debug ### Recommended setup for developers ```bash -cmake -S . -B build/ -D VUL_WERROR=ON -D VUL_TESTS=ON -D CMAKE_BUILD_TYPE=Debug -D UPDATE_DEPS=ON +cmake -S . -B build/ -D VUL_WERROR=ON -D BUILD_TESTS=ON -D UPDATE_DEPS=ON -D CMAKE_BUILD_TYPE=Debug ``` ### Unit Tests diff --git a/CMakeLists.txt b/CMakeLists.txt index 9de2c31..e3d109f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,8 +45,8 @@ add_subdirectory(src) add_subdirectory(include) if (VUL_IS_TOP_LEVEL) - option(VUL_TESTS "Build tests") - if (VUL_TESTS) + option(BUILD_TESTS "Build tests") + if (BUILD_TESTS) enable_testing() add_subdirectory(tests) endif() diff --git a/scripts/CMakeLists.txt b/scripts/CMakeLists.txt index 81690c7..4ce30b9 100644 --- a/scripts/CMakeLists.txt +++ b/scripts/CMakeLists.txt @@ -1,3 +1,4 @@ +# ~~~ # Copyright (c) 2023 LunarG, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -11,18 +12,24 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +# ~~~ option(UPDATE_DEPS "Run update_deps.py for user") if (UPDATE_DEPS) - find_package(PythonInterp 3 REQUIRED) + find_package(Python3 REQUIRED QUIET) + + set(update_dep_py "${CMAKE_CURRENT_LIST_DIR}/update_deps.py") + set(known_good_json "${CMAKE_CURRENT_LIST_DIR}/known_good.json") + + set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${update_dep_py} ${known_good_json}) + + list(APPEND update_dep_command "${update_dep_py}") + list(APPEND update_dep_command "--generator") + list(APPEND update_dep_command "${CMAKE_GENERATOR}") if (CMAKE_GENERATOR_PLATFORM) - set(_target_arch ${CMAKE_GENERATOR_PLATFORM}) - else() - if (MSVC_IDE) - message(WARNING "CMAKE_GENERATOR_PLATFORM not set. Using x64 as target architecture.") - endif() - set(_target_arch x64) + list(APPEND update_dep_command "--arch") + list(APPEND update_dep_command "${CMAKE_GENERATOR_PLATFORM}") endif() if (NOT CMAKE_BUILD_TYPE) @@ -31,51 +38,69 @@ if (UPDATE_DEPS) else() set(_build_type ${CMAKE_BUILD_TYPE}) endif() + list(APPEND update_dep_command "--config") + list(APPEND update_dep_command "${_build_type}") - set(optional_args) - if (NOT VUL_TESTS) - set(optional_args "--optional=tests") + set(UPDATE_DEPS_DIR_SUFFIX "${_build_type}") + if (ANDROID) + set(UPDATE_DEPS_DIR_SUFFIX "android/${UPDATE_DEPS_DIR_SUFFIX}") + endif() + set(UPDATE_DEPS_DIR "${PROJECT_SOURCE_DIR}/external/${UPDATE_DEPS_DIR_SUFFIX}" CACHE PATH "Location where update_deps.py installs packages") + list(APPEND update_dep_command "--dir" ) + list(APPEND update_dep_command "${UPDATE_DEPS_DIR}") + + if (NOT BUILD_TESTS) + list(APPEND update_dep_command "--optional=tests") endif() if (UPDATE_DEPS_SKIP_EXISTING_INSTALL) - set(optional_args ${optional_args} "--skip-existing-install") + list(APPEND update_dep_command "--skip-existing-install") endif() - if (DEFINED CMAKE_TOOLCHAIN_FILE) - set(optional_args ${optional_args} "--cmake_var") - set(optional_args ${optional_args} "CMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}") + list(APPEND cmake_vars "CMAKE_TOOLCHAIN_FILE") + if (ANDROID) + list(APPEND cmake_vars "ANDROID_PLATFORM" "CMAKE_ANDROID_ARCH_ABI" "CMAKE_ANDROID_STL_TYPE" "CMAKE_ANDROID_RTTI" "CMAKE_ANDROID_EXCEPTIONS" "ANDROID_USE_LEGACY_TOOLCHAIN_FILE") endif() - # Add a target so that update_deps.py will run when necessary - # NOTE: This is triggered off of the timestamps of known_good.json and helper.cmake - add_custom_command( - OUTPUT ${PROJECT_SOURCE_DIR}/external/helper.cmake - COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_LIST_DIR}/update_deps.py - --dir ${PROJECT_SOURCE_DIR}/external --arch ${_target_arch} --config ${_build_type} --generator "${CMAKE_GENERATOR}" ${optional_args} - DEPENDS ${CMAKE_CURRENT_LIST_DIR}/known_good.json - ) + set(cmake_var) + foreach(var IN LISTS cmake_vars) + if (DEFINED ${var}) + list(APPEND update_dep_command "--cmake_var") + list(APPEND update_dep_command "${var}=${${var}}") + endif() + endforeach() + if (cmake_var) + list(APPEND update_dep_command "${cmake_var}") + endif() - # Check if update_deps.py needs to be run on first cmake run - if (${CMAKE_CURRENT_LIST_DIR}/known_good.json IS_NEWER_THAN ${PROJECT_SOURCE_DIR}/external/helper.cmake) + file(TIMESTAMP ${update_dep_py} timestamp_1) + file(TIMESTAMP ${known_good_json} timestamp_2) + + string("MD5" md5_hash "${timestamp_1}-${timestamp_2}-${update_dep_command}") + + set(UPDATE_DEPS_HASH "0" CACHE STRING "Default value until we run update_deps.py") + mark_as_advanced(UPDATE_DEPS_HASH) + + if ("${UPDATE_DEPS_HASH}" STREQUAL "0") + list(APPEND update_dep_command "--clean-build") + list(APPEND update_dep_command "--clean-install") + endif() + + if ("${md5_hash}" STREQUAL $CACHE{UPDATE_DEPS_HASH}) + message(DEBUG "update_deps.py: no work to do.") + else() execute_process( - COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_LIST_DIR}/update_deps.py - --dir ${PROJECT_SOURCE_DIR}/external --arch ${_target_arch} --config ${_build_type} --generator "${CMAKE_GENERATOR}" ${optional_args} - WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} + COMMAND ${Python3_EXECUTABLE} ${update_dep_command} RESULT_VARIABLE _update_deps_result ) if (NOT (${_update_deps_result} EQUAL 0)) message(FATAL_ERROR "Could not run update_deps.py which is necessary to download dependencies.") endif() + set(UPDATE_DEPS_HASH ${md5_hash} CACHE STRING "Ensure we only run update_deps.py when we need to." FORCE) + include("${UPDATE_DEPS_DIR}/helper.cmake") endif() - include(${PROJECT_SOURCE_DIR}/external/helper.cmake) -else() - message("********************************************************************************") - message("* NOTE: Not adding target to run update_deps.py automatically. *") - message("********************************************************************************") - find_package(PythonInterp 3 QUIET) endif() -# Dependencies can be installed in arbitrary locations. This is done by update_deps.py / users. if (GOOGLETEST_INSTALL_DIR) list(APPEND CMAKE_PREFIX_PATH ${GOOGLETEST_INSTALL_DIR}) endif() @@ -83,4 +108,8 @@ if (VULKAN_HEADERS_INSTALL_DIR) list(APPEND CMAKE_PREFIX_PATH ${VULKAN_HEADERS_INSTALL_DIR}) endif() -set(CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH} PARENT_SCOPE) +if (CMAKE_CROSSCOMPILING) + set(CMAKE_FIND_ROOT_PATH ${CMAKE_FIND_ROOT_PATH} ${CMAKE_PREFIX_PATH} PARENT_SCOPE) +else() + set(CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH} PARENT_SCOPE) +endif() diff --git a/scripts/known_good.json b/scripts/known_good.json index acac28e..f8fda2a 100644 --- a/scripts/known_good.json +++ b/scripts/known_good.json @@ -19,7 +19,7 @@ "-Dgtest_force_shared_crt=ON", "-DBUILD_SHARED_LIBS=OFF" ], - "commit": "v1.12.0", + "commit": "v1.13.0", "optional": [ "tests" ] diff --git a/scripts/update_deps.py b/scripts/update_deps.py index 3882be6..86b857b 100644 --- a/scripts/update_deps.py +++ b/scripts/update_deps.py @@ -3,6 +3,7 @@ # Copyright 2017 The Glslang Authors. All rights reserved. # Copyright (c) 2018-2023 Valve Corporation # Copyright (c) 2018-2023 LunarG, Inc. +# Copyright (c) 2023-2023 RasterGrid Kft. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -31,11 +32,6 @@ this home repository depend on. It also checks out each dependent repository at a "known-good" commit in order to provide stability in the dependent repositories. -Python Compatibility --------------------- - -This program can be used with Python 2.7 and Python 3. - Known-Good JSON Database ------------------------ @@ -117,6 +113,10 @@ examples of all of these elements. The name of the dependent repository. This field can be referenced by the "deps.repo_name" structure to record a dependency. +- api + +The name of the API the dependency is specific to (e.g. "vulkan"). + - url Specifies the URL of the repository. @@ -233,8 +233,6 @@ option can be a relative or absolute path. """ -from __future__ import print_function - import argparse import json import os.path @@ -264,7 +262,7 @@ DEVNULL = open(os.devnull, 'wb') def on_rm_error( func, path, exc_info): """Error handler for recursively removing a directory. The shutil.rmtree function can fail on Windows due to read-only files. - This handler will change the permissions for tha file and continue. + This handler will change the permissions for the file and continue. """ os.chmod( path, stat.S_IWRITE ) os.unlink( path ) @@ -335,6 +333,7 @@ class GoodRepo(object): self.build_step = json['build_step'] if ('build_step' in json) else 'build' self.build_platforms = json['build_platforms'] if ('build_platforms' in json) else [] self.optional = set(json.get('optional', [])) + self.api = json['api'] if ('api' in json) else None # Absolute paths for a repo's directories dir_top = os.path.abspath(args.dir) self.repo_dir = os.path.join(dir_top, self.sub_dir) @@ -425,9 +424,11 @@ class GoodRepo(object): def CMakeConfig(self, repos): """Build CMake command for the configuration phase and execute it""" if self._args.do_clean_build: - shutil.rmtree(self.build_dir) + if os.path.isdir(self.build_dir): + shutil.rmtree(self.build_dir, onerror=on_rm_error) if self._args.do_clean_install: - shutil.rmtree(self.install_dir) + if os.path.isdir(self.install_dir): + shutil.rmtree(self.install_dir, onerror=on_rm_error) # Create and change to build directory make_or_exist_dirs(self.build_dir) @@ -579,6 +580,10 @@ def CreateHelper(args, repos, filename): install_names = GetInstallNames(args) with open(filename, 'w') as helper_file: for repo in repos: + # If the repo has an API tag and that does not match + # the target API then skip it + if repo.api is not None and repo.api != args.api: + continue if install_names and repo.name in install_names and repo.on_build_platform: helper_file.write('set({var} "{dir}" CACHE STRING "" FORCE)\n' .format( @@ -654,6 +659,12 @@ def main(): type=str.lower, help="Set build files configuration", default='debug') + parser.add_argument( + '--api', + dest='api', + default='vulkan', + choices=['vulkan'], + help="Target API") parser.add_argument( '--generator', dest='generator', @@ -685,6 +696,11 @@ def main(): print('Starting builds in {d}'.format(d=abs_top_dir)) for repo in repos: + # If the repo has an API tag and that does not match + # the target API then skip it + if repo.api is not None and repo.api != args.api: + continue + # If the repo has a platform whitelist, skip the repo # unless we are building on a whitelisted platform. if not repo.on_build_platform: diff --git a/tests/layer/CMakeLists.txt b/tests/layer/CMakeLists.txt index 75db17e..a25e9ff 100644 --- a/tests/layer/CMakeLists.txt +++ b/tests/layer/CMakeLists.txt @@ -16,6 +16,8 @@ set(CMAKE_FOLDER "${CMAKE_FOLDER}/VulkanLayerSettings/tests") find_package(GTest REQUIRED CONFIG) +include(GoogleTest) + # test_layer_setting_util add_executable(test_layer_settings_util) @@ -34,8 +36,6 @@ target_link_libraries(test_layer_settings_util PRIVATE Vulkan::LayerSettings ) -include(GoogleTest) - gtest_discover_tests(test_layer_settings_util) # test_layer_setting_api @@ -56,8 +56,6 @@ target_link_libraries(test_layer_settings_api PRIVATE Vulkan::LayerSettings ) -include(GoogleTest) - gtest_discover_tests(test_layer_settings_api) # test_layer_setting_env @@ -78,8 +76,6 @@ target_link_libraries(test_layer_settings_env PRIVATE Vulkan::LayerSettings ) -include(GoogleTest) - gtest_discover_tests(test_layer_settings_env) # test_layer_setting_file @@ -100,7 +96,5 @@ target_link_libraries(test_layer_setting_file PRIVATE Vulkan::LayerSettings ) -include(GoogleTest) - gtest_discover_tests(test_layer_setting_file)