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

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
Copy link
Contributor Author

glx22 commented Jun 14, 2020

@Nik-mmzd I updated man page naming

@Nik-mmzd
Copy link

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
Copy link
Contributor Author

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 commented Jun 19, 2020

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

@glx22
Copy link
Contributor Author

glx22 commented Jun 19, 2020

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

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})
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

@glx22 glx22 merged commit 97592c4 into OpenTTD:master Jul 2, 2020
@glx22 glx22 deleted the GNUInstallDirs branch August 11, 2021 17:38
archlinux-github pushed a commit to archlinux/aur that referenced this pull request Jul 27, 2022
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.

cmake: make install DESTDIR="something" ignores GLOBAL_DIR option
4 participants