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

Support --prefix correctly in new cmake system #886

Merged

Conversation

ssteinbach
Copy link
Collaborator

@ssteinbach ssteinbach commented Feb 16, 2021

Closes #885

Refactors how the setup.py communicates target locations to the cmake so that --prefix and --user arguments are correctly handled.

POTENTIALLY BREAKING CHANGE: Now if installing via pip/setup.py, the C++ libraries are installed under the python install in a cxx-sdk instead of cxx-libs directory.

I'd like to note that the way of finding the value of the --prefix argument is pretty gross. If there is a better/cleaner way, please let me know!

- as far as our googling can tell, this was the only way we could find
  to get the value of the --prefix commandline argument supplied to
  setup.py.
- needed to change how paths get sent into the cmake part of the build
- also moves the cxx-lib directory to be called cxx-sdk
- use the same logic for where the cxx-sdk location regardless of which
  branch through the setup.py is taken
@ssteinbach ssteinbach added this to the Public Beta 14 milestone Feb 16, 2021
@codecov-io
Copy link

codecov-io commented Feb 16, 2021

Codecov Report

Merging #886 (eed50c9) into master (546b333) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #886   +/-   ##
=======================================
  Coverage   86.63%   86.64%           
=======================================
  Files         183      183           
  Lines       17874    17875    +1     
  Branches     1971     1971           
=======================================
+ Hits        15486    15487    +1     
  Misses       1904     1904           
  Partials      484      484           
Flag Coverage Δ
unittests 86.56% <ø> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
src/opentimelineio/serializableObject.h 90.00% <0.00%> (+0.10%) ⬆️

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 546b333...eed50c9. Read the comment docs.

Copy link
Collaborator

@meshula meshula left a comment

Choose a reason for hiding this comment

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

The thing we should watch for is that the CMake.config files point within cxx-libs on our various platforms. This is a good step forward in any case to handling pip's --prefix complication

@ssteinbach ssteinbach changed the title Setuppy prefix Support --prefix correctly in new cmake system Feb 17, 2021
@ssteinbach ssteinbach merged commit 635a188 into AcademySoftwareFoundation:master Feb 17, 2021
@ssteinbach ssteinbach deleted the setuppy_prefix branch February 17, 2021 18:50
@meshula meshula added the build issues building OTIO label Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build issues building OTIO
Projects
Development

Successfully merging this pull request may close these issues.

setup.py/pip --prefix argument not handled properly
3 participants