2

I am trying to write a boost::process-based program that is able ambiguously redirect input, output and error streams based on whether to not a file to redirect them into is defined, but I'm struggling to figure out how to approach it:

#include <string>
#include <boost/process.hpp>

int main(){
  std::string in;
  std::string out("out.txt");

  namespace bp = boost::process;

  bp::child c(bp::exe("test.exe"), bp::std_in < (in.empty() ? stdin : in),  bp::std_out > (out.empty() ? stdout : out)); // error

  return 0;
}

The ternary operator won't work cause the types are incompatible, but I don't know how to achieve this. I explored boost::variant and boost::any but to no avail.

Community
  • 1
  • 1
Jack Avante
  • 1,405
  • 1
  • 15
  • 32
  • The code is merely an example of concept, not my actual code. I fixed the missing parenthesis, though I do believe my question is valid and reasonable – Jack Avante Jan 23 '21 at 15:07
  • 1
    May I suggest dropping the word "Obviously"? That word rarely adds anything to a question, and it often -- not in this case, but often -- is used to slip misinformation into a question. Plus, if you don't call this fact "obvious", you won't accidentally belittle someone who does not see the different types right away. Just overall a better impression without that word. – JaMiT Jan 23 '21 at 18:08

2 Answers2

1

Boost has chosen a notation that is intuitive to someone familiar with redirection on the command line. However, remember that this is C++. The < and > symbols are still operators; they did not become command-line arguments. The code bp::std_out > stdout is an expression. It evaluates to an object of some sort that records where the standard output should go.


One trick for adapting to different types is to adjust the point where the condition is made. Instead of conditionally selecting the argument to operator< and operator>, conditionally select the argument to the bp::child constructor:

bp::child c(bp::exe("test.exe"),
            in.empty() ? (bp::std_in  < stdin) : (bp::std_in  < in),
            out.empty() ? (bp::std_out > stdout) : (bp::std_out > out));

This would be more obvious if it weren't for operator overloading. Even though both < symbols in the above look the same, they name different operators because the types of their operands are different. Your situation is essentially needing to call either f(bp::std_in, stdin) or g(bp::std_in, in), and there is no way to merge these calls with the ternary operator. It's confusing just because instead of f and g, the operator names are < and <.

You might want to use helper variables to make the code easier to read:

auto in_stream  =  in.empty() ? (bp::std_in  < stdin)  : (bp::std_in  < in);
auto out_stream = out.empty() ? (bp::std_out > stdout) : (bp::std_out > out);

bp::child c(bp::exe("test.exe"), in_stream, out_stream);
JaMiT
  • 14,422
  • 4
  • 15
  • 31
  • That's fantastic! I didn't realize that `bp::std_in < stdin` actually creates a variable that can be used as a stream for the process. Thank you! – Jack Avante Jan 23 '21 at 18:21
  • Though, my compiler seems to fail to compile the code, stating `cannot convert argument 1 from '_Ty' to 'const boost::filesystem::path &'` – Jack Avante Jan 23 '21 at 18:26
  • 1
    @JackAvante Your `_Ty` looks like a template parameter; if so, the full error message should identify to what `_Ty` resolves, which would be the first step in debugging. It's *probably* a separate issue, though, since my compiler is able to process this code. – JaMiT Jan 23 '21 at 18:55
  • I see now. I tried to replace stdin with bp::close but that seems to not convert correctly. Is there a way to do this with bp::close as well, or similar?.. or can I just use an empty string? – Jack Avante Jan 23 '21 at 19:08
  • @JackAvante I'm not sure. If you get the same error with normal use (not using conditionals), you probably have a new StackOverflow question. – JaMiT Jan 23 '21 at 19:22
1

There is an issue with the interface being statically typed and variadic.[¹]

In general the various keyword parameters are not guaranteed to be of compatible types, so ternaries like cond? keyword1 : keyword2 will break down pretty quickly due to type mismatches.

Also, sometimes you will want to dynamically add/remove options.

In my own production code I often opt for traditional control flow and a bit of code duplication, so like

if (condition)
    child = bp::child(/* params 1st style */);
else
    child = bp::child(/* params 2nd style */);

Of course with many option this leads to combinatoric explosion which is hard to maintain. You can trick it using a more elaborate system of variadic lambdas to combine the keyword arguments.

boost::asio::io_service ios;
boost::asio::deadline_timer deadline(ios);

bp::group process_group;

bp::async_pipe input(ios);
std::future<std::string> output, error;

if (_working_directory.empty())
    _working_directory = ".";

auto on_exit = [this, &deadline](int exit, std::error_code ec) {
    if (!_command_timed_out) {
        _exit_code = exit;
    }
    deadline.cancel();
    if (ec) log(LOG_WARNING) << "Child process returned " << ec.message();
    else    log(LOG_DEBUG)   << "Child process returned";
};

auto launcher = [](auto&&... args) { return bp::child(std::forward<decltype(args)>(args)..., bp::posix::fd.restrict_inherit()); };

auto redirect_out = [&](auto f) {
    return [&](auto&&... args) {
        if (_discard_output) {
            if (_redirected_output_fd != -1 && !_redirected_output_fname.empty()) {
                log(LOG_ERR) << "Ignoring output redirection with set_discard_output. This is a bug.";
            }
            return f(std::forward<decltype(args)>(args)..., bp::std_out > bp::null, bp::std_err > bp::null);
        }

        if (_redirected_output_fd != -1 && !_redirected_output_fname.empty()) {
            log(LOG_WARNING) << "Conflicting output redirection, ignoring filename with descriptor";
        }

        if (_redirected_output_fd != -1) {
            return f(std::forward<decltype(args)>(args)..., bp::posix::fd.bind(1, _redirected_output_fd), bp::std_err > error);
        }

        return _redirected_output_fname.empty()
            ? f(std::forward<decltype(args)>(args)..., bp::std_out > output,                   bp::std_err > error)
            : f(std::forward<decltype(args)>(args)..., bp::std_out > _redirected_output_fname, bp::std_err > error);
    };
};

bp::environment bp_env = boost::this_process::environment();
for (auto& p : _env)
    bp_env[p.first] = p.second;

auto c = redirect_out(launcher)(_command_path, _args,
        process_group,
        bp::std_in < input,
        bp::start_dir(_working_directory),
        bp_env,
        ios, bp::on_exit(on_exit)
    );

if (_time_constraint) {
    deadline.expires_from_now(*_time_constraint);
    deadline.async_wait([&](boost::system::error_code ec) {
        if (ec != boost::asio::error::operation_aborted) {
            if (ec) {
                log(LOG_WARNING) << "Unexpected condition in CommandRunner deadline: " << ec.message();
            }

            _command_timed_out = true;
            _exit_code = 1;
            ::killpg(process_group.native_handle(), SIGTERM);

            deadline.expires_from_now(3s); // grace time
            deadline.async_wait([&](boost::system::error_code ec) { if (!ec) process_group.terminate(); }); // timed out
        }
    });
}

boost::asio::async_write(input, bp::buffer(_stdin_data), [&input](auto ec, auto bytes_written){
    if (ec) {
        log(LOG_WARNING) << "Standard input rejected: " << ec.message() << " after " << bytes_written << " bytes written";
    }
    may_fail([&] { input.close(); });
});

ios.run();

if (output.valid()) _stdout_str = output.get();
if (error.valid())  _stderr_str = error.get();

// make sure no grand children survive
if (process_group && process_group.joinable() && !process_group.wait_for(1s))
    process_group.terminate();

Of course you should add exception handling and adapt to your needs.


[¹] Don't get me started. To me this is the mistake of over-engineering. In the presence of the process-exec overhead there is no performance argument to make up for the added complexity and countless bugs [I've seen a few over the years]. There are essential things I don't know how to do with Boost Process.

Of course, the library is still useful so I'll limit my rant :) I felt had some karma to burn today

sehe
  • 374,641
  • 47
  • 450
  • 633
  • Great answer, and I completely agree. Honestly, I Boost.Process 0.5 is a little better than the 0.6 version in my opinion, as it allowed a bit more freedom and efficiency. – Jack Avante Jan 24 '21 at 09:56