Link to X11 targets instead of CMake variables #1606
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The point of COMMON_LIBRARIES was apparently to allow all library linking to be done with a single command; however, continuing this paradigm becomes difficult as more complex conditions are built up - for instance, in MaterialXRenderMsl/CMakeLists.txt, we were altering COMMON_LIBRARIES if APPLE and NOT iOS... which, at best, made the name COMMON_LIBRARIES unclear. Refactored to simply make multiple calls to target_link_libraries, which is the way most other CMakeLists.txt in this repo handle this.
This prevents absolute paths from creeping into the output
MaterialXConfig.cmake, which makes it not portable.
Also, in general in modern CMake, targets are preferred over baked
paths. Plus, at least on Linux, ${OPENGL_LIBRARIES} includes libGLU,
which is not actually a required dependency
ie, ${X11_LIBRARIES} and ${X11_Xt_LIB}
This prevents absolute paths from creeping into the output
MaterialXConfig.cmake, which makes it not portable.
Also, in general in modern CMake, targets are preferred over baked
paths.
Merged
2 tasks
Signed-off-by: Jonathan Stone <[email protected]>
jstone-lucasfilm
changed the title
jstone-lucasfilm
approved these changes
Nov 29, 2023
jstone-lucasfilm
merged commit b8cc4de
into
AcademySoftwareFoundation:main
30 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: please review / merge #1603 before this, as this PR depends on it
(If it is determined that #1603 is unacceptable for some reason, in a way that does NOT apply to this PR, I can rebase / refactor this MR to not rely on #1603)
ie, ${X11_LIBRARIES} and ${X11_Xt_LIB}
This prevents absolute paths from creeping into the output
MaterialXConfig.cmake, which makes it not portable.
Also, in general in modern CMake, targets are preferred over baked
paths.