[KDE/Mac] Review Request 126161: OS X housekeeping

Wed Nov 25 16:10:29 UTC 2015

> On Nov. 25, 2015, 8:39 a.m., David Faure wrote:
> > src/kdeinit/kinit.cpp, line 1621
> > <https://git.reviewboard.kde.org/r/126161/diff/1/?file=418134#file418134line1621>
> >
> >     Yes if you have to run a separate process which will then dlopen the kdeinit module, the whole purpose of kdeinit is moot. You might as well simplify your life by getting rid of most of the  Q_OS_OSX code in this file and simply acting as if the kdeinit module doesn't exist, on Q_OS_OSX.
> >     
> >     The existing code to fallback to executing the binary directly will do exactly the same as your generic proxy, possibly even faster (no dlopen).
> 
> René J.V. Bertin wrote:
>     You're undoubtedly right, I considered doing this myself. It would amount to making the `--no-fork` option the default, no?
>     
>     I don't feel comfortable/ready for that right now, without having had a working equivalent to (the patched) kdeinit4 out there in the wild for observation. Can we leave your suggestion for a 2nd round of housekeeping and cleanup?
> 
> David Faure wrote:
>     Not at all, --no-fork is about kdeinit's own startup, not about the way it starts other applications.
>     
>     In general I don't see much purpose in pushing complex code if we confirm it to serve no purpose at all.
>     But I looked a bit further into it, and in fact kinit's launch() does fork+dlopen or fork+exec, depending on whether the kdeinit module was found.
>     
>     So if fork + exec is a problem on OSX, then indeed that needs to be addressed
>     But if your patch does fork + exec_helper + dlopen, then that is unnecessarily complex.
>     
>     The simple version of what I have in mind is http://www.davidfaure.fr/2015/kinit.osx.cpp.diff i.e. never using QLibrary on OSX. Would that work?

Well, all I can say is that with that patch nothing crashes, `kdeinit5 --kded` still launches kded5 but as a result I now no longer see something like (in `ps` output)

```
12980     1     400c   0  33  0  2510184   6716 -      Ss                  0 ??         0:00.03 /opt/local/bin/kdeinit5 --suicide --nofork
12981 12980     4004   0  48  0  2641864  14856 -      S                   0 ??         0:00.12 /opt/local/libexec/kde5/kf5/klauncher --fd=9 libkdeinit5_klauncher
```

but

```
13211     1     400c   0  33  0  2527592   6724 -      Ss                  0 ??         0:00.02 /opt/local/bin/kdeinit5 --suicide --nofork
13225  1076     4006   0  31  0  2432948    576 -      S+                  0 ttys004    0:00.00 fgrep kdeinit5
```

And `kwrapper5 /path/to/kwrite` now longer launches an kwrite process that can be killed via `killall kwrite` or equivalent. All that is probably to be expected, but that latter observation does feel like a regression of sorts to me.

BTW, I noticed that kded5 will have to be turned into an agent too, it has no business showing up in the Dock.

- René J.V.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126161/#review88784
-----------------------------------------------------------

On Nov. 25, 2015, 12:03 a.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. 25, 2015, 12:03 a.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.cpp PRE-CREATION 
>   src/kdeinit/kinit.cpp a18008a 
>   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 
> 
> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20151125/6bdb019b/attachment.html>