Re: [boost] Boost.Process article: Tutorial and request for comments

Subject: Re: [boost] Boost.Process article: Tutorial and request for comments
From: Boris Schaeling (boris_at_[hidden])
Date: 2009-04-25 08:52:24

On Tue, 21 Apr 2009 20:15:11 +0200, Phil Endecott  
<spam_from_boost_dev_at_[hidden]> wrote:

> [...]First a general comment about how I would prefer that portable  
> system API wrappers are implemented.  No doubt other would have  
> different views.  Ideally, I'd like to see separate thin wrappers for  
> Windows and POSIX that just "C++-ify" those APIs.  Then on top of that  
> implement a portability layer.  My reason is this: I rarely if ever care  
> about Windows and I already know how the POSIX APIs work (i.e. what the  
> functions are called and what the semantics are).  So please don't hide  
> the bottom level thin wrappers that you must have as an implementation  
> detail.

I see. It reminds me a bit of the free-standing functions in  
Boost.Filesystem which I think simply call POSIX and Windows API  
functions? I'm not sure though if I can work on such a layer - I hardly  
have time to work on the upper layer. :-/

> Re the environment: I'm not enthusiastic that you have copied the  
> environment into your own data structure.  Why can't your environment  
> type just wrap getenv() and setenv()?   i.e.

Before I answer your questions I should note that I did not design  
Boost.Process. I went through the entire code last year to make sure that  
what we have works (fixing countless bugs and testing the library with  
numerous compilers and platforms). I try to answer your questions as far  
as I understand the rationale of the design.

> class environment {
>    struct env_var_proxy {
>      const char* varname;
>      env_var_proxy(const char* varname_): varname(varname_) {}
>      operator const char*() const { return getenv(varname); }
>      void operator=(const char* newval) { setenv(varname,newval,1); }
>    }

The type environment is also used to setup a new environment for a child  
process, eg.:

context ctx;
ctx.environment.insert(environment::value_type("PATH", "/newpath"));
launch(..., ctx);

> [...]This example code:
>
>    std::string exec = find_executable_in_path("notepad.exe");
>    std::vector<std::string> args = boost::assign::list_of("notepad.exe");
>    context ctx;
>    ctx.environment = self::get_environment();
>    launch(exec, args, ctx);
>
> Is actually more verbose than directly calling the POSIX functions:
>
>    int child_pid = fork();
>    if (child_pid == -1) throw "fork failed";
>    if (child_pid == 0) {
>      execlp("notepad.exe","notepad.exe",NULL);
>      _exit(1);
>    }
>
> Why don't you boil it down to:
>
>    launch_process("notepad.exe");

Currently the environment variables of the current process are not  
automatically inherited by a child process. I agree that it should be  
easier to launch a new process. But then environment variables should  
probably be inherited by default?

> You write:
>
>> it is very important to refer to the executable with an absolute path -  
>> on all platforms including POSIX systems
>
> Why do you say that?

It's a requirement of Boost.Process. It doesn't need to be an absolute  
path but can also be a relative path of course. What's not guaranteed  
though is that an executable is automatically found when only a filename  
is given. There is a function which searches for an executable and returns  
an absolute path. Would you like to see Boost.Process search for an  
executable automatically?

> It's not clear to me if your terminate() calls wait() or not.  Beware of  
> processes that ignore SIGCHLD.

Currently terminate() only calls kill().

>> 3) Currently Boost.Process is really only useful for creating child
>> processes. It's not possible for example to detect and iterate over  
>> other
>> processes on a system. I guess Boost.Process should be enhanced here.  
>> Then
>> one day the well-known utility 'ps' on POSIX systems could be  
>> implemented
>> in Boost C++. Any comments?
>
> What is the use case for this?

I asked myself the other way round: Why should a library for process  
management only support child processes? But I'd agree that accessing  
other processes has a lower priority.

> [...]
>> 8) There is a method wait() which makes it possible to wait for another
>> process to exit. Currently wait() is provided by the class child though.
>> Shouldn't it really be defined by the class process (so you can wait not
>> only for child processes to exit)?
>
> Are you certain that wait() can be used to monitor non-child processes?   
> I don't believe that it can.

Hm, on POSIX systems it doesn't work but on Windows it works. Another  
problem to think about.

Boris