Am 13.11.2016 um 13:23 schrieb Bjorn Reese:
On 11/07/2016 01:46 AM, Klemens Morgenstern wrote:
I didn't understand it that way, especially since he Bjorn mentioned boost::thread. I'm a bit tired of this discussion, and Boris explained very well, why the initializers-approach was chosen. As far as I understand it he considers everything but args as attributes and wants them all crammed into one type (or something like that) which would be a horrific mess. I should know, I tried to build somethinke like that (based on boost.process), just two ours ago - it doesn't work for a proper process library, there are just too many properties to a process.
"Horrific mess" is in the eye of the beholder. I'm talking about the implementation, not of the usage. Please prove me wrong, but with an implementation not an usage-example. If you find a solution which can be integrated into boost.process, I'll add it to the library.
With the current API it is unclear which parameters are mutually exclusive (e.g. can I pass both a yield context and an error_code?) or what happens if I specify the same attribute multiple times like this:
bp::child c(command, bp::std_out > p, bp::std_out > "output.txt");
Well this will use the second pipe, though I grant you, that's not the best solution here - an error would be better. I could add an assertion, so there are no duplicates of that. On the other hand it is similar to that: child_builder cb; cb.std_out(p); cb.std_out("output.txt"); So it's not that strange.
and of course the argument overwrite issue mentioned by Lee, all of which demonstrates that the current API is error-prone.
The discussion about attributes is a bit fuzzy because the library does not define what they are. After a closer look it appears that there are several categories of attributes crammed into the initializer list.
Well yes, they are not called attributes, but properties. The reason is, that I don't see a different between the arguments passed and all the other stuff that's passed. After all, at least of those properties, you'd call attributes, can in some cases even select a different executable (i.e. the environment variables).
One such category is the status-reporting (e.g. error_code and yield context.) Here is another idea for an API: have separate functions for synchronous and asynchronous invocation. That's not the same. std::error_code is used to avoid exceptions and is only used when the process is launched. On exit you don't get an error_code, but only the exit-code of the process; and another std::error_code in case something went wrong with waiting. So there's no std::error_code vs. yield_context, they do different things and it actually makes sense to use them together. Now in case you use your error_code with system, like that
bp::system("foo", ec, yield); ec being set will indicate that the process was not launched successfully and hence the coroutine wasn't suspended. Also, you do not need to pass an io_service when passing a yield_context, since the system function can get the io_service from the yield_context. The io_service is needed bp::on_exit and the convenience stuff for asynchronous read, e.g. std_out > buffer(thingy). Additionally you do not need to pass an io_service for async-operations to bp::system; if none is passed but a property requires that, system will internally create one and use that.
Let the synchronous invocation accept one command parameter, one optional attributes parameter, zero or more arguments, and one optional error_code. The latter determine whether the call throws or not. I am deliberately omitting the attributes parameter below because it is irrelevant to the sync/async split.
bp::system(command, arguments...); // throws bp::system(command, arguments..., error_code&) nothrow;
Let the asynchronous invocation accept one io_service parameter, one command parameter, one optional attributes parameter, zero or more arguments, and one handler/extension parameter (i.e. no error_code parameter.)
bp::async_system(io, command, arguments..., [](error_code){}); bp::async_system(io, command, arguments..., yield); As stated above, both do not need to get an io-service passed, so I don't see the point of the second one. The first can already be done, if you want that, in the followin way:
bp::child(ios, exe, args..., on_exit=[](int code, const std::error_code &ec){}).detach(); I wouldn't recomment that, but that's possible. And no, bp::spawn doesn't work.