Am 31.10.2016 um 01:18 schrieb Gavin Lambert:
On 31/10/2016 04:39, Klemens Morgenstern wrote:
Am 30.10.2016 um 14:21 schrieb Bjorn Reese:
The bp::child object should not execute in the constructor, but rather via operator(). That way we can use the named parameter idiom to build up redirections and similar modifications. For example:
auto compile = bp::child("c++").output(bp::null); compile("main.cpp"); // Executes
+1 for me for that style too.
I personally don't like that approach though it certainly would work. I think it's partly a matter of taste, here's my (C) association:
struct X {int std_in; const char* exe;}; X x{.std_in = 2, exe ="stuff"};
I.e. your setting properties.
Now the problem I see is this: what happens here?
auto compile = bp::child("c++"); compile.output(bp::null); compile("main.cpp"); compile.env({"PATH", "/foo"});
The fourth call should fail (and probably throw), since it's impossible to change the environment after the process has been started. Otherwise that should work as you'd expect.
One of the great advantages of this style of construction is that it permits conditionals in a fairly natural fashion:
auto compile = bp::child("c++"); if (quiet) compile.output(bp::null); compile.arg("-c"); if (debug) compile.arg("-D_DEBUG"); compile.args("-DFOO", filename); for (auto lib : libraries) compile.arg("-l" + lib); auto child = compile();
or something like that. Or you could have a factory function that returns the un-executed builder to the caller to add additional parameters before finally actually executing it, which aids in composability.
I want to back up my claims here, so for comparison, here you have the documentation of boost.process 0.3 and you can see how many parameters you can actually set in the way you described: http://www.highscore.de/cpp/process/#creating Five. Five options is what you have. And no asynchronous I/O. If you look over 0.5 (with a variadic interface, http://www.highscore.de/boost/process0.5/boost_process/tutorial.html) you have this set of parameters, some of them mutually exclusive (like set_cmd, set_exe/set_args: - bind_stderr - bind_stdin - bind_stdout - bind_fd - close_fd - close_fds - close_fds_if - close_stderr - close_stdin - close_stdout - hide_console - inherit_env - notify_io_service - on_exec_error - on_exec_setup - on_fork_error - on_fork_setup - on_fork_success - on_CreateProcess_error - on_CreateProcess_setup - on_CreateProcess_success - run_exe - set_args - set_cmd_line - set_env - set_on_error - show_window - start_in_dir - throw_on_error And this is without built-in support of asynchronous I/O or /dev/null. I'm very convinced that it is a bad idea to put all of this into one class by default.
So in order to do that, we'd need a child_builder class, that can be converted, similar to boost.format. I consider this style pre C++11, i.e. before variadics. Now granted: it seems not very C++ish, but it's a very unique problem: most properties are inaccesible after you launched the process, which is the reason they are not members.
Variadics are cool, but they contribute to code bloat, since each unique sequence of parameter types requires generating a new overload, and they require templates, which precludes private implementation.
Alright to back my claims up a bit further, here's how a boost.fusion iteration looks like with O1: https://godbolt.org/g/2q4il0 Now granted, boost.process will have a few moves in there, but that's basically as small as it gets. The operator() mostly calls the syscall directly. Sadly the is not as readable when using boost.process.