Skip to content

Commit

Permalink
Fix cross-device rename failure (#593)
Browse files Browse the repository at this point in the history
by creating the temporary file in the same directory as the file to be overwritten.

Includes:

    Add ktxsc tests.
    Improve ktxsc docs.
    Simplify toktx-tests.cmake script.
    Simplify reference image generation scripts.
    Regen skybox_zstd.ktx2 reference with default version numbers.
    Run tests in Mingw build.
    Fix on_failure.ps1 to work for both Appveyor and GitHub Actions and to use intended curl.
    Workaround lack of 'x' mode in Mingw fopen.

Fixes #581.
  • Loading branch information
MarkCallow authored Jun 20, 2022
1 parent 78a3eb6 commit f020c1b
Show file tree
Hide file tree
Showing 12 changed files with 261 additions and 204 deletions.
13 changes: 10 additions & 3 deletions .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ build:
verbosity: detailed

on_failure:
- ps: $env:APPVEYOR_BUILD_FOLDER/ci_scripts/on_failure.ps1
- ps: $env:APPVEYOR_BUILD_FOLDER/ci_scripts/on_failure.ps1 $env:APPVEYOR_BUILD_FOLDER $env:BUILD_DIR
# Following is for GIT_TRACE: 1 above.
# - ps: Get-ChildItem .\.git\lfs\objects\logs\*.log | % { Push-AppveyorArtifact $_.FullName -FileName $_.Name }

Expand All @@ -164,15 +164,22 @@ init:
# - ps: '$blockRdp = $true; iex ((new-object net.webclient).DownloadString(''https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1''))'
# - ps: | #iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))


# N.B.: for some reason indenting "- cmd" on the following lines causes syntax
# errors. Not using PS for downloads because its curl is completely different.
# errors.
install:
- if not "%appveyor_build_worker_image%" == "Visual Studio 2019" cinst doxygen.install
#- ps: |
#run mkversion
#Update-AppveyorBuild -Version "$($env:ospsuite_version).$($env:appveyor_build_version)"

# Use cmd.exe because PS curl is very different.
# Not using PS for install because "curl" is an alias for
# Invoke-WebRequest a completely different command that is difficult to
# use for downloads. When I wrote the .bat script, so I could use the
# real curl, I did not know PS's curl was just an alias and I could get
# the real curl with "curl.exe". Doh! Now it is a case of if it ain't
# broke don't fix it.
#
# To avoid going down a rathole note very well that in at least some of
# the CI environments, cmd.exe does not support pushd and popd. Sigh!
# Also keep quotes in following echos & rems to avoid issues with
Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,19 @@ jobs:
fetch-depth: 0
- uses: seanmiddleditch/gha-setup-ninja@v3
- uses: Honeybunch/setup-mingw@v3
- name: Pull test images from Git LFS
run: git lfs pull --include=tests/srcimages,tests/testimages
- name: Configure Mingw x64
run: cmake -B build -G "Ninja Multi-Config" -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
- name: Build Mingw x64 Debug
run: cmake --build build --config Debug
- name: Build Mingw x64 Release
run: cmake --build build --config Release
- name: Test Mingw build
run: ctest --test-dir build -C Release
- name: Upload test log
if: ${{ failure() }}
run: ci_scripts/on_failure.ps1

linux:

Expand Down
28 changes: 21 additions & 7 deletions ci_scripts/on_failure.ps1
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
# Copyright 2022 The Khronos Group Inc.
# SPDX-License-Identifier: Apache-2.0

if ($args[0]) { $repo_root = $args[0] } else { $repo_root = "." }
if ($args[1]) { $build_dir = $args[1] } else { $build_dir = "build" }

# In Github Actions, this is called only when there is a failed test step.
# In Appveyor we use the Phase environment variable.
echo "Phase = $env:Phase"
if ($env:Phase) {
echo "Now uploading the failed tests"
ls -Name $env:APPVEYOR_BUILD_FOLDER/tests/testimages/toktx*
tar -cvf failed-tests.tar $env:APPVEYOR_BUILD_FOLDER/tests/testimages/toktx*
curl --upload-file failed-tests.tar https://transfer.sh/toktx-failed-tests.tar
echo "Now uploading the test log"
curl --upload-file $env:APPVEYOR_BUILD_FOLDER/$env:BUILD_DIR/Testing/Temporary/LastTest.log https://transfer.sh/ktx-last-test.log
if ($env:GITHUB_ACTIONS -or $env:Phase) {
pushd $repo_root
echo "Now uploading the failed tests"
ls -Name tests/testimages/ktxsc* tests/testimages/toktx*
if (tar -cvf failed-images.tar tests/testimages/ktxsc* tests/testimages/toktx*) {
# N.B. In PS, "curl" is an alias for Invoke-WebRequest. Uploading a
# file via that looks like a p.i.t.a so use the real curl command.
curl.exe --upload-file failed-images.tar https://transfer.sh/ktx-failed-images.tar
}
# Even if there are no failed images and tar exits with false it creates
# the output file.
rm failed-images.tar

echo "`r`nNow uploading the test log"
curl.exe --upload-file $build_dir/Testing/Temporary/LastTest.log https://transfer.sh/ktx-last-test.log
popd
}
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ endif()
# tools tests
if(KTX_FEATURE_TOOLS)
include( ktx2check-tests.cmake )
include( ktxsc-tests.cmake )
include( toktx-tests.cmake )
endif()
95 changes: 95 additions & 0 deletions tests/ktxsc-tests.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# -*- tab-width: 4; -*-
# vi: set sw=2 ts=4 expandtab:

# Copyright 2022 Mark Callow
# SPDX-License-Identifier: Apache-2.0

# toktx share a common scapp class with ktxsc so the toktx tests suffice
# for testing actual compression.

add_test( NAME ktxsc-test-help
COMMAND ktxsc --help
)
set_tests_properties(
ktxsc-test-help
PROPERTIES
PASS_REGULAR_EXPRESSION "^Usage: ktxsc"
)

add_test( NAME ktxsc-test-version
COMMAND ktxsc --version
)
set_tests_properties(
ktxsc-test-version
PROPERTIES
PASS_REGULAR_EXPRESSION "^ktxsc v[0-9][0-9\\.]+"
)

# The near duplication of this and other tests below is due to a "limitation"
# (i.e. a bug) in ctest which checks for neither a zero error code when a
# PASS_REGULAR_EXPRESSION is specified nor a non-zero error code when a
# FAIL_REGULAR_EXPRESSION is specified but only for matches to the REs.
add_test( NAME ktxsc-test-foobar
COMMAND ktxsc --foobar
)
set_tests_properties(
ktxsc-test-foobar
PROPERTIES
WILL_FAIL TRUE
FAIL_REGULAR_EXPRESSION "^Usage: ktxsc"
)
add_test( NAME ktxsc-test-foobar-exit-code
COMMAND ktxsc --foobar
)
set_tests_properties(
ktxsc-test-foobar-exit-code
PROPERTIES
WILL_FAIL TRUE
)

add_test( NAME ktxsc-test-many-in-one-out
COMMAND ktxsc -o foo a.ktx2 b.ktx2 c.ktx2
)
set_tests_properties(
ktxsc-test-many-in-one-out
PROPERTIES
WILL_FAIL TRUE
FAIL_REGULAR_EXPRESSION "^Can't use -o when there are multiple infiles."
)

add_test( NAME ktxsc-test-many-in-one-out-exit-code
COMMAND ktxsc -o foo a.ktx2 b.ktx2 c.ktx2
)
set_tests_properties(
ktxsc-test-many-in-one-out-exit-code
PROPERTIES
WILL_FAIL TRUE
)

set( IMG_DIR "${CMAKE_CURRENT_SOURCE_DIR}/testimages" )

function( gencmpktx test_name reference source args inplace )
if (NOT inplace)
set( workfile ktxsc.${reference} )
add_test( NAME ktxsc-cmp-${test_name}
COMMAND ${BASH_EXECUTABLE} -c "$<TARGET_FILE:ktxsc> --test ${args} -o ${workfile} ${source} && diff ${reference} ${workfile} && rm ${workfile}"
WORKING_DIRECTORY ${IMG_DIR}
)
elseif(${inplace} STREQUAL "cur-dir")
set( workfile ktxsc.ip1.${reference} )
add_test( NAME ktxsc-cmp-${test_name}
COMMAND ${BASH_EXECUTABLE} -c "cp ${source} ${workfile} && $<TARGET_FILE:ktxsc> --test ${args} ${workfile} && diff ${reference} ${workfile} && rm ${workfile}"
WORKING_DIRECTORY ${IMG_DIR}
)
elseif(${inplace} STREQUAL "different-dir")
set( workfile ktxsc.ip2.${reference} )
add_test( NAME ktxsc-cmp-${test_name}
COMMAND ${BASH_EXECUTABLE} -c "cp ${source} ${workfile} && pushd ../.. && $<TARGET_FILE:ktxsc> --test ${args} ${IMG_DIR}/${workfile} && popd && diff ${reference} ${workfile} && rm ${workfile}"
WORKING_DIRECTORY ${IMG_DIR}
)
endif()
endfunction()

gencmpktx( compress-explicit-output skybox_zstd.ktx2 skybox.ktx2 "--zcmp 5" "" "" )
gencmpktx( compress-in-place-cur-dir skybox_zstd.ktx2 skybox.ktx2 "--zcmp 5" "cur-dir" )
gencmpktx( compress-in-place-different-dir skybox_zstd.ktx2 skybox.ktx2 "--zcmp 5" "different-dir" )
50 changes: 4 additions & 46 deletions tests/testimages/genktx2
Original file line number Diff line number Diff line change
Expand Up @@ -13,72 +13,30 @@ depth=../..
# Change dir to the testimages folder, the script location...
cd $(dirname $0)

# Make paths relative to the testimages directory.
ktx_root=$depth
toktx_vs2013=$ktx_root/build/msvs/win/vs2013/x64/Release/toktx.exe
toktx_vs2015=$ktx_root/build/msvs/win/vs2015/x64/Release/toktx.exe
toktx_cmake=$ktx_root/build/cmake/linux/Release/toktx
toktx_cmake_d=$ktx_root/build/cmake/linux/Debug/toktx
toktx_make=$ktx_root/build/make/linux/out/Release/toktx
toktx_make_d=$ktx_root/build/make/linux/out/Debug/toktx

ktx2ktx2_vs2013=$ktx_root/build/msvs/win/vs2013/x64/Release/ktx2ktx2.exe
ktx2ktx2_vs2015=$ktx_root/build/msvs/win/vs2015/x64/Release/ktx2ktx2.exe
ktx2ktx2_cmake=$ktx_root/build/cmake/linux/Release/ktx2ktx2
ktx2ktx2_cmake_d=$ktx_root/build/cmake/linux/Debug/ktx2ktx2
ktx2ktx2_make=$ktx_root/build/make/linux/out/Release/ktx2ktx2
ktx2ktx2_make_d=$ktx_root/build/make/linux/out/Debug/ktx2ktx2

# Ensure generation is not polluted by user environment
unset TOKTX_OPTIONS

if [ -n "$1" -a -x "$1" ]; then
toktx="$1"
elif [ -x "$toktx_vs2013" ]; then
toktx=$toktx_vs2013
elif [ -x "$toktx_vs2015" ]; then
toktx=$toktx_vs2015
elif [ -x "$toktx_cmake" ]; then
toktx=$toktx_cmake
elif [ -x "$toktx_cmake_d" ]; then
toktx=$toktx_cmake_d
elif [ -x "$toktx_make" ]; then
toktx=$toktx_gmake
elif [ -x "$toktx_make_d" ]; then
toktx=$toktx_gmake
elif which toktx >/dev/null; then
toktx=toktx
else
echo $0: None of $toktx_vs2013, $toktx_vs2015, $toktx_gmake or $toktx_cmake found.
echo $0: Nor was toktx found in along PATH.
echo $0: toktx not found along $PATH.
echo $0: Aborting generation
exit 1
fi

if [ -n "$2" -a -x "$2" ]; then
ktx2ktx2="$1"
elif [ -x "$ktx2ktx2_vs2013" ]; then
ktx2ktx2=$ktx2ktx2_vs2013
elif [ -x "$ktx2ktx2_vs2015" ]; then
ktx2ktx2=$ktx2ktx2_vs2015
elif [ -x "$ktx2ktx2_cmake" ]; then
ktx2ktx2=$ktx2ktx2_cmake
elif [ -x "$ktx2ktx2_cmake_d" ]; then
ktx2ktx2=$ktx2ktx2_cmake_d
elif [ -x "$ktx2ktx2_make" ]; then
ktx2ktx2=$ktx2ktx2_gmake
elif [ -x "$ktx2ktx2_make_d" ]; then
ktx2ktx2=$ktx2ktx2_gmake
ktx2ktx2="$2"
elif which ktx2ktx2 >/dev/null; then
ktx2ktx2=ktx2ktx2
else
echo $0: None of $ktx2ktx2_vs2013, $ktx2ktx2_vs2015, $ktx2ktx2_gmake or $ktx2ktx2_cmake found.
echo $0: Nor was ktx2ktx2 found in along PATH.
echo $0: ktx2ktx2 not found along $PATH.
echo $0: Aborting generation
exit 1
fi

# Generate reference files for toktx-tests ...
# Generate some reference files for toktx-tests ...

$ktx2ktx2 --test -f -o orient-down-metadata-u.ktx2 orient-down-metadata.ktx
$ktx2ktx2 --test -f -o orient-up-metadata-u.ktx2 orient-up-metadata.ktx
Expand Down
54 changes: 1 addition & 53 deletions tests/testimages/genktx2icons
Original file line number Diff line number Diff line change
Expand Up @@ -17,67 +17,15 @@ ktxspec_dir=../../../KTX-Specification
# Change dir to the testimages folder, the script location...
cd $(dirname $0)

# Make paths relative to the testimages directory.
ktx_root=$depth
toktx_vs2013=$ktx_root/build/msvs/win/vs2013/x64/Release/toktx.exe
toktx_vs2015=$ktx_root/build/msvs/win/vs2015/x64/Release/toktx.exe
toktx_cmake=$ktx_root/build/cmake/linux/Release/toktx
toktx_cmake_d=$ktx_root/build/cmake/linux/Debug/toktx
toktx_make=$ktx_root/build/make/linux/out/Release/toktx
toktx_make_d=$ktx_root/build/make/linux/out/Debug/toktx

ktx2ktx2_vs2013=$ktx_root/build/msvs/win/vs2013/x64/Release/ktx2ktx2.exe
ktx2ktx2_vs2015=$ktx_root/build/msvs/win/vs2015/x64/Release/ktx2ktx2.exe
ktx2ktx2_cmake=$ktx_root/build/cmake/linux/Release/ktx2ktx2
ktx2ktx2_cmake_d=$ktx_root/build/cmake/linux/Debug/ktx2ktx2
ktx2ktx2_make=$ktx_root/build/make/linux/out/Release/ktx2ktx2
ktx2ktx2_make_d=$ktx_root/build/make/linux/out/Debug/ktx2ktx2

# Ensure generation is not polluted by user environment
unset TOKTX_OPTIONS

if [ -n "$1" -a -x "$1" ]; then
toktx="$1"
elif [ -x "$toktx_vs2013" ]; then
toktx=$toktx_vs2013
elif [ -x "$toktx_vs2015" ]; then
toktx=$toktx_vs2015
elif [ -x "$toktx_cmake" ]; then
toktx=$toktx_cmake
elif [ -x "$toktx_cmake_d" ]; then
toktx=$toktx_cmake_d
elif [ -x "$toktx_make" ]; then
toktx=$toktx_gmake
elif [ -x "$toktx_make_d" ]; then
toktx=$toktx_gmake
elif which toktx >/dev/null; then
toktx=toktx
else
echo $0: None of $toktx_vs2013, $toktx_vs2015, $toktx_gmake or $toktx_cmake found.
echo $0: Nor was toktx found in along PATH.
echo $0: Aborting generation
exit 1
fi

if [ -n "$2" -a -x "$2" ]; then
ktx2ktx2="$1"
elif [ -x "$ktx2ktx2_vs2013" ]; then
ktx2ktx2=$ktx2ktx2_vs2013
elif [ -x "$ktx2ktx2_vs2015" ]; then
ktx2ktx2=$ktx2ktx2_vs2015
elif [ -x "$ktx2ktx2_cmake" ]; then
ktx2ktx2=$ktx2ktx2_cmake
elif [ -x "$ktx2ktx2_cmake_d" ]; then
ktx2ktx2=$ktx2ktx2_cmake_d
elif [ -x "$ktx2ktx2_make" ]; then
ktx2ktx2=$ktx2ktx2_gmake
elif [ -x "$ktx2ktx2_make_d" ]; then
ktx2ktx2=$ktx2ktx2_gmake
elif which ktx2ktx2 >/dev/null; then
ktx2ktx2=ktx2ktx2
else
echo $0: None of $ktx2ktx2_vs2013, $ktx2ktx2_vs2015, $ktx2ktx2_gmake or $ktx2ktx2_cmake found.
echo $0: Nor was ktx2ktx2 found in along PATH.
echo $0: toktx not found along $PATH.
echo $0: Aborting generation
exit 1
fi
Expand Down
40 changes: 15 additions & 25 deletions tests/testimages/genref
Original file line number Diff line number Diff line change
Expand Up @@ -13,42 +13,30 @@ depth=../..
# Change dir to the testimages folder, the script location...
cd $(dirname $0)

# Make paths relative to the testimages directory.
ktx_root=$depth
toktx_vs2013=$ktx_root/build/msvs/win/vs2013/x64/Release/toktx.exe
toktx_vs2015=$ktx_root/build/msvs/win/vs2015/x64/Release/toktx.exe
toktx_cmake=$ktx_root/build/cmake/linux/Release/toktx
toktx_cmake_d=$ktx_root/build/cmake/linux/Debug/toktx
toktx_make=$ktx_root/build/make/linux/out/Release/toktx
toktx_make_d=$ktx_root/build/make/linux/out/Debug/toktx

# Ensure generation is not polluted by user environment
unset TOKTX_OPTIONS

if [ -n "$1" -a -x "$1" ]; then
if [ -n "$1" -a -x "$2" ]; then
toktx="$1"
elif [ -x "$toktx_vs2013" ]; then
toktx=$toktx_vs2013
elif [ -x "$toktx_vs2015" ]; then
toktx=$toktx_vs2015
elif [ -x "$toktx_cmake" ]; then
toktx=$toktx_cmake
elif [ -x "$toktx_cmake_d" ]; then
toktx=$toktx_cmake_d
elif [ -x "$toktx_make" ]; then
toktx=$toktx_gmake
elif [ -x "$toktx_make_d" ]; then
toktx=$toktx_gmake
elif which toktx >/dev/null; then
toktx=toktx
else
echo $0: None of $toktx_vs2013, $toktx_vs2015, $toktx_gmake or $toktx_cmake found.
echo $0: Nor was toktx found in along PATH.
echo $0: toktx not found along $PATH.
echo $0: Aborting generation
exit 1
fi

# Generate the files ...
if [ -n "$2" -a -x "$1" ]; then
ktxsc="$2"
elif which ktxsc >/dev/null; then
ktxsc=ktxsc
else
echo $0: ktxsc not found along $PATH.
echo $0: Aborting generation
exit 1
fi

# Generate the reference files for toktx and sctests ...

$toktx --lower_left_maps_to_s0t0 --nometadata rgb-reference.ktx ../srcimages/rgb.ppm
# The purpose of this is testing automatic mipmap generation. Since SRGB is
Expand Down Expand Up @@ -135,3 +123,5 @@ $toktx --test --depth 7 --encode astc --astc_blk_d 6x6 astc_ldr_6x6
$toktx --test --layers 7 --t2 arraytex_7_reference_u.ktx2 ../srcimages/red16.png ../srcimages/orange16.png ../srcimages/yellow16.png ../srcimages/green16.png ../srcimages/blue16.png ../srcimages/indigo16.png ../srcimages/violet16.png
$toktx --test --depth 7 --t2 3dtex_7_reference_u.ktx2 ../srcimages/red16.png ../srcimages/orange16.png ../srcimages/yellow16.png ../srcimages/green16.png ../srcimages/blue16.png ../srcimages/indigo16.png ../srcimages/violet16.png
$toktx --test --layers 7 --genmipmap --t2 arraytex_7_mipmap_reference_u.ktx2 ../srcimages/red16.png ../srcimages/orange16.png ../srcimages/yellow16.png ../srcimages/green16.png ../srcimages/blue16.png ../srcimages/indigo16.png ../srcimages/violet16.png

$ktxsc -o skybox_zstd.ktx2 --force --test --zcmp 5 skybox.ktx2
Loading

0 comments on commit f020c1b

Please sign in to comment.