Skip to content

Commit

Permalink
Add option to enable mkvk targets (VkFormat-related file generation). (
Browse files Browse the repository at this point in the history
…#840)

The option defaults to OFF. Only project developers need to regenerate
these files.

Fixes #834.

Other included related fixes:
* Modify mkvkformatfiles to write CRLF on Windows.
* Stop using unix2dos in mkvk custom commands and remove near
  duplicate WIN32 custom commands. Developers must ensure they
  use a Windows native Perl to get the correct line endings when
  generating dfdutils' `*.inl` files.
* Generate dfdutils .inl files from vulkan_core not vkformat_enum
  to remove dependency on the latter's generation.
* Add CI jobs to test generation on all platforms since main builds
  will no longer be building those targets.

Unrelated fixes:
* Set `HOMEBREW_NO_AUTO_UPDATE` for macOS Travis CI jobs
  to finally stop the auto update and shave more than 1 hour off the
  total build time.
* Bracket the whole `script` section of `.travis.yml` with `set -e` and
`set +e`
  instead of piecemeal in various `if` clauses.
  • Loading branch information
MarkCallow authored Jan 19, 2024
1 parent 7dedd7e commit e77a531
Show file tree
Hide file tree
Showing 7 changed files with 324 additions and 204 deletions.
21 changes: 18 additions & 3 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jobs:
os: [ windows-latest ]
toolset: [v143, CLangCL]
arch: [ x64 ]
check_mkvk: [ NO ]
options: [
{config: 'Debug,Release',
doc: ON, jni: ON, loadtests: OpenGL+Vulkan, py: ON, tests: ON, tools: ON, tools_cts: ON,
Expand All @@ -50,6 +51,11 @@ jobs:
sse: ON, opencl: ON}
]
include:
- os: windows-latest
generator: 'Visual Studio 17 2022'
toolset: CLangCL
arch: x64
check_mkvk: ONLY
- os: windows-2019
generator: 'Visual Studio 16 2019'
toolset: v142
Expand Down Expand Up @@ -82,6 +88,7 @@ jobs:
CMAKE_TOOLSET: ${{ matrix.toolset }}

ARCH: ${{ matrix.arch }}
CHECK_MKVK: ${{ matrix.check_mkvk }}
CONFIGURATION: ${{ matrix.options.config }}
FEATURE_DOC: ${{ matrix.options.doc }}
FEATURE_JNI: ${{ matrix.options.jni }}
Expand Down Expand Up @@ -122,6 +129,7 @@ jobs:
fetch-depth: 0

- name: Install NSIS with large string support
if: matrix.check_mkvk != 'ONLY'
shell: bash
run: |
retryCount=4
Expand All @@ -147,7 +155,7 @@ jobs:
- name: Force fetch provoking tag's annotation.
# Work around https://github.com/actions/checkout/issues/290.
if: github.ref_type == 'tag'
if: github.ref_type == 'tag' && matrix.check_mkvk != 'ONLY'
run: git fetch -f origin ${{ github.ref }}:${{ github.ref }}

# Need doxygen if docs are supposed to be built.
Expand Down Expand Up @@ -178,6 +186,7 @@ jobs:
}
- name: Install AzureSignTool
if: matrix.check_mkvk != 'ONLY'
id: install-ast
run: |
if ($env:PACKAGE -eq "YES" -and $env:AZURE_KEY_VAULT_URL) {
Expand All @@ -196,18 +205,19 @@ jobs:
echo "LOADTESTS_USE_LOCAL_DEPENDENCIES=ON" >> $env:GITHUB_ENV
- name: Install Dependencies
if: matrix.check_mkvk != 'ONLY'
# This script only installs what's needed by ON FEATUREs.
run: ci_scripts/install_win.ps1

- name: Set up JDK 17.
if: matrix.options.jni == 'ON'
if: matrix.options.jni == 'ON' && matrix.check_mkvk != 'ONLY'
uses: actions/setup-java@v3
with:
distribution: 'temurin'
java-version: '17'

- name: Set up Python 3.11
if: matrix.options.py == 'ON'
if: matrix.options.py == 'ON' && matrix.check_mkvk != 'ONLY'
uses: actions/setup-python@v4
with:
python-version: '3.11.4'
Expand All @@ -219,6 +229,7 @@ jobs:
ci_scripts/smudge_date.ps1
- name: Build Windows
if: matrix.check_mkvk != 'ONLY'
# The installers run as part of "Install Dependencies" add
# environment variables to the registry and augment $Path there.
# Although each step gets a new Powershell instance, that instance
Expand All @@ -239,6 +250,10 @@ jobs:
$env:JAVA_HOME=$step_java_home
ci_scripts/build_win.ps1
- name: Test Generation of VkFormat-related files
if: matrix.check_mkvk == 'ONLY'
run: ci_scripts/check_mkvk.ps1

- name: Test Windows build
# Tests built for arm64 can't be run as the CI runners are all x64.
if: matrix.arch == 'x64' && matrix.options.tests == 'ON'
Expand Down
43 changes: 30 additions & 13 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ addons:
env:
global:
- BUILD_DIR: build
- CHECK_MKVK: NO
- CHECK_REUSE: NO
- FEATURE_TESTS: ON
- GIT_LFS_SKIP_SMUDGE: 1
- HOMEBREW_NO_AUTO_UPDATE: 1
- PACKAGE: NO
- REL_DESC_FILE: "$BUILD_DIR/rel_desc.md"
- VULKAN_SDK_VER: "1.3.243.0"
Expand All @@ -37,6 +39,7 @@ env:
- WASM_BUILD: NO
- WERROR: ON
jobs:
# NOTE: As least 2 configurations must be present or no macOS job is run.
# FEATURE_TESTS is off for arm64 macOS because we can't even build
# them during CI. CI runs on x86_64 and there is a PostBuild command
# that attempts to execute the compiled-for-m1 tests to have gtest
Expand All @@ -45,6 +48,8 @@ env:
# FEATURE_PY is off for arm64 macOS because we'd need a cross-compiled version
# of Python for it to correctly build the Python extensions (the CI
# runs on x86_64)
- CHECK_MKVK=ONLY

- CONFIGURATION=Debug,Release PLATFORM=macOS ARCHS=x86_64
FEATURE_DOC=ON FEATURE_JNI=ON FEATURE_PY=ON FEATURE_LOADTESTS=OpenGL+Vulkan FEATURE_TOOLS=ON
FEATURE_TOOLS_CTS=ON LOADTESTS_USE_LOCAL_DEPENDENCIES=ON SUPPORT_SSE=ON SUPPORT_OPENCL=OFF DEPLOY_DOCS=YES PACKAGE=YES
Expand Down Expand Up @@ -84,6 +89,10 @@ jobs:
dist: jammy
env:
- CHECK_REUSE: ONLY
- os: linux
dist: jammy
env:
- CHECK_MKVK: ONLY
- os: linux
dist: jammy
addons:
Expand Down Expand Up @@ -201,7 +210,7 @@ before_install:
# when the repo owner differs from the user. For details see
# https://github.blog/2022-04-12-git-security-vulnerability-announced/
docker run -dit --name emscripten --user "$(id -u):$(id -g)" -v $(pwd):/src emscripten/emsdk bash
elif [ "$CHECK_REUSE" != "ONLY" ]; then
elif [ "$CHECK_REUSE" != "ONLY" -a "$CHECK_MKVK" != "ONLY" ]; then
sudo apt-get update
fi
;;
Expand All @@ -216,7 +225,7 @@ install:
pip3 install reuse
set +e
fi
if [ "$CHECK_REUSE" != "ONLY" -a "$WASM_BUILD" != "YES" ]; then
if [ "$CHECK_REUSE" != "ONLY" -a "$CHECK_MKVK" != "ONLY" -a "$WASM_BUILD" != "YES" ]; then
if [ "$TRAVIS_CPU_ARCH" = "arm64" ]; then
# JDK was not installed on arm64 runner. Kept for reference.
#sudo apt-get -qq install openjdk-17-jdk
Expand All @@ -235,11 +244,13 @@ install:
fi
;;
osx)
if [ "$LOADTESTS_USE_LOCAL_DEPENDENCIES" = "ON" ]; then
brew install sdl2
brew install assimp
if [ "$CHECK_REUSE" != "ONLY" -a "$CHECK_MKVK" != "ONLY" ]; then
if [ "$LOADTESTS_USE_LOCAL_DEPENDENCIES" = "ON" ]; then
brew install sdl2
brew install assimp
fi
./ci_scripts/install_macos.sh
fi
./ci_scripts/install_macos.sh
;;
esac
Expand All @@ -259,15 +270,15 @@ before_script:

