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

CMakeLists.txt: Use GNUinstalldirs module #327

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

hosiet
Copy link
Contributor

@hosiet hosiet commented Dec 7, 2019

Debian's packger of librime here. This patch would introduce the GNUInstallDirs module into the project's top-level CMakeLists.txt.

GNUInstalldirs is available since CMake 3.0.2. It provides standard install directory variables as defined for GNU software. Using this module would provides standard variable names for bindir, libdir, includedir and datadir, etc., which would be beneficial. There's no needs for manual string concatenation anymore.

This module is especially useful for Debian-based Linux distributions since they are using /usr/lib/<arch-triplet>/ as libdir instead of /usr/lib/ and /usr/lib64/. The GNUInstallDirs module can elegantly suit such needs.

CMake >= 3.0.2 has been available for many years. It is at least available since Ubuntu 16.04 and Debian 8, which covers all supported Debian and Ubuntu versions (Ubuntu 14.04 LTS has reached End-Of-Life already). As a result, such version bump should not cause much troubles even for users of old systems. I would admit that CMake 3.x is not available on CentOS 7 but at least it is available through epel.

This patch modifies the top-level CMakeLists.txt and applies the standard variables accordingly. The minimal CMake version requirement is also bumped to 3.0.2. I'm wondering if you find it okay to merge it. Thanks!


Edit 1: I see that the currrent Travis CI config still uses Trusty (14.04). I would expect build failures with current settings. Maybe it's time to bump the testbed version?

GNUInstallDirs module is available since CMake 3.0.2. It provides
standard install directory variables as defined for GNU software.
Using this module would provides standard variable names for
bindir, libdir, includedir and datadir, etc., which would be
beneficial.

This patch modifies the top-level CMakeLists.txt and apply the
standard variables accordingly. The minimal CMake version requirement
is also bumped to 3.0.2.
@lotem
Copy link
Member

lotem commented Dec 8, 2019

Thanks for the pull request.
I have few questions to his Packager:
Will this PR end up changing the installed dir of the ibus-engine-rime binary on Debian?
If the executable is relocated, what about ibus itself, how does it know where to load its engines?

@hosiet
Copy link
Contributor Author

hosiet commented Dec 8, 2019

I haven't checked thoroughly but here's a quick answer:

This pull request is only about librime, not ibus-rime; the binary of ibus-engine-rime is currently provided by another project thus it won't be affected by this change at all (ibus-engine-rime's install dir won't be relocated).

That said, We may need to reevaluate the impact when making changes in the ibus-rime project.

@lotem
Copy link
Member

lotem commented Dec 11, 2019

Thanks for the answer. I confused ibus-engine-rime with librime.so, the former one is an executable but ibus require it to be in /usr/lib/. librime.so should be okay to follow system convention of libdir.

@lotem lotem merged commit 3cf8eaf into rime:master Dec 12, 2019
@hosiet
Copy link
Contributor Author

hosiet commented Dec 15, 2019

BTW @lotem here is the answer to the question of ibus-engine-rime:

IBus may find the engine executable file even if it is installed into /usr/lib/<arch-triplet>/ibus-rime/ rather than /usr/lib/ibus-rime/. By looking at the file list of ibus-rime package, you will notice a file with path /usr/share/ibus/component/rime.xml. The content on current Debian system is as follows:

<?xml version="1.0" encoding="utf-8"?>
<!-- filename: rime.xml -->
<component>
	<name>im.rime.Rime</name>
	<description>Rime Component</description>
	<exec>/usr/lib/ibus-rime/ibus-engine-rime --ibus</exec>
	<version>1.0</version>
	<author>GONG Chen &lt;[email protected]&gt;</author>
	<license>GPL</license>
	<homepage>https://rime.im</homepage>
	<textdomain>ibus-rime</textdomain>
	<engines>
		<engine>
			<name>rime</name>
			<language>zh</language>
			<license>GPL</license>
			<author>GONG Chen &lt;[email protected]&gt;</author>
			<icon>/usr/share/ibus-rime/icons/rime.png</icon>
			<layout>default</layout>
			<longname>Rime</longname>
			<description>Rime Input Method Engine</description>
			<rank>0</rank>
			<symbol>&#x37A2;</symbol>
		</engine>
	</engines>
</component>

The actual path to the ibus-engine-rime file is provided here. As a result, I guess it would also be okay if you use GNUInstallDirs module in the ibus-rime project. I may submit a Pull Request later.


Some more updates: Such setup may be broken if you are going to ship binaries like ibus-setup-rime (which does not exist yet) but we should be okay for now. For background information, see this one and ibus source code.

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