Skip to content

Commit e382339

Browse files
committed
ci: require all dependencies, with explicit exceptions (AcademySoftwareFoundation#4898)
We recently had a problem where our use of a dependency was broken, and it had gone unnoticed because none of our CI jobs actually had the dependency present and tested it. This can easily happen because so many of our dependencies are "optional" -- our build will just give a warning and silently disable the functionality that would have been supported by the missing dependency. We have long had build-time options `OpenImageIO_REQUIRED_DEPS`, to consider specific dependencies (or "all") required even if they would ordinarily be optional, and `OpenImageIO_OPTIONAL_DEPS`, to make exceptions. But we didn't use these in CI. So this PR makes sets `OpenImageIO_REQUIRED_DEPS=all` to make all dependencies ostensibly required, and then list all exceptions explicitly. This should make it much more difficult in the future to make a mistake where use of a dependency is completely untested in our CI without our being aware of it. And it gives us a visible "hit list" of untested or under-tested dependencies to slowly whittle down. Some changes that came long for the ride: * checked_find_package: explicitly disabled packages are treated as optional. * cuda_macros.cmake: Only look for CUDA on platforms that might conceivably have it (i.e., don't even look on Mac). * Separate linux-aswf from linux-ubuntu into separate job groups, because that makes it easier to have shared commonalities of which dependencies they test. Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 1cd674f commit e382339

File tree

5 files changed

+118
-16
lines changed

5 files changed

+118
-16
lines changed

.github/workflows/build-steps.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ on:
6868
type: string
6969
nametag:
7070
type: string
71+
required_deps:
72+
type: string
73+
optional_deps:
74+
type: string
7175
secrets:
7276
PASSED_GITHUB_TOKEN:
7377
required: false
@@ -102,6 +106,12 @@ jobs:
102106
ABI_CHECK: ${{inputs.abi_check}}
103107
ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION: node16
104108
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
109+
# For CI, sometimes we want to require all dependencies to be present,
110+
# except for a select few listed explicitly. This ensures that we don't
111+
# accidentally have a situation in which we think we are building
112+
# against and testing an optional dependency, but in fact are not.
113+
OpenImageIO_REQUIRED_DEPS: ${{inputs.required_deps}}
114+
OpenImageIO_OPTIONAL_DEPS: ${{inputs.optional_deps}}
105115

106116
steps:
107117
- name: Checkout repo

