-
Notifications
You must be signed in to change notification settings - Fork 460
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
Hide dependency symbol visibility #1409
Hide dependency symbol visibility #1409
Conversation
This is failing on Mac for reasons that I don't quite understand. Should we not be doing this on Mac? Or have I failed so add something else that's needed? I'm looking into it, but if anybody more expert than I am about this symbol visibility can spot the problem, please let me know. |
When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com>
Mystery solved. On Mac, ld does not accept that argument. PR amended. |
Note: I have made this PR against RB-2.0, because that's where I really need it right now. But if accepted, please don't forget to cherry-pick it into master so that will also be in 2.1. |
I'm not sure why the GPU tests failed, but looking at the logs, it doesn't appear to be related to my changes, since it never gets as far as building the software. |
I was under the impression that GPU tests were not run, except on a specific branch. Maybe someone with permissions can restart the job from within GH Actions panel. Expat symbols are there on macOS built library, not sure if it will cause the same issue you were seeing, but the linker doesn't seem to support the flag anyway as you mentioned. Looks like CMake 3.13 would allow us to use |
It's odd that the GPU job ran. It should only run on push to a branch under the primary OCIO repo. I'm going to try closing and reopening this PR to restart all checks, which may help. |
Closing and reopening didn't help, but note that the GPU check is not required. |
When integrating pull requests to create the release 2.0.1, the GPU CI build was also constantly failing. It took me a while at that time, to have a green build... There is definitively a weakness somewhere! |
Note: Here is a thread with potential solutions for macOS but that requests an extra step: From that thread, I tried the approach |
The GPU CI build should be fixed by PR #1412 |
When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com>
When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com>
…cademySoftwareFoundation#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com>
* Adsk Contrib - Improve file rules for v1 configs Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Emergency GPU build fix (#1391) Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Fix some bugs found by Maya and SonarCloud (#1403) * Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix a Windows warning Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * After Effects and Photoshop plug-in updates (#1373) * Set Mac OS deployment target to 10.10 Signed-off-by: Brendan Bolles <brendan@fnordware.com> * More 10.10 targets Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Ditch ADD_EXTRA_BUILTINS Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Getting strange callback when AE effect is pasted Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Use NDEBUG in AE project Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable radio buttons with no config Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Update version Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Move to AE 2021 SDK, set PF_OutFlag2_MUTABLE_RENDER_SEQUENCE_DATA_SLOWER Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Define OCIO_DEPRECATED in After Effects builds Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Verify change from color space menu Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Handle situation where color space has a '/' in it Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Windows family separator fix Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Windows Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable more controls Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Incorporate invert into LUT export Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Get rid of fullPaths stuff Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Fix changed config settings retention Signed-off-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Add OpenColorIOConfig.cmake generation (#1397) * Initial OpenColorIO Config CMake implementation Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove macro, improve documentation Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Use CMake visibility flags (#1411) * Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix Windows build Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update include/OpenColorIO/OpenColorIO.h Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update src/OpenColorIO/FileRules.cpp Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Hide dependency symbol visibility (#1409) (#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Improve DX11 & Cg fragment shader language support (#1406) * Adsk Contrib - Improve DX11 support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve Cg support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve GPU comments Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix unit test failures Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
* Adsk Contrib - Improve file rules for v1 configs Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Emergency GPU build fix (#1391) Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Fix some bugs found by Maya and SonarCloud (#1403) * Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix a Windows warning Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * After Effects and Photoshop plug-in updates (#1373) * Set Mac OS deployment target to 10.10 Signed-off-by: Brendan Bolles <brendan@fnordware.com> * More 10.10 targets Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Ditch ADD_EXTRA_BUILTINS Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Getting strange callback when AE effect is pasted Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Use NDEBUG in AE project Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable radio buttons with no config Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Update version Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Move to AE 2021 SDK, set PF_OutFlag2_MUTABLE_RENDER_SEQUENCE_DATA_SLOWER Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Define OCIO_DEPRECATED in After Effects builds Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Verify change from color space menu Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Handle situation where color space has a '/' in it Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Windows family separator fix Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Windows Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable more controls Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Incorporate invert into LUT export Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Get rid of fullPaths stuff Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Fix changed config settings retention Signed-off-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Add OpenColorIOConfig.cmake generation (#1397) * Initial OpenColorIO Config CMake implementation Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove macro, improve documentation Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Use CMake visibility flags (#1411) * Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix Windows build Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update include/OpenColorIO/OpenColorIO.h Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update src/OpenColorIO/FileRules.cpp Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Hide dependency symbol visibility (#1409) (#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Improve DX11 & Cg fragment shader language support (#1406) * Adsk Contrib - Improve DX11 support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve Cg support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve GPU comments Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix unit test failures Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
…t for v1 (#1445) * Adsk Contrib - Improve File Rules support for v1 configs (#1417) * Adsk Contrib - Improve file rules for v1 configs Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Emergency GPU build fix (#1391) Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Fix some bugs found by Maya and SonarCloud (#1403) * Adsk Contrib - Fix some bugs found by Maya and SonarCloud Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix a Windows warning Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * After Effects and Photoshop plug-in updates (#1373) * Set Mac OS deployment target to 10.10 Signed-off-by: Brendan Bolles <brendan@fnordware.com> * More 10.10 targets Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Ditch ADD_EXTRA_BUILTINS Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Getting strange callback when AE effect is pasted Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Use NDEBUG in AE project Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable radio buttons with no config Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Update version Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Move to AE 2021 SDK, set PF_OutFlag2_MUTABLE_RENDER_SEQUENCE_DATA_SLOWER Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Define OCIO_DEPRECATED in After Effects builds Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Verify change from color space menu Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Handle situation where color space has a '/' in it Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Windows family separator fix Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Invert everything Windows Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Disable more controls Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Incorporate invert into LUT export Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Get rid of fullPaths stuff Signed-off-by: Brendan Bolles <brendan@fnordware.com> * Fix changed config settings retention Signed-off-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Add OpenColorIOConfig.cmake generation (#1397) * Initial OpenColorIO Config CMake implementation Signed-off-by: Rémi Achard <remiachard@gmail.com> * Remove macro, improve documentation Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Use CMake visibility flags (#1411) * Update install doc minimal version numbers Signed-off-by: Rémi Achard <remiachard@gmail.com> * Use CMake visibility presets variables Signed-off-by: Rémi Achard <remiachard@gmail.com> * Fix python package case Signed-off-by: Rémi Achard <remiachard@gmail.com> * Allow advanced user to override symbol visibility Signed-off-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Patrick Hodoul <patrick.hodoul@autodesk.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix Windows build Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update include/OpenColorIO/OpenColorIO.h Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Update src/OpenColorIO/FileRules.cpp Co-authored-by: Michael Dolan <michdolan@gmail.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Hide dependency symbol visibility (#1409) (#1416) When creating libOpenColorIO.so, we lacked the linker commands that hide symbol visibility from the dependent libraries, which is necessary to prevent OCIO from exporting the symbols from Expat and the other dependencies that OCIO needs to use internally but is not trying intentionally to expose via its API. Failing to do this can result in symbol clashes and all sorts of subtle errors if OCIO is used in an app that also uses and is linked against a potentially different version of Expat (or any of the other deps). Signed-off-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Adsk Contrib - Improve DX11 & Cg fragment shader language support (#1406) * Adsk Contrib - Improve DX11 support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve Cg support Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Improve GPU comments Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> * Fix unit test failures Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com> * Add missing file change Signed-off-by: Patrick Hodoul <Patrick.Hodoul@autodesk.com> Co-authored-by: Brendan Bolles <brendan@fnordware.com> Co-authored-by: Rémi Achard <remiachard@gmail.com> Co-authored-by: Michael Dolan <michdolan@gmail.com> Co-authored-by: Larry Gritz <lg@larrygritz.com> Co-authored-by: doug-walker <43830961+doug-walker@users.noreply.github.com>
When creating libOpenColorIO.so, we lacked the linker commands that
hide symbol visibility from the dependent libraries, which is
necessary to prevent OCIO from exporting the symbols from Expat and
the other dependencies that OCIO needs to use internally but is not
trying intentionally to expose via its API.
Failing to do this can result in symbol clashes and all sorts of
subtle errors if OCIO is used in an app that also uses and is linked
against a potentially different version of Expat (or any of the other
deps).
Signed-off-by: Larry Gritz lg@larrygritz.com