FS#66584 - [librime] Built in debug mode

Attached to Project: Arch Linux
Opened by Zhenhui Xie (heyrict) - Friday, 08 May 2020, 04:58 GMT
Last edited by Felix Yan (felixonmars) - Friday, 08 May 2020, 16:46 GMT
Task Type Bug Report
Category Packages: Extra
Status Closed
Assigned To Felix Yan (felixonmars)
Architecture All
Severity Medium
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:

Currently, `librime` is built in debug mode in the packages. This will result in a flooding debug log and a huge stuttering in rime IME.

To solve the issue, line 22 in `PKGBUILD` should be changed to `cmake --build build -DCMAKE_BUILD_TYPE=Release` or so (Correct me if it is not how to make a release build in cmake. I'm not familiar with C++).

Additional info:
* package version(s)

pacman -Qi librime

```
Name : librime
Version : 1:1.5.3-6
Description : Rime input method engine
Architecture : x86_64
URL : https://github.com/rime/librime
Licenses : GPL3
Groups : None
Provides : None
Depends On : boost-libs opencc yaml-cpp leveldb librime-data google-glog marisa
Optional Deps : None
Required By : fcitx-rime
Optional For : None
Conflicts With : None
Replaces : None
Installed Size : 2.75 MiB
Packager : Felix Yan <felixonmars@archlinux.org>
Build Date : Sat 14 Mar 2020 08:59:44 PM CST
Install Date : Fri 01 May 2020 11:11:32 PM CST
Install Reason : Installed as a dependency for another package
Install Script : No
Validated By : Signature
```

* config and/or log files etc.

cat rime.fcitx-rime.INFO | cut -d" " -f 4 | sort | uniq -c | sort -n | tail -n 20

```
74 poet.cc:164]
84 config_component.cc:104]
89 engine.cc:99]
101 syllabifier.cc:187]
126 script_translator.cc:505]
131 script_translator.cc:491]
140 poet.cc:152]
154 dictionary.cc:85]
177 config_data.cc:223]
245 syllabifier.cc:52]
316 syllabifier.cc:138]
316 syllabifier.cc:86]
701 user_dictionary.cc:212]
1965 translator_commons.cc:105]
4664 user_dictionary.cc:243]
4664 user_dictionary.cc:492]
4664 user_dictionary.cc:72]
36495 user_dictionary.cc:215]
36495 user_dictionary.cc:236]
36495 user_dictionary.cc:238]
```

* link to upstream bug report, if any

https://github.com/rime/librime/issues/254 (in Chinese)

Steps to reproduce:
1. start fcitx, switch rime-ime
2. type a few chinese characters
This task depends upon

Closed by  Felix Yan (felixonmars)
Friday, 08 May 2020, 16:46 GMT
Reason for closing:  Fixed
Additional comments about closing:  1:1.5.3-7
Comment by Zhenhui Xie (heyrict) - Friday, 08 May 2020, 05:03 GMT
Oh, I should have set the severity to `HIGH`, as a member of librime developers noted that there could be sensitive information in the log: https://github.com/rime/librime/issues/254#issuecomment-625153406
Comment by Doug Newgard (Scimmia) - Friday, 08 May 2020, 06:12 GMT
So their release tarballs default to debug mode? Sounds like something upstream should be fixing.
Comment by Zhenhui Xie (heyrict) - Friday, 08 May 2020, 06:26 GMT
Well, they said that the package should be built in release mode, and AFAIK release mode should be set in `cmake` command, which is set in PKGBUILD file.

---

Just built the package myself. I can confirm the debug logs no longer exists after adding the `-DCMAKE_BUILD_TYPE=Release` flag

```
build() {
cd $pkgname-$pkgver
cmake . -Bbuild -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=Release
cmake --build build
}
```
Comment by Doug Newgard (Scimmia) - Friday, 08 May 2020, 06:35 GMT
You missed my point. We're building from RELEASE tarballs. It should default to build type release.
Comment by Zhenhui Xie (heyrict) - Friday, 08 May 2020, 08:19 GMT
Well, sorry that I am not familiar with release tarballs.

I've searched further about how cmake works, and found that it is common to make CMAKE_BUILD_TYPE=release in cmakelists.txt .

If that is the case, it should be an upstream-related bug.
Comment by Antonio Rojas (arojas) - Friday, 08 May 2020, 09:52 GMT
Your proposed fix https://github.com/rime/librime/pull/365 is wrong, upstream projects should never mess with CMAKE_BUILD_TYPE themselves against the builder's choices.

Not setting CMAKE_BUILD_TYPE (or equivalently setting it to None) is a perfectly valid choice which is different from both Release and Debug. The quiestion here is, why is librime being built in debug mode when the CMAKE_BUILD_TYPE is None.
Comment by Zhenhui Xie (heyrict) - Friday, 08 May 2020, 10:37 GMT
Sorry, I'm rather confused about the build system of cmake. What can we do to make librime built in release mode without modifying the PKGBUILD file or CMakeList.txt?

It seems that cmake defaults to target debug mode if None is configured, but @Scimmia said that it should be built in release mode when using a *release tarball*. Then how do cmake recognize which mode the package should be built in?
Comment by Zhenhui Xie (heyrict) - Friday, 08 May 2020, 12:37 GMT
Here are some comments from the maintainer of librime lotem:

> For your reference:
> Gentoo package uses the cmake flag -DCMAKE_BUILD_TYPE=Gentoo by default. That's similar to our situation, where CMAKE_BUILD_TYPE is None.
#316 (comment)

> The differences among those CMAKE_BUILD_TYPE options that affect logging in librime is the -DNDEBUG macro, which is only defined by -DCMAKE_BUILD_TYPE=Release.
> I think that makes the point, and answers Antonio's question:
> strictly speaking, librime doesn't create a "debug" build with unspecified CMAKE_BUILD_TYPE; it's only the debug logs depend on that macro definition.

> The fix in Gentoo actually supports a debug build variant corresponding to USE flag debug. That I believe is a non-goal of the PKGBUILD for Arch Linux.

> In the comments, Doug mentioned "release tarball". I'm sure everyone in the thread is aware that a release tarball, in the context of open-source software, doesn't refer to artifacts of a release build, but a source tarball that can be used with multiple build configurations.
> librime uses CMake to build its sources. I also added a wrapper Makefile which provides a default release target calling cmake specifically with -DCMAKE_BUILD_TYPE=Release, so that the conventional make && make install pattern creates a release build. (see instructions in README)
> Somehow, the Arch Linux packager chose to invoke cmake directly in the PKGBUILD instead of calling the wrapper Makefile. This might have the benifit of easily customizing build options for the platform. But that's where the PKGBUILD's resulting compiler options begin to differ from that of a "release" build from the tarball.
Comment by Eli Schwartz (eschwartz) - Friday, 08 May 2020, 16:23 GMT
cmake is an inconvenient build system, since it is quite awkward to handle compiler flags properly.

Anyway, the problem here is that librime is enabling a *debug* feature (very verbose logs) based on the *absence* of release mode. This is backwards and they should add a special -DVERBOSE_LOGS which is automatically toggled in situations such as building with the cmake debug type.

Using -DNDEBUG for this seems kind of unusual as it's more about asserts, while this is something impacting the user.

Loading...