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

Use the GNUInstallDirs CMake module to get the install paths. #133

Merged
merged 1 commit into from Nov 7, 2017

Conversation

wRAR
Copy link
Contributor

@wRAR wRAR commented Oct 30, 2017

No description provided.

@wRAR wRAR mentioned this pull request Oct 30, 2017
set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE)
list(FIND CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES "${CMAKE_INSTALL_PREFIX}/lib" isSystemDir)
list(FIND CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}" isSystemDir)
if("${isSystemDir}" STREQUAL "-1")
Copy link
Member

@dbaarda dbaarda Nov 2, 2017

Choose a reason for hiding this comment

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

What exactly does this if section do? It looks to me like it sets CMAKE_INSTALL_RPATH to exactly the same thing it was set to above.

Copy link
Contributor Author

@wRAR wRAR Nov 2, 2017

Choose a reason for hiding this comment

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

Yes, I'm wondering that too. The relevant commit is 75f6884.

Copy link
Contributor Author

@wRAR wRAR Nov 2, 2017

Choose a reason for hiding this comment

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

Copy link
Member

@dbaarda dbaarda Nov 7, 2017

Choose a reason for hiding this comment

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

Ah, OK. It still doesn't make sense to me, but sure.

@dbaarda
Copy link
Member

@dbaarda dbaarda commented Nov 7, 2017

I'm going to merge this as a minimal change to use the proper cmake install module stuff.

Further work that would be nice is adding the install for the man pages which is missing. However, that's something for another pull request.

@dbaarda dbaarda merged commit 131447a into librsync:master Nov 7, 2017
1 check passed
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.

None yet

2 participants