Skip to content

Fix RPATH on linux to point in bin dir #79

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 2, 2025
Merged

Fix RPATH on linux to point in bin dir #79

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5ff1792
Fix RPATH on linux to point in bin dir
laforel Oct 2, 2025
e62a449
Fix the RPATH in the main cmake
laforel Oct 2, 2025
818d7a8
Disentangle RPATH logic from APPLE logic
laforel Oct 2, 2025
a910c76
Enable build RPATH
laforel Oct 2, 2025
851137a
Remove platform check in test cmake
laforel Oct 2, 2025
dda1ee4
Add LD_LIBRARY_PATH for cleaner test output
laforel Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/ci-steps.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ jobs:
fi
export DISPLAY=:99

# Set library path for USD dependencies
export LD_LIBRARY_PATH="$PWD/build/${{ inputs.build_type }}/vcpkg_installed/x64-linux-hvt/lib:$LD_LIBRARY_PATH"

# Verify GPU setup
echo "GPU Info:"
nvidia-smi --query-gpu=name,driver_version,memory.total --format=csv,noheader
Expand Down
23 changes: 18 additions & 5 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,33 @@ include(VcpkgSetup)
# NOTE: We can't use PROJECT_IS_TOP_LEVEL here because the project is not yet defined.
string(COMPARE EQUAL "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_SOURCE_DIR}" _HVT_PROJECT_IS_TOP_LEVEL)

# Variables that need to be set prior to the project definition.
# RPATH for Linux and macOS
Copy link
Contributor

@hodoulp hodoulp Oct 14, 2025

Choose a reason for hiding this comment

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

🔴 The RPATH computation is needed all the time (i.e., cannot depend on _HVT_PROJECT_IS_TOP_LEVEL).

Copy link
Contributor Author

@laforel laforel Oct 14, 2025

Choose a reason for hiding this comment

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

When building as part gf HVTEx, the RPATH defined there is supposed to be set for other submodules being built.

Copy link
Contributor

@hodoulp hodoulp Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

When building as part gf HVTEx, the RPATH defined there is supposed to be set for other submodules being built.

The OPENUSD_INSTALL_PATH (defined below in the cmake file) takes care of that.

if (_HVT_PROJECT_IS_TOP_LEVEL AND UNIX)
# Set platform-specific RPATH syntax
if (APPLE)
set(_rpath "@loader_path/;@loader_path/../lib/")
set_if_not_defined(CMAKE_MACOSX_RPATH ON "")
else()
# Linux
set(_rpath "$ORIGIN:$ORIGIN/../lib")
endif()
Copy link
Contributor

@hodoulp hodoulp Oct 14, 2025

Choose a reason for hiding this comment

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

🔴 No, The OpenUSD library location is also based on OPENUSD_INSTALL_PATH (which is the contetn of OPENUSD_INSTALL_PATH as-is when using a specific OpneUSD install or vcpkg installed directory).

Copy link
Contributor

@hodoulp hodoulp Oct 14, 2025

Choose a reason for hiding this comment

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

Note: Here is the file the code doesn't know yet where are the OpenUSD libraries i.e., only known after the line 200

Copy link
Contributor

@DDoS DDoS Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This RPATH is relative to the loader (an executable in bin/ or library in lib/), there's no reason to have it be absolute.

Copy link
Contributor

@hodoulp hodoulp Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This RPATH is relative to the loader (an executable in bin/ or library in lib/), there's no reason to have it be absolute.

Agree e.g., HVT library. But the OpenUSD libraries and plugins are not there!

Copy link
Contributor

@DDoS DDoS Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

CMake is supposed to set RPATHs automatically according to the linker command line. We shouldn't need an RPATH for the build.
See: https://cmake.org/cmake/help/latest/variable/CMAKE_SKIP_RPATH.html#variable:CMAKE_SKIP_RPATH

Copy link
Contributor Author

@laforel laforel Oct 14, 2025

Choose a reason for hiding this comment

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

All the USD lib and binaries are in lib and bin folders, we do not use the externals folder for that. It is the same with HVTEx. This also make sense, you can copy these folders and you have everything you need.

Copy link
Contributor

@hodoulp hodoulp Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

All the USD lib and binaries are in lib and bin folders, we do not use the externals folder for that. It is the same with HVTEx. This also make sense, you can copy these folders and you have everything you need.

