Skip to content

Use installed pybind11 and other CMake fixes#2009

Open
jfpanisset wants to merge 6 commits intoAcademySoftwareFoundation:mainfrom
jfpanisset:find-pybind11
Open

Use installed pybind11 and other CMake fixes#2009
jfpanisset wants to merge 6 commits intoAcademySoftwareFoundation:mainfrom
jfpanisset:find-pybind11

Conversation

@jfpanisset
Copy link
Copy Markdown

  • Adds OTIO_FIND_PYBIND11 configuration option to tell CMake to look for a pre-installed pybind11 instead of using the vendored copy. Follows existing pattern for Imath and rapidjson dependencies.
    Fixes #2006.

  • Bugfixes in src/deps/CMakeLists.txt

    • checking for empty submodule directories was broken
    • Imath submodule would not get checked for emptiness
    • pybind11 dependency only required when building Python bindings
  • Generated CMake files now installed in lib/cmake/opentime and lib/cmake/opentimelineio
    instead ofshare/opentime and share/opentimelineio as per CMake documentation.
    Fixes #2008.

- Adds OTIO_FIND_PYBIND11 configuration option to tell CMake
to look for a pre-installed pybind11 instead of using the
vendored copy. Follows existing pattern for Imath and rapidjson dependencies.
[Issue AcademySoftwareFoundation#2006](AcademySoftwareFoundation#2006).

- A couple of bugfixes in src/deps/CMakeLists.txt
  - checking for empty submodule directories was broken
  - rapidjson subdmodule would not get built

- Generated CMake files get installed in
/usr/local/lib/cmake/opentime and opentimelineio
instead of /usr/local/share/opentime and opentimelineio which
is what the CMake documentation suggests.
[Issue AcademySoftwareFoundation#2008](AcademySoftwareFoundation#2008).

Signed-off-by: Jean-Francois Panisset <panisset@gmail.com>
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 31, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@jfpanisset jfpanisset marked this pull request as draft March 31, 2026 19:14
@jfpanisset
Copy link
Copy Markdown
Author

Hmm, wasn't expecting this to break the build so badly. Converted PR to draft, looking into it.

@jfpanisset
Copy link
Copy Markdown
Author

For this failed build:

https://github.com/AcademySoftwareFoundation/OpenTimelineIO/actions/runs/23814704627/job/69410968153?pr=2009

the error is:

CMake Error at src/deps/rapidjson/example/CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.5 has been removed from CMake.

  Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
  to tell CMake that the project requires at least <min> but has been updated
  to work with policies introduced by <max> or earlier.

  Or, add -DCMAKE_POLICY_VERSION_MINIMUM=3.5 to try configuring anyway.

The src/deps/rapidjson submodule was updated to the latest commit, 24b5e7a and in example/CMakeLists.txt I find:

cmake_minimum_required(VERSION 2.8)

I see that my changes are actually trying to build the rapidjson submodule, but since it's a header only library, it doesn't need to get built, opentimelineio just needs to find the headers.

rapidjson as a submodule won't build in our environment due
to old CMake version requirements, and since it's a header-only
library, we only need the headers to be present to allow
opentimelineio to build.

Signed-off-by: Jean-Francois Panisset <panisset@gmail.com>
Signed-off-by: Jean-Francois Panisset <panisset@gmail.com>
Signed-off-by: Jean-Francois Panisset <panisset@gmail.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.14%. Comparing base (267fe4b) to head (da52ec1).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2009   +/-   ##
=======================================
  Coverage   85.14%   85.14%           
=======================================
  Files         181      181           
  Lines       12780    12780           
  Branches     1206     1206           
=======================================
  Hits        10882    10882           
  Misses       1715     1715           
  Partials      183      183           
Flag Coverage Δ
py-unittests 85.14% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 267fe4b...da52ec1. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jfpanisset jfpanisset marked this pull request as ready for review March 31, 2026 20:23
Address PR feedback: better explanation of why we don't
try to build the rapidjson dependency.

Signed-off-by: Jean-Francois Panisset <panisset@gmail.com>
Signed-off-by: Jean-Francois Panisset <panisset@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants