On Wed, 9 Nov 2016 14:50:19 +0100
Klemens Morgenstern
[...] [...]
Some properties do things before and after the process is launched. Others try to manipulate the implementation defined `executor` object. Then another (`shell`) changes what process is launched via argument manipulation. Does it make sense for all of these to be taken in the same variadic argument (especially the last case)? Yes 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 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).
[...] [...]
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
`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.
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`? 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.
bp::launch("foo");
Secondly, I'm not sure why shell would be a distrinct function - that doesn make sense, you can use it in all three launch-functions. A shell ist just another process, why would I handly it any different?
This should already be covered, but the shell function would just return an object storing the path and arguments.
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.
Also I don't think arguments are any different from the other properties, or at least I don't consider them to be. It's a value that determines how a process behaves that I pass at launch, so why would that be any different from the environment? And yes, args args are optional.
I agree, its just that I do not think `cmd` and `shell` are like the other properties. I also think there is benefit to having a single property for path and args that is taken positionally instead of anywhere in the argument list.
Since you're coming up with so many ideas, I would encourage you to start your own library, which would allow us to talk about some tests way of doing things. Currently you're only Feel free to fork mine or the older version from Boris. That might be a good proof-of-concept.
Lee