[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-mac
Subject: Re: [KDE/Mac] 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-mac@kde.org
List Information: https://mail.kde.org/mailman/listinfo/kde-mac
KDE/Mac Information: http://community.kde.org/Mac
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic