New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmake: selectively rewrite install rpath #30028
Conversation
@smithfarm could you help test the update change? after a second thought, i found that the install path of a library cannot be populate to that of RPATH of executables linked against it. so i changed this PR to skip the global RPATH settings for those executables who don't link against the libraries from ceph. |
some executables like ceph_test_mon_memory_target does not link against gtest or gmock, so no need to link agains them. Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov openSUSE 15.1 is now built, along with CentOS, Ubuntu, etc., and reported to Shaman whenever you push a branch to ceph-ci. The last Shaman/Chacra build (obtained by clicking on the link you posted) is green for openSUSE 15.1, I see, but it does not include the latest two commits. Of course I can trigger a build directly in OBS but the result is almost certain to be exactly the same since Jenkins uses "osc build" to do the build on openSUSE 15.1. |
what do you mean by "the latest two commits"? i think https://github.com/ceph/ceph-ci/commits/wip-rpath-kefu includes both commits in this PR. if you think these commits was pushed after i posted the link. that's because i removed #30065 from this PR. as it's not related.
again, i am not following you. 021eb0a is not in upstream master. so i am afraid i cannot revert it. |
@tchaikov Never mind - I was confused (some "RPATH mitigation" commits have not been upstreamed yet). I will watch the latest Shaman build and approve if it's green. |
OK, I can confirm that Shaman is green for the current state of this PR. However, I have two questions: First question: The commit message says:
Can you fix the typo "libradies" -> "libraries" ? Second question: The commit message says:
Is it still expected that this PR is removing the code that sets
|
Third question. Should this PR change all instances of |
Fourth question: All of the I don't understand the root cause of these failures. Before, we were adding Could it be said that, in general, the RPATH build failures are a side effect of libceph-common? |
OK.
yes.
no. |
some executables like ceph_test_mon_memory_target do not link against libraries built from source tree, like librados and libceph-common. so cmake does not set RPATH for them. hence cmake complains like: before this change, `CMAKE_INSTALL_RPATH` is set globally. so cmake is asked to rewrite the RPATH for all installed targets. but this is not needed. as some executables do not link against libceph-common. hence, cmake complains when installing them, like: CMake Error at src/test/mon/cmake_install.cmake:90 (file): file RPATH_CHANGE could not write new RPATH: /usr/lib64/ceph to the file: /home/abuild/rpmbuild/BUILDROOT/ceph-15.0.0-4347.g85a07b9.x86_64/usr/bin/ceph_test_log_rss_usage No valid ELF RPATH or RUNPATH entry exists in the file; after this change, `SKIP_RPATH` is set for those executables which do not link against any libraries created from ceph source tree. so we can avoid setting the RPATH for these executables when `make install`. the same applies to libceph-common. Fixes: https://tracker.ceph.com/issues/41524 Signed-off-by: Kefu Chai <kchai@redhat.com>
well, there are more than 2 questions =)
i haven't audited all of the
sure. i am revising the commit message, hopefully it's more readable.
could you be more specific? |
LGTM |
the second commit message still says:
but I guess that no longer applies in the final version of this PR. In other words, Also, although there may be a semantic difference between |
Still getting FTBFS in OBS even with this patch applied:
Reopened #29922 |
we use rpath for finding libceph-common, which is an internal library
not supposed to be used by 3rd party applications.
before this change,
CMAKE_INSTALL_RPATH
is set globally. so cmake isasked to rewrite the RPATH for all installed targets. but this is not
needed. as some executables do not link against libceph-common. hence,
cmake complains when installing them, like:
CMake Error at src/test/mon/cmake_install.cmake:90 (file):
file RPATH_CHANGE could not write new RPATH:
/usr/lib64/ceph
to the file:
/home/abuild/rpmbuild/BUILDROOT/ceph-15.0.0-4347.g85a07b9.x86_64/usr/bin/ceph_test_log_rss_usage
No valid ELF RPATH or RUNPATH entry exists in the file;
so, in this change, we only apply
INSTALL_RPATH
to libceph-common.Fixes: https://tracker.ceph.com/issues/41524
Signed-off-by: Kefu Chai kchai@redhat.com
Checklist
Show available Jenkins commands
jenkins retest this please
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard backend
jenkins test docs
jenkins render docs