script:
- |
set -e # Instead of trying to chain everything together with &&
# otherwise subsequent commands will swallow bad exit status.
case "${TRAVIS_OS_NAME:-linux}" in
linux)
if [ "$CHECK_REUSE" = "YES" -o "$CHECK_REUSE" = "ONLY" ]; then
echo "Calling reuse lint."
set -e # because the if below swallows a bad exit status.
reuse --suppress-deprecation lint
set +e
fi
if [ "$CHECK_REUSE" != "ONLY" ]; then
if [ "$CHECK_REUSE" != "ONLY" -a "$CHECK_MKVK" != "ONLY" ]; then
if [ "$WASM_BUILD" = "YES" ]; then
./ci_scripts/build_wasm_docker.sh
else
Expand All @@ -276,19 +287,25 @@ script:
fi
;;
osx)
if [ "$PLATFORM" = "macOS" ]; then
./ci_scripts/build_macos.sh
else
./ci_scripts/build_ios.sh
if [ "$CHECK_REUSE" != "ONLY" -a "$CHECK_MKVK" != "ONLY" ]; then
if [ "$PLATFORM" = "macOS" ]; then
./ci_scripts/build_macos.sh
else
./ci_scripts/build_ios.sh
fi
fi
;;
esac
if [ "$CHECK_MKVK" = "ONLY" ]; then
ci_scripts/check_mkvk.sh
fi
if [ "$DEPLOY_DOCS" = "YES" ]; then
# Some files in the pyktx docs have an _ prefix so Jekyll will
# not copy them from the gh-pages branch to the website. This
# file says no Jekyll files here. Treat all as ordinary files.
touch $BUILD_DIR/docs/html/.nojekyll
fi
set +e
# See if this helps with truncated logs.
#after_script:
Expand Down
12 changes: 10 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ else()
set( KTX_LOADTEST_APPS_USE_LOCAL_DEPENDENCIES ON )
endif()

