Skip to content
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

Fix RPATH handling in TriBITS #126

Closed
bartlettroscoe opened this issue Jun 7, 2016 · 11 comments
Closed

Fix RPATH handling in TriBITS #126

bartlettroscoe opened this issue Jun 7, 2016 · 11 comments
Assignees

Comments

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Jun 7, 2016

Next Action Status: ???

Relates To: trilinos/Trilinos#91

Description:

Currently, TriBITS sets the INSTALL_RPATH property on libraries defined with TRIBITS_ADD_LIBRARY()using:

  SET_PROPERTY(TARGET ${LIBRARY_NAME} PROPERTY INSTALL_RPATH
    "${CMAKE_INSTALL_PREFIX}/${${PROJECT_NAME}_INSTALL_LIB_DIR}")

This results in having the RPATH for libraries swapped out in install for libraries.

However, executables installed with TRIBITS_ADD_EXECUTABLE( ... INSTALLABLE ...) do not currently have this property set. This means that executables that get installed do not work correctly once installed. This problem was not found in the early days of TriBITS because Trilinos installed only libraries and not executables.

Also, by default, there is no RPATH set on install for TPL libraries that are not built by the CMake project. That means that you have to set LD_LIBRARY_PATH in the env in order to run these executables. (This is a big problem for CASL VERA, for example.)

This story is to resolve the handling of RPATH as follows:

  1. Define new global project option ${PROJECT_NAME}_SET_INSTALL_RPATH and allow projects to set the default with ${PROJECT_NAME}_SET_INSTALL_RPATH_DEFAULT but make the default TRUE.
  2. If ${PROJECT_NAME}_SET_INSTALL_RPATH==TRUE, then set the property INSTALL_RPATH on all installable libraries defined with TRIBITS_ADD_LIBRARY() and executables defined with TRIBITS_ADD_EXECUTABLE( ... INSTALLABLE ...) .
  3. If ${PROJECT_NAME}_SET_INSTALL_RPATH==TRUE, then set the property CMAKE_INSTALL_RPATH_USE_LINK_PATH to TRUE by default. (This will make it so that CASL VERA no longer needs to set LD_LIBRARY_PATH in load_env.sh.)
  4. Add documentation to the TriBITS Build Reference and the TriBITS Developers Guide on RPATH strategies (see my notes and emails with Brad King). Cover the use cases for install and use locally (replace RPATH) and for install for use on another system (strip RPATH and leave empty).

