On Wed, 24 Mar 2010 03:41:42 +0100, Felipe Tanus <fotanus@gmail.com> wrote:
[...]1.The current implementation of Boost.Process library has the class boost::process::child, and two classes that inherit it, win32_child and posix_child. I don't believe that this is the best solution. There [...]This could be done with programming practices as suggested by Vicente Botet, in a previous topic [1]
I agree that Vicente's proposal looks very interesting. In fact there has been a Boost.Process draft from Ilya Sokolov in 2008 without Windows and POSIX types and functions (his drafts can be found at http://www.boostpro.com/vault/index.php?directory=Process). He later said though that he thinks that removing those types and functions was an error (see http://article.gmane.org/gmane.comp.lib.boost.devel/180309). Unfortunately I didn't ask him at that time why he thinks it was an error (probably because it didn't seem to make sense to me at that time either). Now Ilya doesn't seem to follow the Boost mailing lists anymore. :-/
2.The wait() method, to wait a process (child actually, but this is another problem) to die, is not asynchronous. Sometimes programmers want a synchronous wait() method, so there is no motive for we to remove this. We should implement a new asynchronous wait() method.
I agree.
[...]3.About the process creation, I think the following practice from the documentation should be avoided: “Another important requirement is to always pass at least one argument to the executable. This must be the name of the executable itself”. There is no motive to require a parameter that you can deduce by yourself. The first parameter, in this case, should be implicit.
There should be the possibility to set the executable name somehow (and if only not to force developers to use another library just because it's not possible to do this with Boost.Process). However for convenience I agree it makes sense if developers don't have to set the executable name explicitly (the filename can be used by default).
4.Would really be nice boost::process::launch() be able to receive any kind of string? This tend to let the programmer go for a single platform solution. As a multiplatform library, this may be avoided. I'm not sure about this matter as well, but if the library is here to provide a higher abstraction level, should it also accept API related types? I believe it should not.
I also think we can get rid of the Executable concept which currently must be a std::string anyway.
[...]Named pipes and FIFO (following the item #1, would be the same thing? Something would change? Something to be researched yet)
Here the problem is that on Windows only named pipes support asynchronous I/O. Currently a macro has to be defined to make Boost.Process use a named pipe internally. If we distinguish between anonymous and named pipes anyway we could provide appropriate classes which could be used by developers elsewhere. This is something which could even end up in Boost.Interprocess one day. If we want to support asynchronous I/O on Windows we have to think about all of this.
[...]Test platform issues.
The unit tests of Ilya's Boost.Process drafts (the one in the vault) are pretty good because everything works automatically if I remember correctly (and it doesn't with the original Boost.Process draft as described here: http://article.gmane.org/gmane.comp.lib.boost.devel/180285). Boris