Fix shared lib symlink installation path#556
Fix shared lib symlink installation path#556cary-ilm merged 1 commit intoAcademySoftwareFoundation:mainfrom
Conversation
|
|
Signed-off-by: Jhacson Meza <jhacsonmeza@gmail.com>
jhacsonmeza
force-pushed
the
install_dirs
branch
from
cfdd714 to
bc5f898
Compare
There was a problem hiding this comment.
Pull request overview
Fixes incorrect installation location for shared-library symlinks when using cmake --install . --prefix <custom_path> by avoiding CMAKE_INSTALL_FULL_*DIR variables that can resolve to the configure-time default prefix.
Changes:
- Replace
CMAKE_INSTALL_FULL_BINDIR/CMAKE_INSTALL_FULL_LIBDIRwithCMAKE_INSTALL_BINDIR/CMAKE_INSTALL_LIBDIRfor shared-lib symlink installation destinations. - Apply the same fix consistently across core Imath, Boost.Python PyImath, and pybind11 PyBindImath builds.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/python/PyImath/CMakeLists.txt | Installs PyImath shared-library symlink into prefix-relative install dirs. |
| src/pybind11/PyBindImath/CMakeLists.txt | Installs PyBindImath shared-library symlink into prefix-relative install dirs. |
| src/Imath/CMakeLists.txt | Installs core Imath shared-library symlink into prefix-relative install dirs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set(INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}) | ||
| endif() | ||
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${BASE_LIB_FILENAME} DESTINATION ${INSTALL_DESTINATION}) | ||
| install(CODE "message(STATUS \"Creating symlink ${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") |
There was a problem hiding this comment.
The install-time status message now prints a relative path (because INSTALL_DESTINATION is CMAKE_INSTALL_LIBDIR/CMAKE_INSTALL_BINDIR). Consider prepending ${CMAKE_INSTALL_PREFIX} (and accounting for $ENV{DESTDIR}) so the message matches the real install path when cmake --install --prefix is used.
| install(CODE "message(STATUS \"Creating symlink ${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") | |
| install(CODE "message(STATUS \"Creating symlink \$ENV{DESTDIR}${CMAKE_INSTALL_PREFIX}/${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") |
| set(INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}) | ||
| endif() | ||
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${BASE_LIB_FILENAME} DESTINATION ${INSTALL_DESTINATION}) | ||
| install(CODE "message(STATUS \"Creating symlink ${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") |
There was a problem hiding this comment.
After switching INSTALL_DESTINATION to CMAKE_INSTALL_LIBDIR/CMAKE_INSTALL_BINDIR (relative paths), the subsequent status message prints a relative install path. Consider printing the fully resolved install path by including ${CMAKE_INSTALL_PREFIX} (and $ENV{DESTDIR} if set) for clearer install logs.
| install(CODE "message(STATUS \"Creating symlink ${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") | |
| install(CODE "message(STATUS \"Creating symlink \$ENV{DESTDIR}${CMAKE_INSTALL_PREFIX}/${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") |
| set(INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}) | ||
| endif() | ||
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${BASE_LIB_FILENAME} DESTINATION ${INSTALL_DESTINATION}) | ||
| install(CODE "message(STATUS \"Creating symlink ${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") |
There was a problem hiding this comment.
The install-time status message now prints a relative path (e.g., lib/...) because INSTALL_DESTINATION is set to CMAKE_INSTALL_LIBDIR/CMAKE_INSTALL_BINDIR. Consider including ${CMAKE_INSTALL_PREFIX} (and respecting $ENV{DESTDIR} if applicable) in the message so the output reflects the actual on-disk install location when using cmake --install --prefix.
| install(CODE "message(STATUS \"Creating symlink ${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") | |
| install(CODE "message(STATUS \"Creating symlink \$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/${INSTALL_DESTINATION}/${BASE_LIB_FILENAME} -> ${VERSIONED_LIB_FILENAME}\")") |
|
Looks good, thanks! Copilot's suggest messages also look helpful, if you don't mind accepting them. |
|
I tested the suggested message by Copilot by installing in a local directory (not in to: The printed path of the shared lib is not correct. This is because |
|
Hmm, ok, that's not good. I'm fine with it the way it is, then. Thanks! |
When installing Imath with
cmake --install . --prefix <custom_path>everything is installed correctly atcustom_pathexcept for the shared library symlinks. This is due to the use ofCMAKE_INSTALL_FULL_BINDIRandCMAKE_INSTALL_FULL_LIBDIRwhich default to/usr/localifCMAKE_INSTALL_PREFIXis not set. The proposed changes fixed the problem for me.