- Boost - lists.boost.org

Re: [boost] [Boost-users] [process] Formal review results
by Bjorn Reese 13 Nov '16

13 Nov '16

On 11/09/2016 07:45 PM, Antony Polukhin wrote: > The Boost.Process library is accepted. Unconditionally? Please notice that some of the votes were conditional, and if those conditions are not met, they should be counted as no votes. > The most controversial part of the library is it's interface. Many > comments were addressing: > * passing arguments > * starting process right in `system(...)` function The comments were about starting the process in the child constructor.

4 3

Boost.Config for clang on Linux with libc++
by Belcourt, Kenneth 13 Nov '16

13 Nov '16

Hi, Attached are user.hpp and the console output from running configure with clang-3.9. Note that I tried to configure using libc++ but the configure script immediately failed, so these are the results for clang-3.9 with the default stdlib (libstdc++). Can someone please take a look and see what adjustments might need to be made to Boost.Config to support this configuration? Once it works for the default stdlib, I’d like to rerun it to use libc++. Thanks, Noel Belcourt

3 4

[process] Formal Review
by Nat Goodspeed 13 Nov '16

13 Nov '16

I would like to surface a number of issues, large and small. You are clearly proud to support a number of different syntactic ways to express the same semantic operation. To me this is a minor negative. While in theory it sounds nice to be able to write Process consumer code any way I want, in a production environment I spend more time reading and maintaining other people's code than I do writing brand-new code. Since each person contributing code to our (large) code base will select his/her own preferred Process style, I will be repeatedly confused as I encounter each new usage. I admire the support for synchronous pipe I/O, while remaining skeptical of its practical utility. Synchronous I/O with a child process presents many legitimate opportunities for deadlock: traps for the unwary. I would be content with a combination of: * async I/O on pipes (yes, using Asio) * system() for truly simple calls * something analogous to Python's subprocess.Popen.communicate(): pass an optional string for the child's stdin, retrieve a string (or, say, a stringstream) for each of its stdout and stderr. The example under I/O pipes the stdout from 'nm' to the stdin of 'c++filt'. But the example code seems completely unaware that c++filt could be delayed for any of a number of reasons. It assumes that as soon as nm terminates, c++filt must have produced all the output it ever will. I worry about the Process implementation being confused about such issues. I'm dubious about the native_environment / environment dichotomy. As others have questioned, why isn't 'environment' a typedef for a map<string, string> (or unordered_map)? I understand that you desire to avoid copying the native process environment into such a map until necessary, but to me that suggests something like an environment_view (analogous to string_view) that can perform read-only operations on either back-end implementation. Operations involving splitting and joining on ':' or ';' should be defined purely in terms of strings and ranges of strings. They should not be conflated with environment-map support. The documentation so consistently uses literal ';' as the PATH separator that I worry the code won't correctly process standard Posix PATH strings. At this moment in history, an example showing integration of Process with Boost.Fiber seems as important as examples involving coroutines. Why is the native_handle_t typedef in the boost::this_process namespace? While in full context it makes sense to speak of "assigning" an individual process to a process group, the method name assign() has conventional connotations. Use add() instead. There's a Note that says: "If a default-constructed group is used before being used in a process launch, the behaviour is undefined." I assume you mean "destroyed before being used," but this is a concern. If a program has already instantiated a process group, but for any reason decides not to (or fails to) launch any more child processes, does that put the entire parent process at risk? What remedial action can it take? Move the group object to the heap somewhere and let it leak? Spawn a bogus child process for the purpose of defusing the ticking process group instance? If you're going to reify process group at all, you should wrap more logic around it to give it well-defined cross-platform behavior. And it should definitely be legal to create and destroy one without associating any child process with it. Given support elsewhere for splitting/joining PATH strings, the string_type path parameter to search_path() feels oddly low-level. Maybe accept a range of strings? > At a minimum, kindly state: > - Whether you believe the library should be accepted into Boost > * Conditions for acceptance YES, IF the present Boost.Process preserves the ability of its predecessor to extend it without modifying it. I'm sorry, thorough reading of the documentation plus some browsing through the code leaves me doubtful. Examples of such extensions: * With Boost.Process 0.5 I can use Boost.TypeErasure to pass an "initializer" which is a runtime collection of other initializers. Such a feature in Process 0.6 would support people's requests to be able to assemble an arbitrary configuration with conditional elements and use it repeatedly. * I can create an initializer (actually one for each of Posix and Windows, presenting the same API, so portable code can use either transparently) to implicitly forcibly terminate the child process being launched when the parent process eventually terminates in whatever way. Please understand that I am not asking for the above features to be absorbed into the Process library: I am asking for a Process library extensible enough that I can implement such things with consumer code, without having to modify the library. Perhaps Process 0.6 already is! It's just hard for me tell. Another feature should, in my opinion, be incorporated into the library: * On Windows, if you desire to pass any file handles at all to the new child process, it is completely whimsical what *other* open file handles you may inadvertently pass -- unless you play games with STARTUPINFOEXA and PROC_THREAD_ATTRIBUTE_LIST to enumerate exactly the set of handles you intend to pass. If the library doesn't natively support that, I *must* be able to pass a custom initializer with deep access to the relevant control blocks to set it up. This is a showstopper, as in "you can't remove that file because, unbeknownst to you, some other process has it open." > - Your knowledge of the problem domain I have hand-written process-management code in C++ several times before -- each with multiple implementations to support multiple platforms. I have tested the previous candidate Boost.Process 0.5 with a number of programs exercising a number of features. > You are strongly encouraged to also provide additional information: > - What is your evaluation of the library's: > * Design Design notes are at the top of this mail. > * Implementation I have only glanced over parts of the implementation. It seems somewhat more obscure than the corresponding pieces of Process 0.5, which is why I couldn't quickly satisfy myself as to the library's extensibility. > * Documentation Others have noted that the documentation is very sketchy in places. I too wish for more explanation. Much of the time I spent on this review was reading through the documentation. Apologies if I have misunderstood the library's capabilities. > * Usefulness I have felt for years now that it is essential to get a child-process management library into Boost. It grieves me to have to write platform-specific API calls into different applications. > - Did you attempt to use the library? I did not, sorry, ran out of time -- as you can infer from this review arriving at the end of the final day! > - How much effort did you put into your evaluation of the review? Most of my time was spent reading the documentation. I looked through a couple of the implementation files for further information.

