Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cmake cleanup #837

Merged
merged 14 commits into from
Jan 8, 2021
Merged

Conversation

meshula
Copy link
Collaborator

@meshula meshula commented Dec 8, 2020

This commit allows a full cmake build.

  • consolidate options at the top level
  • detect a "reasonable" place to install the python components
  • restrict gcov coverage to platforms that support it (those built with gcc)
  • use cmake build from setup,py on all platforms (cmake will call make appropriately, so the outcome is identical to how it was)
  • update submodules (any/optional/etc) BEFORE installing them
  • add ability to build OTIO libraries as static
  • add optional debug postfix because windows release/debug builds are not compatible
  • add platform appropriate rpaths (@loader_path on Mac, $ORIGIN on Linux)
  • add ability for cmake to do an install into the python environment so that a cmake build automatically results in a functional python build
  • more granular build control (turn python builds or c++ builds on or off independently)
  • For IDEs, rename the .sln or .xcode file to OpenTimelineIO (was OPENTIMELINEIO_ROOT)

@meshula meshula mentioned this pull request Dec 8, 2020
@ssteinbach ssteinbach added this to the Public Beta 14 milestone Dec 10, 2020
@ssteinbach ssteinbach added this to In progress in WG: Build Improvements via automation Dec 10, 2020
@codecov-io
Copy link

codecov-io commented Dec 13, 2020

Codecov Report

Merging #837 (55489b9) into master (5aa24fb) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #837   +/-   ##
=======================================
  Coverage   84.18%   84.18%           
=======================================
  Files          74       74           
  Lines        3061     3061           
=======================================
  Hits         2577     2577           
  Misses        484      484           
Flag Coverage Δ
unittests 84.18% <ø> (ø)

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


Continue to review full report at Codecov.

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

@meshula
Copy link
Collaborator Author

meshula commented Dec 16, 2020

Related: #813

Before merging, I need to test pip's --target directive to make sure the target is properly transmitted to the cmake script.

@meshula
Copy link
Collaborator Author

meshula commented Dec 16, 2020

Maybe related: #838 ~ If there is a more general PyInstaller issue, we could reopen 838 after this PR is merged.

@Tilix4
Copy link
Contributor

Tilix4 commented Jan 8, 2021

Thanks! I confirm this will solve #813. I've tested the --target option and it works well on my windows.

Thanks, hope this will land soon :)

Copy link
Collaborator

@ssteinbach ssteinbach left a comment

Choose a reason for hiding this comment

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

couple of really small questions around commenting and removing some debug statements

setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
WG: Build Improvements automation moved this from In progress to Review in progress Jan 8, 2021
WG: Build Improvements automation moved this from Review in progress to Reviewer approved Jan 8, 2021
@ssteinbach ssteinbach merged commit bcdd9a7 into AcademySoftwareFoundation:master Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants