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

CMake: install relocatable shared libs for macOS #1089

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SpaceIm
Copy link

@SpaceIm SpaceIm commented Sep 27, 2023

Remove hardcoded CMAKE_INSTALL_NAME_DIR, so that it's @rpath by default (default CMake behavior when cmake_minimum_required() is at least 3.0). set(CMAKE_MACOS_RPATH TRUE) is useless, it's the default value for CMake if cmake_minimum_required() is at least 3.0 as well, no need to hardcode this value.

It reverts #976 (hardcoding CMAKE_INSTALL_NAME_DIR is bad practice).

If someone wants to change this behavior for a specific reason, CMAKE_INSTALL_NAME_DIR can be injected externally, no need to patch build files.

@eustas
Copy link
Collaborator

eustas commented Nov 3, 2023

Hello. I'll prepare a middle-ground PR:

  • remove set(CMAKE_MACOS_RPATH
  • remove set(CMAKE_INSTALL_NAME_DIR
  • specify default INSTALL_NAME if CMAKE_INSTALL_NAME_DIR is not defined

@SpaceIm
Copy link
Author

SpaceIm commented Nov 3, 2023

specify default INSTALL_NAME if CMAKE_INSTALL_NAME_DIR is not defined

Why?

@eustas
Copy link
Collaborator

eustas commented Nov 3, 2023

Because that is what embedders of library do. Why not make their life easier?

eustas
eustas previously approved these changes Nov 3, 2023
@SpaceIm
Copy link
Author

SpaceIm commented Nov 3, 2023

It makes life of package managers harder, because it changes default CMake behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants