<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/126013/">https://git.reviewboard.kde.org/r/126013/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 10th, 2015, 1:26 a.m. UTC, <b>David Edmundson</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;">Ship It!</pre>
</blockquote>
<p>On November 10th, 2015, 1:35 a.m. UTC, <b>Johan Ouwerkerk</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;">Forgot to mention that the patch can be pulled from the fix-cmake-xkb-dep branch at: git@github.com:cmacq2/plasma-desktop.git</p></pre>
</blockquote>
<p>On November 14th, 2015, 3:41 p.m. UTC, <b>Johan Ouwerkerk</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;">Please note: I do not have commit access (just a KDE identity account) so please pull these changes because I cannot commit them!</p></pre>
</blockquote>
<p>On November 16th, 2015, 10:04 a.m. UTC, <b>Sebastian Kügler</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;">I see a problem here. The features offered by kaccess are important and necessary for some users. Are we just disabling it and then be done? I think we need to put this stuff on a porting list and find a better solution than to just not build everything that #includes xcbsomething.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Johan, I think you're getting pretty close to us asking you to get a KDE identity account with commit privileges, just so you know. https://identity.kde.org/index.php?r=developerApplication is currently broken, but I'd say as soon as it's fixed, let's start that process.</p></pre>
</blockquote>
<p>On November 16th, 2015, 10:19 a.m. UTC, <b>Martin Gräßlin</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;"><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;">Are we just disabling it and then be done?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think it's the distros task to built it properly.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">And yes, we might need to rethink which platform specific packages should be required in build. Just saying "hey let's make all X11 deps mandatory" is also not the way to go anymore with Wayland.</p></pre>
</blockquote>
<p>On November 16th, 2015, 4:16 p.m. UTC, <b>Johan Ouwerkerk</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;"><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;">Are we just disabling it and then be done?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That is not what this change is about. The change rectifies an inconsistency between the top level cmake configure stuff which checks the build environment for dependencies, and the C++ code which uses these unconditionally (e.g. kaccess).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The plasma-desktop build process deals with two classes of X11 deps: mandatory and optional. As it happens the XKB dep is classified optional in the top level cmake files, meaning configure will accept a build environment without XKB. This change makes sure that the 'optional' part is, indeed, consistently optional and does not result in build failure later (during make).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">People who need kaccess must necessarily install the XKB headers/development files anyway. If the XKB dep is found then kacess is still included in the build and nothing changes (effectively).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(The alternative is to classify XKB as mandatory.)</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I see, thanks for the clarification.</p></pre>
<br />
<p>- Sebastian</p>
<br />
<p>On November 10th, 2015, 12:52 a.m. UTC, Johan Ouwerkerk 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 Plasma.</div>
<div>By Johan Ouwerkerk.</div>
<p style="color: grey;"><i>Updated Nov. 10, 2015, 12:52 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</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;">The XCB-XKB dependency of plasma-desktop is optional, however kacess depends directly and unconditionally on the xcb/xkb.h header file.
This causes the build of plasma-desktop to fail at compile stage if the xkb header is not present.
As per the convention in the kcm modules shipped by plasma-desktop, inclusion of the kacess subdirectory is made conditional on XKB being present and properly detected during the cmake configure stage.</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;">Built without xkb header, using kdesrc-build</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>CMakeLists.txt <span style="color: grey">(193238a8dcbb2c6e2bda01c390c57ff2ecfebeb0)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126013/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>