Relocatable pkg config files, @rpath/<libraryname> install name on macOS#1546
Relocatable pkg config files, @rpath/<libraryname> install name on macOS#1546Dead2 merged 3 commits intozlib-ng:developfrom
@rpath/<libraryname> install name on macOS#1546Conversation
…use @rpath/libname as install name on macOS
| "-Wl,--version-script,\"${CMAKE_CURRENT_SOURCE_DIR}/zlib${SUFFIX}.map\"") | ||
| elseif(IS_ABSOLUTE "${CMAKE_INSTALL_LIBDIR}" OR NOT WITH_RPATH) | ||
| # Match configure/make's behavior (i.e. don't use @rpath on mac when using absolute path). | ||
| set_target_properties(zlib PROPERTIES INSTALL_NAME_DIR "@rpath/${CMAKE_INSTALL_FULL_LIBDIR}") |
There was a problem hiding this comment.
This was completely bogus... @rpath/<absolute path> makes the executable/library that links to zlib-ng add a load command @rpath/absolute/path/to/zlib-ng/lib, and @rpath is substituted by absolute rpaths set in that dependent. So you end up with for example:
/absolute/path/to/zlib-ng/lib/absolute/path/to/zlib-ng/lib/
as a search path :|
|
|
||
| # Fix install directory after generating zlib.pc/zlib-ng.pc | ||
| if (NOT IS_ABSOLUTE CMAKE_INSTALL_LIBDIR AND WITH_RPATH) | ||
| set(CMAKE_INSTALL_LIBDIR "/${CMAKE_INSTALL_LIBDIR}") |
There was a problem hiding this comment.
This also looked very surprising to me.
There was a problem hiding this comment.
Default prefix is /usr, which made it impossible to use directory structure where libraries are located in lib subdirectory of the output directory. This primarily affected builds where zlib-ng was used as a dependency.
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## develop #1546 +/- ##
========================================
Coverage 83.89% 83.90%
========================================
Files 132 132
Lines 10850 10850
Branches 2794 2794
========================================
+ Hits 9103 9104 +1
+ Misses 1048 1047 -1
Partials 699 699
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| /*) | ||
| LDSHAREDFLAGS="-dynamiclib -install_name ${libdir}/${SHAREDLIBM} -compatibility_version ${VER1} -current_version ${VER3}" ;; | ||
| *) | ||
| LDSHAREDFLAGS="-dynamiclib -install_name @rpath/${libdir}/${SHAREDLIBM} -compatibility_version ${VER1} -current_version ${VER3}" ;; |
There was a problem hiding this comment.
This was bogus in multiple ways:
- Relative libdir is not relative to
${prefix}but current working directory on install. - If relative libdir worked, you'd end up with
@rpath/lib/libz.dylib, which is very non-standard... the expectation is that the dependent uses an rpath of the form<prefix>/lib, not<prefix>. - For absolute libdir, it's still better to use
@rpath/${SHAREDLIBM}, since that's still more relocatable.
haampie
changed the title
CMAKE_INSTALL_*, absolute paths CMAKE_INSTALL_FULL_*; use @rpath/<libraryname> as install name on macOS
haampie
force-pushed
the
fix/install-name-issues
branch
2 times, most recently
from
cfac1b9 to
f1dc3db
Compare
haampie
changed the title
CMAKE_INSTALL_*, absolute paths CMAKE_INSTALL_FULL_*; use @rpath/<libraryname> as install name on macOSCMAKE_INSTALL_*, absolute paths CMAKE_INSTALL_FULL_*; use @rpath/<libraryname> as install name on macOS; use prefix not destdir in tests
haampie
force-pushed
the
fix/install-name-issues
branch
from
f1dc3db to
b1d108e
Compare
haampie
changed the title
CMAKE_INSTALL_*, absolute paths CMAKE_INSTALL_FULL_*; use @rpath/<libraryname> as install name on macOS; use prefix not destdir in tests@rpath/<libraryname> install name on macOS, prefix vs destdir in tests
|
|
||
| # Refer to prefix symbolically to ease relocation by end user, | ||
| # as Makefile-generated .pc file does. | ||
| string(FIND "${CMAKE_INSTALL_INCLUDEDIR}" "${CMAKE_INSTALL_PREFIX}/" INCLUDEDIR_POS) |
There was a problem hiding this comment.
The idea is to drop this complexity. If a user wants to customize the install layout while keeping things relocatable, then just use CMAKE_INSTALL_INCLUDEDIR=myincludedir instead of an absolute path, as suggested by the docs: https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html#result-variables
haampie
changed the title
@rpath/<libraryname> install name on macOS, prefix vs destdir in tests@rpath/<libraryname> install name on macOS
…paths for CMAKE_INSTALL_[LIB|INCLUDE]DIR
haampie
force-pushed
the
fix/install-name-issues
branch
from
322a932 to
0bae8f7
Compare
| sysctl -n machdep.cpu.leaf7_features | ||
| sysctl -n machdep.cpu.extfeatures | ||
| CMAKE_ARGS="-DCMAKE_INSTALL_LIBDIR=lib -DPKGCONFIG_INSTALL_DIR=/lib/pkgconfig -DWITH_RPATH=on ${CMAKE_ARGS}" | ||
| CONFIGURE_ARGS="--libdir=lib ${CONFIGURE_ARGS}" |
There was a problem hiding this comment.
Not sure how this ever passed tests? --libdir=lib with make DESTDIR=hello/ puts the libraries in hello/lib/* instead of hello/${prefix}/lib/*. --libdir takes absolute paths (or maybe --libdir='${prefix}/custompath')
There was a problem hiding this comment.
This worked because Makefile doesn't add directory separator after prefix. This means that prefix also had to always end with directory separator.
In your example, hello/ is the prefix, so should not be duplicated.
Dead2
added
Build Env
Compatibility
|
That is quite the cleanup, I'll have to spend some time later for trying to understand the implications of all the changes. But at first glance it is looking quite neat. Not sure whether you are aware of Edit: Actually, perhaps some of these changes should also be considered to move into |
|
Hadn't looked at If I understand correctly, it sets defaults for the (not Looks like it isn't necessary to change any of that: the deprecated variables and pkg config path variables should work whether relative or absolute |
|
For what it's worth, Spack's pipeline is green with this patch for macOS (and linux). It uses configure not cmake, and it also builds and links hundreds of dependents of zlib-ng. And most importantly, it relies on rpaths for locating libraries, as it builds packages in funny directories that include a hash, like Previously it was broken on macOS |
3 participants
Closes #1523
Make the install name
@rpath/<libz.dylib>on macOS for CMake and configure. This is what CMake 3.0+ does by default (CMP0042)1 and is a very sensible.When using GNUInstallDirs, there's
CMAKE_INSTALL_*for relative paths, andCMAKE_INSTALL_FULL_*for absolute paths (joined withCMAKE_INSTALL_PREFIX+ some special rules for multi-arch systems). Absolute paths inCMAKE_INSTALL_*DIRare allowed but discouraged, since that makes things non-relocatable. This PR drops the complexity of generating relocatable pc files when using absolute paths in those variables. If you want a custom layout, but a relocatable install, use e.g.CMAKE_INSTALL_[LIB|INCLUDE]DIR=relative/path, it's as simple as that.In contrast to
CMake+GNUInstallDirs, the configure script expects absolute paths for--libdir(or./configure '--libdir=${prefix}/lib'literally?). I'm not familiar enough with autotools, but looks like it's standard: Standard Directory Variables. Or maybe this is to mimic upstream zlib? In any case, with this PR, both the generated autotools & cmake pc file are relocatable by default, unless--libdir=/absolute/pathorCMAKE_INSTALL_LIBDIR=/absolute/pathis given.Footnotes
For what it's worth, there are two approaches to make libraries easily locatable in non-standard directories, both are rather hacky:
@rpath/libfoo.dylibinstead oflibfoo.dylib, so that dependent libraries end up with a load command of@rpath/libfoo.dylibafter linking, and rpaths can be used automatically./absolute/path/of/libfoo.dylibso that dependent libraries end up with a load command of/absolute/path/of/libfoo.dylib, meaning that the dynamic linker doesn't have to search but can immediately open it.As far as I know, the second option is the most hacky and non-relocatable (using an absolute path as a library name...), the first option is the default of CMake. Option 1 is very common. ↩