KProcess overhaul [PATCH]

[Ralf Habacker]

Ralf Habacker schrieb:
> Thiago Macieira schrieb:
>> Ralf Habacker wrote:
>>  
>>> 1. As Oswald initial stated rename KProcess KProcIO and
>>> KProcessController to relating K3... and move to kde3support and
>>> introduce new implementation as KProcess. This requires to port all
>>> KProcess references in kdecore to the new style (much work) or to make
>>> kde3support a dependency of kdecore (seems not be much work) and rename
>>> all references into other packages to K3Process (seems solvable).
>>>     
>>
>> kdecore cannot depend on kde3support. The K3Process class, if we make 
>> one, has to be in kde3support.
>>
>> So your option here is: port every use of KProcess in kdecore, kdeui 
>> and kio to the new class.
>>
>>   
> Below there all required changes listed.
>
> kdecore/ktimezones.cpp - unix only - default blocking call without any 
> redirection, easy to fix    [1]
> kdecore/kpty.cpp - unix only - redirects fd's, does QProcess support 
> all required needs ? [3]
> kdecore/kconfigbackend.cpp - all platforms - default blocking call 
> without any redirection, easy to fix [1]
>
> kdeui/kbugreport.cpp - all platforms - starts non blocking call and 
> perform action on process finished  [1]
> kdeui/kedittoolbar.cpp - all platforms  - starts non blocking call and 
> perform action on process finished  depending on exit state [2]
> kdeui/kspell.cpp - all platforms  - starts non blocking call and 
> perform action on process finished [1]
>
> kio/kfile/kdiskfreesp.cpp - all platforms - starts non blocking call, 
> catch stderr and perform action on process finished [2]
> kio/kfile/kfilesharedlg.cpp - unix only - starts non blocking call and 
> perform action on process finished [2]
> kio/kfile/kpropertiesdialog.cpp - all platforms - needs cleanup (old 
> KProcess method  is used in KRun context) [4]
> kio/kio/defaultprogress.cpp - all platforms - start non blocking call [1]
> kio/kio/kfileshare.cpp - unix only - starts non blocking call, catch 
> stdout and perform action on process finished [2]
> kio/kio/kmimetype.cpp - all platforms - start non blocking call [1]
> kio/kio/kmimetypechooser.cpp - all platforms - needs cleanup (old 
> KProcess function is used in KRun context) [4]
> kio/kio/krun.cpp - all platforms - starts non blocking call and 
> perform action on process finished  depending on exit state [2]
> kio/kio/slave.cpp - all platforms - start non blocking call [1]
> kio/kioexec/main.cpp - all platforms - start blocking call [1]
> kio/misc/kpac/discovery.cpp - unix only - starts non blocking call, 
> catch stdout and perform action on process finished [2]
> kio/tests/kdcopcheck.cpp - all platforms - starts non blocking call, 
> catch stdout and perform action on process finished [2]
>
> Inidicators:
> [1] easy to fix
> [2] more work to fix
> [3] needs unix developer
> [4] need design change
>
While trying to port some of the above mentioned locations to QProcess I 
encountered a Qt issue using the QProcess pid() method, which is 
implemented different on win32 systems

#if defined(Q_OS_WIN32)
typedef struct _PROCESS_INFORMATION* Q_PID;
#else
typedef qint64 Q_PID;
#endif
...
Q_PID pid() const;


This requires to wrap every reference to pid() depending on the platform 
which is very annoying.

#ifdef Q_WS_WIN
if (process->pid())
    pid = process->pid()->dwProcessId;
#else
    pid = process->pid();
#endif

I think this should be changed into a platfrom independent method 
returning the process id and an additional, may be platform related,  
method  to return additional information pInfo() or similar.

A second issue:  I tried to verify the related changes with kdelibs test 
applications (kio/kruntest) and found that some test applications (at 
least i found it in kio/tests for example ksyscocatest, kmimetypetest) 
are refering to applications, desktop files not installed by kdelibs 
(like kate, Office/kword.desktop), which means these testcases are not 
usable.

from kio/kruntest.cpp
// ------ this test works only if your terminal application is set to 
"x-term" ------

Ralf