<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/125163/">https://git.reviewboard.kde.org/r/125163/</a>
</td>
</tr>
</table>
<br />
<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'm in favor of the approach as this solves the problem that again and again [1] we have attempts of OSX-specific disabling of X11 in various frameworks and applications.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The big problem on OSX seems to be that one doesn't want suport for X11, as it's not the windowing system. The code we have in our frameworks which is X11 specific performs runtime checks, thus is dead code on OSX anyway. For (to me unknown) reasons X11 gets pulled in during build on OSX though, causing either useless or crashy code to be included. Thus there are attempts to patch the frameworks instead of just fixing the cmake build command.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I understand that it's a behavior change and thus dangerous. But that's the same for each framework which gets "fixed" in the way I describe above. In the end from what I understood from our OSX devs: they don't want X11 and it's from their perspective a bug that it gets picked up. To me it sounds like we should really break our rules to provide an overall better product. Thousand micro-edits in the frameworks are more dangerous than the one time explicit break.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For what is worth I suggest that a warning could be printed by CMake, that those modules are not searched for and what's the command to override it.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">[1] There is already a huge list of reviews which I blocked because they were just wrong, but there are probably way more frameworks which got "fixed" where I just didn't notice the review.</p></pre>
<br />
<p>- Martin Gräßlin</p>
<br />
<p>On September 12th, 2015, 11:19 p.m. CEST, Samuel Gaist 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 Extra Cmake Modules.</div>
<div>By Samuel Gaist.</div>
<p style="color: grey;"><i>Updated Sept. 12, 2015, 11:19 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
extra-cmake-modules
</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;">Disable X11,XCB etc. detection on OS X</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;">KCrash build on OS X 10.8</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>kde-modules/KDECMakeSettings.cmake <span style="color: grey">(0c997931abee8673ccecc66d122108c6f72bf9b1)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125163/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>