.github/workflows/ci.yml

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,9 @@ jobs:
239239
240240
241241
#
242-
# Linux Tests
242+
# Linux Tests using ASWF-docker containers
243243
#
244-
linux:
244+
linux-aswf:
245245
if: ${{ ! contains(github.ref, 'windows-only') && ! contains(github.ref, 'macos-only') }}
246246
name: "${{matrix.desc}}"
247247
uses: ./.github/workflows/build-steps.yml
@@ -273,6 +273,10 @@ jobs:
273273
ctest_test_timeout: ${{ matrix.ctest_test_timeout }}
274274
coverage: ${{ matrix.coverage || 0 }}
275275
sonar: ${{ matrix.sonar || 0 }}
276+
# Override required_deps to be 'all' and explicitly list as optional
277+
# only the ones we are intentionally not testing for those jobs.
278+
required_deps: ${{ matrix.required_deps || 'all' }}
279+
optional_deps: ${{ matrix.optional_deps || 'DCMTK;JXL;Libheif;Nuke;OpenCV;openjph;OpenVDB;Qt5;R3DSDK;'}}${{matrix.optional_deps_append}}
276280
strategy:
277281
fail-fast: false
278282
matrix:
@@ -287,6 +291,7 @@ jobs:
287291
fmt_ver: 10.1.1
288292
pybind11_ver: v2.10.0
289293
setenvs: export PUGIXML_VERSION=v1.13
294+
optional_deps_append: 'LibRaw;Ptex;Qt6'
290295
- desc: VFX2023 icc/C++17 py3.10 exr3.1 ocio2.3 qt5.15
291296
nametag: linux-vfx2023.icc
292297
runner: ubuntu-latest
@@ -303,6 +308,7 @@ jobs:
303308
DISABLE_libuhdr=1
304309
# For icc, use fp-model precise to eliminate needless LSB errors
305310
# that make test results differ from other platforms.
311+
optional_deps_append: "LibRaw;Ptex;Qt6"
306312
- desc: VFX2023 icx/C++17 py3.10 exr3.1 ocio2.3 qt5.15
307313
nametag: linux-vfx2023.icx
308314
runner: ubuntu-latest
@@ -319,6 +325,7 @@ jobs:
319325
UHDR_CMAKE_CXX_COMPILER=g++
320326
# Building libuhdr with icx results in test failures
321327
# so we force using gcc/g++.
328+
optional_deps_append: "LibRaw;Ptex;Qt6"
322329
- desc: VFX2024 gcc11/C++17 py3.11 exr3.2 ocio2.3
323330
nametag: linux-vfx2024
324331
runner: ubuntu-latest
@@ -330,6 +337,7 @@ jobs:
330337
pybind11_ver: v2.12.0
331338
benchmark: 1
332339
setenvs: export PUGIXML_VERSION=v1.14
340+
optional_deps_append: "LibRaw"
333341
- desc: VFX2024 clang/C++17 py3.11 exr3.2 ocio2.3
334342
nametag: linux-vfx2024.clang
335343
runner: ubuntu-latest
@@ -343,6 +351,7 @@ jobs:
343351
pybind11_ver: v2.12.0
344352
benchmark: 1
345353
setenvs: export PUGIXML_VERSION=v1.14
354+
optional_deps_append: "LibRaw"
346355
- desc: VFX2025 gcc11/C++17 py3.11 exr3.3 ocio2.4
347356
nametag: linux-vfx2025
348357
runner: ubuntu-latest
@@ -354,6 +363,7 @@ jobs:
354363
pybind11_ver: v2.13.6
355364
benchmark: 1
356365
setenvs: export PUGIXML_VERSION=v1.15
366+
optional_deps_append: "openjph;Qt6"
357367
- desc: VFX2026 gcc14/C++20 py3.13 exr3.4 ocio2.4
358368
nametag: linux-vfx2026
359369
runner: ubuntu-latest
@@ -364,6 +374,7 @@ jobs:
364374
pybind11_ver: v3.0.0
365375
benchmark: 1
366376
# setenvs: export
377+
optional_deps_append: "Qt5;Qt6"
367378
- desc: Sanitizers
368379
nametag: sanitizer
369380
runner: ubuntu-latest
@@ -378,6 +389,7 @@ jobs:
378389
OIIO_CMAKE_FLAGS="-DSANITIZE=address,undefined -DOIIO_HARDENING=3 -DUSE_PYTHON=0"
379390
CTEST_EXCLUSIONS="broken|png-damaged"
380391
OpenImageIO_BUILD_LOCAL_DEPS=PNG
392+
optional_deps_append: "LibRaw"
381393

382394
# Test ABI stability. `abi_check` is the version or commit that we
383395
# believe is the current standard against which we don't want to
@@ -397,7 +409,52 @@ jobs:
397409
abi_check: 9bfcce725a3806a3f70c7e838d9d98d6d95c917a
398410
setenvs: export OIIO_CMAKE_FLAGS="-DOIIO_BUILD_TOOLS=0 -DOIIO_BUILD_TESTS=0 -DUSE_PYTHON=0"
399411
USE_OPENCV=0 USE_FFMPEG=0 USE_PYTHON=0 USE_FREETYPE=0
412+
optional_deps_append: "openjph;Qt6"
400413

