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

...

...

I know there is some difficulty because the Posix fork + exec design does not work the same as way as the Windows API. But I think there is some value in specifying some basic concept of an `executor` and its relationship with a `property`. For instance, the executor will have some function (call operator) that starts a new process using internally stored string arguments and returns a handle to the new process. The string arguments should arguably be non-modifiable once the executor is created, etc. The string are only set after the executor is created, I don't know how that would work. Also the flags need to be accumulated through disjunction, I don't see how it would be better to this on construction time, and not in on_setup. It's internal, so why bother with a strange workaround there? What I noticed in the source is that some of the properties require special logic because they are manipulating other properties instead of

Yes the "executor". For example, the `shell` property is manipulating the path and arg properties but nothing else.

Earlier I mentioned on working out a loose way of specifying a `property`. I think such a specification would not consider properties that only manipulate other properties to be a property (since it could do that separately from the `child` constructor). Well that depends on how you model it, I consider the question if a

process is launched through the shell or not to be a property of this; it must be set prior to launching and it affects how it runs. There is no need to model this exactly after the excutor; that's is actually not possible. Also it does not modify a property, but rather modifies an initializer, which is build from several properties. So do I understand you correctly, that you would want this syntax? bp::system(cmd="foo"); bp::system(shell="foo"); If so - why? Why should'n you be able to launch the same process on it's own or through the shell? It just makes more sense to me to enable the shell as an additional feature.

...

...

I was asking whether you could store a handle to the `group` in some object whose call operator starts the process (like Niall originally mentioned). So the handle would be present, but potentially invalid depending on how it was implemented. Yes it could, but I stated enought times why this would be a horrible idea. Yes, and I disagreed with your analysis. I recall the primary objection was poor performance - specifically that space for each type of possible

[...] [...] property was needed. This is not true, the binding approach (at least how I saw it), would return a new callable type after each binding. In other words, the binding was conceptually calling `push_back` on a fusion like tuple. Moves + ref-qualifiers meant that dynamic memory would not copied in many cases. So `launcher("foo").group(...).stdout(...)` would return the equivalent of `tuple` while moving `std::string` memory as needed.

After thinking about it, all that I (and I think Bjorn and Niall) were trying to accomplish is separating the path and arguments from the remainder of the properties. If the path and arguments were in a single property, `shell` and `cmd` could return that property instead of requiring custom logic internally.

Well that is one reason, though I also assumed, that you wouldn't construct a tuple like that. The primary reason I really dislike the idea, is that every conceivable property would need to be in a member defninition of this tuple, and that is horrible.

...

...

...

`launch` returns a callable that accepts 0 or more `properties` to launch the process. Example:

class launcher { std::string exe_; std::vectorstd::string args_; public: launcher(std::string exe, std::vectorstd::string args) : exe_(std::move(exe)), args_(std::move(args)) { }

template child operator()(Props&&... props) const { return detail::exec(exe_, args_, std::forward<Props>(props)...); } }; struct launch_ { template launcher operator()(std::string exe, Args&&... args) const { return {std::move(exe), {std::forward<Args>(args)...}}; } }; constexpr launch_ launch{};

// syntax: // launch("ls", "-la", "/")(stdout(my_pipe)); Yeah, that still doesn't work. When do you launcht process? Please

[...] [...] [...] think those Ideas through, if you want me to take them serious. If you launch at the call of "launch("ls", "-la", "/") than you couldn't bind stdout. If you launch after the binding of stdout, how would you append another paremeter? It's broken. How is it broken? Look at the code snippet, the second function call takes a variadic argument list like the `child` constructor. The string arguments are the only thing "bound" to an object. So appending another property would be done like:

launch("ls, "-la", "/")(stdout(my_pipe1), stdin(my_pipe2));

That style is simply separating string arguments into a separate variadic function call. Or another way of looking at it - there is a single `property` for specifying the path and args that has a fixed positional location. What about the cmd style, i.e. bp::system("gcc --version"); ?

...

...

I don't see a good reason for the `child` class to actually launch the process. A global or static function that returns a `child` object should do it instead. This would remove the need for the `shell` property, which would be a distinct function. Well we had a global function for that in previous versions, but that wasn't like that much; especially since people got reminded of std::async. I have to agree, it's not the common C++ idiom to use functions to construct object, you also don't write

std::thread t = std::launch_thread([]{});

Also this would be bad, and probably a common mistake. It would launch the process and immediatly terminate it.

Why would it immediately terminate the process? Or do you mean a non-joined `child` should terminate the process like `std::thread`? Exactly. Also this would produce a deadlock if waiting in ~child and foo

Oh ok, sry, misunderstood that one. But then why is that any better? Just fyi: since you can pass environment variable to the property-list, this doesn't even have to be the same executable. throws. bopstream os; bp::child c("foo", std_in < os); os << foo() << endl; //boom, deadlock.

Admittedly, I did forget about that case - its probably best to follow `std::thread` closely and terminate the current process if not `join`ed or `detach`ed. I don't there is anything wrong with a function returning a `child`, provided the behavior of ignoring the `child` is well documented and defined. However, there is little difference to the code snippet above and:

bp::child c1(bp::exec("ls", "-la", "/"), bp::stdout(my_pipe1)); bp::child c2(bp::shell("ls", "-la", "/"), bp::stdout(my_pipe2)); bp::child c3("ls", {"-la, "/"}, bp::stdout(my_pipe3));

Where the constructor in `c3` forwards to the constructor in `c1`; `c2` uses the `bp::exec` object returned by `bp::shell`; `c1` uses the `bp::exec` property to set the path and args of the executor.

Ok, so I consider exe/args, cmd and shell different properties, but in the end they all get put into one initializer. (https://github.com/klemens-morgenstern/boost-process/blob/develop/include/bo...) So if anything this could be made public, though at the time the values get put into the exe-args initializrs they are already parsed and formatted. I really don't see the benefit.

...

...

Also, is there an advantage to allowing string arguments anywhere in the function call? A single `std::vectorstd::string` argument can be initialized implicitly with a `{...}`. This would easily separate the arguments from the rest of the properties without the lazy binding provided above:

shell(std::string cmd, Props&&...) launch(std::string exe, std::vectorstd::string args, Props&&...)

Which can be invoked like:

bp::shell("ls -la /", bp::stdout(my_pipe)); bp::launch("ls", {"-la", "/"}, bp::stdout(my_pipe)); bp::launch("foo", {}, bp::stdout(my_pipe));

Is requiring a `{}` pair less desirable than the variadic syntax? Yes there is, bp::system("gcc", "main.cpp", "-o", "main.o"); looks awesome. Also keep in mind, that you have the cmd-syntax, which is a single string and that you can path a boost::filesystem path. What you are basically proposing are positional arguments, which might make sense for exe/args or cmd, but not for the test of that. There is no logical order of the other arguments and secondly it is variadic to allow the user to skip some arguments. Requiring a `{}` hardly seems detrimental to me, but I guess people have different tastes. I hope you at least consider positional arguments for the path and args. For the remainder - I agree the variadic constructor is probably preferable since its behavior can be closer to std::thread. The only reason you can't use that is that the parameter list is forwarded internally, hence the type-deduction fails. That syntax is already valid:

bp::system("foo", std::vectorstd::string{"--arg", "bar"}); or auto args = {"--arg", "bar"}; bp::system("foo", args); Though I just checked my code, and that would work for `std::initializer_list` but not for `std::initializer_list`, so I'll add a bugfix and test for the latter.