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

Subject: Re: [boost] Boost.Process article: Tutorial and request for comments
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2009-04-25 13:09:05

Hi Boris,
----- Original Message ----- 
From: "Boris Schaeling" <boris_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Saturday, April 25, 2009 5:48 PM
Subject: Re: [boost] Boost.Process article: Tutorial and request for comments

> 
> On Tue, 21 Apr 2009 23:21:14 +0200, vicente.botet  
> <vicente.botet_at_[hidden]> wrote:
> 
>> [...]I will organize the specific classes on a specific platform  
>> namespace
>> common for the portable part
>>
>> namespace process {
>> namespace common {
>>     class process;
>>     class child;
>> }
>>
>> posix for the posix based OS
>>
>> #ifdef BOOST_PLATFORM_POSIX // or something like
>> namespace process {
>> namespace posix {
>>     class process : public common::process{...};
>>     class child: public common::child{...};
>> }
>> }
>> #endif
<snip>
>> and have typedef  or a using directive at the namespace process level  
>> for each one of the available classes as
>>
>> namespace process {
>> #if defined(__POSIX__) // or something like
>> typedef posix::process process;
>> // or using posix::process;
>> #elsif defined() // other patforms
>> ...
>> #else
>> typedef common::process process;
>> or // using common::process;
>> #endif
>> }
>>
>> When the user use a non portable feature on a portable programm it will  
>> need to check conditionally for the non portable feature as
>>
>> #ifdef BOOST_PROCESS_HAS_SPECIFC_FEATURE_1
>> // ...
>> #else
>> // ...
>> #endif
> 
> I see. Thanks for the suggestion! I don't know if this design has been  
> considered by the original developers or if they had good reasons to  
> reject it and go with the current design. The problems with Boost.Process  
> really seem to be unique as too many features are platform-dependent. I  
> don't know any other Boost library which had to face similar problems.

I think that you need to write some examples using non portables features that needs to run on several platforms. When you try this, you see that it is not really interesting to work with classes like process_windows and process_posix. Just a class named process is needed. The design platform specific namespaces is secondary. You can get the same using conditional compilation. What it is more important to me is that the library must provide a class process wich provided all the feautures available on the current platform. Of course the user can do himself this using declaration depending on the platform but I would prefer the library provides it. 

Imagine this use case. One application use the portable class 'children' because all the uses are portable. The user needs to add a new feature that force to use a non portable feature. With the current interface the user needs to change the use of 'children' for the specific child by 'children_posix' or 'chindren_windows' depending on the platform. With my using import directive, no change will be needed.
 
>> [...]If the command fails it can be recovered
>>
>> Commands could be composed using pipe "|",
>>     shell::cat("/tmp.file.txt") | shell::wc();
>>
>> redirection ">"
>>
>>     shell::cat("/tmp.file.txt") | shell::wc() > str;
>>
>> tee
>>
>>     shell::cat("/tmp.file.txt") | shell::grep () | shell::tee("file");
>>
>> conditional concatenation "&&",
>>
>>     shell::cd("/tmp") && shell::ls();
>>
>> which executes ls only if cd /tmp succeeds.
> 
> Another nice idea! However I'm pretty sure that I unfortunately won't be  
> able to work on this in the near future given the amount of more  
> fundamental problems. :-/

If you don't mind and as these seems orthogonal I will try to see how all these shell related features can be implemented on top of the Process library.
 
>> [...]Last some minor questions:
>> What about renaming handle by native_handle?
> 
> I'm open to it. You want native_handle to be a first-class citizen? Then  
> it should move out of the detail namespace, too?

Yes, for all the system related libraries we should have a native_handle_t type and a function native_handle to get it. This is needed to manage with all the features not covered by the library. You should follow the design of the Thread library for which there are handles for the threads, mutex, ... system classes.
  
>> What is wrong on using the Boost.Filesystem library to idientify  
>> executables?
> 
> Support for boost::filesystem::path could be added, too. However it  
> doesn't solve the narrow/wide string problem if that's what you meant?

Yes, I don't know Boost.Filesystem, but I expected that it provided a way to name any file. If this is not the case,  forget my comment: :(
 
>> Why the do we need to duplicate the name of the executables as an  
>> argument?
> 
> There's no way to distinguish otherwise if the first argument is the  
> executable name or if it's meant to be the first command line argument.  
> Currently Boost.Process simply copies all arguments into the array which  
> is passed as a parameter to main(int argc, char *argv[]).

Why this is not hidden by the library?

>> Respect to relative or absolute path of executables,why not let the  
>> undelying platform to handle with this?
> 
> Then developers would need to resolve paths themselves (or use a helper  
> function) if they want to write platform-independent code?

I don't think that using absolute paths help to write platform independent code. IMO it is better to rely on the PATH environment variable (at least on unix-like OS exec takes this already in account).

Best,
Vicente