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

On Mon, 7 Nov 2016 14:26:11 +0100 Klemens Morgenstern wrote:

Am 07.11.2016 um 13:44 schrieb Lee Clagett:

...

On Mon, 7 Nov 2016 01:46:42 +0100 Klemens Morgenstern 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