KProcess overhaul [PATCH]

[Ralf Habacker]

Oswald Buddenhagen schrieb:

It seems I haven't understand you complete. Please have patience with me 
:-)
> On Mon, Apr 17, 2006 at 07:03:43PM +0200, Ralf Habacker wrote:
>   
>> Oswald Buddenhagen schrieb:
>>     
>>> On Mon, Apr 17, 2006 at 01:57:25PM +0200, Ralf Habacker wrote:
>>>       
>>>> 8-10 not implemented (on windows not required)
>>>>
>>>>         
>>> this is simply not true. only pty support is not needed; redirections
>>> in general *are* needed, now even more than before, as we want
>>> portability, so we can't simply use the unix shell syntax anymore.
>>>  
>>>       
>> Not sure, what you mean exactly here and where this support is used or
>> required in the kde sources. Can you give an example ?
>>
>>     
> i can't come up with an example where i can assure you that it is
> meaningful for windows, but if you grep for setUseShell you will find
> many redirections for unix. :)
>
>   
>>>> new use cases 11. run a process using a command line shell
>>>>
>>>>    
>>>>         
>>> it should be noted that this bears *considerable* portability
>>> problems.
>>>  
>>>       
>> yes, which will still exists in some area, where the unix command have
>> other names as on windows.
>>
>>     
> yes. after all, i don't think we'll write too many cross-platform shell
> commands ...
>
>   
>>> the quoting rules are *totally* different 
>>>       
>> How are they different ?
>>
>>     
> really, read the man pages.
>
>   
>>> the only thing the shell should be used for are user-supplied commands.
>>> for internal stuff, either the code should be rewritten not to use a
>>> shell (as stated before, the majority of the cases is a poor man's
>>> workaround for the lack of redirection support in kprocess), or, if it
>>> is not hard-coded (like in .desktop files), it should be written in unix
>>> shell syntax and interpreted internally - i intend to put a *simple*
>>> shell execution semantics interpreter into KShell.
>>>  
>>>       
>> Agreed, but KProcess need an interface for executing a shell. It is okay 
>> as it is ?
>>
>>     
> i don't have objections.
> well, in fact i hoped to go more into the direction of set<SomeMode>
> functions and one async exec() with very few options, but unfortunately
> qt went exactly the opposite direction, so for consistency we should do
> the same. oh, well.
>
>   
Please note that my code is currently more a proposal based on recent 
QProcess. It has the advantage that is is QT conform, but requires some 
rewriting of KDE code. I have played with an alternative approach using 
recent KProcess api using QProcess internal, which would work too after 
some KProcess api changes I would suggest.

In another mail I asked for alternative proposals, but I haven't got any 
response until now :-(.

 From my windows perspective I would like to use as much as possible 
given code (which means QProcess api) and like to make it as small as it 
could be (as you stated also above ), but from the unix perspective 
there may be (an you already stated this) things go the other way.


>>> 1) on unix, -c is required (where it is /c on windoze). remember that
>>> everything in windows that is not fundametally new (is there something
>>> like that at all?) is based on unix concepts. :-P
>>> btw, you botched the if-else cascade. 
>>>       
>> which comes from the old KProcess code. If other shells beside /bin/sh 
>> aren't required, this code could be reduced very much.
>>
>>     
> re-read the code and the message. c is mean. ;)
>   
sorry, you'r re right I have overseen this

(line 287)       arglist[1] = (char *) "-c";


>>> but there's no reason for
>>> shellOptions anyway, as it is required to be consistent among shells.
>>>  
>>>       
>> QProcess::start()'s or QProcess:execute()'s first parameter must be an 
>> executable without any parameters, otherwise it couldn't be located in 
>> the path variable.
>> This require to save parameters elsewhere in this case shellOptions. Is 
>> this okay for you ?
>>
>>     
> on unix it is always shell -c command, on windows shell /c command - so
> why do you want to save it in a variable?
>   
to avoid #ifdefs in the related start functions otherwise it would be

void KProcess::startShell( const QString & program, const QStringList & 
arguments )
{
#ifdef Q_OS_WIN
    return start(KProcessPrivate::shell, QStringList() << "/c" << 
program << arguments);
#else
    return start(KProcessPrivate::shell, QStringList() << "-c" << 
program << arguments);
#endif

and the same for  KProcess::executeShell()

Now is is

setupShell()
{
#ifdef Q_OS_WIN
 set shell variable
#else
   set unix shell for unix derivate
#endif
}

void KProcess::startShell( const QString & program, const QStringList & 
arguments )
{
    return start(KProcessPrivate::shell, QStringList() << 
KProcessPrivate::shellOptions << program << arguments);
}

I like more to setup relevants options on one place and to use it 
without conditions later , but this could also be implemented in another 
way. :-)

>   
>>> 2) you botched the command composition. it is 
>>> "shell -c <all-arguments-concatenated>", not "shell -c <arg> <arg> ...".
>>> on windows that does not matter, as everything is more or less simply
>>> concatenated into one string anyway, but on unix it does.
>>>  
>>>       
>> I don't know how to implement this on unix.
>>
>>     
> you could simly read the old code.
> honestly, you are scaring me by playing with this stuff. get yourself *a
> bit* into the topic first, for *all* relevant platforms.
>
>   
Hmmh, KProcess::start() and KProcess::execute() a QStringList as 
arguments, so it would be easy to add this support in these methods.

void KProcess::startShell( const QString & program, const QStringList & 
arguments )
{
    args = quoteArguments(arguments);
    return start(KProcessPrivate::shell, QStringList() << "/c" << 
program << args );
}


Additional on windows quoting is already implemented in 
QProcess::execute() and QProcess::start() (see see 
http://doc.trolltech.com/4.1/qprocess.html)

Do you expect a full working solutions for all platforms in this state ? 
The implementation i send works on windows, but needs some fixes on unix.
In one of the last mails I asked who will implement the new KProcess on unix

>>   2. Who will implement the unix and mac related parts ?
>me for unix, but not before tt makes up and reveals its plans.

So I thought, we are in a state were we are discussing possible 
solutions. Sorry if I misunderstood you.

The executeShell() and startShell() methods are such a proposal, maybe 
it would be better to add this to KShell, which could be the Shell 
Starter class as KShellProcess is recent.

Any comments ?

BTW: If you like to discuss some details private, we can do this also in 
german.  :-)

Ralf



Ralf