-
-
Notifications
You must be signed in to change notification settings - Fork 364
CMakeLists.txt: Avoid hardcoding installation dirs #315
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: Avoid hardcoding installation dirs #315
Conversation
Hi @ZhongRuoyu, thanks for brining this up. This seams to change most of linux-only paths... which (because of how I'm integrating to windows managers needs to be fixed). Wondering if instead we can do this changes only for Apple. Something like: if (APPLE)
include(GNUInstallDirs)
install(TARGETS glslViewer DESTINATION ${CMAKE_INSTALL_BINDIR})
else
install(TARGETS glslViewer DESTINATION bin)
target_link_libraries(glslViewer PRIVATE atomic)
install(FILES "${PROJECT_SOURCE_DIR}/assets/glslViewer.png" DESTINATION /usr/share/pixmaps)
install(FILES "${PROJECT_SOURCE_DIR}/assets/glslViewer.desktop" DESTINATION /usr/share/applications)
... |
Hi @patriciogonzalezvivo, thanks for your attention. However, the build actually failed on Linux (Homebrew not only supports macOS), so I'm afraid these changes are necessary. Build failure aside: I'm not familiar with how software should be installed on Windows, but normally on Unix systems, software installation isn't done this way. Firstly, not all users want, or have enough privilege, to install to If you still want to preserve the original behaviour, you may set I hope that clarifies! |
Hard-coding install locations is not a nice thing to do, and negates one of the significant benefits of using a cross-platform build system like CMake. If you'd like to retain the existing behaviour of installing into option(CMAKE_INSTALL_PREFIX "Location to install" "/usr") However, it is important to respect user configuration of |
Good points, thank you @ZhongRuoyu and @carlocab. I will test it today locally, solve the .desktop and .thumbnail dependencies |
So here is the pickle, there is no `/usr/local/share/mime/package/ folder'. It would be ok, hard_coding only this path? Install the project...
-- Install configuration: ""
-- Up-to-date: /usr/local/bin/glslViewer
-- Installing: /usr/local/share/pixmaps/glslViewer.png
-- Installing: /usr/local/share/applications/glslViewer.desktop
-- Installing: /usr/local/share/mime/packages/glslViewer-types.xml
xdg-mime: file 'share/mime/packages/glslViewer-types.xml' does not exist
Directory 'share/mime/packages' does not exist!
The databases in [share/applications] could not be updated.
-- Installing: /usr/local/share/thumbnailers/glslViewer.thumbnailer
-- Installing: /usr/local/bin/glslThumbnailer
-- Installing: /usr/local/bin/glslScreenSaver
-- Installing: /usr/local/share/glslViewer/glslScreenSaver.frag
-- Installing: /usr/local/share/glslViewer/glslScreenSaver.yaml |
This allows the user to specify alternative directories when `/usr/share` is not a suitable option. Also, remove `sudo` invocations. It should be more preferable to let the user do `sudo cmake --install <build-directory>` instead, if needed. This is safer, and allows users without root privileges to build and install to custom locations. See: - https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html - https://www.gnu.org/prep/standards/html_node/Directory-Variables.html Signed-off-by: Ruoyu Zhong <zhongruoyu@outlook.com>
80cfa78
to
d3879be
Compare
Sorry, I must have missed that, since
Actually, we can use the |
Thank you so much @ZhongRuoyu ! that worked perfectly! I added a couple of lines to solve the issues finding GlslViewer png icon through scripting the it on the cmake file. If you can double check that this is ok with brew I will make a new tag for this bug fix. Thank you again |
I've tested this locally, and I think it's good to go. Thanks, @patriciogonzalezvivo! |
Sweet! I'm making the tag and release soon |
Thanks, @patriciogonzalezvivo ! |
Hi! Submitting this to fix build in Homebrew/homebrew-core#118048 (log). Homebrew's test bot won't allow writes to system directories like
/usr/bin
. That could be resolved if installation directories are customizable, which is usually the case.For details, please see the commit message. Thanks!
Signed-off-by: Ruoyu Zhong zhongruoyu@outlook.com