[boost] [process] Formal Review

Subject: [boost] [process] Formal Review
From: Klaim - Joël Lamotte (mjklaim_at_[hidden])
Date: 2016-11-03 14:26:51

Boost.Process Review
====================

...At a minimum, kindly state:
- Whether you believe the library should be accepted into Boost
  * Conditions for acceptance

YES it should be accepted, though documentation need improvements.

- Your knowledge of the problem domain

I implemented naive and less naives ways to manage different processes in
the past.
I currently work on libraries which are all about multi-process
communication
and we have a poor-man's process management (launching, tracking, etc.) api
that I have to maintain
on Linux, Windows and MacOSX.
I also have home projects which are organized as client-server with
master-slave relationship,
clients spawning servers and pinging them to keep processes alive mutually
(using network communication).

So basically, I implemented basic process management libraries, but I'm
more a process
management library user, in all my projects, paid or not.
I have been exposed to a lot of problems related and solved a lot of them.

I also used the last 2 Boost.Process versions proposed to Boost (though I
voted no for these ones)
but in cases where I just needed a cross-platform way to launch an
arbitrary executable (no
tracking nor communication).

You are strongly encouraged to also provide additional information:
- What is your evaluation of the library's:
  * Design

The weird choice of variadic arguments in launching functions have been
explained
so I'm more at ease with it, though maybe ways to simplify it could be
attemted in the future.

I like that the design of boost::process::child follow, where it make
sense, the design of std::thread.
Both could be considered as execution resource, so it's fine to me that
they share
similar interface phylosophies of design (like spawning on construction and
move-only).

I would love it if there was a way (even slow) to track processes lifetime
for processes that
I don't have code for. Though it's ok for me if it's a future extension.

I like the minimalism of the scope, though useful extensions could be added
later.

  * Implementation

I briefly eyescanned the code, looked more closely at the termination
trigerriung code
because of the discussion we had on termination reqest vs kill.
I don't bother much about performance for this library as launching
processes is rarely
done in a tight high-performance loop.

  * Documentation

I did a report of the documentation a few weeks or months ago, I do not
know if
changes have been made since.
Anyway I think the documentation should have a part focused on explaining
the concepts
because even if most programmers knows what a process is, all related
concepts (like what
is the "environnement") are so specific to processes that they are often
ignored as long as
naive usage works.
So as soon as the user need more specific usage, most, I believe, will not
know what word
to use to find the right concept.
I think this library documentation should follow something similar to
Boost.MSM which introduce
to the basic concepts, though without walls of text.

  * Tests

I didn't have time to try it yet in real applications, but so far didn't
find
any problem in basic usage.

  * Usefulness

It is very useful for almost all my projects.

- Did you attempt to use the library?
YES

 If so:
  * Which compiler(s)

Visual Studio 2015 for now.

  * What was the experience? Any problems?

I only did minimal testing, nothing deep, no problems.

- How much effort did you put into your evaluation of the review?

Several hours diffused through several months.

Joël Lamotte