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

Subject: Re: [boost] [process] Formal Review starts today, 27 October
From: Klemens Morgenstern (klemens.morgenstern_at_[hidden])
Date: 2016-11-09 08:50:19

>>> So I cannot modify how the process is actually launched? Or does
>>> `exec` provided to `on_setup` have a modifiable dynamically
>>> dispatch function (I didn't see this in the implementation)?
>> Here you go.
>> https://github.com/klemens-morgenstern/boost-process/blob/develop/include/boost/process/detail/windows/executor.hpp#L235
>>
>> The handler modifies the executor, which then launches the function.
>>
> 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?
>>> Why would the group have to be passed in the same call with the
>>> arguments? The arguments are already being copied to a std::string,
>>> so what harm is there in "binding" these arguments to an object
>>> before invoking the process?
>> It has to be present when CreateProcess or fork is called, that's the
>> reason. Sure you can copy it, that's why there the syntax
>> bp::system(exe="foo", args="bar");
> 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.
>
>> And you can also bind them to an object beforehand:
>>
>> std::vector<std::string> args = {"bar"};
>> bp::system(exe="foo", args=args);
>>>      bp::group g;
>>>      bp::launch("foo", "args")(g);
>>>
>>> Also, is it possibly invalid to "bind" `g` here (because it could
>>> have a dead handle by invocation time)?
>> How would launching work with in this code? Launching in the
>> destructor? What would bp::launch("foo", "args") return?
> `launch` returns a callable that accepts 0 or more `properties` to
> launch the process. Example:
>
>    class launcher {
>      std::string exe_;
>      std::vector<std::string> args_;
>    public:
>      launcher(std::string exe, std::vector<std::string> args)
>        : exe_(std::move(exe)), args_(std::move(args)) {
>      }
>
>      template<typename... Props>
>      child operator()(Props&&... props) const {
>        return detail::exec(exe_, args_, std::forward<Props>(props)...);
>      }
>    };
>    struct launch_ {
>      template<typename... Args>
>      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.
> 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.

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?
> Also, is there an advantage to allowing string arguments anywhere in
> the function call? A single `std::vector<std::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::vector<std::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.

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.

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.