Nope.
With the HVT project used as standalone, the OpenUSD libraries are not in lib or bin folders.
With the HVT project used as sub-module of HVTEx, the OpenUSD libraries are all in lib or bin folders (because an extra copy is done in HVTEx).

Copy link
Contributor

@hodoulp hodoulp Oct 15, 2025

Choose a reason for hiding this comment

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

CMake is supposed to set RPATHs automatically according to the linker command line. We shouldn't need an RPATH for the build. See: https://cmake.org/cmake/help/latest/variable/CMAKE_SKIP_RPATH.html#variable:CMAKE_SKIP_RPATH

Based on my experiments on macOS, the two must be explicit.

Copy link
Contributor

@DDoS DDoS Oct 15, 2025

Choose a reason for hiding this comment

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

I've never had to explicitly set the RPATH, as long as you leave the default CMake RPATH settings on.


# Common RPATH settings for all UNIX platforms
set_if_not_defined(CMAKE_SKIP_BUILD_RPATH TRUE "")
set_if_not_defined(CMAKE_BUILD_WITH_INSTALL_RPATH TRUE "")
set_if_not_defined(CMAKE_INSTALL_RPATH "@loader_path/" "")
set_if_not_defined(CMAKE_INSTALL_RPATH "${_rpath}" "")
set_if_not_defined(CMAKE_INSTALL_RPATH_USE_LINK_PATH FALSE "")
endif()

# macOS specific (ignored on other platforms)
# Apple-specific configuration
if (_HVT_PROJECT_IS_TOP_LEVEL AND APPLE)
set_if_not_defined(CMAKE_OSX_DEPLOYMENT_TARGET "12.0" "Minimum OSX deployment version")

# Validate architecture (universal builds not supported due to libjpeg-turbo)
list(LENGTH CMAKE_OSX_ARCHITECTURES osx_arch_count)
if(osx_arch_count GREATER 1)
message(FATAL_ERROR "Universal builds are not supported because of libjpeg-turbo")
endif()
set_if_not_defined(CMAKE_OSX_DEPLOYMENT_TARGET "12.0" "Minimum OSX deployment version")
set_if_not_defined(CMAKE_MACOSX_RPATH ON "")
endif()

# Enable only the typical build types.
Expand Down
20 changes: 2 additions & 18 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,24 +67,8 @@ add_executable(${_TARGET}
get_cmake_property(_MULTI_CONFIG GENERATOR_IS_MULTI_CONFIG)

if (UNIX)
Copy link
Contributor

@DDoS DDoS Oct 2, 2025

Choose a reason for hiding this comment

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

I would just remove this empty block entirely.

Copy link
Contributor Author

@laforel laforel Oct 2, 2025

Choose a reason for hiding this comment

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

Yeah tried but it fails on platform check bellow, will come up with cleaner code and stop failing in the else...

Copy link
Contributor

@DDoS DDoS Oct 2, 2025

Choose a reason for hiding this comment

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

Just remove the check, it's not needed.

# Do not copy the third-party libraries by updating the rpath with:
# 1. The OpenUSD library location.
# 2. The default library location.
# 3. The OpenUSD plugins location.
# Note: Handle the multi-generator cases.

if(_MULTI_CONFIG)
set(openusd_path_gen "$<IF:$<CONFIG:Debug>,${OPENUSD_INSTALL_PATH_DEBUG},${OPENUSD_INSTALL_PATH}>")
list(APPEND _RPATH "${openusd_path_gen}/lib")
list(APPEND _RPATH "${openusd_path_gen}/plugin")
list(APPEND _RPATH "${_HVT_OUTPUT_DIR}/lib/$<CONFIG>")
else()
list(APPEND _RPATH "${OPENUSD_INSTALL_PATH}/lib")
list(APPEND _RPATH "${OPENUSD_INSTALL_PATH}/plugin")
list(APPEND _RPATH "${_HVT_OUTPUT_DIR}/lib")
endif()

set_target_properties(${_TARGET} PROPERTIES INSTALL_RPATH "${_RPATH}")
# RPATH is set globally in root CMakeLists.txt for all UNIX platforms
# No additional per-target RPATH configuration needed
elseif (WIN32)
# On the Windows platform, the safest approach is to copy libraries where are the executables.

Expand Down