Re: [boost] [process] Formal Review starts today, 27 October

On Sat, 12 Nov 2016 14:22:24 +0100 Klemens Morgenstern wrote:

Am 12.11.2016 um 13:35 schrieb Lee Clagett:

...

On Thu, 10 Nov 2016 10:05:00 +0100 Klemens Morgenstern wrote:

[...] [...] [...]

...

Yes, whether a process is being launched in a shell must be set before starting the process, but this is irrelevant. Is `shell` a property of the `child` or a property of the path / arguments pair? In other words, does the internal logic of the `child` constructor actually need to know whether its launching a shell or just some other executable? The implementation would suggest that `child` does not need to know. Well the child doesn't know of any of the properties you pass it...

[...]

...

You can launch the process on its own or through the shell with that syntax. Its just that `shell` is modifying the path + args properties given to the `child`. I know, that's why there's shell, like this:

bp::system(cmd="foo", bp::shell); bp::system(exe="../foo", "--bar", bp::shell);

This same usage/syntax in `process::system` is irrelevant.

So obivously the question isn't why is this useful, but why is your syntax better?

I think the syntax I proposed is a better model of what is actually happening. I do not think there are any aesthetical gains to my proposal for `process::shell`, I think its a "push" in that regard.

...

The benefit is it removes the special internal logic for a few specific properties and makes what the code is _actually_ doing more clear. I've also been pushing for a more formal specification of `properties` because some of the IO variants are doing too much in `child`. The constructor for `std::thread` does not throw once the thread is created, but what about some of the IO properties for `process::child`? Several of the `on_exec`/`on_success` hooks are doing various calls that can throw - does `child` block internally in the constructor until the other process completes? Does it detach? Terminate? And why is so much being done inside of the `child` constructor when the `child` should only need to know the pipe handles for communication. I'm mainly thinking of the code for piping to future or mutable buffer. The child-class does not know of anything there, the child(...) ctor actually just forwards to values to a launch function. It's just a

[...] [...] form of RAII, since I think that makes sense. It's equivalent to boost::thread([](int), 42);

I do not think this is equivalent to `std::thread`. The constructor for `std::thread` takes the bare minimum to start a new processing thread, while the constructor for `process::child` accepts parameters that are unnecessary - `std::future` for instance. I think its worth determining the minimum set of values needed before process launching, and restricting the constructor to accept those values. If `stdout` of a process needs to go to `std::futurestd::string`, this does not need to be indicated before process launching, only a designated pipe needs to be specified, etc.

If there's an error before the launch it will not launch, if afterwards it will terminate the child immediatly.

Lee