| Submitter | Simon Busch |
|---|---|
| Date | May 4, 2011, 3:50 p.m. |
| Message ID | <1304524228-25642-1-git-send-email-morphis@gravedo.de> |
| Download | mbox | patch |
| Permalink | /patch/3193/ |
| State | Superseded |
| Headers | show |
Comments
On Wed, 2011-05-04 at 17:50 +0200, Simon Busch wrote: > +# We need to set the rpath to the correct directory as cmake does not provide any > +# directory as rpath by default > + echo "set( CMAKE_INSTALL_RPATH ${libdir} )" >> ${WORKDIR}/toolchain.cmake Shouldn't this be inside some kind of guard for native? You don't want rpath in target binaries, do you? p.
On 04.05.2011 18:09, Phil Blundell wrote: > On Wed, 2011-05-04 at 17:50 +0200, Simon Busch wrote: >> +# We need to set the rpath to the correct directory as cmake does not provide any >> +# directory as rpath by default >> + echo "set( CMAKE_INSTALL_RPATH ${libdir} )" >> ${WORKDIR}/toolchain.cmake > > Shouldn't this be inside some kind of guard for native? You don't want > rpath in target binaries, do you? Thanks for the hint! You mean something like: if not bb.data.inherits_class('native', d) and not bb.data.inherits_class('cross', d): [...] ? regards, Simon
On Wed, 2011-05-04 at 18:19 +0200, Simon Busch wrote: > On 04.05.2011 18:09, Phil Blundell wrote: > > On Wed, 2011-05-04 at 17:50 +0200, Simon Busch wrote: > >> +# We need to set the rpath to the correct directory as cmake does not provide any > >> +# directory as rpath by default > >> + echo "set( CMAKE_INSTALL_RPATH ${libdir} )" >> ${WORKDIR}/toolchain.cmake > > > > Shouldn't this be inside some kind of guard for native? You don't want > > rpath in target binaries, do you? > > Thanks for the hint! > > You mean something like: > > if not bb.data.inherits_class('native', d) > and not bb.data.inherits_class('cross', d): I think you have the sense backwards but yes, that's the general idea. Though I must admit I don't quite understand why the rpath setting from BUILD_LDFLAGS is not turning up in cmake's final link. It might be worth investigating why that isn't working for you in case this patch is just papering over some deeper problem. p.
On 04.05.2011 18:54, Phil Blundell wrote: > On Wed, 2011-05-04 at 18:19 +0200, Simon Busch wrote: >> On 04.05.2011 18:09, Phil Blundell wrote: >>> On Wed, 2011-05-04 at 17:50 +0200, Simon Busch wrote: >>>> +# We need to set the rpath to the correct directory as cmake does not provide any >>>> +# directory as rpath by default >>>> + echo "set( CMAKE_INSTALL_RPATH ${libdir} )" >> ${WORKDIR}/toolchain.cmake >>> >>> Shouldn't this be inside some kind of guard for native? You don't want >>> rpath in target binaries, do you? >> >> Thanks for the hint! >> >> You mean something like: >> >> if not bb.data.inherits_class('native', d) >> and not bb.data.inherits_class('cross', d): > > I think you have the sense backwards but yes, that's the general idea. > Though I must admit I don't quite understand why the rpath setting from > BUILD_LDFLAGS is not turning up in cmake's final link. It might be > worth investigating why that isn't working for you in case this patch is > just papering over some deeper problem. I alreay searched through the cmake documentation and it says that the rpath is set with the content of the CMAKE_INSTALL_RPATH variable. The default value of CMAKE_INSTALL_RPATH is ""[1]. As currently nothing sets it to another value the rpath is never set during the build for the resulting binaries. I send another patch which does the solution mentioned above the right way. regards, Simon [1]: http://www.cmake.org/Wiki/CMake_RPATH_handling
On Wed, 2011-05-04 at 19:00 +0200, Simon Busch wrote: > I alreay searched through the cmake documentation and it says that the > rpath is set with the content of the CMAKE_INSTALL_RPATH variable. The > default value of CMAKE_INSTALL_RPATH is ""[1]. As currently nothing sets > it to another value the rpath is never set during the build for the > resulting binaries. I send another patch which does the solution > mentioned above the right way. Oh right, I see. So cmake is actually stripping the rpaths during the install step unless you tell it that you want them included? In that case, yeah, your plan seems reasonable. p.
On 05.05.2011 11:50, Phil Blundell wrote: > On Wed, 2011-05-04 at 19:00 +0200, Simon Busch wrote: >> I alreay searched through the cmake documentation and it says that the >> rpath is set with the content of the CMAKE_INSTALL_RPATH variable. The >> default value of CMAKE_INSTALL_RPATH is ""[1]. As currently nothing sets >> it to another value the rpath is never set during the build for the >> resulting binaries. I send another patch which does the solution >> mentioned above the right way. > > Oh right, I see. So cmake is actually stripping the rpaths during the > install step unless you tell it that you want them included? > > In that case, yeah, your plan seems reasonable. Ok, so you are fine with the patch? regards, Simon
On Thu, 2011-05-05 at 13:21 +0200, Simon Busch wrote: > On 05.05.2011 11:50, Phil Blundell wrote: > > On Wed, 2011-05-04 at 19:00 +0200, Simon Busch wrote: > >> I alreay searched through the cmake documentation and it says that the > >> rpath is set with the content of the CMAKE_INSTALL_RPATH variable. The > >> default value of CMAKE_INSTALL_RPATH is ""[1]. As currently nothing sets > >> it to another value the rpath is never set during the build for the > >> resulting binaries. I send another patch which does the solution > >> mentioned above the right way. > > > > Oh right, I see. So cmake is actually stripping the rpaths during the > > install step unless you tell it that you want them included? > > > > In that case, yeah, your plan seems reasonable. > > Ok, so you are fine with the patch? Yup. My only remaining concern was that it seemed a bit unwholesome to do: + echo "set( CMAKE_INSTALL_RPATH ${OECMAKE_RPATH} )" >> ${WORKDIR}/toolchain.cmake when OECMAKE_RPATH might be empty, and maybe it'd have been better to bracket that echo with "if [ -n ${OECMAKE_RPATH} ] ...". But if you've tested it and cmake does the right thing then I guess it's fine as it stands. Acked-by: Phil Blundell <philb@gnu.org> p.
On 05.05.2011 13:34, Phil Blundell wrote: > On Thu, 2011-05-05 at 13:21 +0200, Simon Busch wrote: >> On 05.05.2011 11:50, Phil Blundell wrote: >>> On Wed, 2011-05-04 at 19:00 +0200, Simon Busch wrote: >>>> I alreay searched through the cmake documentation and it says that the >>>> rpath is set with the content of the CMAKE_INSTALL_RPATH variable. The >>>> default value of CMAKE_INSTALL_RPATH is ""[1]. As currently nothing sets >>>> it to another value the rpath is never set during the build for the >>>> resulting binaries. I send another patch which does the solution >>>> mentioned above the right way. >>> >>> Oh right, I see. So cmake is actually stripping the rpaths during the >>> install step unless you tell it that you want them included? >>> >>> In that case, yeah, your plan seems reasonable. >> >> Ok, so you are fine with the patch? > > Yup. My only remaining concern was that it seemed a bit unwholesome to > do: > > + echo "set( CMAKE_INSTALL_RPATH ${OECMAKE_RPATH} )" >> ${WORKDIR}/toolchain.cmake > > when OECMAKE_RPATH might be empty, and maybe it'd have been better to > bracket that echo with "if [ -n ${OECMAKE_RPATH} ] ...". But if you've > tested it and cmake does the right thing then I guess it's fine as it > stands. > > Acked-by: Phil Blundell <philb@gnu.org> Ok, thanks for your help. Patch is now pushed but somehow cgit got the line breaks wrong while local git is showing them correctly ... regards, Simon
Patch
diff --git a/classes/cmake.bbclass b/classes/cmake.bbclass index 40fadea..5f7066b 100644 --- a/classes/cmake.bbclass +++ b/classes/cmake.bbclass @@ -44,6 +44,10 @@ cmake_do_generate_toolchain_file() { echo "set( CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY )" >> ${WORKDIR}/toolchain.cmake # Use qt.conf settings echo "set( ENV{QT_CONF_PATH} ${WORKDIR}/qt.conf )" >> ${WORKDIR}/toolchain.cmake + +# We need to set the rpath to the correct directory as cmake does not provide any +# directory as rpath by default + echo "set( CMAKE_INSTALL_RPATH ${libdir} )" >> ${WORKDIR}/toolchain.cmake } addtask generate_toolchain_file after do_patch before do_configure
In the default configuration cmake does not set a rpath for its builded binaries. This leads to errors at runtime when the binaries not find their needed runtime libraries. Signed-off-by: Simon Busch <morphis@gravedo.de> --- classes/cmake.bbclass | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)