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

Follow PDAL's CMake RPATH strategy #2009

Merged
merged 2 commits into from
Mar 3, 2020
Merged

Follow PDAL's CMake RPATH strategy #2009

merged 2 commits into from
Mar 3, 2020

Conversation

hobu
Copy link
Contributor

@hobu hobu commented Mar 2, 2020

PDAL's RPATH strategy on CMake has worked well for us, and PROJ should consider following suit.
Review it at https://github.com/PDAL/PDAL/blob/master/cmake/rpath.cmake

Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor nit is that cmakelint will suggest lower-case functions set(...) and if(APPLE) for consistency.

I don't have any preference on handling RPATH, but I'll take your word on PDAL's experience.

Further reading on this topic is found here, which describes various RPATH strategies.

@kbevers
Copy link
Member

kbevers commented Mar 3, 2020

Is this considered a bug fix for 7.0.1 or a feature for 7.1.0?

@hobu
Copy link
Contributor Author

hobu commented Mar 3, 2020

Feature for 7.1, but the Conda builds might have to apply this patch to 7.0.x for a while.

@kbevers kbevers added this to the 7.1.0 milestone Mar 3, 2020
@kbevers
Copy link
Member

kbevers commented Mar 3, 2020

Feature for 7.1, but the Conda builds might have to apply this patch to 7.0.x for a while.

Right, thanks. I'm keeping an eye on the conda-forge PROJ package and will keep them posted about this when the 7.0.0 package is on it's way.

@hobu
Copy link
Contributor Author

hobu commented Mar 3, 2020

them (conda) == me also, so I'm happy to help babysit it there too.

@kbevers kbevers merged commit 57c12c2 into master Mar 3, 2020
@hobu hobu deleted the cmake-rpath branch March 3, 2020 19:34
rouault added a commit to rouault/PROJ that referenced this pull request Feb 13, 2022
… appear to be obsolete given current CMake versions (fixes OSGeo#3029)

And specifically remove set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
which was the only setting at a non-default value.
rouault added a commit to rouault/PROJ that referenced this pull request Feb 13, 2022
… appear to be obsolete given current CMake versions (fixes OSGeo#3029)

And specifically remove set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
which was the only setting at a non-default value.
rouault added a commit that referenced this pull request Feb 14, 2022
CMake: remove all Mac specific settings added per #2009, …
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants