<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119025/">https://git.reviewboard.kde.org/r/119025/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 19th, 2014, 12:17 a.m. EEST, <b>Raphael Kubo da Costa</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(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)</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The ibus-panel can't build on OpenBSD because some required definitions obtained from pkgconfig file are not used.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time</li>
</ol>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On July 19th, 2014, 9:41 p.m. EEST, <b>Vadim Zhukov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Raphael, thank you for your input!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please note that FreeBSD and OpenBSD and quiet different. So you can't test on one OS instead of another.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
[...]<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
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?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please note that FreeBSD and OpenBSD and quiet different. So you can't test on one OS instead of another.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">/usr</code>, reliance on non-POSIX features without checking for their availability etc).</p></pre>
<br />
<p>- Raphael</p>
<br />
<p>On July 19th, 2014, 9:43 p.m. EEST, Vadim Zhukov wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for kde-workspace, Plasma and Hui Ni.</div>
<div>By Vadim Zhukov.</div>
<p style="color: grey;"><i>Updated July 19, 2014, 9:43 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kimtoy
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1. IBUS_DEFINITIONS is not used in ibus-panel/CMakeLists.txt<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
2. IBUS_DEFINITIONS is not saved in cache, and thus isn't available at compile time</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch resolves both issues and makes ibus-panel compile on OpenBSD.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(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)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">((as there was no feedback for more than a week, I've added "kde-workspace" group to list of reviewers, too))</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">OpenBSD/i386-CURRENT, KDE 4.13 (it doesn't have kimtoy package, of course, but the code is same)</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>ibus-panel/CMakeLists.txt <span style="color: grey">(3a1ee49)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119025/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>