<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/126161/">https://git.reviewboard.kde.org/r/126161/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/126161/diff/1/?file=418132#file418132line21" style="color: black; font-weight: bold; text-decoration: underline;">src/kdeinit/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">21</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">endif</span> <span class="p">()</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">15</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="c"># There doesn't appear to be any reason for the code below, but I'm leaving it in for reference for now</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">Just remove. For reference there's git history.</p></pre>
</div>
</div>
<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;">+1 all in all it looks good to me.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's mostly only touching OS X code anyway though, and I am quite clueless there, so more eyes will help.</p></pre>
<p>- Aleix Pol Gonzalez</p>
<br />
<p>On November 25th, 2015, 12:03 a.m. CET, René J.V. Bertin 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 Software on Mac OS X and KDE Frameworks.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Nov. 25, 2015, 12:03 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kinit
</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;">This patch addresses several issues with the OS X adaptations:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">-1 replaces the obsolete Q_OS_MAC with Q_OS_OSX
-2 builds the relevant applications <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">nongui</code> instead of as app bundles
-3 turns klauncher into an "agent" by setting <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">LSUIElement</code> to true programmatically
-4 ports a patch that has been in MacPorts' <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">port:kdelibs4</code> since October 14th 2009, which prevents a kdeinit crash that is caused by calling exec after <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">fork()</code> in an application that has used non-POSIX APIs and/or calling those APIs in the exec'ed application. This patch (originally by MacPorts developers Jeremy Lainé and Jeremy Lavergne) rearranges call order and uses a proxy application to do the actual exec.</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;">On OS X 10.9.5 with Qt 5.5.1 and KF5rameworks 5.16.0 . With this patch, starting <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">kded5</code> will launch kdeinit5 and klauncher5 as expected, but <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">kdeinit5 --kded</code> does not yet launch <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">kded5</code>. This is probably acceptable for typical KF5 use on OS X (kded5 can be launched as a login item or as a LaunchAgent) but I will have another look at why the kded isn't started.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I am not yet able to perform further testing; practice will for instance have to show whether point 2 above needs revision (apps that need to be installed as app bundles).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Similarly it will have to be seen whether point 3 above has any drawbacks. Applications running as agents do not show up in the Dock or App Switcher. Thus, klauncher will not be able to "turn itself into" an application that does have a full GUI presence with my current modification. I don't know if that's supposed to be possible though.
NB: I have been building the KDE4 klauncher in a way that makes it impossible to construct a GUI at all, so I'm not expecting issues in KF5 as long as klauncher's role hasn't evolved too much.</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>src/kdeinit/CMakeLists.txt <span style="color: grey">(f94db71)</span></li>
<li>src/kdeinit/kdeinit5_proxy.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/kdeinit/kinit.cpp <span style="color: grey">(a18008a)</span></li>
<li>src/klauncher/CMakeLists.txt <span style="color: grey">(746edfa)</span></li>
<li>src/klauncher/klauncher.h <span style="color: grey">(e155f72)</span></li>
<li>src/klauncher/klauncher.cpp <span style="color: grey">(8b3d343)</span></li>
<li>src/klauncher/klauncher_main.cpp <span style="color: grey">(f69aaa5)</span></li>
<li>src/start_kdeinit/CMakeLists.txt <span style="color: grey">(46d6cb3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126161/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>