2016-11-05 0:13 GMT+03:00 Boris Schäling :

...

"Antony Polukhin" wrote in message news:CAKqmYPaksmoU=A=dqvAiDVMO7BEEZDepSfL64LVXazj2_kOqaA@mail.gmail.com... [...] If you're willing to write the review but could not make it on time - please give a hint. In that case it is possible to postpone the final decision for a few days.

I'm a bit late but definitely want to send my 2 cents to the mailing list, too! I'll try to do it tomorrow (Saturday) and latest on Sunday.

Waiting for the 2 cents :) Thanks in advance! -- Best regards, Antony Polukhin

Dear Boris, dear Klemens,

On 06 Nov 2016, at 20:59, Boris Schäling wrote:

There were not only incremental improvements though but also tangible results: We always wanted to support asynchronous I/O in [Boost.]Process. This included waiting asynchronously for a child process to exit. So we worked on a new I/O service object to support asynchronous waiting on Windows. The I/O service object was added to Boost.Asio in 2012 (known as boost::asio::windows::object_handle).

A huge step forward was the introduction of executors and initializers - an idea from Jeff Flinn in 2011. All [Boost.]Process versions since then are based on executors and initializers. His idea changed the design of [Boost.]Process quite a lot as it provides a generic solution to the problem of how to make [Boost.]Process extensible. It is important to understand Jeff's idea and the problem it solves as any other C++ process management library has to solve the very same problem to be at least as good in that aspect.

It turns out that when creating a child process an infinite number of configuration options can be set. The Windows and Linux functions being called when a child process is created accept only a limited number of arguments. If however, for example, the root directory of a child process on Linux should be changed, the function chroot() has to be called. The configuration option "root directory" is set through an additional function call. It's easy to create a C++ process management library which just supports a few cross-platform configuration options. It's much more difficult to create a C++ process management library which allows a library user to customize the process of creating child processes any way he likes. Jeff's idea was to use objects as named parameters. If [Boost.]Process provides a function which accepts any objects (or actually any types), library users can create their own named parameters and pass them to that function. They could for example create a named parameter which changes the root directory for a child process.

Jeff's idea isn't necessarily the only solution to how to provide an extensible interface. And of course one could argue that this is a negligible problem a process management library doesn't need to solve. For [Boost.]Process extensibility has always been one of the major goals though. We want to avoid that developers start using [Boost.]Process only to find out later that they can't do what they need to do because the library isn't extensible enough. Ideally any alternative process management library states if and how it solves this problem.

Here are some of the other not so obvious goals of [Boost.]Process:

* It should be entirely clear to a library user how resources are managed (which resources could leak when and how, and how the library helps a user to avoid leaking resources; and I'm not talking about resources you could manage with a smart pointer or alike).

* It should be possible to do asynchronous I/O (this includes reading, writing and waiting for child processes to exit).

* Similar concepts found across operating systems should be represented by one concept (for example, standard streams), different concepts should be represented by different concepts (for example, signals on Linux and object handles on Windows - these are the OS services used for asynchronous waiting).

I'm not going to discuss these goals and the current solutions in detail (this email is already too long; I will talk about resource management in [Boost.]Process at Meeting C++ in case anyone wants to know more).

thank you for this summary of the design principles and goals. I would like to see this added to the Design Rationale in the documentation for [boost.]process. Laying out design principles and goals is very useful: - They are easier to grasp than the implementation. - They allow reviewers to check whether they agree with the principles and goals. - Reviewers can check, whether the implementation actually follows the principles. Best regards, Hans

Am 07.11.2016 um 21:09 schrieb Nat Goodspeed:

On Sun, Nov 6, 2016 at 11:59 AM, Boris Schäling wrote:

...

While Klemens made some changes to the version I handed over to him and made his own steps forward, I helped him avoiding making steps backwards. Boris, I believe you're saying that Boost.Process 0.6 is just as extensible as your 0.5, and that I should stop worrying about that point. Thank you.

However there are two points that still bother me:

* Klemens says that the dynamic-container-of-initializers idea will not work in Process 0.6. I'm not sure why. Process 0.5 initializers also used templated executor arguments; it's true that the any_initializer adapter had to state a specific executor type. (I had distinct windows vs. posix any_initializer types.) Nonetheless any_initializer could still be used with any templated initializer type. Isn't that still true? You're right, those were templated too. BUT the executor was actually not a template, so you could've written on_success(windows::executor& e). That is now different. I don't know how a type erasure could be done here, but I could write a variant. The reason is, that some properties/initializers require access the the sequence, mainly to obtain the reference of the io_service.

* Klemens states that the machinery needed to write custom initializers is currently in the detail namespace. I want it to be promoted out of the detail namespace: I want support for custom initializers to be a documented, supported feature of the library. If the library is not yet "done" in that respect, then let's consider it again when it's more fully baked. Ok, so you have three things you can do: use custom handler for events, as in

child c("foo", on_error([](auto & exec, const std::error_code & ec){std::cerr << ec.message() << std::endl;}); which is already public & documented (or at least mentioned in the documentation). Secondly there is the basic inheritance of "handler" (currently private, but needlessly so) struct foo : boost::process::detail::handler { template<typename Exec> void on_error(Exec & exec, const std::error_code & ec) { std::cerr << ec.message() << std::endl;} }; }; foo f; child c("foo", foo); This has additional features like async_handler, where you can write an on_exit handler etc. and there's a function which get's the passed io-service; so it's a bit more rich than in 0.5. Now the third option (and most complicated) would be that you declare a tag for a type, provide a builder and a initializer. I won't explain that here in detail, but let's say "foo" can be internally constructed from a sequence of "bar". Then you could us it this way: bar b1, b2; child c("bar", b1, b2); //equal too something like foo f = {b1, b2}; child c("bar", f); The reason this is not public, is because I'm not sure what should be public and what not and I'm not sure if it works as a public interface the way it does internally. I think that needs some experience from someone other than me, actually implementing an extension, to know that. I didn't think that it would've been the best idea to put this in the version of the library to be reviewed, especially since I need the experience of someone else. I really think it makes much more sense to get the library accepted in its structure before we - and by we I mean me and a user, e.g. you, - work out the details of the public extension interface.

Am 07.11.2016 um 22:14 schrieb Nat Goodspeed:

On Mon, Nov 7, 2016 at 12:52 PM, Klemens Morgenstern wrote:

...

...

* any_initializer could still be used with any templated initializer type. Isn't that still true? You're right, those were templated too. BUT the executor was actually not a template, so you could've written on_success(windows::executor& e). That is now different. I don't know how a type erasure could be done here, but I could write a variant. The reason is, that some properties/initializers require access the the sequence, mainly to obtain the reference of the io_service. Please try that? I want a custom initializer to be able to access such

Am 07.11.2016 um 21:09 schrieb Nat Goodspeed: things too.

...

...

* Klemens states that the machinery needed to write custom initializers is currently in the detail namespace. I want it to be promoted out of the detail namespace: I want support for custom initializers to be a documented, supported feature of the library. If the library is not yet "done" in that respect, then let's consider it again when it's more fully baked. Ok, so you have three things you can do: use custom handler for events, as in

child c("foo", on_error([](auto & exec, const std::error_code & ec){std::cerr << ec.message() << std::endl;});

which is already public & documented (or at least mentioned in the documentation). I saw that, but was misled by the fact that the documentation showed a nullary lambda.

In any case I'm more interested in a custom initializer that can self-consistently intercept more than one such event.

...

Secondly there is the basic inheritance of "handler" (currently private, but needlessly so)

struct foo : boost::process::detail::handler { template<typename Exec> void on_error(Exec & exec, const std::error_code & ec) { std::cerr << ec.message() << std::endl;} }; };

foo f; child c("foo", foo);

This has additional features like async_handler, where you can write an on_exit handler etc. and there's a function which get's the passed io-service; so it's a bit more rich than in 0.5. Good! I think you're saying that this feature is stable? Let's just move it out of "detail." As stable as the library, since it is using it ;).

...

Now the third option (and most complicated) would be that you declare a tag for a type, provide a builder and a initializer. I won't explain that here in detail, but let's say "foo" can be internally constructed from a sequence of "bar".

The reason this is not public, is because I'm not sure what should be public and what not and I'm not sure if it works as a public interface the way it does internally. I think that needs some experience from someone other than me, actually implementing an extension, to know that. I didn't think that it would've been the best idea to put this in the version of the library to be reviewed, especially since I need the experience of someone else. I really think it makes much more sense to get the library accepted in its structure before we - and by we I mean me and a user, e.g. you, - work out the details of the public extension interface. Then may I suggest a namespace like "experimental" (borrowing a convention used in recent C++ documents)?

My expectation from other Boost libraries is that I am explicitly supposed to avoid namespace "detail." The connotation is: hands off!

"experimental" suggests "API is still unstable" while leaving the feature open for user testing. Sure, makes sense. Or I just put everything into boost::process::extensions and declare the whole namespace as experimental.