Resolve run paths relative to the loader, not the install prefix - #232
Resolve run paths relative to the loader, not the install prefix#232gdevenyi wants to merge 2 commits into
Conversation
Every component baked CMAKE_INSTALL_PREFIX into its install RPATH, so a moved install could no longer find its own libraries. That, plus the prefix baked into minc-toolkit-config.sh, is what makes the toolkit non-relocatable today. Replace the absolute prefix with $ORIGIN (@loader_path on Apple) in all 22 places, keeping whichever library directory each already used, so the directory referred to is unchanged and only how it is reached differs. One value serves everything: binaries install to <prefix>/bin, libraries to <prefix>/lib, and <origin>/../lib is right from both -- from a library it normalises back to its own directory. Being a single entry it also avoids the list separator that a ;-joined RPATH would need to survive ExternalProject. minc-toolkit-config.sh now derives MINC_TOOLKIT from its own location; it installs at the top of the prefix, so that is the prefix. This is bash-only via BASH_SOURCE -- every other shell keeps the configure-time path, as before. csh is left alone: locating a sourced script there is too fragile to be worth it, and the binaries no longer depend on the variable anyway. Packaging policy is deliberately untouched. CPACK_SET_DESTDIR, CPACK_PACKAGING_INSTALL_PREFIX and CPACK_PACKAGE_RELOCATABLE=OFF still pin the .deb/.rpm to a fixed prefix, which is a maintainer decision about how releases install rather than a consequence of this change. Relaxing them is now possible, but is a separate call. Verified: - build, install, move the whole prefix, run: the binary still resolves its library through $ORIGIN/../lib; - $ORIGIN survives ExternalProject with the Makefile generator -- CMake emits \$$ORIGIN, so make collapses it and the shell passes it through literally, confirmed both in the generated ZLIB-cfgcmd.txt and by a sub-project echoing back what it received; - all 24 Build*.cmake still parse, and build_zlib() instantiates; - the config script self-locates under bash and follows a relocated prefix, falls back cleanly under zsh, passes bash --posix -n and shellcheck -s sh. Not verified: a full superbuild install and relocation, which is hours of build time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019katBUnczdvUS7WQ4RpVoa
|
The idea here is that our cloud builds will have tarballs available which can be unpacked into any directory. This will facilitate installations in module systems. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates MINC toolkit installation to be more relocatable by deriving the prefix from the config script location and by switching installed RPATHs to be origin-relative.
Changes:
- Compute
MINC_TOOLKITfrom the config script’s directory when running under bash. - Introduce
MT_RPATH_ORIGINand use origin-relativeCMAKE_INSTALL_RPATHin the top-level build. - Propagate the origin-relative install RPATH into multiple ExternalProject builds.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| minc-toolkit-config.unix.sh.cmake | Derives MINC_TOOLKIT from the script location under bash for relocatability. |
| minc-toolkit-config.apple.sh.cmake | Same relocatable MINC_TOOLKIT logic for macOS config script. |
| CMakeLists.txt | Defines MT_RPATH_ORIGIN and sets an origin-relative CMAKE_INSTALL_RPATH. |
| cmake-modules/BuildZLIB.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildPatchMorphology.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildOpenJPEG.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildOpenBLAS.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildNIFTI.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildNETCDF.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildLibarchive.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildJPEG.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildITKv4.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildHDF5.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildGIFTI.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildFREEGLUT.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildFFTW3F.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildFFTW3D.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildElastix.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildEZMINC.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildC3D.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildBICGL.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildBEAST.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildANTS.cmake | Switches ExternalProject install RPATH to origin-relative. |
| cmake-modules/BuildABC.cmake | Switches ExternalProject install RPATH to origin-relative. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ -n "${BASH_SOURCE:-}" ] && [ -f "${BASH_SOURCE}" ]; then | ||
| MINC_TOOLKIT=$(cd -- "$(dirname -- "${BASH_SOURCE}")" && pwd -P) |
There was a problem hiding this comment.
Checked this against bash 5.3.15, and the failure mode does not occur — cd+pwd -P already normalises both cases.
The concern needs BASH_SOURCE to be a bare basename while the script is not in the current directory. Bash only produces a bare name when it found the file in the cwd, in which case the cwd is the right answer:
| how it is sourced | BASH_SOURCE holds |
resolved MINC_TOOLKIT |
|---|---|---|
| explicit relative path | ../onpath/x.sh |
.../onpath ✅ |
bare name, found via PATH |
.../elsewhere/../onpath/x.sh (full) |
.../onpath ✅ |
| bare name, found in cwd | x.sh (bare) |
cwd, which is where the script lives ✅ |
So the PATH case keeps the full path rather than the basename, and the bare-name case only happens when dirname . is already correct. Both come out right after cd -- "$(dirname -- ...)" && pwd -P.
Happy to add a command -v / realpath normalisation as belt-and-braces if you would rather not depend on that behaviour, but on the evidence it is not currently reachable.
| -DCMAKE_SKIP_INSTALL_RPATH:BOOL=OFF | ||
| -DMACOSX_RPATH:BOOL=ON | ||
| -DCMAKE_INSTALL_RPATH:PATH=${install_prefix}/lib${LIB_SUFFIX} | ||
| -DCMAKE_INSTALL_RPATH:PATH=${MT_RPATH_ORIGIN}/../lib${LIB_SUFFIX} |
There was a problem hiding this comment.
Agreed, changed in the follow-up commit — all 21 sites now use -DCMAKE_INSTALL_RPATH:STRING=.
Worth noting the :PATH spelling predates this branch (I only changed the value, not the type) and it does work: the full build linked below produced correct run paths with it, and the generated ZLIB-cfgcmd.txt carried $ORIGIN/../lib through untouched. But STRING is what a list of run paths actually is, and it removes any question about a PATH-typed entry normalising $ORIGIN / @loader_path.
|
Verified locally with a real build, install and relocation. The "not verified" caveat in the description is now covered. Configuration — maximum system dependencies (12 of 13 1. No absolute RPATH survives. The 88 without an RPATH are all scripts — 90 Perl, 7 shell — not ELF objects. Every actual binary and shared library got a relative run path. CMake's install step logs it explicitly: 2. Nothing unresolved with 3. Relocation works, including the ITK-heavy case. 118 ITK shared libraries install into the prefix. After moving the whole prefix: 85 libraries resolving through one 4. The config script follows the move. Sourced from the relocated prefix, 5. Test suite: 150/150 pass on the relocatable build, One incidental finding, unrelated to this PR: the bundled NetCDF does not compile under GCC 16 — |
|
only subprojects install into external during the build. |
CMAKE_INSTALL_RPATH is a list of run paths, not a filesystem path, and $ORIGIN / @loader_path are not paths a PATH-typed cache entry should be invited to normalise. The :PATH spelling predates this branch and does work -- the full build below produced correct run paths with it -- but STRING is what the value actually is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019katBUnczdvUS7WQ4RpVoa
|
@vfonov correct, and following that up found a real gap — plus a flaw in my own verification. On
Correction to my earlier numbers. I reported One object still had an absolute run path: Everything else in the earlier report stands — the relocation test, the 85 ITK libraries resolving from the moved prefix, binaries executing with no environment set, 150/150 tests. It passed despite this gap because a build with system HDF5, NetCDF and zlib leaves So this PR needs BIC-MNI/libminc#153 alongside it to actually deliver a relocatable install. |
The target-scoped INSTALL_RPATH baked in CMAKE_INSTALL_PREFIX, so libminc2 kept pointing at the configure-time prefix. Move the install and that path dangles: the library then has to be found some other way, by LD_LIBRARY_PATH or by the RPATH of whatever loaded it. libminc and the libraries it links install to the same directory, so $ORIGIN (@loader_path on Apple) names it without naming the prefix, and survives a move. Found while verifying the superbuild's equivalent change (BIC-MNI/minc-toolkit-v2#232): after that, libminc2.so.5.3.0 was the single object in a full install of 419 binaries and libraries still carrying an absolute run path, because this target property overrides the superbuild's CMAKE_INSTALL_RPATH. It went unnoticed because a build using system HDF5, NetCDF and zlib leaves libminc2 with no dependency inside the prefix at all, so the dangling path costs nothing. With those bundled it would matter. Verified: standalone shared build installs with RUNPATH [$ORIGIN]. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019katBUnczdvUS7WQ4RpVoa
|
CI has now run for the first time on this branch — it had never triggered before (zero runs recorded, while #230 and #224 from the same fork did run), so I closed and reopened to fire it. Result matches the base branch exactly, with no regression from the RPATH change:
The macOS failure is the pre-existing N3 numerical one, byte-identical to what Relevant to this PR specifically: no dynamic-loader errors on macOS. Nothing matching The only |
Why
Every component bakes
CMAKE_INSTALL_PREFIXinto its install RPATH —CMakeLists.txt:160for the main tree, and a-DCMAKE_INSTALL_RPATH:PATH=${install_prefix}/lib${LIB_SUFFIX}in each of the 21Build*.cmakeExternalProjects. Move the install and nothing finds its libraries any more. Together with the prefix baked intominc-toolkit-config.sh, that is what makes the toolkit non-relocatable.This came out of the review on #224: with
LD_LIBRARY_PATHgone, the RPATH is what resolves everything, so it is worth it being robust.What
RPATH. The absolute prefix becomes
$ORIGIN(@loader_pathon Apple) in all 22 places, keeping whichever library-directory expression each file already used. The directory referred to is unchanged; only how it is reached differs — a mechanical, semantics-preserving substitution.One value covers everything: binaries install to
<prefix>/binand libraries to<prefix>/lib, and<origin>/../libis correct from both — from a library inlib/it normalises back to its own directory. Being a single entry, it also sidesteps theLIST_SEPARATORa;-joined RPATH would need to surviveExternalProject_Add.Config script.
minc-toolkit-config.shderivesMINC_TOOLKITfrom its own location — it installs at the top of the prefix, so its directory is the prefix. Bash-only viaBASH_SOURCE; every other shell keeps the configure-time path exactly as now.cshis left alone. Locating a sourced script there is too fragile to be worth it, and the binaries no longer depend on the variable to find their libraries.Not in scope
Packaging policy.
CPACK_SET_DESTDIR,CPACK_PACKAGING_INSTALL_PREFIXandCPACK_PACKAGE_RELOCATABLE=OFFstill pin the.deb/.rpmto a fixed prefix. That is a decision about how releases install rather than a consequence of this change — relaxing it is now possible, but should be your call in a separate PR.Verification
RUNPATH: [$ORIGIN/../lib], and the library still resolves after the move.$ORIGINsurvivesExternalProjectwith the Makefile generator, which was the main risk. CMake emits\$$ORIGIN, make collapses$$to$, and the shell passes it through literally. Confirmed two ways: the generatedZLIB-cfgcmd.txtcontains-DCMAKE_INSTALL_RPATH:PATH=$ORIGIN/../lib, and a sub-project echoing backCMAKE_INSTALL_RPATHreceives$ORIGIN/../libintact. NoCMAKE_CACHE_ARGSworkaround needed.Build*.cmakestill parse, andbuild_zlib()instantiates cleanly.bash --posix -nandshellcheck -s sh.Not verified: a full superbuild install-and-relocate, which is hours of build time. The mechanism is proven in isolation and the substitution is uniform, but a real install is worth doing before merging — the same
lddcheck suggested on #224 covers it:then move the prefix and run it again. Both should be silent.
Relationship to #224
Independent, either order. #224 removes
LD_LIBRARY_PATHbecause the absolute RPATH already covers it; this makes that RPATH survive a move as well. Merging both leaves the toolkit resolving its libraries with no environment variables at all.🤖 Generated with Claude Code
https://claude.ai/code/session_019katBUnczdvUS7WQ4RpVoa