Skip to content

Fixes for API exports - #2039

Merged
darbyjohnston merged 2 commits into
AcademySoftwareFoundation:mainfrom
darbyjohnston:windows-shared-exports
Sep 22, 2026
Merged

darbyjohnston merged 2 commits into
AcademySoftwareFoundation:mainfrom
darbyjohnston:windows-shared-exports

Conversation

@darbyjohnston

@darbyjohnston darbyjohnston commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

This PR has various fixes for the API exports:

  • Change OTIO_EXPORTS and OPENTIME_EXPORTS to PRIVATE
  • Add missing exports
  • Remove the exports from templates which are inlined
  • Add missing includes

Assisted-by: Claude:claude-opus-5 [debugging]

@codecov-commenter

codecov-commenter commented Sep 11, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.52%. Comparing base (b424801) to head (98b84aa).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2039   +/-   ##
=======================================
  Coverage   83.52%   83.52%           
=======================================
  Files         182      182           
  Lines       13533    13533           
  Branches     1255     1255           
=======================================
  Hits        11303    11303           
  Misses       2057     2057           
  Partials      173      173           
Flag Coverage Δ
py-unittests 83.52% <ø> (ø)

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

Files with missing lines Coverage Δ
src/opentimelineio/color.h 66.66% <ø> (ø)
src/opentimelineio/composition.h 82.75% <ø> (ø)
src/opentimelineio/deserialization.cpp 62.88% <ø> (ø)
src/opentimelineio/imageSequenceReference.h 56.52% <ø> (ø)
src/opentimelineio/serializableCollection.h 75.00% <ø> (ø)
src/opentimelineio/serializableObject.h 83.78% <ø> (ø)
...rc/opentimelineio/serializableObjectWithMetadata.h 100.00% <ø> (ø)
src/opentimelineio/stringUtils.cpp 54.54% <ø> (ø)
src/opentimelineio/timeline.h 83.33% <ø> (ø)
src/opentimelineio/transition.h 100.00% <ø> (ø)
... and 1 more

Continue to review full report in Codecov by Harness.

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

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

@meshula meshula left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Darby, this is all correct, thank you

OTIO_EXPORTS and OPENTIME_EXPORTS were PUBLIC, so everything consuming
OTIO compiled its API as dllexport where it has to be dllimport. They
become PRIVATE, which says the library is being built rather than
consumed; the OTIO_STATIC and OPENTIME_STATIC definitions stay PUBLIC,
since a consumer does have to know the API carries no declspec at all.

With that corrected the members marked only at the class level came up
missing, 74 of them, the Color statics and TypeRegistry among them:
OTIO_API_TYPE is empty on Windows, where only the per-member OTIO_API
carries the declspec. Those members are marked.

The three find_children templates lose OTIO_API in turn, a template
defined in a header being something that cannot be imported, and two
source files that define exported functions without including the
header that marks them are given the include.

No change to what either library exports on macOS or Linux: the
symbol tables of a shared build are identical before and after.

Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
SerializableObject's destructor, its Reader's any overload and type
check, and its Writer's int64_t overload had no OTIO_API on them. A
build that hides its symbols by default, which main does since AcademySoftwareFoundation#2041,
leaves them out of the library and upgrade_downgrade_example cannot be
linked.

Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
@darbyjohnston darbyjohnston added the bug A problem, flaw, or broken functionality. label Sep 22, 2026
@darbyjohnston darbyjohnston added this to the 0.19.0 milestone Sep 22, 2026
@darbyjohnston
darbyjohnston merged commit 64bb3cd into AcademySoftwareFoundation:main Sep 22, 2026
51 checks passed
darbyjohnston added a commit that referenced this pull request Sep 22, 2026
* Put Windows executables and their DLLs in one build directory

Nothing set an output directory, so on a multi-config generator the test
and example executables land in build/tests/<config> while the shared
libraries land in build/src/<lib>/<config>. Windows looks for a DLL next
to the executable or on PATH, neither of which covers that, so the tests
cannot start from the build tree when OTIO_SHARED_LIBS is ON.

Windows executables and DLLs now go to build/bin. The other platforms
resolve a shared library through its install name or RPATH and are left
alone, as are static builds, where an archive is not a runtime artifact.

Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>

* Build and test shared libraries in CI

OTIO_SHARED_LIBS defaults to ON, but every build in the repository turned
it off: setup.py and the only C++ job. The default configuration was
therefore never built or tested, and it is the only one where the export
annotations mean anything, as #2039 showed.

The C++ job gains a shared dimension, six legs rather than three. Code
coverage stays on the static Linux leg alone rather than being collected
twice.

Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>

* Set the coverage flag from the matrix rather than an expression

The ternary form of && and || is easy to misread, and it is only correct
while both of its values are truthy. The matrix carries the flag instead.

Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>

---------

Signed-off-by: Darby Johnston <darbyjohnston@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug A problem, flaw, or broken functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants