Skip to content

Commit

Permalink
Fix: ensure ktxdiff runs on macOS and enable CTS in Linux arm64 CI. (#…
Browse files Browse the repository at this point in the history
…946)

Set INSTALL_RPATH in `ktxdiff`. Its absence meant macOS could not find
libktx, if not installed on the system. Tests were succeeding on macOS
x86_64 because all the outputs are exact matches for the golden files so
`ktxdiff` was never invoked.

Enable FEATURE_TOOLS_CTS for Linux arm64 build as
KhronosGroup/KTX-Software-CTS#30 fixes the
previous failure.

Fix syntax errors in Linux INSTALL_RPATH values.
  • Loading branch information
MarkCallow authored Sep 16, 2024
1 parent 2ee4332 commit ffb9152
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 18 deletions.
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ jobs:
- VULKAN_SDK_VER: "1.3.290"
- CMAKE_GEN: Ninja
- CONFIGURATION=Release
FEATURE_DOC=ON FEATURE_JNI=ON FEATURE_PY=ON FEATURE_LOADTESTS=OpenGL+Vulkan FEATURE_TOOLS=ON
FEATURE_DOC=ON FEATURE_JNI=ON FEATURE_PY=ON FEATURE_LOADTESTS=OpenGL+Vulkan FEATURE_TOOLS=ON FEATURE_TOOLS_CTS=ON
SUPPORT_SSE=ON SUPPORT_OPENCL=OFF WERROR=ON PACKAGE=YES
- os: linux
arch: arm64
Expand All @@ -134,7 +134,7 @@ jobs:
- CMAKE_GEN: Ninja
- CONFIGURATION=Release
FEATURE_DOC=ON FEATURE_JNI=ON FEATURE_PY=ON FEATURE_LOADTESTS=OpenGL
FEATURE_TOOLS=ON FEATURE_TOOLS_CTS=OFF
FEATURE_TOOLS=ON FEATURE_TOOLS_CTS=ON
SUPPORT_SSE=OFF SUPPORT_OPENCL=OFF WERROR=ON PACKAGE=YES
- os: linux
dist: jammy
Expand Down
2 changes: 1 addition & 1 deletion tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function(set_test_properties test_target)
)
elseif(LINUX)
set_target_properties(${test_target} PROPERTIES
INSTALL_RPATH "$ORIGIN;$ORIGIN/../lib"
INSTALL_RPATH "\$ORIGIN;\$ORIGIN/../lib"
)
endif()
endfunction()
Expand Down
2 changes: 1 addition & 1 deletion tests/cts
2 changes: 2 additions & 0 deletions tests/ktxdiff/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,5 @@ target_compile_definitions(
PRIVATE
$<TARGET_PROPERTY:ktx,INTERFACE_COMPILE_DEFINITIONS>
)

set_test_properties(ktxdiff)
13 changes: 4 additions & 9 deletions tests/loadtests/glloadtests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -263,17 +263,12 @@ function( create_gl_target target version sources common_resources test_images
$<TARGET_FILE_DIR:${target}>/../resources
)

# To keep the resources (test images and models) close to the
# executable and to be compliant with the Filesystem Hierarchy
# Standard https://refspecs.linuxfoundation.org/FHS_3.0/fhs/index.html
# we have chosen to install the apps and data in /opt/<target>.
# Each target has a `bin` directory with the executable and a
# `resources` directory with the resources. We install a symbolic
# link to the executable in ${CMAKE_INSTALL_LIBDIR}, usually
# /usr/local/bin.
# See important comment and TODO:s starting at line 365
# in ./vkloadtests.cmake regarding installation of these
# targets. Search for "keep the resources".

set_target_properties( ${target} PROPERTIES
INSTALL_RPATH "\$ORIGIN:${CMAKE_INSTALL_FULL_LIBDIR}"
INSTALL_RPATH "\$ORIGIN;${CMAKE_INSTALL_FULL_LIBDIR}"
)

######### IMPORTANT ######
Expand Down
18 changes: 14 additions & 4 deletions tests/loadtests/vkloadtests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,21 @@ else()
# we have chosen to install the apps and data in /opt/<target>.
# Each target has a `bin` directory with the executable and a
# `resources` directory with the resources. We install a symbolic
# link to the executable in ${CMAKE_INSTALL_LIBDIR}, usually
# /usr/local/bin.

# link to the executable in ${CMAKE_INSTALL_BINDIR}, usually
# /usr/local/bin, instead of adding /opt/<target>/bin to $PATH.
#
# TODO: Figure out how to handle libktx so installs of tools only,
# tools + loadtests and loadtests only are supported. Only put
# library in /usr/local/lib? Duplicate it in /opt/<provider>/lib
# from where it is shared by gl3loadtests and vkloadtests? Only
# put it in /opt/<provider>/lib with link from
# ${CMAKE_INSTALL_LIBDIR}? NOTE: if we put lib in /opt/<provider>
# then consider putting the executables in /opt/provider/<target>.

# TODO: Before adding this target to the release packages, ensure
# this RPATH will work for alternate install root.
set_target_properties( vkloadtests PROPERTIES
INSTALL_RPATH "${CMAKE_INSTALL_FULL_LIBDIR}"
INSTALL_RPATH "\$ORIGIN;${CMAKE_INSTALL_FULL_LIBDIR}"
)

######### IMPORTANT ######
Expand Down
15 changes: 14 additions & 1 deletion tools/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ endif()

function(set_tool_properties tool_target)
if(APPLE)
# The first RPATH entry is so executables run in the build
# directories will work and will find the just built libraries
# instead of whatever may be installed on the system. The
# second RPATH entry is for finding the installed library.
set_target_properties(${tool_target} PROPERTIES
XCODE_ATTRIBUTE_ENABLE_HARDENED_RUNTIME "YES"
# Creates an LC_RPATH entry in the Mac-O binary for each
Expand All @@ -39,10 +43,19 @@ function(set_tool_properties tool_target)
# - Default path: /lib;/usr/lib.
# $ORIGIN is equivalent to @executable_path.
#
# Use relative path to installed lib so users can change
# installation location. Using CMAKE_INSTALL_FULL_LIBDIR
# would not work when changing the location during package
# install only when changing the installation location
# during `cmake --build` or `cmake --install`. The second
# entry may not be necessary as users installing to an
# alternate location will likely have it set in their
# LD_LIBRARY_PATH or /etc/ld.so.conf.
#
# Check DT_RUNPATH with one of
# - readelf -d <file> | head -20
# - objdump -x <file> | grep 'R.*PATH'
INSTALL_RPATH "$ORIGIN;$ORIGIN/../lib"
INSTALL_RPATH "\$ORIGIN;\$ORIGIN/../lib"
)
endif()
endfunction()
Expand Down

0 comments on commit ffb9152

Please sign in to comment.