This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Fix llvm_setup_rpath function
ClosedPublic

Authored by azharudd on Nov 3 2016, 2:02 PM.

Details

Summary

Set _install_rpath to CMAKE_INSTALL_RPATH if it is defined, so that eventually
INSTALL_RPATH is set to CMAKE_INSTALL_RPATH.
The "if(NOT DEFINED CMAKE_INSTALL_RPATH)" was missing a corresponding else
clause.
This also cleans up the fix made in r285908.

Patch by Azharuddin Mohammed

Diff Detail

Event Timeline

azharudd updated this revision to Diff 76884.Nov 3 2016, 2:02 PM
azharudd retitled this revision from to [CMake] Fix llvm_setup_rpath function.
azharudd updated this object.
azharudd added reviewers: john.brawn, beanz.
azharudd added a subscriber: llvm-commits.
azharudd updated this object.Nov 3 2016, 2:03 PM
beanz edited edge metadata.Nov 7 2016, 11:10 AM

I think there is actually a more general solution to this. At the top of the function we can just have:

if(CMAKE_INSTALL_RPATH)
  return()
endif()

Then we can remove the conditionals on CMAKE_INSTALL_RPATH, and just allow the target to inherit from the global setting, which is the default.

azharudd updated this revision to Diff 77083.Nov 7 2016, 12:51 PM
azharudd edited edge metadata.

I think there is actually a more general solution to this. At the top of the function we can just have:

if(CMAKE_INSTALL_RPATH)
  return()
endif()

Then we can remove the conditionals on CMAKE_INSTALL_RPATH, and just allow the target to inherit from the global setting, which is the default.

sounds good. updated.

beanz accepted this revision.Nov 7 2016, 4:44 PM
beanz edited edge metadata.

LGTM!

This revision is now accepted and ready to land.Nov 7 2016, 4:44 PM
This revision was automatically updated to reflect the committed changes.