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

Subject: Re: [boost] [process] Formal Review starts today, 27 October
From: Lee Clagett (forum_at_[hidden])
Date: 2016-11-08 13:57:06

On Mon, 7 Nov 2016 14:26:11 +0100
Klemens Morgenstern <klemens.morgenstern_at_[hidden]> wrote:

> Am 07.11.2016 um 13:44 schrieb Lee Clagett:
> > On Mon, 7 Nov 2016 01:46:42 +0100
> > Klemens Morgenstern <klemens.morgenstern_at_[hidden]> wrote:
> >  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> > I read through the explanation by Boris and the object as named
> > parameters approach. The documentation never formally specifies a
> > "property", or how a user could create a custom property for
> > interacting with library (or did I miss this?). There is a section
> > labeled "extensions" which list three functions: `on_setup`,
> > `on_error`, and `on_success`. The documentation mentions no
> > arguments for this functions. So how exactly do I extend this
> > library? Did you mean the maintainers could add features easier?
> > Separating different `properties` into unique calls would reduce
> > the maintenance for the internal metaprogramming necessary for the
> > current design.  
> Well there are two things: first I forgot to add the signatures for
> the extensions and the example is wrong, but with those you can start
> with that.
> 
> Secondly, the I'll add a part about extensions, but that is currently 
> not public. The interface is easily extensible, I didn't add
> documention for that and the necessary classes are in
> boost::proces::detail. The reason is, that this part of the library
> might change, and I don't think give I can give a guarantee that I
> won't break your code at some point if you use that. I.e. I don't
> want to, but it may be unavoidable. I'll add some implementation
> notes for that soon.

I think defining the concept of `properties` and how it will be
extended will help in the documentation and refining the implementation.

> Here's how you'd do it:
> 
> struct my_handler : boost::process::detail::handler
> {
>     //overload the event handlers
>      template<typename Exec>
>      void on_success(Executor & exec) const {cout << "it works" <<
> endl;} };
> 
> my_handler mh;
> bp::child c("foo", mh);
> 
> And with two template specializations & one more class you can add a 
> marked type, which is not directly an initializer. That's a bit mor 
> complicated, but possible.

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)?

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`.

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`?

> >  
>  [...]  
>  [...]  
> > Its worth repeating that Bjorn suggested something like:
> >
> >      bp::system("gcc").env(...)("--version");
> >
> > which does not require named parameters for the arguments
> > properties.  
> Do you really want me to repeat for the 42th time why this is a bad
> idea? [...]

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.

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.

> > I do not understand this paragraph. Are you saying that requiring
> > explicit `arguments properties` would affect these other
> > properties?  
> well instead of writing
> 
> bp::group g;
> bp::child c("foo", g);
> 
> you'd need to write
> 
> bp::group g;
> bp::child c(cmd="foo", some_artificial_group_ref_name=g);
> 

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?

   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)?

Lee