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

CMakeLists.txt: set LIB_INSTALL_DIR to /usr/local/lib which is common… #4

Merged
merged 1 commit into from
Jun 27, 2019

Conversation

slaufmann
Copy link

… among Linux systems

With the old value the *.so files got installed to /usr/local/include which is usually used for header files. A common directory for manually installed libraries is /usr/local/lib.

@DerDakon
Copy link

This is only marginally better. Just drop all of these variables and use the GNUInstallDirs module shipped with CMake.

@SemaiCZE
Copy link
Owner

Hi, oops, we're definitely using wrong directory. I aggree with @DerDakon that GNUInstallDirs would be better than just hardcoding the path. @slaufmann do you want to update this PR or do you want me to do the change?
Anyways, thaks for your help!

@slaufmann
Copy link
Author

Thank you for the feedback. I added a commit that uses GNUInstallDirs that you could review.

@DerDakon
Copy link

The idea would be to just remove the options entirely and use the variables from GNUInstallDirs, this already has code to cache those.

@slaufmann
Copy link
Author

I applied the approach suggested by DerDakon and squashed the changes together. The branch should be ready to merge.

CMakeLists.txt Outdated
install(DIRECTORY ${INCLUDE_DIR} DESTINATION include)
install(TARGETS ${PROJECT_NAME} DESTINATION lib COMPONENT library)
install(DIRECTORY ${INCLUDE_DIR} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
install(TARGETS ${PROJECT_NAME} DESTINATION lib COMPONENT ${CMAKE_INSTALL_LIBDIR})

Choose a reason for hiding this comment

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

You replaced the wrong argument here.

Copy link
Author

Choose a reason for hiding this comment

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

You're right, fixed it. Thank you.

…llDirs module.

Requires CMake 2.8.5 as the module was introduced in that version.
@SemaiCZE
Copy link
Owner

@slaufmann Thanks for your code and @DerDakon for your review!

@SemaiCZE SemaiCZE merged commit e85196e into SemaiCZE:master Jun 27, 2019
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

3 participants