Skip to content

Conversation

hageboeck
Copy link
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
Contributor

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

Copy link

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
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