-
Notifications
You must be signed in to change notification settings - Fork 125
CI : Update to GafferHQ/dependencies 10.0.0 #1481
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
CI : Update to GafferHQ/dependencies 10.0.0 #1481
Conversation
cb434d1 to
ccf0455
Compare
SConstruct
Outdated
| if env["PLATFORM"] == "win32" : | ||
| pxrVersionHeader = env.FindFile( "pxr/pxr.h", dependencyIncludes ) | ||
| if pxrVersionHeader is not None and "#define PXR_USE_INTERNAL_BOOST_PYTHON\n" in open( str( pxrVersionHeader ) ) : | ||
| # Windows builds currently require both boost_python and USD's internal library, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the story here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit of an odd one. From this patch, it appears this is how Cortex has been built for the dependencies-10 Windows releases Linking only to boost_python results in one set of missing symbols, and linking only to usd_python results in another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be my guess :
IECoreUSD.libdoes need to link tousd_python.so, to satisfy USD's own linking requirements. But it doesn't need to link toboost_pythonbecause our Python bindings aren't inlibIECoreUSD.so. So your patch where you replaced one library with the other depending on PXR_USE_INTERNAL_BOOST_PYTHON was right.- But
_IECoreUSD.lib(the python module) also needs to link tousd_python.so, again to satisfy USD's own linking requirements. So as well as replacing inusdEnv, we need to add inusdPythonModuleEnv.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be my guess :
Thanks! That appears to have done the trick, I've rebased this PR and updated the approach in 304f1da to link IECoreUSD to either usd_python or boost_python, and _IECoreUSD to both when necessary.
We'll be able to reinstate it at some point in the future when macOS builds of dependencies-10.x.x are available.
We take the same approach as gafferhq/gaffer to install the required SDK version as it's no longer provided by the runner.
de8454c to
fdcc8f7
Compare
In this version, the Python bindings use Pixar's fork of Boost Python unconditionally, so we need to link to that instead.
fdcc8f7 to
304f1da
Compare
No description provided.