4 8

Visual Studio 2015 Update 3 has removed std::unary_function and std::binary_function
by Marshall Clow 12 Nov '16

12 Nov '16

... if you build with /std:c++latest. See https://svn.boost.org/trac/boost/ticket/12586 for an example. Other vendors will be removing these types in C++17 mode in the future (or at least requiring a -D RETAIN_REMOVED_STUFF=1 or something like that) A quick grep for these terms found 153 instances of "std::unary_function" and 118 of "std::binary_function" across several libraries, including accumulators, algorithm, config, container, function, gil, graph, icl, mpi, msm, polygon, ptr_container, serialization, smart_ptr, tr1, unordered, utility - and probably others. I think that we ought to have a boost-wide solution for this (i.e, everyone should solve this the same way). I can think of three possibilities: * Deny, deny, deny: Boost doesn't work with C++17. Not my recommendation. * Since std::unary|binary_function is an empty struct with a set of two (or three) typedefs, just define the typedefs everywhere. Note that the typedefs (result_type, first_argument_type, second_argument_type) were deemed to not be useful in C++17 due to the existence of advanced type traits, etc. * Define a boost:unary_function and boost::binary_function struct that mimics the one that was in namespace std, and change over to use that. * Something else. Comments? -- Marshall

15 74

Adding any_dynamic_cast?
by Peter Dimov 12 Nov '16

12 Nov '16

I stumbled across https://github.com/bytemaster/Boost.DynamicAny which implements, in a fully backward compatible manner, dynamic cast functionality for boost::any. That is, it can give you X* from an 'any' holding a value of type Y if Y derives from X. It achieves this by simply changing template<typename ValueType> class holder : public placeholder { ValueType held; to template<typename ValueType> class holder : public placeholder, public ValueType which then enables dynamic_cast from holder<>* to OtherValueType*. This is pretty cool, and I don't remember seeing this idea anywhere else. Perhaps we should adopt it? One might even argue that this is how any_cast should always have worked, but I suppose we can't change it now for compatibility reasons. Although it's an interesting question whether C++17 'any' shouldn't be enhanced in this manner.

