Skip to content

Commit

Permalink
Handle problems caused by runner and Docker image updates (#933)
Browse files Browse the repository at this point in the history
An Emscripten Docker image update to Clang 20 and an update to the
GitHub Actions Windows runner image broke our CI builds. This PR
provides fixes or workarounds. While working on the Clang 20 issue it
was noticed that the FP options for `libktx` were not being set as
intended, except for the ASTC encoder. This PR also fixes that problem
so the same FP options are used throughout `libktx`. Lastly it contains
improvements to `build_wasm_docker.sh` made while debugging and fixing
the FP settings issue.

clang 20 has a new "overriding option" warning which is raised if one
float-point option modifies a setting made by another. We are setting
`-ffp-model=precise`, which leaves contraction on, and
`-ffp-contract=off` thus we get the warning. This usage is normal for
cross-platform invariance thus the fix is to disable the spam warning.

The Windows runner image update broke the python build due to removal of
a long deprecated package from the Python/PIP `setuptools` package and
our use of the `cffi` package which has a dependency on `setuptools`
including the deleted package. See
python-cffi/cffi#117. This PR works around the
problem by creating a `constraint.txt` file which is passed to PIP and
tells it to limit the `setuptools` package to a version that still
contains the deleted package.
  • Loading branch information
MarkCallow authored Sep 6, 2024
1 parent ea77b3c commit da4755f
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 29 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ jobs:
core.exportVariable('VCPKG_BINARY_SOURCES', "clear;x-gha,readwrite");
core.exportVariable('VCPKG_ROOT', process.env.VCPKG_INSTALLATION_ROOT || '');
- name: Setup PIP_CONSTRAINT (workaround https://github.com/python-cffi/cffi/issues/117)
if: matrix.options.py == 'ON' && matrix.check_mkvk != 'ONLY'
run: |
echo 'setuptools<74' > $env:RUNNER_TEMP/constraint.txt
echo "PIP_CONSTRAINT=$env:RUNNER_TEMP/constraint.txt" >> $env:GITHUB_ENV
- name: Install Dependencies
if: matrix.check_mkvk != 'ONLY'
# This script only installs what's needed by ON FEATUREs.
Expand Down
40 changes: 17 additions & 23 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,10 @@ endif()

# For Visual Studio prior to 2022 (compiler < 19.30) /fp:strict
# For Visual Studio 2022 (compiler >= 19.30) /fp:precise
# For Visual Studio 2022 ClangCL seems to have accidentally enabled contraction by default,
# so behaves differently to CL.exe. Use the -Xclang argument to workaround and allow access
# GNU-style switch to control contraction and force disable.
# For Visual Studio 2022 ClangCL has enabled contraction by default,
# which is the same as standard clang, so behaves differently to
# CL.exe. Use the -Xclang argument to access GNU-style switch to
# control contraction and force disable.

# On CMake 3.25 or older CXX_COMPILER_FRONTEND_VARIANT is not always set
if(CMAKE_CXX_COMPILER_FRONTEND_VARIANT STREQUAL "")
Expand All @@ -338,6 +339,7 @@ set(is_gnu_fe1 "$<STREQUAL:${CMAKE_CXX_COMPILER_FRONTEND_VARIANT},GNU>")
set(is_gnu_fe2 "$<STREQUAL:${CMAKE_CXX_COMPILER_FRONTEND_VARIANT},AppleClang>")
# Compiler accepts GNU-style command line options
set(is_gnu_fe "$<OR:${is_gnu_fe1},${is_gnu_fe2}>")
#add_custom_target(debug_isgnufe1 COMMAND ${CMAKE_COMMAND} -E echo "is_gnufe1_exp = $<STREQUAL:${CMAKE_CXX_COMPILER_FRONTEND_VARIANT},GNU>")

# Compiler is Visual Studio cl.exe
set(is_msvccl "$<AND:${is_msvc_fe},$<CXX_COMPILER_ID:MSVC>>")
Expand All @@ -346,24 +348,18 @@ set(is_clangcl "$<AND:${is_msvc_fe},$<CXX_COMPILER_ID:Clang>>")
# Compiler is upstream clang with the standard frontend
set(is_clang "$<AND:${is_gnu_fe},$<CXX_COMPILER_ID:Clang,AppleClang>>")

if(${is_msvccl} AND $<VERSION_LESS:$<CXX_COMPILER_VERSION>,19.30>)
add_compile_options(/fp:strict)
endif()
if(${is_msvccl} AND $<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,19.30>)
add_compile_options(/fp:precise)
endif()
if(${is_clangcl})
add_compile_options(/fp:precise)
endif()
if(${is_clangcl} AND $<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,14.0.0>)
add_compile_options(-Xclang -ffp-contract=off)
endif()
if(${is_clang} AND $<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,10.0.0>)
add_compile_options(-ffp-model=precise)
endif()
if(${is_gnu_fe})
add_compile_options(-ffp-contract=off)
endif()
add_compile_options(
$<$<AND:${is_msvccl},$<VERSION_LESS:$<CXX_COMPILER_VERSION>,19.30>>:/fp:strict>
$<$<AND:${is_msvccl},$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,19.30>>:/fp:precise>
$<${is_clangcl}:/fp:precise>
$<$<AND:${is_clangcl},$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,14.0.0>>:-Xclang$<SEMICOLON>-ffp-contract=off>
$<$<AND:${is_clang},$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,10.0.0>>:-ffp-model=precise>
$<${is_gnu_fe}:-ffp-contract=off>
# Hide noise from clang and clangcl 20 warning the 2nd fp option changes
# one of the settings made the first.
$<$<AND:${is_clang},$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,20.0.0>>:-Wno-overriding-option>
)
#add_custom_target(debug_gnufe_ffpcontract COMMAND ${CMAKE_COMMAND} -E echo "ffp_contract = $<${is_gnu_fe}:-ffp-contract=off>")

set(KTX_BUILD_DIR "${CMAKE_BINARY_DIR}")

Expand Down Expand Up @@ -906,8 +902,6 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
)
endif()
if (${clang_version} VERSION_GREATER_EQUAL "14.0")
# These are for Emscripten which is ahead of xcode in its clang
# version. Also future proofing for when xcode catches up.
set_source_files_properties(
${BASISU_ENCODER_CXX_SRC}
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-unused-parameter;-Wno-deprecated-copy-with-user-provided-copy"
Expand Down
1 change: 1 addition & 0 deletions external/astc-encoder/Source/cmake_core.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ macro(astcenc_set_properties ASTCENC_TARGET_NAME ASTCENC_IS_VENEER)
$<${is_gnu_fe}:-Wno-float-equal>
$<${is_gnu_fe}:-Wno-deprecated-declarations>
$<${is_gnu_fe}:-Wno-atomic-implicit-seq-cst>
$<${is_clang}:-Wno-overriding-option>

# Clang 10 also throws up warnings we need to investigate (ours)
$<${is_gnu_fe}:-Wno-cast-align>
Expand Down
45 changes: 39 additions & 6 deletions scripts/build_wasm_docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,45 @@
# Exit if any command fails.
set -e

atexit () {
function atexit () {
if [ -z $dockerrunning ]; then
docker stop emscripten > /dev/null
docker rm emscripten > /dev/null
fi
}

# Set parameters from command-line arguments, if any.
for i in $@; do
eval $i
function usage() {
echo "Usage: $0 [Option] [PARAMETER=value] [target...]"
echo ""
echo "Build KTX-Software using Emscripten docker package."
echo "Defaults to building everything (Release config) if no targets specified."
echo "Options:"
echo " --help, -h Print this usage message."
echo " --verbose ,-v Cause the underlying make to run in verbose mode."
exit $1
}

# cd repo root so script will work whereever the current directory
path_to_repo_root=..
cd -- "$(dirname -- "${BASH_SOURCE[0]}")/$path_to_repo_root"

for arg in $@; do
case $arg in
--help | -h)
usage 0
;;
--verbose | -v)
verbose_make="-- VERBOSE=1"
shift
;;
*\=*)
# Set parameter from command-line arguments
eval $arg
shift ;;
*)
targets="$targets --target $arg"
shift ;;
esac
done