414+
415+
#
416+
# Linux Tests using GHA Ubuntu runners directly
417+
#
418+
linux-ubuntu:
419+
if: ${{ ! contains(github.ref, 'windows-only') && ! contains(github.ref, 'macos-only') }}
420+
name: "${{matrix.desc}}"
421+
uses: ./.github/workflows/build-steps.yml
422+
with:
423+
nametag: ${{ matrix.nametag || 'unnamed!' }}
424+
runner: ${{ matrix.runner || 'ubuntu-latest' }}
425+
container: ${{ matrix.container }}
426+
cc_compiler: ${{ matrix.cc_compiler }}
427+
cxx_compiler: ${{ matrix.cxx_compiler }}
428+
cxx_std: ${{ matrix.cxx_std || '17' }}
429+
build_type: ${{ matrix.build_type || 'Release' }}
430+
depcmds: ${{ matrix.depcmds }}
431+
extra_artifacts: ${{ matrix.extra_artifacts }}
432+
fmt_ver: ${{ matrix.fmt_ver }}
433+
opencolorio_ver: ${{ matrix.opencolorio_ver }}
434+
openexr_ver: ${{ matrix.openexr_ver }}
435+
pybind11_ver: ${{ matrix.pybind11_ver }}
436+
python_ver: ${{ matrix.python_ver }}
437+
setenvs: ${{ matrix.setenvs }}
438+
simd: ${{ matrix.simd }}
439+
skip_build: ${{ matrix.skip_build }}
440+
skip_tests: ${{ matrix.skip_tests }}
441+
abi_check: ${{ matrix.abi_check }}
442+
benchmark: ${{ matrix.benchmark }}
443+
build_docs: ${{ matrix.build_docs }}
444+
clang_format: ${{ matrix.clang_format }}
445+
generator: ${{ matrix.generator }}
446+
ctest_args: ${{ matrix.ctest_args }}
447+
ctest_test_timeout: ${{ matrix.ctest_test_timeout }}
448+
coverage: ${{ matrix.coverage || 0 }}
449+
sonar: ${{ matrix.sonar || 0 }}
450+
# Override required_deps to be 'all' and explicitly list as optional
451+
# only the ones we are intentionally not testing for those jobs.
452+
required_deps: ${{ matrix.required_deps || 'all' }}
453+
optional_deps: ${{ matrix.optional_deps || 'CUDAToolkit;DCMTK;JXL;Nuke;OpenGL;openjph;OpenVDB;Ptex;pystring;Qt5;R3DSDK;' }}${{matrix.optional_deps_append}}
454+
strategy:
455+
fail-fast: false
456+
matrix:
457+
include:
401458
# Test formatting. This test entry doesn't build at all, it
402459
# just runs clang-format on everything, and passes if nothing is
403460
# misformatted. Upon failure, the build artifact will be the full
@@ -440,6 +497,10 @@ jobs:
440497
WEBP_VERSION=v1.6.0
441498
FREETYPE_VERSION=VER-2-14-0
442499
USE_OPENVDB=0
500+
# Ensure we are testing all the deps we think we are. We would
501+
# like this test to have minimal missing dependencies.
502+
required_deps: all
503+
optional_deps: 'CUDAToolkit;DCMTK;JXL;Nuke;OpenCV;OpenGL;OpenVDB;R3DSDK'
443504
- desc: bleeding edge gcc14 C++23 py3.12 OCIO/libtiff/exr-main avx2
444505
nametag: linux-bleeding-edge
445506
runner: ubuntu-24.04
@@ -467,6 +528,10 @@ jobs:
467528
FREETYPE_VERSION=master
468529
QT_VERSION=0 INSTALL_OPENCV=0
469530
# The installed OpenVDB has a TLS conflict with Python 3.8
531+
# Ensure we are testing all the deps we think we are. We would
532+
# like this test to have minimal missing dependencies.
533+
required_deps: all
534+
optional_deps: 'CUDAToolkit;DCMTK;JXL;Nuke;OpenCV;OpenGL;OpenVDB;R3DSDK'
470535
- desc: all local builds gcc12 C++17 avx2 exr3.2 ocio2.3
471536
nametag: linux-local-builds
472537
runner: ubuntu-22.04
@@ -482,7 +547,6 @@ jobs:
482547
PTEX_VERSION=v2.4.2
483548
PUGIXML_VERSION=v1.14
484549
WEBP_VERSION=v1.4.0
485-
486550
- desc: clang15 C++17 avx2 exr3.1 ocio2.3
487551
nametag: linux-clang15
488552
runner: ubuntu-22.04
@@ -597,6 +661,11 @@ jobs:
597661
ctest_test_timeout: ${{ matrix.ctest_test_timeout || '800' }}
598662
coverage: ${{ matrix.coverage || 0 }}
599663
sonar: ${{ matrix.sonar || 0 }}
664+
# We're able to use Homebrew to install ALMOST every dependency, so the
665+
# only optional ones in our Mac CI tests are commercial things we can't
666+
# test in GHA CI.
667+
required_deps: ${{ matrix.required_deps || 'all' }}
668+
optional_deps: ${{ matrix.optional_deps || 'Nuke;R3DSDK;' }}${{matrix.optional_deps_append}}
600669
strategy:
601670
fail-fast: false
602671
matrix:
@@ -663,6 +732,11 @@ jobs:
663732
ctest_test_timeout: ${{ matrix.ctest_test_timeout }}
664733
coverage: ${{ matrix.coverage || 0 }}
665734
sonar: ${{ matrix.sonar || 0 }}
735+
# Windows is a PITA, so we expect very few dependencies to be present or
736+
# built. But we would like to add more dependencies and reduce this list
737+
# of exceptions in the future.
738+
required_deps: ${{ matrix.required_deps || 'all' }}
739+
optional_deps: ${{ matrix.optional_deps || 'CUDAToolkit;DCMTK;FFmpeg;GIF;JXL;Libheif;LibRaw;Nuke;OpenCV;OpenGL;OpenJPEG;openjph;OpenCV;OpenVDB;Ptex;pystring;Qt5;Qt6;TBB;R3DSDK;${{matrix.optional_deps_append}}' }}
666740
strategy:
667741
fail-fast: false
668742
matrix:

