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

Subject: Re: [boost] [process] Formal Review starts today, 27 October
From: Klemens Morgenstern (klemens.morgenstern_at_[hidden])
Date: 2016-10-31 18:44:10

Am 31.10.2016 um 23:01 schrieb Remi Chateauneu:
> Hi All,
>
> "Whether you believe the library should be accepted into Boost"
>
> It could be, but as some very important features are missinge, the
> implementation should be prepared from now on.
>
> "Your knowledge of the problem domain"
> About 20 yrs C++ development Linux and Windows.
>
> This library has a strong focus on the streaming which is quite rare and
> interesting. However, other very important features of multi-processing
> are missing should be planned: At least to ensure that, in the future, they
> can be added without breaking existing code. Example of these desired
> fatures are (Taken from my own needs)
>
Thank you for the feedback, though I have to say, I wasn't planning to 
put the focus on streaming, it's just essential for processes.
Actually the library does provide a (badly documented) way to extend the 
library, at least the settings. And of course you can obtain the native 
handles, so you can start to develop and use an extensions and then we 
can integrate that.

> * List of  subprocesses of a given process.

That's usually a painful iteration of handles on windows, since there's 
no real API-support. Would make sense, but it's really not portable. I 
added process groups so you can do the essentials with them (i.e. wait & 
terminate).

> * Get its parent pid (Already discussed).
> * Get the entire process tree of a machine.

I thought about that, but that would basically be a third section of 
features, after process and this_process, you'd have system_processes. I 
am really afraid that this would completely get out of hand and should 
rather be a custom library. Especially since you then actually get 
problems with access rights. I'd think that would better be put into 
another library.

> * It is very useful to be notified when a given process dies (unotify on
> Linux).

You can do that with boost.asio using on_exit.

> * On windows, we have the feature e.startup_info.dwFlags =
> STARTF_RUNFULLSCREEN, which gives some control on the console, which Is
> very good. Could we have more documentation about it, and plans to have the
> same feature on Linux ? Generally speaking have some control on the process
> terminal if there is one.

I don't think there is an equivalent. It's a windows extension and a 
simple flag that tells windows how a gui-application shall be handled. 
It links to the documentation in the msdn, which is all the information 
I have, too.

> * Have some knowledge about users and user groups, as strings and integer
> identifiers: the concept is similar on Windows and Unix.

I guess that would make sense in a library which allows analyzation of 
other processes and walking of the process tree etc.

> * Have some control on process priority( Unix's nice command )
I actually thought about that, but I'd need a mapping of the priority 
levels - they are too different between windows and posix. If somebody 
has an idea how to do that, I'd be all for it.

See here: 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100(v=vs.85).aspx

>
> Indeed, many of these features are not portable, but most of time,
> developers use only a common core of features, which is quite enough. Maybe
> take inspiration from portable libraries such as Python psutil, which
> exposes a common interface for all operating systems.
>
> Interoperability:
>
> * It might be worth checking portability with other operating systems. BSD
> ? VMS ???
It implements the posix-standard and I tested on debian, but if there 
are any problems I add workarounds.
> * Must ensure that this lib is interoperable with boost::interprocess
> (Concept of permissions might be worth using ?)
I don't see any reason why it wouldn't, but I didn't test for that.
> * Does it build on pre-C++11 compilers ? Many projects are not able to
> upgrade to C++11 immediately and have to stick to previous C++ standard.
It does not. It was painful enough going from C++14 to C++11, but C++98 
would just be impossible.
>
> Some critics (Nothing is perfect):
>
> * The merit of the ">" operator for optional arguments is to be visually
> similar to an indirection, but in the context of C++ programming, although
> elegant, it is I think confusing and could be mismatched with <> templates
> or comparisons. Creating a process is a not-so-frequent operation and we
> should not be afraid to be verbose and very clear, I believe.
It's on version, the others are std_out="logfile" and 
std_out("logfile"). I really didn't think that this point would be such 
an issue.

> * Syntax ‘(bp::std_out & bp::std_err) > "output.txt"’ . Can we explain the
> exact difference with the syntax: ‘bp::std_out > "output.txt", bp::std_err
>> "output.txt"’’
Ou, that's a neat cornercase I haven't thought of. Here you'd have two 
file handles, which would lead to OS-defined behaviour, with std_out & 
std_err you'd have one. But if you use a pipe instead of "output.txt" it 
would not differ. I'll add a note.

> * More comments in the code and examples please.
> * Examples async_io.cpp or arg.cpp do not build with boost 1.62:
> include\boost\process\detail\windows\compare_handles.hpp(24): error C2039:
> 'BY_HANDLE_FILE_INFORMATION_': is not a member of 'boost::detail::winapi'
You need to get the newest version of boost/winapi to build it. Just 
overwrite libs/winapi with the develop branch of winapi.
> * ‘typedef basic_environment<wchar_t> wenvironment;’ : Is It really useful
> ?? POSIX getenv() only uses char.

It is not at all, like all other wchar_t features on posix. But my 
attitude is: if it's part of the library on windows, it must be on 
posix. Hence you can use wchar_t on both platforms.

> * Why is the class basic_environment so big, as it is basically a simple
> std::map< std::string, std::string > ? (Naïve question).

Because it uses the native Implementation to store the values, which is 
either char** or a null-seperated array of null-seperated-strings. It 
does that, so it can be directly assigned when launching the process. 
And so you can modify the environment of the current process.

>
> Anyway, many congratulations for this very interesting work, which is
> definitely on the right direction.

Thanks!
>
> Regards
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>