Together with setting the CMake 3.3 policy (see trilinos/Trilinos#91), then these changes should fix all of the issues with RPATH that people are having with Trilinos and CASL VERA.

Tasks:

  1. Implement above behaviors and add some automated tests to try to detect the correct behavior (e.g. install executable with RPATH and show that it runs correctly, then install without RPATH and show that you need to set LD_LIBRARY_PATH to get it to run correctly).
  2. Add documentation to the TriBITS Build Reference explaining how to work with RPATH and how to set up a TriBITS project for the use case by default or not.
  3. Add documentation for ${PROJECT_NAME}_SET_INSTALL_RPATH in the TriBITS Developers Guide and point to the TriBITS Build Reference for a discussion of RPATH handling.
  4. Test behavior on Linux with SEMS and on OSX with Trilinos using TriBITS github branch to ensure that RPATH is being handled correctly automatically.
  5. Snapshot TriBITS into Trilinos 'master' and then update CMake issue with shared libraries, --prefix on Apple trilinos/Trilinos#91 to describe that the issue should be resolved on 'master'. (Also, ask if this should also be fixed on the Trilinos 12.6 release branch as well.)
@bartlettroscoe
Copy link
Member Author

I am going to slam this out since this would likely fix a problem for xSDKTrilinos on an OSX system (see trilinos/xSDKTrilinos#8).

@bartlettroscoe
Copy link
Member Author

So much for slamming this out ...

I am working on a slightly revised plan:

  • Define new global project option ${PROJECT_NAME}_SET_INSTALL_RPATH and allow projects to set the default with ${PROJECT_NAME}_SET_INSTALL_RPATH_DEFAULT but make the default TRUE.
  • If ${PROJECT_NAME}_SET_INSTALL_RPATH==TRUE, then set default for CMAKE_INSTALL_RPATH to the library install directory.
  • Set the default for CMAKE_INSTALL_RPATH_USE_LINK_PATH to CMAKE_INSTALL_RPATH_USE_LINK_PATH_DEFAULT and set the default for CMAKE_INSTALL_RPATH_USE_LINK_PATH_DEFAULT to TRUE. (For a raw CMake project, the default for CMAKE_INSTALL_RPATH_USE_LINK_PATH is empty '' which is the same things as FALSE.)

Above, I changed the original plan in #126 and decided not to set the default for CMAKE_INSTALL_RPATH_USE_LINK_PATH based on ${PROJECT_NAME}_SET_INSTALL_RPATH. Instead I will to document that this should be set by the user if they want this variable to be set and let the TriBITS Project decide what the default should be.

This plan is to use the explicit full install path in RPATH. My exploration of CMake behaviors with RPATH is going well. I will attach my detailed notes when I am finished.

When reading the documentation:

https://cmake.org/Wiki/CMake_RPATH_handling

I noticed that it recommends using $ORIGIN (and similar things on OSX) for relative paths in the RPATH to allow the installation to be relocated. I had a conversation with @bradking about this topic in below email chain. The take away from that conversation is that while using $ORIGIN is nice, it may not be a desirable default. And there is some warnings from the above documentation site that there may be some portability issues with this approach. Therefore, using the explicit install location in the RPATH is likely the best thing to do. But I will at least document the relative $ORIGIN path approach for people to consider. I may even try to have TriBITS implement that as an option in the near future.


-----Original Message-----
From: Brad King [mailto:[email protected]]
Sent: Wednesday, October 19, 2016 1:48 PM
To: Bartlett, Roscoe A
Cc: Bill Hoffman
Subject: Re: [EXTERNAL] Re: Can CPack binary installs reset RPATH in shared libs
and executables?

On 10/19/2016 01:43 PM, Bartlett, Roscoe A wrote:

make the default TriBITS behavior just write in the absolute path for
the install dir for RPATH.

Yes, that should be the default for local installations. The relocatable settings
are really meant for use by applications that want to deploy the binaries as part
of a packaged distribution. Then it is up to the packagers to do the right thing.

Do you think that would be helpful and appreciated by users?

Anyone creating packages of applications using software based on TriBITS will
appreciate having infrastructure for adding relocatable RPATHs.

[...]

-Brad


From: Bartlett, Roscoe A
Sent: Wednesday, October 19, 2016 1:43 PM
To: 'Brad King'
Cc: Bill Hoffman
Subject: RE: [EXTERNAL] Re: Can CPack binary installs reset RPATH in shared libs
and executables?

Brad,

Thanks for the info. This is very helpful.

Given this conversation, I think it might be better and play it safe and make the
default TriBITS behavior just write in the absolute path for the install dir for
RPATH. That is super safe and is likely to handle 95% of installs for Trilinos and
other TriBITS projects that I know about. But I would like the documentation
the relative RPATH strategies and provide some information on how to do it. I
might even add an option to TriBITS to put in relative RPATH paths automatically
on Linux and Mac OSX depending on how well my experiments go. This RPATH
stuff is not straightforward and people need some guidance on how to do this
and some push button solutions might be good. Do you think that would be
helpful and appreciated by users?

[...]

Thanks,

-Ross


From: Brad King [mailto:[email protected]]
Sent: Wednesday, October 19, 2016 12:59 PM
To: Bartlett, Roscoe A
Cc: Bill Hoffman
Subject: Re: [EXTERNAL] Re: Can CPack binary installs reset RPATH in shared libs
and executables?

On 10/19/2016 12:12 PM, Bartlett, Roscoe A wrote:

Are there any concrete projects that I can look at that use the
"$ORIGIN,$LIB on Linux and @executable_path,@loader_path on macOS"
approach? It sounds like that should be the default for TriBITS projects.

ParaView uses @executable_path on macOS to find the shared libraries installed
within its .app:

https://gitlab.kitware.com/paraview/paraview/blob/v5.2.0-RC2/CMake/ParaViewMacros.cmake#L458-459

On Linux it still uses an environment-setting launcher for the main application,
but we use $ORIGIN for a few utilities, e.g.:

https://gitlab.kitware.com/paraview/paraview/blob/v5.2.0-RC2/Utilities/ProcessXML/CMakeLists.txt#L46-47

(We just haven't gotten around to porting to $ORIGIN for the main app.)

On Windows we just install the dlls next to the exes.

BTW, I remembered that $LIB is only for the "lib" part to switch between "lib"
and "lib64". It is only $ORIGIN that gets replaced with a binary's own location.

I caution that the above examples are all "it works for this case" level of effort.
A generalized solution for TriBITS may require more care.

-Brad


From: Bartlett, Roscoe A
Sent: Wednesday, October 19, 2016 12:13 PM
To: 'Brad King'; Bill Hoffman
Subject: RE: [EXTERNAL] Re: Can CPack binary installs reset RPATH in shared libs
and executables?

Brad,

Thanks for the info.

Are there any concrete projects that I can look at that use the "$ORIGIN,$LIB on
Linux and @executable_path,@loader_path on macOS" approach? It sounds
like that should be the default for TriBITS projects.

But this does not solve the problem of third-party libraries and their locations
but I guess those generally are assumed to be in the default library paths for the
system or people will use LD_LIBRARY_PATH to find them.

Thanks,

-Ross


From: Brad King [mailto:[email protected]]
Sent: Wednesday, October 19, 2016 11:44 AM
To: Bartlett, Roscoe A; Bill Hoffman
Subject: [EXTERNAL] Re: Can CPack binary installs reset RPATH in shared libs and
executables?

On 10/19/2016 11:36 AM, Bartlett, Roscoe A wrote:

It occurred to me that it would be great if you could define a binary
distribution executable based on CMake that could make this RPATH
substitution on the final target machine.

Very few projects ever try to do that because it is fairly unreliable.
It means a simple binary tarball cannot simply be extracted and used on the end-
user machine. Some kind of update step has to be done first.
If they move it around on their machine they need to update again, and we may
not be able to reliably do that since original placeholders will be gone.

There are two approaches that applications typically use for this:

  • Use $ORIGIN on Linux and @executable_path,@loader_path on macOS.
    These are placeholders in the binaries that get substituted by the
    dynamic loader with the current locations of the binaries. They can
    be used to encode relative RPATHs. CMake supports placing these
    in the INSTALL_RPATH (or INSTALL_NAME_DIR) field.
  • Use no RPATH and instead have an application launcher script
    that sets LD_LIBRARY_PATH (or platform equivalent) based on its
    own location in order to help the dynamic loader find the libraries.
    Then it exec-s the real executable in the constructed environment.

-Brad


From: Bartlett, Roscoe A
Sent: Wednesday, October 19, 2016 11:37 AM
To: 'Brad King'; Bill Hoffman
Subject: Can CPack binary installs reset RPATH in shared libs and executables?

Hello Brad and Bill,

I am finally getting to the bottom of RPATH issues with Trilinos (through TriBITS) and I am documenting the various install and distributions use cases and how to deal with RPATH for each of these. The nice feature of CMake is that it can replace RPATH for the final installation directory when you know what it is using CMAKE_INSTALL_RPATH. But you can only use that if you know what the final directory structure will look like.

It occurred to me that it would be great if you could define a binary distribution executable based on CMake that could make this RPATH substitution on the final target machine. I guess you would have to know the max length of RPATH to make sure there is room for the substitution but otherwise, it seems that the functionality to implement is already in CMake and that could be used to create a binary installer that does this as well. Do any of the CMake CPack binary installer implementations support replacing RPATH in the shared libs and execs on the final install on the target machine through the binary distribution package?

Thanks,

-Ross

@ibaned
Copy link
Contributor

ibaned commented Oct 21, 2016

I assume this issue encompasses adding TPL RPATHs to Trilinos libraries, for example libzoltan.so would have the RPATH to find libparmetis.so if the ParMETIS TPL is enabled ? If so I am interested in the resolution of this issue, thank you for doing this work.

@bartlettroscoe
Copy link
Member Author

@ibaned,

Yes, this will resolve that issue. The new default will be the add the directories for all the external TPLs to RPATH. But you can do that right now by just configuring with:

-DCMAKE_INSTALL_RPATH_USE_LINK_PATH=TRUE

That is a magical CMake option that I just learned about recently. The new TriBITS will make that the default.

Would you be willing to review the documentation for RPATH once I push it to this GitHub repo master branch? That would be good before I snapshot TriBITS into Trilinos proper.

@ibaned
Copy link
Contributor

ibaned commented Oct 22, 2016

Excellent, I've also been making that the default in my codes (actually, the whole "full RPATH" from Kitware's documentation). I wasn't sure TriBITS would respect it on the command line, but I'll try that in the meantime. I'd be happy to review, just let me know when it's pushed.

@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Oct 23, 2016

I wasn't sure TriBITS would respect it on the command line, but I'll try that in the meantime.

From the perspective of the user who is just configuring using CMake, a project that uses TriBITS should behave just like any regular old CMake project, except perhaps with different defaults. If you set any raw CMake variable, then it should have the same effect as it would with any other CMake project. However, there are some known examples where this is not true that need to be fixed (e.g. #131). If you know of other examples like this, please let me know.

I'd be happy to review, just let me know when it's pushed.

Thanks! I will add a comment here once I do the push to the TriBITS GitHub master branch. At that point the online documentation will be updated. Then you can test against Trilinos you want by TriBITS locally and using that version.

bartlettroscoe added a commit that referenced this issue Oct 27, 2016
Need to add unit tests for this yet.
bartlettroscoe added a commit that referenced this issue Oct 27, 2016
bartlettroscoe added a commit that referenced this issue Oct 27, 2016
This showed some surprising behavior of CMake and/or the Linux linker on this
machine.  Somehow it put in the path to libsimplecxx.so into RPATH when it
should not have done so.
bartlettroscoe added a commit that referenced this issue Oct 29, 2016
This failed on the Travis CI server that has no RPATH set at all.
bartlettroscoe added a commit that referenced this issue Oct 29, 2016
When no RPATH is set on the Travis CI server, then there is no RPATH field at
all.

Hopefully this will fix the Travis CI build once and for all.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Nov 2, 2016
It seems that the `date` command does not work on OSX like it does on Linux.
This is important to know.

I found this out while working TriBITSPub#126 (but this change is not directly releated
to TriBITSPub#126).
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Nov 2, 2016
With this change that test for CMAKE_INSTALL_RPATH now passes on OSX!
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Nov 2, 2016
This was done as part of TriBITSPub#126 but is not directly related to that story.
bartlettroscoe added a commit that referenced this issue Nov 2, 2016
bartlettroscoe added a commit that referenced this issue Nov 3, 2016
This changes the default behavior of TriBITS projects to set RPATH on install
so that everything works right out of the install directory without having to
set any extra env vars.

Several things done as part of this:

* Added SimpleTpl to help teste RPATH.  (This will also come in handy for
  doing more TPL testing.)

* Add installable exec to SimpleCxx package to help test RPATH.

* Added documentation for users and developers (see documenation files).

* Added detailed automated tests to test the different use cases described in
  the documentation.

I manually tested this locally on Linux, Mac OSX (gaia) and on Travis CI (by
pushing the branch fix-rpath-126 to the main TriBITS GitHub repo).  I think
this is pretty solid.  See #126 for more motivation and
details.
bartlettroscoe added a commit that referenced this issue Nov 3, 2016
It seems that the `date` command does not work on OSX like it does on Linux.
This is important to know.

I found this out while working #126 (but this change is not directly releated
to #126).
bartlettroscoe added a commit that referenced this issue Nov 3, 2016
This was done as part of #126 but is not directly related to that story.
bartlettroscoe added a commit that referenced this issue Nov 3, 2016
This changes the default behavior of TriBITS projects to set RPATH on install
so that everything works right out of the install directory without having to
set any extra env vars.

Several things done as part of this:

* Added SimpleTpl to help teste RPATH.  (This will also come in handy for
  doing more TPL testing.)

* Add installable exec to SimpleCxx package to help test RPATH.

* Added documentation for users and developers (see documenation files).

* Added detailed automated tests to test the different use cases described in
  the documentation.

I manually tested this locally on Linux, Mac OSX (gaia) and on Travis CI (by
pushing the branch fix-rpath-126 to the main TriBITS GitHub repo).  I think
this is pretty solid.  See #126 for more motivation and
details.
bartlettroscoe added a commit that referenced this issue Nov 3, 2016
It seems that the `date` command does not work on OSX like it does on Linux.
This is important to know.

I found this out while working #126 (but this change is not directly releated
to #126).
bartlettroscoe added a commit that referenced this issue Nov 3, 2016
This was done as part of #126 but is not directly related to that story.
@bartlettroscoe
Copy link
Member Author

I just pushed the commit eb300ad to the 'master' branch so this is ready to review. The updated documentation on RPATH handling can be seen here:

I wrote some pretty good automated tests for these different RPATH use cases and ran them on OSX and Linux as shown here (see "RPATH" tests):

@ibaned and @bradking, if you have some time can you please take a few minutes and look over the above documentation and then provide any comments in this Issue ticket?

Also, if you want to test against Trilinos, just clone TriBITS under Trilinos and then configure with:

-DTrilinos_TRIBITS_DIR:STRING=TriBITS/tribits

Thanks!

@ibaned
Copy link
Contributor

ibaned commented Nov 3, 2016

Thanks @bartlettroscoe !
Here are tiny spelling/grammar comments for the documentation:

  • In "Setting Install RPATH" should the first bullet be CMAKE_INSTALL_RPATH (the R is missing) ?
  • In that same section, I would rename items (2) and (3) to "targets will move after installation" and "targets and TPLs will move after installation", or at least shorten item (3)'s title
  • In "RPATH Handling", the last sentence can start with "For"; remove the "And".

The content makes sense and I'm impressed that testing actually checks the objdump of the target.
I'll test the new system when I get a chance, and I can confirm that setting CMAKE_INSTALL_RPATH_USE_LINK_PATH already works with the current system (I have several different Trilinos installs).

bartlettroscoe added a commit that referenced this issue Nov 3, 2016
I fixed the typos things that @ibaned found and made the other changes he
suggested in #126.

I also did a full read over and fixed a few more typos and improved a few
things.

Build/Test Cases Summary
Enabled Packages:
Enabled all Packages
0) MPI_DEBUG => passed: passed=222,notpassed=0 (0.33 min)
1) SERIAL_RELEASE => passed: passed=222,notpassed=0 (0.31 min)
@bartlettroscoe
Copy link
Member Author

@ibaned,

Thanks for the review! I fixed the typos and make your other suggested changes in the commit 4769da4.

@bradking, let me know if you ca do a review. I will leave this "in review" until then.

@bradking
Copy link
Contributor

bradking commented Nov 3, 2016

@bartlettroscoe I've just finished reading through the documentation. It looks fine after the fixup commit.

@bartlettroscoe
Copy link
Member Author

Thanks @bradking. I am now closing at complete.

No more RPATH problems with shared libraries :-)

bartlettroscoe added a commit to trilinos/Trilinos that referenced this issue Nov 4, 2016
Brings in:

* Fixed default RPATH (see TriBITSPub/TriBITS#126)

* Updated gitdist --dist-repos behavior (see TriBITSPub/TriBITS#151).  For
  this last change, one should update their install of the gitdist script.  If
  you don't, then running gitdist will process the base Trilinos git repo
  twice (which is typically harmless).

There are other small changes as well but the above ones were the motivation
for updating the TriBITS snapshot.
bartlettroscoe added a commit that referenced this issue Apr 6, 2017
These tests are not set up to work correctly on cygwin (and RPATH likely will
not work on Windows under cygwin).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants