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

9 Nov 2016

      On Tue, 8 Nov 2016 21:38:19 +0100
Klemens Morgenstern  wrote:
...
Am 08.11.2016 um 19:57 schrieb Lee Clagett:
...
On Mon, 7 Nov 2016 14:26:11 +0100
Klemens Morgenstern  wrote:
[...]  
 [...]  
 [...]  
 [...]  
 [...]
...
I think defining the concept of `properties` and how it will be
extended will help in the documentation and refining the
implementation.  
Uhm ok, well it's a property of a process, I don't what's so
ambiguous about that. But they're all basically private.
[...]
...
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/bo...
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)?

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.
...
...
I apologize for not being explicit earlier, what I noticed was that
`argument properties` are different from other type of properties.
They manipulate the launching of the process. Some of the other
properties manipulate before/after state (`on_setup`, `on_error`,
`on_success`). Does this mean that there are actually two different
concepts here? A `launcher` (execv, shell, etc) concept, and a
`property` concept? There is certainly some special magic for
`shell`.
most of the properties modify the process at launch - I think
actually all on windows. You have a set of parameters you set up in
the executor which then get invoked in "CreateProcess". Thus the
distinction you make here is just wrong.
But to be fair, the executor is not documented (yet).
...
Which leads to ... why does the `child` constructor launch the
process? Is `child launch(...)` and `child shell(...)` more
appropriate? Or even `child launch("foo",
"args")(...properties...)`, where `properties` can only change
state via `on_setup`, `on_error`, `on_success`?
[...]  
 [...]  
 [...]  
 [...]
...
I _think_ your issue with named parameters is that they inherently
require lazy process launching, and some of the parameters
cannot/should not be "bound" to an object that is later used to
launch a process. Is that accurate? The remaining arguments I
recall seeing: (1) not extensible, (2) inefficient, (3) access to
values that should be hidden - I do not agree with. Conceptually,
the only difference is the lazy nature of the named parameter
approach which requires "binding" of some values to an object
before launching the process. So yes, this could be undesirable if
resources are cleaned up before invocation.  
Yes that is another reason, since some properties (as io_service,
group) can only be taken by reference. Why you disagree with the
other points is beyond me.
...
I suspect the named parameters will be easier to implement since it
does not require massive filtering and dispatching, although I
could be incorrect. Named parameters also means the removal of a
accept-anything constructor with potentially confusing internal
errors. And it should be possible to disable named parameters by
returning a new callable type, so that a parameter can only be
invoked once.  
Well massive filtering is an over-statement, but elsewise your
correct. But: passing a wrong type gives you an error of the type
"undefined refernce to boost::process::detail::initializer_tag<foo>",
while the actual ugly error comes from forgetting your io_service in
the sequence, but you need it. Than you get the horrible error, and I
don't know a way around this - except for SFINAE, but then you get an
even less explanatory error.
[...]  
 [...]
...
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.
...
And you can also bind them to an object beforehand:
std::vectorstd::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::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));

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.

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?

Lee