option( KTX_GENERATE_VK_FILES
"Include targets for generating VkFormat related files. For project developers only."
OFF
)
mark_as_advanced(FORCE KTX_GENERATE_VK_FILES)

# Platform specific settings

if(APPLE)
Expand Down Expand Up @@ -253,7 +259,9 @@ else()
endif()

# Depends on the settings, so it must be included after
include(cmake/mkvk.cmake)
if(KTX_GENERATE_VK_FILES)
include(cmake/mkvk.cmake)
endif()

# Global compile & link options including optimization flags

Expand Down Expand Up @@ -457,7 +465,7 @@ add_library( ktx_read ${LIB_TYPE}
macro(common_libktx_settings target enable_write library_type)

if(TARGET mkvk)
# Creating vulkan headers is only required when Vulkan SDK updates.
# Creating vulkan headers only required after Vulkan Spec/SDK updates.
add_dependencies(${target} mkvk)
endif()

Expand Down
62 changes: 62 additions & 0 deletions ci_scripts/check_mkvk.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Copyright 2024 The Khronos Group Inc.
# SPDX-License-Identifier: Apache-2.0

<#
.SYNOPSIS
Check generation of VkFormat related files.
.DESCRIPTION
Regenerates all VkFormat related files and compares them with the
version in Git. Used to verify correct functioning of the generation
scripts in CI.
.INPUTS
None
.OUTPUTS
None
#>

param (
# Default of $null results in an empty string when not set, so be explicit.
[string] $BUILD_DIR = ""
# With positional parameters, BUILD_DIR will be $null if no parameter.
# [Parameter(Position=0)] [string[]]$BUILD_DIR
)

function Get-ParamValue {
<#
.SYNOPSIS
Get a parameter value.
.DESCRIPTION
Returns one of the following in this priority order:
1. Value set on command line, if any.
2. Value from same-named environment variable, if any
3. $DefaultValue param.
#>
param ( $ParamName, $DefaultValue )
$res = get-variable $ParamName -ValueOnly -ErrorAction 'SilentlyContinue'
if ($res -eq "" -or $res -eq $null) {
$res = [Environment]::GetEnvironmentVariable($ParamName)
if ($res -eq $null) {
$res = $DefaultValue
}
}
return $res
}

$BUILD_DIR = Get-ParamValue BUILD_DIR "build/checkmkvk"

cmake . -B $BUILD_DIR -D KTX_FEATURE_TESTS=OFF -D KTX_FEATURE_TOOLS=OFF -D KTX_GENERATE_VK_FILES=ON
# Clean first is to ensure all files are generated so everything is tested.
cmake --build $BUILD_DIR --target mkvk --clean-first
rm $BUILD_DIR -Recurse -Confirm:$false
# Verify no files were modified. Exit with 1, if so.
git diff --quiet HEAD
if (!$?) {
git status
git diff
exit 1
}
22 changes: 22 additions & 0 deletions ci_scripts/check_mkvk.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#! /usr/bin/env bash
# Copyright 2024 The Khronos Group Inc.
# SPDX-License-Identifier: Apache-2.0

# Check generation of VkFormat related files.
#
# Regenerates all VkFormat related files and compares them with the
# version in Git. Used to verify correct functioning of the generation
# scripts in CI.

BUILD_DIR=${BUILD_DIR:-build/checkmkvk}

cmake . -B $BUILD_DIR -D KTX_FEATURE_TESTS=OFF -D KTX_FEATURE_TOOLS=OFF -D KTX_GENERATE_VK_FILES=ON
# Clean first is to ensure all files are generated so everything is tested.
cmake --build $BUILD_DIR --target mkvk --clean-first
rm -rf $BUILD_DIR
# Verify no files were modified. Exit with 1, if so.
if ! git diff --quiet HEAD; then
git status
git diff
exit 1
fi
Loading

0 comments on commit e77a531

Please sign in to comment.