<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/103333/">http://git.reviewboard.kde.org/r/103333/</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 24th, 2011, 8:28 a.m., <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;">This seems like a dangerous change to me.
With it, you could run multiple instance of "kmail --nofork", and they would step on each other's toes. The whole point of kuniqueapplication (in normal apps, not in konsole), is to prevent multiple instances of the application from running.
A developer using --nofork in valgrind or gdb shouldn't run the risk that an app that wasn't mean to run twice, ends up running twice.
konsole is doing rather unusual stuff with kuniqueapplication so I can't comment on what would be the proper fix for konsole, but I'm pretty sure it should only affect konsole, not other kuniqueapplications.</pre>
</blockquote>
<p>On December 24th, 2011, 2:45 p.m., <b>Thomas Lübking</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;">Seconded - "Do NOT ship it!"
$ <random kuniqueapplication> --help-kde
...
--nofork Do not run in the background.
There's no indication this means to spawn a new process and it's (despite the ambigious term) obviously not meant like this (but meaning "do not fork to background")
-> Solution:
Konsole should add an extra parameter or map it's --nofork to NonUniqueInstance in the ::start() call.
Turning "--nofork" to "--new-process" for general kuniqueapplication is rather a no-go, because applications like eg. k3b or amarok could possibly really rely on it (ioctl/database messup)
I do see your point in either having an extra konsole process as well as in redirecting IO, but please fix it in konsole *only*</pre>
</blockquote>
<p>On December 24th, 2011, 3:54 p.m., <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;">Don't get confused by the variable name. AFAICS "forceNewProcess" only means that it will register to DBus as kfoo-PID instead of kfoo.
Nofork doesn't fork, the patch doesn't change that, AFAICS.
</pre>
</blockquote>
<p>On December 24th, 2011, 7:04 p.m., <b>Thomas Lübking</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;">Looking at the patch, setting that flag would get him into the block he desires so whatever the effect of the patch is, setting the flag should gain the same, yesno?!</pre>
</blockquote>
<p>On December 24th, 2011, 7:09 p.m., <b>Askar Safin</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;">Yes, you are right. So, please add to konsole/src/main.cpp to function forceNewProcess() check, wherever or not there is option "--nofork".</pre>
</blockquote>
<p>On December 24th, 2011, 7:13 p.m., <b>Askar Safin</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;">Thomas, if you apply the patch, konsole will start in new process if there is --nofork option independently of terminal status. But, as I said, my patch is bad idea, edit konsole/src/main.cpp</pre>
</blockquote>
<p>On December 24th, 2011, 7:46 p.m., <b>Thomas Lübking</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;">I didn't mean to question that result, just that that's not the purpose of --nofork in general. Konsole somehow abuses it ;-)
PS: i'm not the maintainer of konsole, so i cannot just add that check there across whoever /is/ the maintainer, sorry.</pre>
</blockquote>
<p>On December 25th, 2011, 9:08 a.m., <b>Askar Safin</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;">What can I do to apply change to konsole?
I cannot post it to reviewboard, because I cannot write patch (trivial solution 'return (isatty(1) && !args->isSet("new-tab")) || args->isSet("no-fork")' suddenly doesn't work, because option "no-fork" is in another container).
I cannot post it to bugs.kde.org, because there bugs stay opened very long time.
How to contact with konsole maintainers?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Use
!args->isSet("fork");
The option is named fork and default to true, --no-fork is the way to set it to false.
According to git log the last one from `konsole --author` who committed to the code was Kurt Hindenburg <kurt.hindenburg@gmail.com>. Just a month ago, so he should still be around :-)</pre>
<br />
<p>- David</p>
<br />
<p>On December 25th, 2011, 9:01 a.m., Askar Safin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs and Konsole.</div>
<div>By Askar Safin.</div>
<p style="color: grey;"><i>Updated Dec. 25, 2011, 9:01 a.m.</i></p>
<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;">See https://bugs.kde.org/show_bug.cgi?id=288200</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=288200">288200</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdeui/kernel/kuniqueapplication.cpp <span style="color: grey">(777fc35)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103333/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>