Skip to content

[CMake] Fix RPATH issues in build system#138

Open
zacharyvincze wants to merge 6 commits intoROCm:developfrom
zacharyvincze:zv/cmake/fix-rpath-issues
Open

[CMake] Fix RPATH issues in build system#138
zacharyvincze wants to merge 6 commits intoROCm:developfrom
zacharyvincze:zv/cmake/fix-rpath-issues

Conversation

@zacharyvincze
Copy link
Copy Markdown
Contributor

Changes

  • Explicitly set build/install RPATHs for targets so that LD_LIBRARY_PATH is not a requirement.
  • Move OpenMP dependency to private for roccv target.
  • Organize bin and lib folders in build tree.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates rocCV’s CMake packaging/build configuration to improve in-tree runnable artifacts (tests/samples/benchmarks/python module) by standardizing output directories and setting build/install RPATHs, while also tightening roccv’s exported dependency surface.

Changes:

  • Standardize runtime/library output locations for tests, samples, benchmarks, and the Python module.
  • Add/adjust BUILD_RPATH and INSTALL_RPATH to improve runtime library discovery (especially for ROCm/LLVM libs).
  • Make OpenMP a private dependency of roccv and expose C++20 via target compile features; add roccv_VERSION in the package config.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/roccv/cpp/CMakeLists.txt Redirect test executables to bin/tests/... and add build RPATH.
src/CMakeLists.txt Make OpenMP private, publish C++20 requirement, and revise build/install RPATHs for libroccv.
samples/CMakeLists.txt Set samples output dir and build RPATH in the sample helper function.
samples/cropandresize/cpp/CMakeLists.txt Set sample app output dir and build RPATH.
benchmarks/CMakeLists.txt Set benchmark output dir and build RPATH.
python/CMakeLists.txt Set python module output dir and adjust build/install RPATH.
cmake/roccvConfig.cmake.in Export roccv_VERSION and remove OpenMP as a transitive dependency.
CHANGELOG.md Mark 0.5.0 as “Unreleased” and document OpenMP visibility change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@simonCatBot
Copy link
Copy Markdown

Review: [CMake] Fix RPATH issues in build system

This PR improves the build system configuration:

Changes:

  • Explicit RPATH settings so LD_LIBRARY_PATH is not required
  • OpenMP moved to private linkage for roccv target
  • Better organization of bin/lib folders

Assessment: LGTM - Build system improvements are always welcome.

Proper RPATH handling makes the library more portable and easier to use. Moving OpenMP to private linkage is correct - consumers should not need to link it transitively. Would benefit from CI testing on different Linux distributions to ensure the RPATH logic works across environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants