Subject: Re: [boost] [process] Formal Review starts today, 27 October
From: Klemens Morgenstern (klemens.morgenstern_at_[hidden])
Date: 2016-11-12 08:22:24
Am 12.11.2016 um 13:35 schrieb Lee Clagett:
> On Thu, 10 Nov 2016 10:05:00 +0100
> Klemens Morgenstern <klemens.morgenstern_at_[hidden]> wrote:
>
>> [...]
>> [...]
>> [...]
>>> What I noticed in the source is that some of the properties require
>>> special logic because they are manipulating other properties
>>> instead of the "executor". For example, the `shell` property is
>>> manipulating the path and arg properties but nothing else.
>>>
>>> Earlier I mentioned on working out a loose way of specifying a
>>> `property`. I think such a specification would not consider
>>> properties that only manipulate other properties to be a property
>>> (since it could do that separately from the `child` constructor).
>> Well that depends on how you model it, I consider the question if a
>> process is launched through the shell or not to be a property of this;
>> it must be set prior to launching and it affects how it runs. There
>> is no need to model this exactly after the excutor; that's is
>> actually not possible.
> Yes, whether a process is being launched in a shell must be set before
> starting the process, but this is irrelevant. Is `shell` a property of
> the `child` or a property of the path / arguments pair? In other words,
> does the internal logic of the `child` constructor actually need to know
> whether its launching a shell or just some other executable? The
> implementation would suggest that `child` does not need to know.
Well the child doesn't know of any of the properties you pass it...
>
>> Also it does not modify a property, but rather modifies an
>> initializer, which is build from several properties.
>>
>> So do I understand you correctly, that you would want this syntax?
>>
>> bp::system(cmd="foo");
>> bp::system(shell="foo");
>>
>> If so - why? Why should'n you be able to launch the same process on
>> it's own or through the shell? It just makes more sense to me to
>> enable the shell as an additional feature.
> You can launch the process on its own or through the shell with that
> syntax. Its just that `shell` is modifying the path + args properties
> given to the `child`.
I know, that's why there's shell, like this:
bp::system(cmd="foo", bp::shell);
bp::system(exe="../foo", "--bar", bp::shell);
So obivously the question isn't why is this useful, but why is your
syntax better?
>>> Admittedly, I did forget about that case - its probably best to
>>> follow `std::thread` closely and terminate the current process if
>>> not `join`ed or `detach`ed. I don't there is anything wrong with a
>>> function returning a `child`, provided the behavior of ignoring the
>>> `child` is well documented and defined. However, there is little
>>> difference to the code snippet above and:
>>>
>>> bp::child c1(bp::exec("ls", "-la", "/"), bp::stdout(my_pipe1));
>>> bp::child c2(bp::shell("ls", "-la", "/"), bp::stdout(my_pipe2));
>>> bp::child c3("ls", {"-la, "/"}, bp::stdout(my_pipe3));
>>>
>>> Where the constructor in `c3` forwards to the constructor in `c1`;
>>> `c2` uses the `bp::exec` object returned by `bp::shell`; `c1` uses
>>> the `bp::exec` property to set the path and args of the executor.
>>>
>> Ok, so I consider exe/args, cmd and shell different properties, but
>> in the end they all get put into one initializer.
>> (https://github.com/klemens-morgenstern/boost-process/blob/develop/include/boost/process/detail/windows/basic_cmd.hpp#L92)
>> So if anything this could be made public, though at the time the
>> values get put into the exe-args initializrs they are already parsed
>> and formatted. I really don't see the benefit.
> The benefit is it removes the special internal logic for a few specific
> properties and makes what the code is _actually_ doing more clear. I've
> also been pushing for a more formal specification of `properties`
> because some of the IO variants are doing too much in `child`. The
> constructor for `std::thread` does not throw once the thread is
> created, but what about some of the IO properties for `process::child`?
> Several of the `on_exec`/`on_success` hooks are doing various calls that
> can throw - does `child` block internally in the constructor until the
> other process completes? Does it detach? Terminate? And why is so much
> being done inside of the `child` constructor when the `child` should
> only need to know the pipe handles for communication. I'm mainly
> thinking of the code for piping to future or mutable buffer.
The child-class does not know of anything there, the child(...) ctor
actually just forwards to values to a launch function. It's just a form
of RAII, since I think that makes sense. It's equivalent to
boost::thread([](int), 42);
If there's an error before the launch it will not launch, if afterwards
it will terminate the child immediatly.