src/build-scripts/gh-installdeps.bash

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,28 @@ if [[ "$ASWF_ORG" != "" ]] ; then
1717

1818
#ls /etc/yum.repos.d
1919

20-
if [[ "$ASWF_VFXPLATFORM_VERSION" == "2021" || "$ASWF_VFXPLATFORM_VERSION" == "2022" ]] ; then
21-
# CentOS 7 based containers need the now-nonexistant centos repo to be
20+
# time sudo dnf upgrade --refresh || true
21+
time sudo dnf install --nogpgcheck https://mirrors.rpmfusion.org/free/el/rpmfusion-free-release-$(rpm -E %rhel).noarch.rpm -y || true
22+
23+
if [[ "$ASWF_VFXPLATFORM_VERSION" == "2022" ]] ; then
24+
# CentOS 7 based containers need the now-nonexistent centos repo to be
2225
# excluded or all the subsequent yum install commands will fail.
2326
yum-config-manager --disable centos-sclo-rh || true
2427
sed -i 's,^mirrorlist=,#,; s,^#baseurl=http://mirror\.centos\.org/centos/$releasever,baseurl=https://vault.centos.org/7.9.2009,' /etc/yum.repos.d/CentOS-Base.repo
2528
fi
2629

27-
sudo yum install -y giflib giflib-devel || true
30+
time time sudo yum install -y giflib giflib-devel || true
2831
if [[ "${USE_OPENCV}" != "0" ]] ; then
29-
sudo yum install -y opencv opencv-devel || true
32+
time sudo yum install -y opencv opencv-devel || true
3033
fi
3134
if [[ "${USE_FFMPEG}" != "0" ]] ; then
32-
sudo yum install -y ffmpeg ffmpeg-devel || true
35+
time sudo dnf install -y ffmpeg ffmpeg-devel || true
3336
fi
3437
if [[ "${USE_FREETYPE:-1}" != "0" ]] ; then
35-
sudo yum install -y freetype freetype-devel || true
38+
time sudo yum install -y freetype freetype-devel || true
39+
fi
40+
if [[ "${USE_LIBRAW:-0}" != "0" ]] ; then
41+
time sudo yum install -y LibRaw LibRaw-devel || true
3642
fi
3743
if [[ "${EXTRA_DEP_PACKAGES}" != "" ]] ; then
3844
time sudo yum install -y ${EXTRA_DEP_PACKAGES} || true

src/cmake/cuda_macros.cmake

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
# https://github.com/AcademySoftwareFoundation/OpenImageIO
44

55

6-
set_option (OIIO_USE_CUDA "Include Cuda support if found" OFF)
6+
set_option (OIIO_USE_CUDA "Include CUDA support if found" OFF)
77
set_cache (CUDA_TARGET_ARCH "sm_60" "CUDA GPU architecture (e.g. sm_60)")
88
set_cache (CUDAToolkit_ROOT "" "Path to CUDA toolkit")
99

