Skip to content

[CMake] Append to CMAKE_INSTALL_RPATH without force-overriding cache.#19500

Closed
hageboeck wants to merge 1 commit into
root-project:masterfrom
hageboeck:rpath
Closed

[CMake] Append to CMAKE_INSTALL_RPATH without force-overriding cache.#19500
hageboeck wants to merge 1 commit into
root-project:masterfrom
hageboeck:rpath

Conversation

@hageboeck
Copy link
Copy Markdown
Member

@guitargeek, I would like to suggest this small change for #19135.
It's preferable not to touch any cache variables, as they are user-settable (and can be altered after configuring). Given that RootBuildOptions is included at the outermost scope, it's sufficient to declare a new variable with the same name as the cache.

On each CMake run, this variable will be recomputed from the cache variable + whatever we append, leaving the CMakeCache.txt invariant.

I also took the opportunity to clarify what the rpath option does, since this was not really explained (see e.g. #19327).

It's preferable not to touch any cache variables, as they are
user-settable (and can be altered after configuring). Given that
RootBuildOptions is included at the outermost scope, it's sufficient to
declare a new variable with the same name as the cache.
@hageboeck hageboeck requested a review from guitargeek August 1, 2025 11:52
@hageboeck hageboeck self-assigned this Aug 1, 2025
@hageboeck hageboeck requested a review from bellenot as a code owner August 1, 2025 11:52
@guitargeek
Copy link
Copy Markdown
Contributor

I also have an alternative solution that avoids messing around with the CMAKE_INSTALL_RPATH completely:

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 1, 2025

Test Results

    20 files      20 suites   3d 3h 51m 26s ⏱️
 3 223 tests  3 223 ✅ 0 💤 0 ❌
62 798 runs  62 798 ✅ 0 💤 0 ❌

Results for commit 5ad21d5.

@hageboeck
Copy link
Copy Markdown
Member Author

Closing in favour of #19501

@hageboeck hageboeck closed this Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants