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
_______________________________________________
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
