Review Request 119025: Actually pass IBUS_DEFINITIONS when compiling ibus-panel

[Raphael Kubo da Costa]

On July 19, 2014, 12:17 a.m., Vadim Zhukov wrote:
> > (As a general note, for build system related stuff like this you can also try including the "buildsystem" group, which can be more responsive at times)
> > 
> > > The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used.
> > > 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt
> > 
> > Can you post the error you get here? I've tried building kimtoy here on FreeBSD expecting to hit the same issue(s), but it all went along just fine.
> > 
> > > 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time
> > 
> > This doesn't make much sense; all values found at configuration time in CMake are then used to generate your make/ninja/whatever files, regardless of whether they are cached or not.
> 
> Vadim Zhukov wrote:
>     Raphael, thank you for your input!
>     
>     The issue was caused by the fact that X includes are placed in the /usr/X11R6/include directory on OpenBSD. This catalog is mentioned in PC_IBUS_DEFINITIONS variable but isn't propagated to the caller of find_package(IBus). Yes, I was wrong: the CACHE part may and should be omitted. This patch was added by me a long time ago (7 Feb 2012 according to git log; guess the KDE version used then), when I was much less expirienced in CMake... I've started a massive push of OpenBSD local patches upstream recently during OpenBSD hackathon, when I got time for such cleanup.
>     
>     At the present time, the /usr/X11R6/include gets to the include_directories() from another place, so the patch isn't required at all. But, still, the PC_IBUS_DEFINITIONS should be respected, IMHO. What do you think?
>     
>     Please note that FreeBSD and OpenBSD and quiet different. So you can't test on one OS instead of another.

> The issue was caused by the fact that X includes are placed in the /usr/X11R6/include directory on OpenBSD. This catalog is mentioned in PC_IBUS_DEFINITIONS variable but isn't propagated to the caller of find_package(IBus).
> [...]
> At the present time, the /usr/X11R6/include gets to the include_directories() from another place, so the patch isn't required at all. But, still, the PC_IBUS_DEFINITIONS should be respected, IMHO. What do you think?

While that is not wrong, the approach we normally take with CMake is that pkg-config's presence is optional, and we don't depend on its output to be able to build a module. In practice, this means one should look for IBus and X11 separately, as well as add their compiler flags/link against them independently. If kimtoy already does that, I just wouldn't make any change.

> Please note that FreeBSD and OpenBSD and quiet different. So you can't test on one OS instead of another.

You don't need to preach to the choir :-) I'm well aware of the differences between them, my point is that in many cases packaging problems in one also impact the other (missing includes because people assume all software is in `/usr`, reliance on non-POSIX features without checking for their availability etc).


- Raphael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119025/#review62669
-----------------------------------------------------------


On July 19, 2014, 9:43 p.m., Vadim Zhukov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119025/
> -----------------------------------------------------------
> 
> (Updated July 19, 2014, 9:43 p.m.)
> 
> 
> Review request for kde-workspace, Plasma and Hui Ni.
> 
> 
> Repository: kimtoy
> 
> 
> Description
> -------
> 
> The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used. This happens due to the following reasons:
> 
> 1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt
> 2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time
> 
> This patch resolves both issues and makes ibus-panel compile on OpenBSD.
> 
> (I found no suitable review group and therefore used "plasma" instead, as it was in "plasma-addons" previously; please, feel free to correct me if I'm wrong and sorry for any possible inconvenience)
> 
> ((as there was no feedback for more than a week, I've added "kde-workspace" group to list of reviewers, too))
> 
> 
> Diffs
> -----
> 
>   ibus-panel/CMakeLists.txt 3a1ee49 
> 
> Diff: https://git.reviewboard.kde.org/r/119025/diff/
> 
> 
> Testing
> -------
> 
> OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same)
> 
> 
> Thanks,
> 
> Vadim Zhukov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140728/ed160ca0/attachment.htm>