[process] Formal Review starts today, 27 October
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) * List of subprocesses of a given process. * Get its parent pid (Already discussed). * Get the entire process tree of a machine. * It is very useful to be notified when a given process dies (unotify on Linux). * 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. * Have some knowledge about users and user groups, as strings and integer identifiers: the concept is similar on Windows and Unix. * Have some control on process priority( Unix's nice command ) 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 ??? * Must ensure that this lib is interoperable with boost::interprocess (Concept of permissions might be worth using ?) * 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. 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. * 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"’’ * 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' * ‘typedef basic_environmentwenvironment;’ : Is It really useful ?? POSIX getenv() only uses char. * Why is the class basic_environment so big, as it is basically a simple std::map< std::string, std::string > ? (Naïve question). Anyway, many congratulations for this very interesting work, which is definitely on the right direction. Regards
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).a...
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
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
participants (2)
-
Klemens Morgenstern
-
Remi Chateauneu