[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 126161: OS X housekeeping
From: "David Faure" <faure () kde ! org>
Date: 2016-01-02 11:39:41
Message-ID: 20160102113941.30660.88757 () mimi ! kde ! org
[Download RAW message or body]
--===============6492288206395034334==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Dec. 25, 2015, 2:42 p.m., René J.V. Bertin wrote:
> > src/kdeinit/kinit_mac.mm, lines 662-666
> > <https://git.reviewboard.kde.org/r/126161/diff/4/?file=418469#file418469line662>
> >
> > I'd love to add `[NSApp activateIgnoringOtherApps:YES]` and/or `[NSApp \
> > unhide]` here, preceded by `[NSApplication sharedApplication]` so that \
> > the spawned `executable` appears in front and not behind the windows of \
> > the "parent" application.
> > 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 `fork()` \
> > and an `exec()`.
> > 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.
> > 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 (`/usr/bin/open`).
> > NB: this may well be why `[NSProcessInfo setProcessName:]` doesn't \
> > appear to have the intended effect.
> > So, David, what's worse here — introducing an additional layer or \
> > putting up with applications that play hide and seek?
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.
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...)
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126161/#review90098
-----------------------------------------------------------
On Nov. 26, 2015, 4:20 p.m., René J.V. Bertin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126161/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 4:20 p.m.)
>
>
> Review request for KDE Software on Mac OS X and KDE Frameworks.
>
>
> Repository: kinit
>
>
> Description
> -------
>
> This patch addresses several issues with the OS X adaptations:
>
> -1 replaces the obsolete Q_OS_MAC with Q_OS_OSX
> -2 builds the relevant applications `nongui` instead of as app bundles
> -3 turns klauncher into an "agent" by setting `LSUIElement` to true \
> programmatically
> -4 ports a patch that has been in MacPorts' `port:kdelibs4` since October \
> 14th 2009, which prevents a kdeinit crash that is caused by calling exec \
> after `fork()` 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.
>
> Diffs
> -----
>
> src/kdeinit/CMakeLists.txt f94db71
> src/kdeinit/kdeinit5_proxy.mm PRE-CREATION
> src/kdeinit/kinit.cpp a18008a
> src/kdeinit/kinit_mac.mm PRE-CREATION
> src/klauncher/CMakeLists.txt 746edfa
> src/klauncher/klauncher.h e155f72
> src/klauncher/klauncher.cpp 8b3d343
> src/klauncher/klauncher_main.cpp f69aaa5
> src/start_kdeinit/CMakeLists.txt 46d6cb3
> src/wrapper.cpp 95b7ec2
>
> Diff: https://git.reviewboard.kde.org/r/126161/diff/
>
>
> Testing
> -------
>
> On OS X 10.9.5 with Qt 5.5.1 and KF5rameworks 5.16.0 . With this patch, \
> starting `kded5` will launch kdeinit5 and klauncher5 as expected, but \
> `kdeinit5 --kded` does not yet launch `kded5`. 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.
> 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).
> 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.
>
> Thanks,
>
> René J.V. Bertin
>
>
--===============6492288206395034334==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<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, \
2:42 p.m. UTC, <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>
</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;">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> <br />
<p>- David</p>
<br />
<p>On November 26th, 2015, 4:20 p.m. UTC, 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, 4: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>
--===============6492288206395034334==--
[Attachment #3 (text/plain)]
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic