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

Avoid hardcoded path to conform to current CMake pratice #782

Merged
merged 1 commit into from
Apr 27, 2023
Merged

Avoid hardcoded path to conform to current CMake pratice #782

merged 1 commit into from
Apr 27, 2023

Conversation

topazus
Copy link
Contributor

@topazus topazus commented Apr 27, 2023

It maybe be better to use the ${CMAKE_INSTALL_LIBDIR}, ${CMAKE_INSTALL_INCLUDEDIR}, instead of hardcoding the directory. ${CMAKE_INSTALL_LIBDIR}, ${CMAKE_INSTALL_INCLUDEDIR}, et al. are defined by the GNUInstallDirs that represents the platform-specific installation directory and provides install directory variables defined by the GNU Coding Standards.

Ref:
https://gitlab.kitware.com/cmake/cmake/-/merge_requests/8111
https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html
libsdl-org/SDL@6956f4a

@mwestphal
Copy link
Contributor

Definitely a good idea !

@mwestphal
Copy link
Contributor

strange unrelated CI issues, investigating.

@mwestphal
Copy link
Contributor

I've rerun, looks like an intermitent docker repository failure

@topazus
Copy link
Contributor Author

topazus commented Apr 27, 2023

Thanks for your quick looking at this.

@mwestphal
Copy link
Contributor

I see a few other usages of lib in application/CMakeLists.txt, I think they should be fixed too.

@mwestphal
Copy link
Contributor

BTW @topazus , I see that you are a fedora packager. Do you plan to add F3D to fedora packages ? :)

@topazus
Copy link
Contributor Author

topazus commented Apr 27, 2023

I see a few other usages of lib in application/CMakeLists.txt, I think they should be fixed too.

You mean the usage of lib here? I don't know kwnowledge about apple platform.

if (APPLE)
if (F3D_MACOS_BUNDLE)
set_target_properties(f3d PROPERTIES INSTALL_RPATH @loader_path/../../../lib)
else ()
set_target_properties(f3d PROPERTIES INSTALL_RPATH @loader_path/../lib)
endif ()

@mwestphal
Copy link
Contributor

mwestphal commented Apr 27, 2023

No worries, I will take care of it.

@mwestphal mwestphal merged commit 18c3e68 into f3d-app:master Apr 27, 2023
@topazus
Copy link
Contributor Author

topazus commented Apr 27, 2023

BTW @topazus , I see that you are a fedora packager. Do you plan to add F3D to fedora packages ? :)

Yes, I would like to make f3d package into Fedora official repository. Maybe I can do this this weekend.

@mwestphal
Copy link
Contributor

Perfect, thanks @topazus !

@topazus topazus deleted the fix_cmake branch May 22, 2023 00:48
mwestphalnew pushed a commit to mwestphalnew/f3d that referenced this pull request Feb 10, 2024
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.

2 participants