[Development] Qt5.15 deprecating & Qt6 removing QProcess::setupChildProcess

[Thiago Macieira]

On Tuesday, 18 February 2020 03:17:38 PST Edward Welbourne wrote:
> Thiago Macieira (17 February 2020 20:35) wrote:
> > Sorry, this just occurred to me. This is a request for 5.15 feature freeze
> > exception.
> > 
> > Re: https://bugreports.qt.io/browse/QTBUG-17331
> 
> Closed: 30-07-2013 03:38, >9 years ago.

Because I had no clean way of implementing it until a couple of months ago, 
when Linux 5.4 was released.

> > Re: 97645478de3ceffce11f58eab140c4c775e48be5
> > ("QProcess: use FFD_USE_FORK when the class is not QProcess itself")
> 
> Committed to 5.15, has not appeared in any release yet.
> Modifies (only) corelib/io/qprocess_unix.cpp, only internal to
> QProcessPrivate::startProcess(); no API change.

This commit shows the consequence of the existing API.

> > I don't think we can remove the ability to run user code in the child
> > process.  But can we remove the virtual? Can we just have a callback
> > like on Windows?
> 
> I take it that would involve an API change.
> I see QProcess has differences in API between MS and Unix.
> Reducing that inconsistency would be good.

Correct, an API change. We can add more functionality to QProcess to minimise 
the need to have user functions run, but that's not what I am talking about 
here. We can let the native API continue to exist. All I want is that the Unix 
one be by way of a setter of a callback, instead of by derivation.

> > Cons:
> > - breaks feature freeze, adds SIC for Qt 6
> 
> Please clarify: if we include some new API in 5.15 to support this, what
> will be the SiC change in Qt 6 ?

The idea is to deprecate setupChildProcess in 5.15 so we can remove it 6.0. 
The removal is the SIC part.

> > - of the 6 times I can find of QProcess being derived from in Qt & Qt
> > 
> >   Creator, 5 are to override setupChildProcess anyway and the last one
> >   is in tst_QProcess
> 
> Does anyone have sources from outside Qt project that are currently
> forced to derive from QProcess for the sake of this ?  If so, please try
> to work out how Thiago's change would affect you and give the rest of us
> a summary.

Here are a couple more, searching KDE sources:

$ grep -r 'public QProcess'
kcoreaddons/src/lib/io/kprocess.h:class KCOREADDONS_EXPORT KProcess : public 
QProcess
khtml/src/java/kjavaprocess.h:class KJavaProcess : public QProcess //QObject
kpty/src/kpty.cpp:class UtemptProcess : public QProcess
kwin/utils.h:class KWIN_EXPORT Process : public QProcess

The first two do not override setupChildProcess, the latter two do. 
UtemptProcess needs to set up stdin, stdout and stderr to a specific file 
descriptor, which comes from openpty(3), whereas the Utils::Process is just 
unblocking two signals that I imagine KWin blocks for its own purposes.

That adds to:

qtbase/src/corelib/doc/snippets/code/src_corelib_io_qprocess.cpp
qtbase/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
qtbase/tests/auto/corelib/tools/qsharedpointer/externaltests.cpp
qt-creator/src/libs/ssh/sshprocess.h
qt-creator/src/libs/utils/qtcprocess.h
qt-creator/src/libs/utils/synchronousprocess.cpp

The documentation example is trying to document why one would want to override 
the function and it's showing an example of chroot()[1]. The QSharedPointer 
code attempts to reset stdin to /dev/tty. SshProcess and 
TerminalControllingProcess are just doing setsid(). QtcProcess is doing a nice 
of 5[2].

The case of unblocking signals, closing all extraneous file descriptors, 
setsid()/setpgrp(), detaching from the controlling TTY, etc., we can add flags 
for and implement directly in QProcess without the user having to override 
anything. The other cases seem too specific to add flags for and require a 
callback.

[1] setting umask to 0 seems wrong. I'd set it to 022 or 077.

[2] the qWarning after it failed is a deadlock magnet => fixed in
https://codereview.qt-project.org/c/qt-creator/qt-creator/+/291111
-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel System Software Products