2 2

Request for a new submodule, tools/depinst
by Peter Dimov 12 Nov '16

12 Nov '16

I'd like to request the creation of a new submodule, tools/depinst, that will contain a Python script depinst.py which would, given a module in libs/, install the dependencies needed for running that module's tests. Currently, the .travis.yml files I use enumerate the dependencies manually, which is a bit tiresome: https://github.com/boostorg/function/blob/develop/.travis.yml - git submodule init libs/algorithm - git submodule init libs/align ... - git submodule init libs/type_traits - git submodule init libs/typeof - git submodule init libs/utility - git submodule init libs/winapi - git submodule init tools/build - git submodule update --depth 1 This will be replaced by just - git submodule update --init --depth 1 tools/depinst - git submodule update --init --depth 1 tools/build - python tools/depinst/depinst.py function

5 19

[CI] Activation for Boost.Units
by Jürgen Hunold 11 Nov '16

11 Nov '16

Hi Rene, could you please activate CI via Travis CI and Appveyor for Boost.Units? I hope that I have successfully adapted the scripts from Boost.Predef and that they work for Units as well. Is there any way to get myself some admin rights so I can scheduled cron jobs on TravisCI? I'd like to have at least a weekly run in order to check that no other library break units. And I noticed that the scripts don't work for my fork on GitHub. It needs to detect that and add a remote to the submodule in the Boost checkout somehow. Do you have any plans for that? Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! * voice: ++49 4257 300 ! Fährstraße 1 * fax : ++49 4257 300 ! 31609 Balge/Sebbenhausen * jhunold(a)gmx.eu ! Germany

2 1

[Beast] std::to_string not available in gcc 4.8?
by Vinnie Falco 11 Nov '16

11 Nov '16

A user is complaining that std::to_string is not available in their distribution. Beast doesn't support toolchains that don't have full support for C++11. However, if this is the only thing missing maybe I can just use a Boost equivalent to get him up and running. I see boost::to_string in <boost/exception/to_string.hpp> is that a public interface? It doesn't look documented. What do folks typically do when they want to call std::to_string on an unsupported distribution?

4 4

libs/regex/build/Jamfile.v2 missing "import testing"
by Peter Dimov 10 Nov '16

10 Nov '16

In a partial installation (not all submodules installed), running b2 in the Boost root says: libs\regex\build\Jamfile.v2:93: in modules.load ERROR: rule "unit-test" unknown in module "Jamfile<D:\tmp2\boost-root\libs\regex\build>". because libs/regex/build/Jamfile.v2 uses the unit-test rule, defined in testing.jam, but does not have "import testing ;". Adding that line fixes it. I've no idea why it works in a full installation - probably because someone else imports testing first before it gets to regex.

2 1

[process] Formal review results
by Antony Polukhin 09 Nov '16

09 Nov '16

The Boost.Process library is accepted. Congratulations to Klemens, and thanks to everybody who reviewed the library! The votes were as follows: + Johannes - Niall Douglas +/- Tom Kent +/- Nat Goodspeed + Paul A. Bristow +/- Bjorn Reese + Edward Diener + Boris Schäling +/- Raphaël Londeix + Klaim - Joël Lamotte Legend: + accept - decline +/- accept conditionally Separate thanks to Gavin Lambert for providing a solution to deadlocking problem. Additional thanks to Nat Goodspeed, Niall Douglas, Gavin Lambert, Raphaël Londeix and Bjorn Reese for spending significant amount of time on discussing, reviewing or testing the library. -- Quick review summary -- During the review multiple technical issues were raised and as far as I know, Klemens already working on them. The biggest win of the review - is a fix for possible deadlock with pipes. The most controversial part of the library is it's interface. Many comments were addressing: * passing arguments * starting process right in `system(...)` function Current approaches have benefits and drawbacks, other approaches have different benefits and drawbacks. At this point some people agree that the current approach is the lesser evil. Reviewers also wished to have better/richer documentation. P.S.: Please continue with the discussion and report issues that would help to improve the library. -- Best regards, Antony Polukhin

3 3