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-30 21:13:02

Am 31.10.2016 um 01:18 schrieb Gavin Lambert:
> On 31/10/2016 04:39, Klemens Morgenstern wrote:
>> Am 30.10.2016 um 14:21 schrieb Bjorn Reese:
>>> The bp::child object should not execute in the constructor, but rather
>>> via operator(). That way we can use the named parameter idiom to build
>>> up redirections and similar modifications. For example:
>>>
>>>   auto compile = bp::child("c++").output(bp::null);
>>>   compile("main.cpp"); // Executes
>
> +1 for me for that style too.
>
>> I personally don't like that approach though it certainly would work. I
>> think it's partly a matter of taste, here's my (C) association:
>>
>> struct X {int std_in; const char* exe;};
>> X x{.std_in = 2, exe ="stuff"};
>>
>> I.e. your setting properties.
>>
>> Now the problem I see is this: what happens here?
>>
>> auto compile = bp::child("c++");
>> compile.output(bp::null);
>> compile("main.cpp");
>> compile.env({"PATH", "/foo"});
>
> The fourth call should fail (and probably throw), since it's 
> impossible to change the environment after the process has been 
> started.  Otherwise that should work as you'd expect.
So you'd be replacing a compile-time error (or to be precise: an 
interface constraint) with a runtime-error?
>
> One of the great advantages of this style of construction is that it 
> permits conditionals in a fairly natural fashion:
>
> auto compile = bp::child("c++");
> if (quiet)
>     compile.output(bp::null);
> compile.arg("-c");
> if (debug)
>     compile.arg("-D_DEBUG");
> compile.args("-DFOO", filename);
> for (auto lib : libraries)
>     compile.arg("-l" + lib);
> auto child = compile();
>
> or something like that.  Or you could have a factory function that 
> returns the un-executed builder to the caller to add additional 
> parameters before finally actually executing it, which aids in 
> composability.
Ou, that's even worse, with a chain as proposed by Bjorn, it could've at 
least be a compile-time construction, but you want that actually all 
built at runtime. This discussion was held many times before, so please 
don't take this as a personal attack, but here are my reasons this would 
be the worst design decisions:
It would incurr a runtime-overhead, require you to include and compile 
literally everything you may use (currently if unused boost.asio is only 
forward-declared if you don't include async.hpp), require more memory, 
the usage of variants and is not extensible at all. Just consider all 
the possible features in the library: every single one of them has to be 
a member of the child-class at all them. And of course not having a 
property does not only mean, that you don't have to store a object of 
some sort (e.g. environment), but in severla cases also will skip one 
(or several) function calls (e.g. dup2), which in my opinion is best 
solved at compile-time. And then, after you launched the thing, the 
average user will be utterly confused, because he assumes he can use the 
setters after the process has launched.
It might look more natural but it just doesn't solve the problem at 
hand; you have an idea of what you would want to do with the library and 
you know how that should look. But I cannot assume I know every 
use-case, that's why there are so many parameters (and they might become 
more in future versions, i don't know). If you want you solution it will 
take about 30 lines to implement it using boost.process.

>> So in order to do that, we'd need a child_builder class, that can be
>> converted, similar to boost.format. I consider this style pre C++11,
>> i.e. before variadics. Now granted: it seems not very C++ish, but it's a
>> very unique problem: most properties are inaccesible after you launched
>> the process, which is the reason they are not members.
>
> Variadics are cool, but they contribute to code bloat, since each 
> unique sequence of parameter types requires generating a new overload, 
> and they require templates, which precludes private implementation.
Well I trust my optimizer with the code-bloat; if you have several 
similar use-cases you can just do that exactly one thing: build a small 
class, which constructs your process; if you have something that does 
not change a type (like args), that would not cause a problem; and if 
you want to optimize that, you can just use "extern template".
>
> For something like this, using them seems like the wrong style to me, 
> and a fluent builder seems like a better approach.  But this path 
> might contain bikesheds.
>
> On a related note: given a variable number of arguments in a 
> container, how would I pass this to the existing launch functions?  I 
> don't see an example of this in the docs, and this seems like the most 
> common case.
>
It has an extra overload for std::vector, and if no other overload is 
known it tries to use it as a range. I.e. this would work:

std::list<std::string> args;
bp::args+=args;

There isn't any example of this, but it's written in the reference: 
http://klemens-morgenstern.github.io/process/boost/process/args.html#namespaceboost_1_1process_1args_set_var_value
>
>
> _______________________________________________
> Unsubscribe & other changes: 
> http://lists.boost.org/mailman/listinfo.cgi/boost