# Set defaults
Expand All @@ -33,7 +62,7 @@ WERROR=${WERROR:-OFF}

BUILD_DIR=${BUILD_DIR:-build/web-$CONFIGURATION}

trap atexit EXIT
trap atexit EXIT SIGINT

# Check if emscripten container is already running as CI will already have started it.
if [ "$(docker container inspect -f '{{.State.Status}}' emscripten 2> /dev/null)" != "running" ]
Expand Down Expand Up @@ -74,12 +103,16 @@ mkdir -p $BUILD_DIR
# emcmake uses the "Unix Makefiles" generator on Linux which does not
# support multiple configurations.
echo "Configure and Build KTX-Software (Web $CONFIGURATION)"

# Uncomment for debugging some generator expressions.
#targets="--target debug_isgnufe1 --target debug_gnufe_ffpcontract"

docker exec -it emscripten sh -c "emcmake cmake -B$BUILD_DIR . \
-D CMAKE_BUILD_TYPE=$CONFIGURATION \
-D KTX_FEATURE_DOC=OFF \
-D KTX_FEATURE_LOADTEST_APPS=$FEATURE_LOADTESTS \
-D KTX_WERROR=$WERROR \
&& cmake --build $BUILD_DIR"
&& cmake --build $BUILD_DIR $targets $verbose_make"

if [ "$PACKAGE" = "YES" ]; then
echo "Pack KTX-Software (Web $CONFIGURATION)"
Expand Down

0 comments on commit da4755f

Please sign in to comment.