AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.47k stars 294 forks source link

Offer alternative for cxxabi on Windows (Swift+Clang). #1787

Closed furby-tm closed 1 month ago

furby-tm commented 4 months ago

Summarize your change.

This fixes the clang compilation of OpenTimelineIO on Microsoft Windows, when compiling with Swift. Previously, the cxxabi.h header was erroneously getting included in the Windows clang compilation of OpenTimelineIO's stringUtils.cpp file because it was being conditionally compiled for clang across all platforms, and this header does not exist on the Windows platform.

linux-foundation-easycla[bot] commented 4 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.71%. Comparing base (c0e97b0) to head (69144f2). Report is 21 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787/graphs/tree.svg?width=650&height=150&src=pr&token=mfnfUB4h7u&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation)](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ```diff @@ Coverage Diff @@ ## main #1787 +/- ## ========================================== - Coverage 84.11% 81.71% -2.41% ========================================== Files 198 176 -22 Lines 22241 12319 -9922 Branches 4687 3022 -1665 ========================================== - Hits 18709 10066 -8643 + Misses 2610 1717 -893 + Partials 922 536 -386 ``` | [Flag](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [py-unittests](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | `81.71% <ø> (-2.41%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [src/opentimelineio/stringUtils.cpp](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787?src=pr&el=tree&filepath=src%2Fopentimelineio%2FstringUtils.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL29wZW50aW1lbGluZWlvL3N0cmluZ1V0aWxzLmNwcA==) | `44.44% <ø> (ø)` | | ... and [67 files with indirect coverage changes](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Last update [d213805...69144f2](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1787?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation).
furby-tm commented 3 months ago

If the team prefers to remove the cxxabi demangling, I can update this PR to deprecate it instead. Please feel free to ping me in this issue once the team has reached a consensus.

jminor commented 1 month ago

From some offline discussion, it seems there is no objection to removing the demangling. @furby-tm we would gladly accept this PR as-is, or with the demangling removed. Please let us know which you'd like.

furby-tm commented 1 month ago

I'll go ahead and remove the demangling @jminor

furby-tm commented 1 month ago

Closing in favor of https://github.com/AcademySoftwareFoundation/OpenTimelineIO/pull/1800