<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/123075/">https://git.reviewboard.kde.org/r/123075/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 20th, 2015, 7:07 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">as in other similar requests: -2 from my side</p></pre>
</blockquote>
<p>On March 20th, 2015, 7:09 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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To extend: I think the way is wrong. If it now builds on MacOS the required is wrong. It should be an optional find_package properly ifdefed.</p></pre>
</blockquote>
<p>On March 20th, 2015, 12:37 p.m. UTC, <b>Christoph Cullmann</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;">Actually, you don't want that it is optional as you really don't want that it ever is found on MacOS. If you install an XQuartz for legacy apps, it will be found, and you will have a completly mess as result ;=)</p></pre>
</blockquote>
<p>On March 20th, 2015, 12:59 p.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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Christoph, that argument is wrong. The same would be the case with Windows and any other platform which optionally offers X11 (this includes Linux!). CMake can handle the situation quite well to disable an unwanted build dependency. If a user installs XLib headers (which is not the same as just installing XQuartz) we should expect the user to disable the cmake build option.</p></pre>
</blockquote>
<p>On March 20th, 2015, 1:02 p.m. UTC, <b>Christoph Cullmann</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;">That means that nobody will get a senseful build on Mac, if he doesn't disable that optional dependency. Thats the opposite of a normal optional dependency, that leads to missing features if not found but not complete mess if found.
I tried to compile frameworks stuff on Mac, and IMHO, really, that makes it close to unusable, that you need to tweak each cmake call just to have something usable, if you have too much stuff installed. I never had to do that on Linux/Other Unices.</p></pre>
</blockquote>
<p>On March 20th, 2015, 1:05 p.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;">That means that nobody will get a senseful build on Mac</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What since when is XLib as a build dependency available by default on OS X?</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;">Actually, if you work with Frameworks there, you will install something over MacPorts or Homebrew, and yes, you will have XQuartz installed, to run some legacy apps and there is really no user understandable way to install XQuartz without its devel headers, the .dmg will just install everything, I was suprised myself that I have X devel headers just by installing an X-Server. My first workaround was just to deinstall XQuartz, later I started to tweak CMake options or do exactly the same fixes as above. And yes, I think, per default, without any magic options, frameworks should just build to a usable state. And I see 0 reason to ever have even an optional "with x11" build of an frameworks application. But I might be wrong. Therefore I don't vote here or say ship it, just wanted to state my concerns ;=)</p></pre>
<br />
<p>- Christoph</p>
<br />
<p>On March 19th, 2015, 10:59 p.m. UTC, Harald Fernengel 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 Frameworks and Michael Palimaka.</div>
<div>By Harald Fernengel.</div>
<p style="color: grey;"><i>Updated March 19, 2015, 10:59 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdesu
</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;">do not require X11 on Mac OS X</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">(9623483d6f11f9cdb7d7dc19decfd7cf5e86d079)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123075/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>