10-
if (OIIO_USE_CUDA)
10+
if (OIIO_USE_CUDA AND NOT APPLE)
1111
set (CUDA_PROPAGATE_HOST_FLAGS ON)
1212
set (CUDA_VERBOSE_BUILD ${VERBOSE})
1313
checked_find_package(CUDAToolkit

src/cmake/dependency_utils.cmake

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ function (dump_matching_variables pattern)
7676
endfunction ()
7777

7878

79-
# Helper: Print a report about missing dependencies and give insructions on
79+
# Helper: Print a report about missing dependencies and give instructions on
8080
# how to turn on automatic local dependency building.
8181
function (print_package_notfound_report)
8282
message (STATUS)
@@ -286,7 +286,7 @@ macro (checked_find_package pkgname)
286286
#
287287
cmake_parse_arguments(_pkg # prefix
288288
# noValueKeywords:
289-
"REQUIRED;CONFIG;PREFER_CONFIG;DEBUG;NO_RECORD_NOTFOUND;NO_FP_RANGE_CHECK"
289+
"REQUIRED;OPTIONAL;CONFIG;PREFER_CONFIG;DEBUG;NO_RECORD_NOTFOUND;NO_FP_RANGE_CHECK"
290290
# singleValueKeywords:
291291
"ENABLE;ISDEPOF;VERSION_MIN;VERSION_MAX;RECOMMEND_MIN;RECOMMEND_MIN_REASON;BUILD_LOCAL"
292292
# multiValueKeywords:
@@ -303,17 +303,24 @@ macro (checked_find_package pkgname)
303303
set (${pkgname}_FIND_QUIETLY true)
304304
set (${pkgname_upper}_FIND_QUIETLY true)
305305
endif ()
306-
if ("${pkgname}" IN_LIST ${PROJECT_NAME}_REQUIRED_DEPS OR "ALL" IN_LIST ${PROJECT_NAME}_REQUIRED_DEPS)
306+
if ("${pkgname}" IN_LIST ${PROJECT_NAME}_REQUIRED_DEPS
307+
OR "ALL" IN_LIST ${PROJECT_NAME}_REQUIRED_DEPS
308+
OR "all" IN_LIST ${PROJECT_NAME}_REQUIRED_DEPS)
307309
set (_pkg_REQUIRED 1)
308310
endif ()
309-
if ("${pkgname}" IN_LIST ${PROJECT_NAME}_OPTIONAL_DEPS OR "ALL" IN_LIST ${PROJECT_NAME}_OPTIONAL_DEPS)
311+
if ("${pkgname}" IN_LIST ${PROJECT_NAME}_OPTIONAL_DEPS
312+
OR "ALL" IN_LIST ${PROJECT_NAME}_OPTIONAL_DEPS
313+
OR "all" IN_LIST ${PROJECT_NAME}_OPTIONAL_DEPS)
310314
set (_pkg_REQUIRED 0)
315+
set (_pkg_OPTIONAL 1)
311316
endif ()
312317
# string (TOLOWER "${_pkg_BUILD_LOCAL}" _pkg_BUILD_LOCAL)
313318
if ("${pkgname}" IN_LIST ${PROJECT_NAME}_BUILD_LOCAL_DEPS
319+
OR ${PROJECT_NAME}_BUILD_LOCAL_DEPS STREQUAL "ALL"
314320
OR ${PROJECT_NAME}_BUILD_LOCAL_DEPS STREQUAL "all")
315321
set (_pkg_BUILD_LOCAL "always")
316322
elseif ("${pkgname}" IN_LIST ${PROJECT_NAME}_BUILD_MISSING_DEPS
323+
OR ${PROJECT_NAME}_BUILD_MISSING_DEPS STREQUAL "ALL"
317324
OR ${PROJECT_NAME}_BUILD_MISSING_DEPS STREQUAL "all")
318325
set_if_not (_pkg_BUILD_LOCAL "missing")
319326
endif ()
@@ -341,6 +348,11 @@ macro (checked_find_package pkgname)
341348
set (_quietskip true)
342349
endif ()
343350
endif ()
351+
if (NOT _enable)
352+
set (_pkg_OPTIONAL 1)
353+
set (_pkg_REQUIRED 0)
354+
message(STATUS "Forcing optional of disabled ${pkgname}")
355+
endif ()
344356
set (_config_status "")
345357
unset (_${pkgname}_version_range)
346358
if (_pkg_BUILD_LOCAL AND NOT _pkg_NO_FP_RANGE_CHECK)
@@ -361,7 +373,7 @@ macro (checked_find_package pkgname)
361373
#
362374
set (${pkgname}_FOUND FALSE)
363375
set (${pkgname}_LOCAL_BUILD FALSE)
364-
if (_enable OR _pkg_REQUIRED)
376+
if (_enable OR (_pkg_REQUIRED AND NOT _pkg_OPTIONAL))
365377
# Unless instructed not to, try to find the package externally
366378
# installed.
367379
if (${pkgname}_FOUND OR ${pkgname_upper}_FOUND OR _pkg_BUILD_LOCAL STREQUAL "always")

0 commit comments

Comments
 (0)