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

Add: [CMake] More improvements for install #8218

Merged
merged 2 commits into from Jul 2, 2020
Merged

Conversation

@glx22
Copy link
Contributor

@glx22 glx22 commented Jun 12, 2020

Default path will be different, but the switch to cmake already changed some. And package maintainers usually don't rely on default values.

Also added a way to rename openttd executable.

Closes #8204

@glx22 glx22 force-pushed the glx22:GNUInstallDirs branch from 9d9edc6 to 5bd8468 Jun 12, 2020
@glx22 glx22 force-pushed the glx22:GNUInstallDirs branch from 5bd8468 to 75399a2 Jun 13, 2020
@glx22
Copy link
Contributor Author

@glx22 glx22 commented Jun 14, 2020

@Nik-mmzd I updated man page naming

@Nik-mmzd
Copy link

@Nik-mmzd Nik-mmzd commented Jun 14, 2020

All seems to work as expected (CMAKE_INSTALL_DATADIR, CMAKE_INSTALL_DOCDIR, CMAKE_INSTALL_MANDIR, BINARY_NAME, man page naming)

Found one more missing thing:
Old makefile creates ${binary_name}.desktop file in ${prefix}/share/applications and installs icons to ${prefix}/share/icons/${theme}/${icon_resolution}/${binary_name}.png: https://github.com/OpenTTD/OpenTTD/blob/release/1.10/Makefile.bundle.in#L84
https://github.com/OpenTTD/OpenTTD/blob/release/1.10/Makefile.bundle.in#L194
cmake build does not

CMakeLists.txt Outdated Show resolved Hide resolved
@glx22 glx22 changed the title Add: [CMake] Use GNUInstallDirs to allow install paths customisation Add: [CMake] More improvements for install Jun 19, 2020
@glx22 glx22 force-pushed the glx22:GNUInstallDirs branch from 75399a2 to 60d9098 Jun 19, 2020
@glx22
Copy link
Contributor Author

@glx22 glx22 commented Jun 19, 2020

I assumed if someone wants to change binary name, it should be reflected to install paths.

@LordAro
Copy link
Member

@LordAro LordAro commented Jun 19, 2020

Why should we provide an ability to rename the output executable?

@glx22
Copy link
Contributor Author

@glx22 glx22 commented Jun 19, 2020

It was supported before cmake move and some packagers use this feature, mainly for nightly packages.

CMakeLists.txt Show resolved Hide resolved
set(MAN_BINARY_FILE ${CMAKE_BINARY_DIR}/docs/${BINARY_NAME}.6)
install(CODE
"
execute_process(COMMAND ${CMAKE_COMMAND} -E copy ${MAN_SOURCE_FILE} ${MAN_BINARY_FILE})

This comment has been minimized.

@glx22

glx22 Jun 19, 2020
Author Contributor

yeah configure_file works here too, anyway the file is always copied during install, because gzip compresses it in-place :)

This comment has been minimized.

@h3xx

h3xx Jun 19, 2020
Contributor

Very few of the build systems I've ever worked with (as a packager myself) have automatically compressed the man page. The few times they have, about half the time it's done incorrectly, e.g. not with gzip -9, or using bzip2 as the compressor, or something, and I have to redo it in my script.

For instance, wesnoth/wesnoth doesn't compress its man pages upon install, and that's just fine with me.

@LordAro
LordAro approved these changes Jul 2, 2020
@glx22 glx22 merged commit 97592c4 into OpenTTD:master Jul 2, 2020
8 checks passed
8 checks passed
Commit checker
Details
OpenTTD CI Build #20200619.3 succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.9) Linux linux-amd64-clang-3.9 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.