<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 25th, 2015, 3:42 p.m. CET, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/4/?file=418469#file418469line662" style="color: black; font-weight: bold; text-decoration: underline;">src/kdeinit/kinit_mac.mm</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">662</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// attempt to the correct application name</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">663</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QFileInfo</span> <span class="n">fi</span><span class="p">(</span><span class="n">QString</span><span class="o">::</span><span class="n">fromUtf8</span><span class="p">(</span><span class="n">executable</span><span class="p">));</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">664</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">[[</span><span class="bp">NSProcessInfo</span> <span class="n">processInfo</span><span class="p">]</span> <span class="nl">setProcessName</span><span class="p">:(</span><span class="bp">NSString</span><span class="o">*</span><span class="p">)</span><span class="n">fi</span><span class="p">.</span><span class="n">baseName</span><span class="p">().</span><span class="n">toCFString</span><span class="p">()];</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">665</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">qApp</span><span class="o">-></span><span class="n">setApplicationName</span><span class="p">(</span><span class="n">fi</span><span class="p">.</span><span class="n">baseName</span><span class="p">());</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">666</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">execvp</span><span class="p">(</span><span class="n">executable</span><span class="p">.</span><span class="n">constData</span><span class="p">(),</span> <span class="n">d</span><span class="p">.</span><span class="n">argv</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<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'd love to add <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">[NSApp activateIgnoringOtherApps:YES]</code> and/or <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">[NSApp unhide]</code> here, preceded by <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">[NSApplication sharedApplication]</code> so that the spawned <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">executable</code> appears in front and not behind the windows of the "parent" application.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think that's a very sensible thing to do, but sadly those calls are part of (or call into) the SDKs that are off-limits between a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">fork()</code> and an <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">exec()</code>.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This really makes me reconsider to use a wrapper, because it quickly grows old 1) having to wait for an expected application to appear, 2) realise it must have opened somewhere in the background and 3) go dig it up.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If only I could be sure that the spawned application is NOT supposed to inherit the environment and other context from kdeinit5. In that case I could rewrite the command to execute so that it uses LaunchServices ... via a proxy that's already available (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">/usr/bin/open</code>).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">NB: this may well be why <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">[NSProcessInfo setProcessName:]</code> doesn't appear to have the intended effect.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So, David, what's worse here — introducing an additional layer or putting up with applications that play hide and seek?</p></pre>
</blockquote>
<p>On January 2nd, 2016, 12:39 p.m. CET, <b>David Faure</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;">Starting apps via kdeinit is only an optimization. It's perfectly OK to start apps directly with QProcess instead. So yes, the spawned application is NOT supposed to inherit the environment from kdeinit5.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As previously stated, I strongly recommend to avoid the whole complication of "stuff I can't do between fork and exec" on OSX, but not using fork and exec at all (wrapper or not), and just starting the other process directly with QProcess. If that doesn't bring the new app to the front, maybe that's a QProcess bug or missing feature? If you think about this as a "pure Qt application developer", such a developer would expect QProcess to be able to start a GUI app and have it pop up to the front. Maybe you can write a small test with QProcess, to check if that works out of the box (maybe all your troubles actually come from the indirection via kdeinit...)</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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 will indeed have to have another look at what QProcess does exactly in qprocess_unix, but a priori it looks a variant on the fork+exec scheme. There is support for app bundles, but that works by obtaining the bundle executable and launching it as it if were a regular application.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The underlying issue is not with Qt, it's something specific to OS X. Maybe there's a reason for it: it could be a cheap way to avoid stealing focus from the "parent" process. The app bundle detection could be used to channel spawning the new process through LaunchServices so that it does open in the foreground, and that could even be extended to cover symlinks that point inside an app bundle (i.e. open <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">foo.app</code> instead of <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">/path/to/foo.app/Contents/MacOS/foo_bundle_exec</code>). Qt's design choices may make that impossible though: QProcess's implementatin (notably <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QProcessPrivate::startProcess()</code>) is quite complex and if it is supposed to support Posix-style communication between parent and child (including pipes) then LaunchServices is probably out of the question. I don't think there is any such assumption in kdeinit, correct?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have to come back a bit on the idea of using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">/usr/bin/open</code> : it tends to use Terminal.app to launch shell scripts and "certain" regular executables ("BSD utilities" in Mac speak). That's evidently not an option. I do have wrapper utilities lying around that I hacked together, I'll see if and how I can refactor relevant code from them into kinit_mac.mm .</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On November 26th, 2015, 5:20 p.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. 26, 2015, 5:20 p.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.mm <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/kdeinit/kinit.cpp <span style="color: grey">(a18008a)</span></li>
<li>src/kdeinit/kinit_mac.mm <span style="color: grey">(PRE-CREATION)</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>
<li>src/wrapper.cpp <span style="color: grey">(95b7ec2)</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>