From 4bed07281216121334885e4a882b9e220e309434 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Tue, 20 Nov 2018 22:07:59 +1100 Subject: [PATCH 01/17] adding execute and check_output of the new API --- Makefile | 3 +- demo.cpp | 34 +-- subprocess.hpp | 477 ++++++++++++++++++++++-------------- test.cpp | 207 +++++++++++----- test_programs/.gitignore | 1 + test_programs/Makefile | 14 ++ test_programs/README | 1 + test_programs/print_env.cpp | 16 ++ 8 files changed, 481 insertions(+), 272 deletions(-) create mode 100644 test_programs/.gitignore create mode 100644 test_programs/Makefile create mode 100644 test_programs/README create mode 100644 test_programs/print_env.cpp diff --git a/Makefile b/Makefile index 221007e..c9ec6e8 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,6 @@ CXX=g++ -CXXFLAGS=-g -std=c++11 -# temporary for pnappa +CXXFLAGS=-g -std=c++11 -Wall -pedantic LIBS=-lpthread .PHONY: all clean diff --git a/demo.cpp b/demo.cpp index ccbf445..f63b4ef 100644 --- a/demo.cpp +++ b/demo.cpp @@ -9,8 +9,13 @@ void echoString(std::string in) { } int main() { - // execute bc and pass it some equations + std::vector args = {}; std::list inputs = {"1+1\n", "2^333\n", "32-32\n"}; + std::vector env = {}; + subprocess::execute("/usr/bin/bc", args.begin(), args.end(), inputs.begin(), inputs.end(), echoString, env.begin(), env.end()); + + // execute bc and pass it some equations + inputs = {"1+1\n", "2^333\n", "32-32\n"}; subprocess::execute("/usr/bin/bc", {}, inputs, echoString); // grep over some inputs @@ -19,19 +24,18 @@ int main() { // execute a process and extract all lines outputted inputs.clear(); // provide no stdin - int status; - std::vector vec = subprocess::checkOutput("/usr/bin/time", {"sleep", "1"}, inputs, status); + std::vector vec = subprocess::check_output("/usr/bin/time", {"sleep", "1"}, inputs); + //std::vector vec = subprocess::check_output("/usr/bin/time", {"sleep", "1"}, inputs, status); for (const std::string& s : vec) { std::cout << "output: " << s << '\t'; std::cout << "line length:" << s.length() << std::endl; } - std::cout << "process finished with an exit code of: " << status << std::endl; - // execute sleep asynchronously, and block when needing the output - std::future futureStatus = subprocess::async("/bin/sleep", {"3"}, inputs, [](std::string) {}); - // if this wasn't async, this line wouldn't print until after the process finished! - std::cout << "executing sleep..." << std::endl; - std::cout << "sleep executed with exit status: " << futureStatus.get() << std::endl; + //// execute sleep asynchronously, and block when needing the output + //std::future futureStatus = subprocess::async("/bin/sleep", {"3"}, inputs, [](std::string) {}); + //// if this wasn't async, this line wouldn't print until after the process finished! + //std::cout << "executing sleep..." << std::endl; + //std::cout << "sleep executed with exit status: " << futureStatus.get() << std::endl; // simulate pipes between programs: lets launch cat to provide input into a grep process! // Note! This will read all output into a single vector, then provide this as input into the second @@ -39,14 +43,14 @@ int main() { // If you want a function that isn't as memory intensive, consider streamOutput, which provides an // iterator interface inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; - vec = subprocess::checkOutput("/bin/cat", {}, inputs, status); + vec = subprocess::check_output("/bin/cat", {}, inputs); inputs = std::list(vec.begin(), vec.end()); subprocess::execute("/bin/grep", {"-i", "^Hello, world$"}, inputs, echoString); // stream output from a process - inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; - subprocess::ProcessStream ps("/bin/grep", {"-i", "^Hello, world$"}, inputs); - for (std::string out : ps) { - std::cout << "received: " << out; - } + //inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; + //subprocess::ProcessStream ps("/bin/grep", {"-i", "^Hello, world$"}, inputs); + //for (std::string out : ps) { + //std::cout << "received: " << out; + //} } diff --git a/subprocess.hpp b/subprocess.hpp index a8c630e..1b3760e 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -4,11 +4,16 @@ #include #include #include +#include #include #include #include #include #include +#include +#include +#include +#include // unix process stuff #include @@ -179,8 +184,46 @@ class Process { pid_t pid; TwoWayPipe pipe; + const std::string commandPath; + std::vector processArgs; + std::vector envVariables; + + // construct the argument list (unfortunately, the C api wasn't defined with C++ in mind, so + // we have to abuse const_cast) see: https://stackoverflow.com/a/190208 + // this returns a null terminated vector that contains a list of non-const char ptrs + template + static std::vector toNullTerminatedCharIterable(Iter begin, Iter end) { + // TODO: insert test to check if our iterable store strings..? + // well it'll fail in the push_back stage anyway + + std::vector charArrayPlex; + + // the process name must be first for execv + //charArrayPlex.push_back(const_cast(input.c_str())); + for (auto it = begin; it != end; ++it) { + charArrayPlex.push_back(const_cast((*it).c_str())); + } + // must be terminated with a nullptr for execv + charArrayPlex.push_back(nullptr); + + return charArrayPlex; + } + public: - Process() = default; + template + Process(const std::string& commandPath, ArgIt argBegin, ArgIt argEnd, EnvIt envBegin, EnvIt envEnd) : commandPath(commandPath) { + pid = 0; + pipe.initialize(); + + // generate a vector that is able to be passed into exec for the process arguments + processArgs = toNullTerminatedCharIterable(argBegin, argEnd); + // process args must start with the processes name + processArgs.insert(processArgs.begin(), const_cast(commandPath.c_str())); + + // ditto for the env variables + envVariables = toNullTerminatedCharIterable(envBegin, envEnd); + envVariables.insert(envVariables.begin(), const_cast(commandPath.c_str())); + } /** * Starts a seperate process with the provided command and @@ -191,24 +234,8 @@ class Process { * @return TODO return errno returned by child call of execv * (need to use the TwoWayPipe) * */ - template - void start(const std::string& commandPath, Iterable& args) { - pid = 0; - pipe.initialize(); - // construct the argument list (unfortunately, - // the C api wasn't defined with C++ in mind, so - // we have to abuse const_cast) see: - // https://stackoverflow.com/a/190208 - std::vector cargs; - // the process name must be first for execv - cargs.push_back(const_cast(commandPath.c_str())); - for (const std::string& arg : args) { - cargs.push_back(const_cast(arg.c_str())); - } - // must be terminated with a nullptr for execv - cargs.push_back(nullptr); - - pid = fork(); + void start() { + this->pid = fork(); // child if (pid == 0) { pipe.setAsChildEnd(); @@ -217,12 +244,9 @@ class Process { // in case the parent dies prctl(PR_SET_PDEATHSIG, SIGTERM); - execv(commandPath.c_str(), cargs.data()); - // Nothing below this line - // should be executed by child - // process. If so, it means that - // the execl function wasn't - // successfull, so lets exit: + execvpe(commandPath.c_str(), processArgs.data(), envVariables.data()); + // Nothing below this line should be executed by child process. If so, it means that + // the execl function wasn't successfull, so lets exit: exit(1); } pipe.setAsParentEnd(); @@ -256,123 +280,196 @@ class Process { } }; +/* begin https://stackoverflow.com/a/16974087 */ +// a way to provide optional iterators to a function that do nothing +struct DevNull { + template T& operator=(T const&) { } + template operator T&() { static T dummy; return dummy; } +}; + +struct DevNullIterator { + DevNull operator*() const { return DevNull();} + DevNullIterator& operator++() { return *this; } + DevNullIterator operator++(int) const { return *this; } + DevNullIterator* operator->() { return this; } + // always equivalent (a for loop should instantly terminate!) + bool operator==(DevNullIterator&) const { return true; } + bool operator!=(DevNullIterator&) const { return false; } +}; +/* end https://stackoverflow.com/a/16974087 */ + +/* hm, I copied this from somewhere, dunno where */ +template +struct is_iterator { + static char test(...); + + template ::difference_type, + typename=typename std::iterator_traits::pointer, + typename=typename std::iterator_traits::reference, + typename=typename std::iterator_traits::value_type, + typename=typename std::iterator_traits::iterator_category + > static long test(U&&); + + constexpr static bool value = std::is_same())),long>::value; +}; +/* begin https://stackoverflow.com/a/29634934 */ +namespace detail +{ + // To allow ADL with custom begin/end + using std::begin; + using std::end; + + template + auto is_iterable_impl(int) + -> decltype ( + begin(std::declval()) != end(std::declval()), // begin/end and operator != + ++std::declval()))&>(), // operator ++ + *begin(std::declval()), // operator* + std::true_type{}); + + template + std::false_type is_iterable_impl(...); + +} + +template +using is_iterable = decltype(detail::is_iterable_impl(0)); +/* end https://stackoverflow.com/a/29634934 */ + +static std::list dummyVec = {}; + /** * Execute a subprocess and optionally call a function per line of stdout. * @param commandPath - the path of the executable to execute, e.g. "/bin/cat" - * @param commandArgs - the extra arguments for an executable e.g. {"argument 1", "henlo"} - * @param stdinInput - a list of inputs that will be piped into the processes' stdin + * @param firstArg - the begin iterator for a list of arguments + * @param lastArg - the end iterator for a list of arguments + * @param stdinBegin - an InputIterator to provide stdin + * @param stdinEnd - the end of the InputIterator range for stdin * @param lambda - a function that is called with every line from the executed process (default NOP function) - * @param env - a list of environment variables that the process will execute with (default nothing) + * @param envBegin - the begin of an iterator containing process environment variables to set + * @param envEnd - the end of the env iterator */ -int execute(const std::string& commandPath, const std::vector& commandArgs, const std::vector& stdinInput, const std::function& lambda = [](std::string){}, const std::vector& env = {}); +template::iterator, + class EnvIt = std::list::iterator, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> +int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, + StdinIt stdinBegin = dummyVec.begin(), StdinIt stdinEnd = dummyVec.end(), + const std::function& lambda = [](std::string){}, + EnvIt envBegin = dummyVec.begin(), EnvIt envEnd = dummyVec.end()) { + + Process childProcess(commandPath, firstArg, lastArg, envBegin, envEnd); + childProcess.start(); + + // write our input to the processes stdin pipe + for (auto it = stdinBegin; it != stdinEnd; ++it) { + childProcess.write(*it); + } + // close the stdin for the process + childProcess.sendEOF(); + + // iterate over each line output by the child's stdout, and call the functor + std::string processOutput; + while ((processOutput = childProcess.readLine()).size() > 0) { + lambda(processOutput); + } + + return childProcess.waitUntilFinished(); +} /** * Execute a subprocess and optionally call a function per line of stdout. * @param commandPath - the path of the executable to execute, e.g. "/bin/cat" - * @param commandArgs - the extra arguments for an executable e.g. {"argument 1", "henlo"} - * @param stdinBegin - an InputIterator to provide stdin - * @param stdinEnd - the end of the InputIterator range for stdin + * @param commandArgs - the extra arguments for an executable e.g. {"argument 1", "henlo"} (default no arguments) + * @param stdinInput - a list of inputs that will be piped into the processes' stdin (default no stdin) * @param lambda - a function that is called with every line from the executed process (default NOP function) * @param env - a list of environment variables that the process will execute with (default nothing) */ -template -int execute(const std::string& commandPath, const std::vector& commandArgs, InputIt stdinBegin, InputIt stdinEnd, const std::function& lambda = [](std::string){}, const std::vector& env = {}); +template, + class StdinIterable = std::vector, + class EnvIterable = std::vector, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> +int execute(const std::string& commandPath, const ArgIterable& commandArgs = {}, + const StdinIterable& stdinInput = {}, const std::function& lambda = [](std::string){}, + const EnvIterable& env = {}) { + return execute(commandPath, commandArgs.begin(), commandArgs.end(), stdinInput.begin(), stdinInput.end(), lambda, env.begin(), env.end()); +} /** * Execute a subprocess and retrieve the output of the command * @param commandPath - the path of the executable to execute, e.g. "/bin/cat" - * @param commandArgs - the extra arguments for an executable e.g. {"argument 1", "henlo"} - * @param stdinInput - a list of inputs that will be piped into the processes' stdin - * @param env - a list of environment variables that the process will execute with (default nothing) + * @param firstArg - the begin iterator for a list of arguments + * @param lastArg - the end iterator for a list of arguments + * @param stdinBegin - an InputIterator to provide stdin + * @param stdinEnd - the end of the InputIterator range for stdin + * @param lambda - a function that is called with every line from the executed process (default NOP function) + * @param envBegin - the begin of an iterator containing process environment variables to set + * @param envEnd - the end of the env iterator */ -std::vector check_output(const std::string& commandPath, const std::vector& commandArgs, const std::vector& stdioInput, const std::vector& env = {}); +template::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> +std::vector check_output(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, + StdinIt stdinBegin = DevNullIterator(), StdinIt stdinEnd = DevNullIterator(), + EnvIt envBegin = DevNullIterator(), EnvIt envEnd = DevNullIterator()) { + std::vector retVec; + //int status = execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { retVec.push_back(std::move(s)); }, envBegin, envEnd); + execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { retVec.push_back(std::move(s)); }, envBegin, envEnd); + return retVec; +} /** * Execute a subprocess and retrieve the output of the command * @param commandPath - the path of the executable to execute, e.g. "/bin/cat" * @param commandArgs - the extra arguments for an executable e.g. {"argument 1", "henlo"} - * @param stdinBegin - an InputIterator to provide stdin - * @param stdinEnd - the end of the InputIterator range for stdin + * @param stdinInput - a list of inputs that will be piped into the processes' stdin * @param env - a list of environment variables that the process will execute with (default nothing) */ -template -std::vector check_output(const std::string& commandPath, const std::vector& commandArgs, InputIt stdioBegin, InputIt stdioEnd, const std::vector& env = {}); - -// TODO: what if the process terminates? consider error handling potentials... -class ProcessStream { - public: - ProcessStream(const std::string& commandPath, const std::vector& commandArgs); - - // write a line to the subprocess's stdin - void write(const std::string& inputLine); - // read a line and block until received (or until timeout reached) - template - std::string read(std::chrono::duration timeout=-1); - // if there is a line for reading - template - bool ready(std::chrono::duration timeout=0); - - ProcessStream& operator<<(const std::string& inputLine); - ProcessStream& operator>>(std::string& outputLine); -}; - -/** - * Execute a process, inputting stdin and calling the functor with the stdout - * lines. - * @param commandPath - an absolute string to the program path - * @param commandArgs - a vector of arguments that will be passed to the process - * @param stringInput - a feed of strings that feed into the process (you'll typically want to end them with a - * newline) - * @param lambda - the function to execute with every line output by the process - * @return the exit status of the process - * */ -int execute(const std::string& commandPath, const std::vector& commandArgs, - std::list& stringInput /* what pumps into stdin */, - std::function lambda) { - Process childProcess; - childProcess.start(commandPath, commandArgs); - - // while our string queue is working, - while (!stringInput.empty()) { - // write our input to the process's stdin pipe - std::string newInput = stringInput.front(); - stringInput.pop_front(); - childProcess.write(newInput); - } - - childProcess.sendEOF(); - - // iterate over each line output by the child's stdout, and call - // the functor - std::string input; - while ((input = childProcess.readLine()).size() > 0) { - lambda(input); - } - - return childProcess.waitUntilFinished(); +template, + class StdinIterable = std::vector, + class EnvIterable = std::vector, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> +std::vector check_output(const std::string& commandPath, const ArgIterable& commandArgs = {}, + const StdinIterable& stdinInput = {}, const EnvIterable& env = {}) { + return check_output(commandPath, commandArgs.begin(), commandArgs.end(), stdinInput.begin(), stdinInput.end(), env.begin(), env.end()); } -/* convenience fn to return a list of outputted strings */ -std::vector checkOutput(const std::string& commandPath, - const std::vector& commandArgs, - std::list& stringInput /* what pumps into stdin */, int& status) { - std::vector retVec; - status = execute( - commandPath, commandArgs, stringInput, [&](std::string s) { retVec.push_back(std::move(s)); }); - return retVec; -} +//// TODO: what if the process terminates? consider error handling potentials... +//class ProcessStream { +// public: +// ProcessStream(const std::string& commandPath, const std::vector& commandArgs); +// +// // write a line to the subprocess's stdin +// void write(const std::string& inputLine); +// // read a line and block until received (or until timeout reached) +// template +// std::string read(std::chrono::duration timeout=-1); +// // if there is a line for reading +// template +// bool ready(std::chrono::duration timeout=0); +// +// ProcessStream& operator<<(const std::string& inputLine); +// ProcessStream& operator>>(std::string& outputLine); +//}; /* spawn the process in the background asynchronously, and return a future of the status code */ -std::future async(const std::string commandPath, const std::vector commandArgs, - std::list stringInput, std::function lambda) { - // spawn the function async - we must pass the args by value into the async lambda - // otherwise they may destruct before the execute fn executes! - // whew, that was an annoying bug to find... - return std::async(std::launch::async, - [&](const std::string cp, const std::vector ca, std::list si, - std::function l) { return execute(cp, ca, si, l); }, - commandPath, commandArgs, stringInput, lambda); -} +//std::future async(const std::string commandPath, const std::vector commandArgs, +// std::list stringInput, std::function lambda) { +// // spawn the function async - we must pass the args by value into the async lambda +// // otherwise they may destruct before the execute fn executes! +// // whew, that was an annoying bug to find... +// return std::async(std::launch::async, +// [&](const std::string cp, const std::vector ca, std::list si, +// std::function l) { return execute(cp, ca, si, l); }, +// commandPath, commandArgs, stringInput, lambda); +//} /* TODO: refactor up this function so that there isn't duplicated code - most of this is identical to the * execute fn execute a program and stream the output after each line input this function calls select to @@ -380,81 +477,81 @@ std::future async(const std::string commandPath, const std::vector& commandArgs, - std::list& stringInput) { - childProcess.start(commandPath, commandArgs); - - // while our string queue is working, - while (!stringInput.empty()) { - // write our input to the - // process's stdin pipe - std::string newInput = stringInput.front(); - stringInput.pop_front(); - childProcess.write(newInput); - } - // now we finished chucking in the string, send - // an EOF - childProcess.sendEOF(); - } - - ~ProcessStream() { - childProcess.waitUntilFinished(); - } - - struct iterator { - ProcessStream* ps; - bool isFinished = false; - // current read line of the process - std::string cline; - - iterator(ProcessStream* ps) : ps(ps) { - // increment this ptr, because nothing exists initially - ++(*this); - } - // ctor for end() - iterator(ProcessStream* ps, bool) : ps(ps), isFinished(true) {} - - const std::string& operator*() const { - return cline; - } - - /* preincrement */ - iterator& operator++() { - // iterate over each line output by the child's stdout, and call the functor - cline = ps->childProcess.readLine(); - if (cline.empty()) { - isFinished = true; - } - return *this; - } - - /* post increment */ - iterator operator++(int) { - iterator old(*this); - ++(*this); - return old; - } - - bool operator==(const iterator& other) const { - return other.ps == this->ps && this->isFinished == other.isFinished; - } - - bool operator!=(const iterator& other) const { - return !((*this) == other); - } - }; - - iterator begin() { - return iterator(this); - } - - iterator end() { - return iterator(this, true); - } -}; +//class ProcessStream { +// Process childProcess; +// +//public: +// ProcessStream(const std::string& commandPath, const std::vector& commandArgs, +// std::list& stringInput) { +// childProcess.start(commandPath, commandArgs); +// +// // while our string queue is working, +// while (!stringInput.empty()) { +// // write our input to the +// // process's stdin pipe +// std::string newInput = stringInput.front(); +// stringInput.pop_front(); +// childProcess.write(newInput); +// } +// // now we finished chucking in the string, send +// // an EOF +// childProcess.sendEOF(); +// } +// +// ~ProcessStream() { +// childProcess.waitUntilFinished(); +// } +// +// struct iterator { +// ProcessStream* ps; +// bool isFinished = false; +// // current read line of the process +// std::string cline; +// +// iterator(ProcessStream* ps) : ps(ps) { +// // increment this ptr, because nothing exists initially +// ++(*this); +// } +// // ctor for end() +// iterator(ProcessStream* ps, bool) : ps(ps), isFinished(true) {} +// +// const std::string& operator*() const { +// return cline; +// } +// +// /* preincrement */ +// iterator& operator++() { +// // iterate over each line output by the child's stdout, and call the functor +// cline = ps->childProcess.readLine(); +// if (cline.empty()) { +// isFinished = true; +// } +// return *this; +// } +// +// /* post increment */ +// iterator operator++(int) { +// iterator old(*this); +// ++(*this); +// return old; +// } +// +// bool operator==(const iterator& other) const { +// return other.ps == this->ps && this->isFinished == other.isFinished; +// } +// +// bool operator!=(const iterator& other) const { +// return !((*this) == other); +// } +// }; +// +// iterator begin() { +// return iterator(this); +// } +// +// iterator end() { +// return iterator(this, true); +// } +//}; } // end namespace subprocess diff --git a/test.cpp b/test.cpp index 3564bdc..ded2e58 100644 --- a/test.cpp +++ b/test.cpp @@ -9,7 +9,8 @@ #include "subprocess.hpp" -TEST_CASE("basic echo execution", "[subprocess::execute]") { +/* TODO: test all permuations of arguments */ +TEST_CASE("[iterable] basic echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; subprocess::execute("/bin/echo", {"hello"}, inputs, [&](std::string s) { outputs.push_back(s); }); @@ -18,7 +19,7 @@ TEST_CASE("basic echo execution", "[subprocess::execute]") { REQUIRE(outputs.front() == "hello\n"); } -TEST_CASE("no trailing output newline echo execution", "[subprocess::execute]") { +TEST_CASE("[iterable] no trailing output newline echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; subprocess::execute("/bin/echo", {"-n", "hello"}, inputs, [&](std::string s) { outputs.push_back(s); }); @@ -27,7 +28,7 @@ TEST_CASE("no trailing output newline echo execution", "[subprocess::execute]") REQUIRE(outputs.front() == "hello"); } -TEST_CASE("non existent executable", "[subprocess::execute]") { +TEST_CASE("[iterable] non existent executable", "[subprocess::execute]") { // try and run a non-existent executable, what should happen..? std::list inputs; std::vector outputs; @@ -39,7 +40,7 @@ TEST_CASE("non existent executable", "[subprocess::execute]") { REQUIRE(outputs.size() == 0); } -TEST_CASE("stdin execute simple cat", "[subprocess::execute]") { +TEST_CASE("[iterable] stdin execute simple cat", "[subprocess::execute]") { std::list inputs = {"henlo wurld\n", "1,2,3,4\n"}; std::vector outputs; int retval = subprocess::execute("/bin/cat", {}, inputs, [&](std::string s) { outputs.push_back(s); }); @@ -50,7 +51,7 @@ TEST_CASE("stdin execute simple cat", "[subprocess::execute]") { REQUIRE(outputs.at(1) == "1,2,3,4\n"); } -TEST_CASE("stdin execute simple cat no trailing newline for last input", "[subprocess::execute]") { +TEST_CASE("[iterable] stdin execute simple cat no trailing newline for last input", "[subprocess::execute]") { // executing a command with the last one missing a newline still should work the same, as the stdin stream gets closed. std::list inputs = {"henlo wurld\n", "1,2,3,4"}; std::vector outputs; @@ -62,86 +63,162 @@ TEST_CASE("stdin execute simple cat no trailing newline for last input", "[subpr REQUIRE(outputs.at(1) == "1,2,3,4"); } -TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { - // execute bc and pass it some equations - std::list inputs = {"1+1\n", "2^333\n", "32-32\n"}; - // this one is interesting, there's more than one line of stdout for each input (bc line breaks after a certain number of characters) - std::vector output_expected = {"2\n", "17498005798264095394980017816940970922825355447145699491406164851279\\\n", "623993595007385788105416184430592\n", "0\n"}; - int retval; - std::vector out = subprocess::checkOutput("/usr/bin/bc", {}, inputs, retval); - - REQUIRE(retval == 0); - REQUIRE(out.size() == output_expected.size()); - REQUIRE(out == output_expected); -} - -TEST_CASE("asynchronous is actually asynchronous", "[subprocess::async]") { +TEST_CASE("[iterator] basic echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; + std::vector args = {"hello"}; + std::vector env = {}; + int status; - std::atomic isDone(false); - std::future retval = subprocess::async("/usr/bin/time", {"sleep", "3"}, inputs, [&](std::string s) { isDone.store(true); outputs.push_back(s); }); - // reaching here after the async starts, means that we prrroooooobbbaabbbly (unless the user has a very, very slow computer) won't be finished - REQUIRE(isDone.load() == false); - REQUIRE(retval.get() == 0); + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }, env.begin(), env.end()); + REQUIRE(outputs.size() == 1); + // echo appends a newline by default + REQUIRE(outputs.front() == "hello\n"); - // time has different outputs for different OSes, pluuus they will take different times to complete. all we need is some stdout. - REQUIRE(outputs.size() > 0); + outputs.clear(); + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }); + REQUIRE(outputs.size() == 1); + // echo appends a newline by default + REQUIRE(outputs.front() == "hello\n"); + + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end()) ; + REQUIRE(status == 0); + + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end()) ; + REQUIRE(status == 0); + + status = subprocess::execute("/bin/echo", args.begin(), args.end()); + REQUIRE(status == 0); + + status = subprocess::execute("/bin/echo"); + REQUIRE(status == 0); } -TEST_CASE("output iterator contains everything", "[subprocess::ProcessStream]") { - // stream output from a process - std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; - subprocess::ProcessStream ps("/bin/grep", {"-i", "^Hello, world$"}, inputs); - std::vector expectedOutput = {"hello, world\n", "Hello, world\n"}; +/* TODO: make the rest iterators */ +TEST_CASE("[iterator] no trailing output newline echo execution", "[subprocess::execute]") { + std::list inputs; std::vector outputs; - for (std::string out : ps) { - outputs.push_back(out); - } + subprocess::execute("/bin/echo", {"-n", "hello"}, inputs, [&](std::string s) { outputs.push_back(s); }); - REQUIRE(outputs == expectedOutput); + REQUIRE(outputs.size() == 1); + REQUIRE(outputs.front() == "hello"); } -TEST_CASE("output iterator handles empty output", "[subprocess::ProcessStream]") { - std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; - subprocess::ProcessStream ps("/bin/grep", {"-i", "^bingo bango bongo$"}, inputs); - std::vector expectedOutput = {}; +TEST_CASE("[iterator] non existent executable", "[subprocess::execute]") { + // try and run a non-existent executable, what should happen..? + std::list inputs; std::vector outputs; - for (std::string out : ps) { - FAIL("why do we have output!!! - " << out); - outputs.push_back(out); - } + int retval = subprocess::execute("/bin/wangwang", {}, inputs, + [](std::string) { FAIL("this functor should never have been called"); }); - REQUIRE(outputs == expectedOutput); + // process should have failed..? + REQUIRE(retval != 0); + REQUIRE(outputs.size() == 0); } -TEST_CASE("output iterator all operator overload testing", "[subprocess::ProcessStream]") { - // stream output from a process - std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; - subprocess::ProcessStream ps("/bin/grep", {"-i", "Hello, world"}, inputs); - std::list expectedOutput = {"hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; +TEST_CASE("[iterator] stdin execute simple cat", "[subprocess::execute]") { + std::list inputs = {"henlo wurld\n", "1,2,3,4\n"}; + std::vector outputs; + int retval = subprocess::execute("/bin/cat", {}, inputs, [&](std::string s) { outputs.push_back(s); }); - auto beg = ps.begin(); - auto end = ps.end(); + REQUIRE(retval == 0); + REQUIRE(outputs.size() == 2); + REQUIRE(outputs.at(0) == "henlo wurld\n"); + REQUIRE(outputs.at(1) == "1,2,3,4\n"); +} - REQUIRE(beg != end); +TEST_CASE("[iterator] stdin execute simple cat no trailing newline for last input", "[subprocess::execute]") { + // executing a command with the last one missing a newline still should work the same, as the stdin stream gets closed. + std::list inputs = {"henlo wurld\n", "1,2,3,4"}; + std::vector outputs; + int retval = subprocess::execute("/bin/cat", {}, inputs, [&](std::string s) { outputs.push_back(s); }); - REQUIRE(*beg == expectedOutput.front()); - expectedOutput.pop_front(); - REQUIRE(beg != end); + REQUIRE(retval == 0); + REQUIRE(outputs.size() == 2); + REQUIRE(outputs.at(0) == "henlo wurld\n"); + REQUIRE(outputs.at(1) == "1,2,3,4"); +} - ++beg; - REQUIRE(*beg == expectedOutput.front()); - expectedOutput.pop_front(); - REQUIRE(beg != end); - beg++; - REQUIRE(*beg == expectedOutput.front()); - expectedOutput.pop_front(); +TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { + // execute bc and pass it some equations + std::list inputs = {"1+1\n", "2^333\n", "32-32\n"}; + // this one is interesting, there's more than one line of stdout for each input (bc line breaks after a certain number of characters) + std::vector output_expected = {"2\n", "17498005798264095394980017816940970922825355447145699491406164851279\\\n", "623993595007385788105416184430592\n", "0\n"}; + std::vector out = subprocess::check_output("/usr/bin/bc", {}, inputs); - beg++; - REQUIRE(beg == end); - REQUIRE(expectedOutput.size() == 0); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); } +/* tests pending API update */ +//TEST_CASE("asynchronous is actually asynchronous", "[subprocess::async]") { +// std::list inputs; +// std::vector outputs; +// +// std::atomic isDone(false); +// std::future retval = subprocess::async("/usr/bin/time", {"sleep", "3"}, inputs, [&](std::string s) { isDone.store(true); outputs.push_back(s); }); +// // reaching here after the async starts, means that we prrroooooobbbaabbbly (unless the user has a very, very slow computer) won't be finished +// REQUIRE(isDone.load() == false); +// REQUIRE(retval.get() == 0); +// +// // time has different outputs for different OSes, pluuus they will take different times to complete. all we need is some stdout. +// REQUIRE(outputs.size() > 0); +//} +// +//TEST_CASE("output iterator contains everything", "[subprocess::ProcessStream]") { +// // stream output from a process +// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; +// subprocess::ProcessStream ps("/bin/grep", {"-i", "^Hello, world$"}, inputs); +// std::vector expectedOutput = {"hello, world\n", "Hello, world\n"}; +// std::vector outputs; +// for (std::string out : ps) { +// outputs.push_back(out); +// } +// +// REQUIRE(outputs == expectedOutput); +//} +// +//TEST_CASE("output iterator handles empty output", "[subprocess::ProcessStream]") { +// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; +// subprocess::ProcessStream ps("/bin/grep", {"-i", "^bingo bango bongo$"}, inputs); +// std::vector expectedOutput = {}; +// std::vector outputs; +// for (std::string out : ps) { +// FAIL("why do we have output!!! - " << out); +// outputs.push_back(out); +// } +// +// REQUIRE(outputs == expectedOutput); +//} +// +//TEST_CASE("output iterator all operator overload testing", "[subprocess::ProcessStream]") { +// // stream output from a process +// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; +// subprocess::ProcessStream ps("/bin/grep", {"-i", "Hello, world"}, inputs); +// std::list expectedOutput = {"hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; +// +// auto beg = ps.begin(); +// auto end = ps.end(); +// +// REQUIRE(beg != end); +// +// REQUIRE(*beg == expectedOutput.front()); +// expectedOutput.pop_front(); +// REQUIRE(beg != end); +// +// ++beg; +// REQUIRE(*beg == expectedOutput.front()); +// expectedOutput.pop_front(); +// REQUIRE(beg != end); +// +// beg++; +// REQUIRE(*beg == expectedOutput.front()); +// expectedOutput.pop_front(); +// +// beg++; +// REQUIRE(beg == end); +// REQUIRE(expectedOutput.size() == 0); +//} + // TODO: write more test cases (this seems pretty covering, let's see how coverage looks) diff --git a/test_programs/.gitignore b/test_programs/.gitignore new file mode 100644 index 0000000..bdec747 --- /dev/null +++ b/test_programs/.gitignore @@ -0,0 +1 @@ +print_env diff --git a/test_programs/Makefile b/test_programs/Makefile new file mode 100644 index 0000000..0b83418 --- /dev/null +++ b/test_programs/Makefile @@ -0,0 +1,14 @@ +CXX=g++ +CXXFLAGS=-g -std=c++11 -Wall -pedantic +LIBS=-lpthread + +.PHONY: all clean + +all: print_env + +print_env: print_env.cpp + $(CXX) $(CXXFLAGS) print_env.cpp -o print_env $(LIBS) + +clean: + rm -rvf print_env + diff --git a/test_programs/README b/test_programs/README new file mode 100644 index 0000000..bb48469 --- /dev/null +++ b/test_programs/README @@ -0,0 +1 @@ +A bunch of programs that do things so we can unit-test different functionalities diff --git a/test_programs/print_env.cpp b/test_programs/print_env.cpp new file mode 100644 index 0000000..4846c42 --- /dev/null +++ b/test_programs/print_env.cpp @@ -0,0 +1,16 @@ +/** + * Print environment variable values for each arg + * usage: ./print_env SHELL + * -> /usr/bin/bash + */ + +// to allow us to use getenv +#include +#include + +int main(int argc, char* argv[]) { + for (int i = 1; i < argc; ++i) { + char* env_val = getenv(argv[i]); + std::cout << (env_val == nullptr ? "" : env_val) << std::endl; + } +} From 12b05ec36803a19f0a768467ca02b79644a1c40e Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Tue, 20 Nov 2018 22:43:35 +1100 Subject: [PATCH 02/17] renamed test -> check, and wrote execute tests, currently linker errors lol --- .travis.yml | 2 +- Makefile | 9 +++---- README.md | 24 +++++++++-------- subprocess.hpp | 39 +++++++++------------------- test.cpp | 51 ++++++++++++++++++++++++++++++++++--- test_programs/print_env.cpp | 2 +- 6 files changed, 79 insertions(+), 48 deletions(-) diff --git a/.travis.yml b/.travis.yml index e288794..f574468 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,7 +20,7 @@ jobs: - stage: UnitTests os: linux sudo: false - script: make && make test + script: make && make check addons: apt: packages: diff --git a/Makefile b/Makefile index c9ec6e8..ec1f3c6 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ CXXFLAGS=-g -std=c++11 -Wall -pedantic LIBS=-lpthread .PHONY: all clean -all: demo test +all: demo check clean: rm -fv demo test coverage @@ -12,12 +12,11 @@ clean: demo: demo.cpp subprocess.hpp $(CXX) $(CXXFLAGS) demo.cpp -o demo $(LIBS) +check: test + valgrind ./test + test: test.cpp subprocess.hpp $(CXX) $(CXXFLAGS) test.cpp -o test $(LIBS) - # run the testsuite (-s makes it nice and verbose) - # XXX: should we make this verbose? - ./test -s - valgrind ./test coverage: test.cpp subprocess.hpp $(CXX) $(CXXFLAGS) -fprofile-arcs -ftest-coverage test.cpp -o coverage $(LIBS) diff --git a/README.md b/README.md index 8910ff9..04b10e6 100644 --- a/README.md +++ b/README.md @@ -72,14 +72,17 @@ Unfortunately, I am not as familiar with Windows to write code for it, if you wa ### TODO: ....detail what each of the functions _should_ be used for. ```C++ -old API (current for now): -int execute(const std::string& commandPath, const std::vector& commandArgs, std::list& stringInput, std::function lambda) -std::vector checkOutput(const std::string& commandPath, const std::vector& commandArgs, std::list& stringInput, int& status) -std::future async(const std::string commandPath, const std::vector commandArgs, std::list stringInput, std::function lambda) +// Iterable can be stuff that can be iterated over (std::vector, etc.), all arguments other than commandPath are optional! +int execute(const std::string& commandPath, const Iterable& commandArgs, const Iterable& stdinInput, std::function lambda, const Iterable& environmentVariables); +// accepts iterators, the argument iterators are mandatory, the rest are optional +int execute(const std::string& commandPath, Iterator argsBegin, Iterator argsEnd, Iterator stdinBegin, Iterator stdinEnd, std::function lambda, Iterator envStart, Iterator envEnd); -// ctor for ProcessStream class -class ProcessStream(const std::string& commandPath, const std::vector& commandArgs, std::list& stringInput) +// Iterable can be stuff that can be iterated over (std::vector, etc.), all arguments other than commandPath are optional! +std::vector check_output(const std::string& commandPath, const Iterable& commandArgs, const Iterable& stdinInput, const Iterable& environmentVariables); +// accepts iterators, the argument iterators are mandatory, the rest are optional +std::vector check_output(const std::string& commandPath, Iterator argsBegin, Iterator argsEnd, Iterator stdinBegin, Iterator stdinEnd, Iterator envStart, Iterator envEnd); +// we currently don't have asynchronous, daemon spawning, or iterable interaction yet. coming soon(TM) ``` # License @@ -88,8 +91,7 @@ This is dual-licensed under a MIT and GPLv3 license - so FOSS lovers can use it, I don't know too much about licenses, so if I missed anything, please let me know. # Future Features -Some stuff that I haven't written yet, but I wanna: - - [X] Output streaming. Provide an iterator to allow iteration over the output lines, such that we don't have to load all in memory at once. - - [ ] Thread-safe async lambda interactions. Provide a method to launch a process in async, but still allow writing to the list of stdin without a race condition. - - [ ] A ping-ponging interface. This should allow incrementally providing stdin, then invoking the functor if output is emitted. Note that will likely not be possible if there's not performed asynchronously, or without using select. Using select is a bit annoying, because how do we differentiate between a command taking a while and it providing no input? - - [ ] Provide a way to set environment variables (i can pretty easily do it via using `execvpe`, but what should the API look like?) +Some stuff that I haven't written yet, but I wanna (see [this issue for a more in depth explanation of each](https://github.com/pnappa/subprocesscpp/issues/3)): + - Asynchronous execution + - Daemon spawning helpers (execute process and disown it, only need to consider where stdout should go). + - Interactive processes (manually feed stdin and retrieve stdout) diff --git a/subprocess.hpp b/subprocess.hpp index 1b3760e..dd3a34d 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -280,24 +280,6 @@ class Process { } }; -/* begin https://stackoverflow.com/a/16974087 */ -// a way to provide optional iterators to a function that do nothing -struct DevNull { - template T& operator=(T const&) { } - template operator T&() { static T dummy; return dummy; } -}; - -struct DevNullIterator { - DevNull operator*() const { return DevNull();} - DevNullIterator& operator++() { return *this; } - DevNullIterator operator++(int) const { return *this; } - DevNullIterator* operator->() { return this; } - // always equivalent (a for loop should instantly terminate!) - bool operator==(DevNullIterator&) const { return true; } - bool operator!=(DevNullIterator&) const { return false; } -}; -/* end https://stackoverflow.com/a/16974087 */ - /* hm, I copied this from somewhere, dunno where */ template struct is_iterator { @@ -337,7 +319,9 @@ template using is_iterable = decltype(detail::is_iterable_impl(0)); /* end https://stackoverflow.com/a/29634934 */ -static std::list dummyVec = {}; +// smallest possible iterable for the default arg values for the API functions that accept iterators +using DummyContainer = std::list; +static DummyContainer dummyVec = {}; /** * Execute a subprocess and optionally call a function per line of stdout. @@ -350,8 +334,8 @@ static std::list dummyVec = {}; * @param envBegin - the begin of an iterator containing process environment variables to set * @param envEnd - the end of the env iterator */ -template::iterator, - class EnvIt = std::list::iterator, +template::value, void>::type, typename = typename std::enable_if::value, void>::type, typename = typename std::enable_if::value, void>::type> @@ -410,13 +394,14 @@ int execute(const std::string& commandPath, const ArgIterable& commandArgs = {}, * @param envBegin - the begin of an iterator containing process environment variables to set * @param envEnd - the end of the env iterator */ -template::value, void>::type, - typename = typename std::enable_if::value, void>::type, - typename = typename std::enable_if::value, void>::type> +template::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> std::vector check_output(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, - StdinIt stdinBegin = DevNullIterator(), StdinIt stdinEnd = DevNullIterator(), - EnvIt envBegin = DevNullIterator(), EnvIt envEnd = DevNullIterator()) { + StdinIt stdinBegin = dummyVec.begin(), StdinIt stdinEnd = dummyVec.end(), + EnvIt envBegin = dummyVec.begin(), EnvIt envEnd = dummyVec.end()) { std::vector retVec; //int status = execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { retVec.push_back(std::move(s)); }, envBegin, envEnd); execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { retVec.push_back(std::move(s)); }, envBegin, envEnd); diff --git a/test.cpp b/test.cpp index ded2e58..6a4e427 100644 --- a/test.cpp +++ b/test.cpp @@ -19,6 +19,34 @@ TEST_CASE("[iterable] basic echo execution", "[subprocess::execute]") { REQUIRE(outputs.front() == "hello\n"); } +TEST_CASE("[iterable] basic echo execution varargs", "[subprocess::execute]") { + // test that execute will compile file with vargs + std::list inputs; + std::vector outputs; + std::vector env = {"lol=lol"}; + subprocess::execute("/bin/echo", {"hello"}, inputs, [&](std::string s) { outputs.push_back(s); }, env); + REQUIRE(outputs.size() == 1); + // echo appends a newline by default + REQUIRE(outputs.front() == "hello\n"); + + int status = subprocess::execute("/bin/echo", {"hello"}, inputs, [&](std::string s) { outputs.push_back(s); }); + outputs.clear(); + REQUIRE(outputs.size() == 1); + REQUIRE(status == 0); + + outputs.clear(); + status = subprocess::execute("/bin/echo", {"hello"}, inputs); + REQUIRE(status == 0); + + outputs.clear(); + status = subprocess::execute("/bin/echo", {"hello"}); + REQUIRE(status == 0); + + outputs.clear(); + status = subprocess::execute("/bin/echo"); + REQUIRE(status == 0); +} + TEST_CASE("[iterable] no trailing output newline echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; @@ -63,6 +91,16 @@ TEST_CASE("[iterable] stdin execute simple cat no trailing newline for last inpu REQUIRE(outputs.at(1) == "1,2,3,4"); } +TEST_CASE("[iterable] test env variables are sent to program correctly", "[subprocess::execute]") { + // executing a command with the last one missing a newline still should work the same, as the stdin stream gets closed. + std::vector outputs; + int retval = subprocess::execute("./test_programs/print_env", {}, {}, [&](std::string s) { outputs.push_back(s); }, {"LOL=lol"}); + + REQUIRE(retval == 0); + REQUIRE(outputs.size() == 1); + REQUIRE(outputs.at(0) == "LOL,lol\n"); +} + TEST_CASE("[iterator] basic echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; @@ -106,9 +144,10 @@ TEST_CASE("[iterator] no trailing output newline echo execution", "[subprocess:: TEST_CASE("[iterator] non existent executable", "[subprocess::execute]") { // try and run a non-existent executable, what should happen..? + std::list args; std::list inputs; std::vector outputs; - int retval = subprocess::execute("/bin/wangwang", {}, inputs, + int retval = subprocess::execute("/bin/wangwang", args.begin(), args.end(), inputs.begin(), inputs.end(), [](std::string) { FAIL("this functor should never have been called"); }); // process should have failed..? @@ -117,9 +156,10 @@ TEST_CASE("[iterator] non existent executable", "[subprocess::execute]") { } TEST_CASE("[iterator] stdin execute simple cat", "[subprocess::execute]") { + std::list args; std::list inputs = {"henlo wurld\n", "1,2,3,4\n"}; std::vector outputs; - int retval = subprocess::execute("/bin/cat", {}, inputs, [&](std::string s) { outputs.push_back(s); }); + int retval = subprocess::execute("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }); REQUIRE(retval == 0); REQUIRE(outputs.size() == 2); @@ -129,9 +169,10 @@ TEST_CASE("[iterator] stdin execute simple cat", "[subprocess::execute]") { TEST_CASE("[iterator] stdin execute simple cat no trailing newline for last input", "[subprocess::execute]") { // executing a command with the last one missing a newline still should work the same, as the stdin stream gets closed. + std::list args; std::list inputs = {"henlo wurld\n", "1,2,3,4"}; std::vector outputs; - int retval = subprocess::execute("/bin/cat", {}, inputs, [&](std::string s) { outputs.push_back(s); }); + int retval = subprocess::execute("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }); REQUIRE(retval == 0); REQUIRE(outputs.size() == 2); @@ -140,6 +181,10 @@ TEST_CASE("[iterator] stdin execute simple cat no trailing newline for last inpu } + + + + TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { // execute bc and pass it some equations std::list inputs = {"1+1\n", "2^333\n", "32-32\n"}; diff --git a/test_programs/print_env.cpp b/test_programs/print_env.cpp index 4846c42..9dc6f99 100644 --- a/test_programs/print_env.cpp +++ b/test_programs/print_env.cpp @@ -11,6 +11,6 @@ int main(int argc, char* argv[]) { for (int i = 1; i < argc; ++i) { char* env_val = getenv(argv[i]); - std::cout << (env_val == nullptr ? "" : env_val) << std::endl; + std::cout << argv[i] << "," << (env_val == nullptr ? "" : env_val) << std::endl; } } From 52ad3c2f4d1c29c0af8b401e9db910ae2986ab81 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Wed, 21 Nov 2018 21:36:36 +1100 Subject: [PATCH 03/17] clang tidy and minor adjustments --- .travis.yml | 2 +- subprocess.hpp | 166 ++++++++++++++++++++++++------------------------- test.cpp | 79 ++++++++++++----------- 3 files changed, 127 insertions(+), 120 deletions(-) diff --git a/.travis.yml b/.travis.yml index f574468..e1b98d0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,7 +20,7 @@ jobs: - stage: UnitTests os: linux sudo: false - script: make && make check + script: make check addons: apt: packages: diff --git a/subprocess.hpp b/subprocess.hpp index dd3a34d..6deff78 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -9,11 +9,9 @@ #include #include #include -#include #include -#include -#include #include +#include // unix process stuff #include @@ -191,7 +189,7 @@ class Process { // construct the argument list (unfortunately, the C api wasn't defined with C++ in mind, so // we have to abuse const_cast) see: https://stackoverflow.com/a/190208 // this returns a null terminated vector that contains a list of non-const char ptrs - template + template static std::vector toNullTerminatedCharIterable(Iter begin, Iter end) { // TODO: insert test to check if our iterable store strings..? // well it'll fail in the push_back stage anyway @@ -199,7 +197,7 @@ class Process { std::vector charArrayPlex; // the process name must be first for execv - //charArrayPlex.push_back(const_cast(input.c_str())); + // charArrayPlex.push_back(const_cast(input.c_str())); for (auto it = begin; it != end; ++it) { charArrayPlex.push_back(const_cast((*it).c_str())); } @@ -211,10 +209,11 @@ class Process { public: template - Process(const std::string& commandPath, ArgIt argBegin, ArgIt argEnd, EnvIt envBegin, EnvIt envEnd) : commandPath(commandPath) { + Process(const std::string& commandPath, ArgIt argBegin, ArgIt argEnd, EnvIt envBegin, EnvIt envEnd) + : commandPath(commandPath) { pid = 0; pipe.initialize(); - + // generate a vector that is able to be passed into exec for the process arguments processArgs = toNullTerminatedCharIterable(argBegin, argEnd); // process args must start with the processes name @@ -222,7 +221,6 @@ class Process { // ditto for the env variables envVariables = toNullTerminatedCharIterable(envBegin, envEnd); - envVariables.insert(envVariables.begin(), const_cast(commandPath.c_str())); } /** @@ -240,13 +238,13 @@ class Process { if (pid == 0) { pipe.setAsChildEnd(); - // ask kernel to deliver SIGTERM - // in case the parent dies + // ask kernel to deliver SIGTERM in case the parent dies + // so we don't get zombies prctl(PR_SET_PDEATHSIG, SIGTERM); execvpe(commandPath.c_str(), processArgs.data(), envVariables.data()); // Nothing below this line should be executed by child process. If so, it means that - // the execl function wasn't successfull, so lets exit: + // the exec function wasn't successful, so lets exit: exit(1); } pipe.setAsParentEnd(); @@ -285,36 +283,33 @@ template struct is_iterator { static char test(...); - template ::difference_type, - typename=typename std::iterator_traits::pointer, - typename=typename std::iterator_traits::reference, - typename=typename std::iterator_traits::value_type, - typename=typename std::iterator_traits::iterator_category - > static long test(U&&); + template ::difference_type, + typename = typename std::iterator_traits::pointer, + typename = typename std::iterator_traits::reference, + typename = typename std::iterator_traits::value_type, + typename = typename std::iterator_traits::iterator_category> + static long test(U&&); - constexpr static bool value = std::is_same())),long>::value; + constexpr static bool value = std::is_same())), long>::value; }; /* begin https://stackoverflow.com/a/29634934 */ -namespace detail -{ - // To allow ADL with custom begin/end - using std::begin; - using std::end; - - template - auto is_iterable_impl(int) - -> decltype ( - begin(std::declval()) != end(std::declval()), // begin/end and operator != - ++std::declval()))&>(), // operator ++ - *begin(std::declval()), // operator* - std::true_type{}); - - template - std::false_type is_iterable_impl(...); - -} - +namespace detail { +// To allow ADL with custom begin/end +using std::begin; +using std::end; + +template +auto is_iterable_impl(int) + -> decltype(begin(std::declval()) != end(std::declval()), // begin/end and operator != + ++std::declval()))&>(), // operator ++ + *begin(std::declval()), // operator* + std::true_type{}); + +template +std::false_type is_iterable_impl(...); + +} // namespace detail + template using is_iterable = decltype(detail::is_iterable_impl(0)); /* end https://stackoverflow.com/a/29634934 */ @@ -330,20 +325,19 @@ static DummyContainer dummyVec = {}; * @param lastArg - the end iterator for a list of arguments * @param stdinBegin - an InputIterator to provide stdin * @param stdinEnd - the end of the InputIterator range for stdin - * @param lambda - a function that is called with every line from the executed process (default NOP function) + * @param lambda - a function that is called with every line from the executed process (default NOP + * function) * @param envBegin - the begin of an iterator containing process environment variables to set * @param envEnd - the end of the env iterator */ -template::value, void>::type, - typename = typename std::enable_if::value, void>::type, - typename = typename std::enable_if::value, void>::type> -int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, - StdinIt stdinBegin = dummyVec.begin(), StdinIt stdinEnd = dummyVec.end(), - const std::function& lambda = [](std::string){}, - EnvIt envBegin = dummyVec.begin(), EnvIt envEnd = dummyVec.end()) { - +template ::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> +int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, + StdinIt stdinBegin = dummyVec.begin(), StdinIt stdinEnd = dummyVec.end(), + const std::function& lambda = [](std::string) {}, + EnvIt envBegin = dummyVec.begin(), EnvIt envEnd = dummyVec.end()) { Process childProcess(commandPath, firstArg, lastArg, envBegin, envEnd); childProcess.start(); @@ -366,21 +360,23 @@ int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, /** * Execute a subprocess and optionally call a function per line of stdout. * @param commandPath - the path of the executable to execute, e.g. "/bin/cat" - * @param commandArgs - the extra arguments for an executable e.g. {"argument 1", "henlo"} (default no arguments) + * @param commandArgs - the extra arguments for an executable e.g. {"argument 1", "henlo"} (default no + * arguments) * @param stdinInput - a list of inputs that will be piped into the processes' stdin (default no stdin) - * @param lambda - a function that is called with every line from the executed process (default NOP function) + * @param lambda - a function that is called with every line from the executed process (default NOP + * function) * @param env - a list of environment variables that the process will execute with (default nothing) */ -template, - class StdinIterable = std::vector, - class EnvIterable = std::vector, - typename = typename std::enable_if::value, void>::type, - typename = typename std::enable_if::value, void>::type, - typename = typename std::enable_if::value, void>::type> -int execute(const std::string& commandPath, const ArgIterable& commandArgs = {}, - const StdinIterable& stdinInput = {}, const std::function& lambda = [](std::string){}, - const EnvIterable& env = {}) { - return execute(commandPath, commandArgs.begin(), commandArgs.end(), stdinInput.begin(), stdinInput.end(), lambda, env.begin(), env.end()); +template , class StdinIterable = std::list, + class EnvIterable = std::list, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> +int execute(const std::string& commandPath, const ArgIterable& commandArgs = {}, + const StdinIterable& stdinInput = {}, + const std::function& lambda = [](std::string) {}, const EnvIterable& env = {}) { + return execute(commandPath, commandArgs.begin(), commandArgs.end(), stdinInput.begin(), stdinInput.end(), + lambda, env.begin(), env.end()); } /** @@ -390,21 +386,23 @@ int execute(const std::string& commandPath, const ArgIterable& commandArgs = {}, * @param lastArg - the end iterator for a list of arguments * @param stdinBegin - an InputIterator to provide stdin * @param stdinEnd - the end of the InputIterator range for stdin - * @param lambda - a function that is called with every line from the executed process (default NOP function) + * @param lambda - a function that is called with every line from the executed process (default NOP + * function) * @param envBegin - the begin of an iterator containing process environment variables to set * @param envEnd - the end of the env iterator */ -template::value, void>::type, - typename = typename std::enable_if::value, void>::type, - typename = typename std::enable_if::value, void>::type> +template ::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> std::vector check_output(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, - StdinIt stdinBegin = dummyVec.begin(), StdinIt stdinEnd = dummyVec.end(), - EnvIt envBegin = dummyVec.begin(), EnvIt envEnd = dummyVec.end()) { + StdinIt stdinBegin = dummyVec.begin(), StdinIt stdinEnd = dummyVec.end(), + EnvIt envBegin = dummyVec.begin(), EnvIt envEnd = dummyVec.end()) { std::vector retVec; - //int status = execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { retVec.push_back(std::move(s)); }, envBegin, envEnd); - execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { retVec.push_back(std::move(s)); }, envBegin, envEnd); + // int status = execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { + // retVec.push_back(std::move(s)); }, envBegin, envEnd); + execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, + [&](std::string s) { retVec.push_back(std::move(s)); }, envBegin, envEnd); return retVec; } @@ -415,19 +413,19 @@ std::vector check_output(const std::string& commandPath, ArgIt firs * @param stdinInput - a list of inputs that will be piped into the processes' stdin * @param env - a list of environment variables that the process will execute with (default nothing) */ -template, - class StdinIterable = std::vector, - class EnvIterable = std::vector, - typename = typename std::enable_if::value, void>::type, - typename = typename std::enable_if::value, void>::type, - typename = typename std::enable_if::value, void>::type> -std::vector check_output(const std::string& commandPath, const ArgIterable& commandArgs = {}, - const StdinIterable& stdinInput = {}, const EnvIterable& env = {}) { - return check_output(commandPath, commandArgs.begin(), commandArgs.end(), stdinInput.begin(), stdinInput.end(), env.begin(), env.end()); +template , class StdinIterable = std::vector, + class EnvIterable = std::vector, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type, + typename = typename std::enable_if::value, void>::type> +std::vector check_output(const std::string& commandPath, const ArgIterable& commandArgs = {}, + const StdinIterable& stdinInput = {}, const EnvIterable& env = {}) { + return check_output(commandPath, commandArgs.begin(), commandArgs.end(), stdinInput.begin(), + stdinInput.end(), env.begin(), env.end()); } //// TODO: what if the process terminates? consider error handling potentials... -//class ProcessStream { +// class ProcessStream { // public: // ProcessStream(const std::string& commandPath, const std::vector& commandArgs); // @@ -445,7 +443,7 @@ std::vector check_output(const std::string& commandPath, const ArgI //}; /* spawn the process in the background asynchronously, and return a future of the status code */ -//std::future async(const std::string commandPath, const std::vector commandArgs, +// std::future async(const std::string commandPath, const std::vector commandArgs, // std::list stringInput, std::function lambda) { // // spawn the function async - we must pass the args by value into the async lambda // // otherwise they may destruct before the execute fn executes! @@ -462,10 +460,10 @@ std::vector check_output(const std::string& commandPath, const ArgI * output, it may be not input into the functor until another line is fed in. You may modify the delay to try * and wait longer until moving on. This delay must exist, as several programs may not output a line for each * line input. Consider grep - it will not output a line if no match is made for that input. */ -//class ProcessStream { +// class ProcessStream { // Process childProcess; // -//public: +// public: // ProcessStream(const std::string& commandPath, const std::vector& commandArgs, // std::list& stringInput) { // childProcess.start(commandPath, commandArgs); diff --git a/test.cpp b/test.cpp index 6a4e427..9f02ac7 100644 --- a/test.cpp +++ b/test.cpp @@ -23,17 +23,19 @@ TEST_CASE("[iterable] basic echo execution varargs", "[subprocess::execute]") { // test that execute will compile file with vargs std::list inputs; std::vector outputs; - std::vector env = {"lol=lol"}; + std::vector env = {"LOL=lol"}; subprocess::execute("/bin/echo", {"hello"}, inputs, [&](std::string s) { outputs.push_back(s); }, env); REQUIRE(outputs.size() == 1); // echo appends a newline by default REQUIRE(outputs.front() == "hello\n"); - int status = subprocess::execute("/bin/echo", {"hello"}, inputs, [&](std::string s) { outputs.push_back(s); }); outputs.clear(); + int status = + subprocess::execute("/bin/echo", {"hello"}, inputs, [&](std::string s) { outputs.push_back(s); }); REQUIRE(outputs.size() == 1); REQUIRE(status == 0); + // XXX: this causes a linker error..? outputs.clear(); status = subprocess::execute("/bin/echo", {"hello"}, inputs); REQUIRE(status == 0); @@ -80,7 +82,8 @@ TEST_CASE("[iterable] stdin execute simple cat", "[subprocess::execute]") { } TEST_CASE("[iterable] stdin execute simple cat no trailing newline for last input", "[subprocess::execute]") { - // executing a command with the last one missing a newline still should work the same, as the stdin stream gets closed. + // executing a command with the last one missing a newline still should work the same, as the stdin stream + // gets closed. std::list inputs = {"henlo wurld\n", "1,2,3,4"}; std::vector outputs; int retval = subprocess::execute("/bin/cat", {}, inputs, [&](std::string s) { outputs.push_back(s); }); @@ -92,9 +95,11 @@ TEST_CASE("[iterable] stdin execute simple cat no trailing newline for last inpu } TEST_CASE("[iterable] test env variables are sent to program correctly", "[subprocess::execute]") { - // executing a command with the last one missing a newline still should work the same, as the stdin stream gets closed. + // executing a command with the last one missing a newline still should work the same, as the stdin stream + // gets closed. std::vector outputs; - int retval = subprocess::execute("./test_programs/print_env", {}, {}, [&](std::string s) { outputs.push_back(s); }, {"LOL=lol"}); + int retval = subprocess::execute("./test_programs/print_env", {"LOL"}, {}, + [&](std::string s) { outputs.push_back(s); }, {"LOL=lol"}); REQUIRE(retval == 0); REQUIRE(outputs.size() == 1); @@ -108,21 +113,24 @@ TEST_CASE("[iterator] basic echo execution", "[subprocess::execute]") { std::vector env = {}; int status; - status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }, env.begin(), env.end()); + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end(), + [&](std::string s) { outputs.push_back(s); }, env.begin(), env.end()); REQUIRE(outputs.size() == 1); // echo appends a newline by default REQUIRE(outputs.front() == "hello\n"); + // test the optional arguments compile outputs.clear(); - status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }); + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end(), + [&](std::string s) { outputs.push_back(s); }); REQUIRE(outputs.size() == 1); // echo appends a newline by default REQUIRE(outputs.front() == "hello\n"); - status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end()) ; + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end()); REQUIRE(status == 0); - status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end()) ; + status = subprocess::execute("/bin/echo", args.begin(), args.end(), inputs.begin(), inputs.end()); REQUIRE(status == 0); status = subprocess::execute("/bin/echo", args.begin(), args.end()); @@ -159,7 +167,8 @@ TEST_CASE("[iterator] stdin execute simple cat", "[subprocess::execute]") { std::list args; std::list inputs = {"henlo wurld\n", "1,2,3,4\n"}; std::vector outputs; - int retval = subprocess::execute("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }); + int retval = subprocess::execute("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end(), + [&](std::string s) { outputs.push_back(s); }); REQUIRE(retval == 0); REQUIRE(outputs.size() == 2); @@ -168,11 +177,13 @@ TEST_CASE("[iterator] stdin execute simple cat", "[subprocess::execute]") { } TEST_CASE("[iterator] stdin execute simple cat no trailing newline for last input", "[subprocess::execute]") { - // executing a command with the last one missing a newline still should work the same, as the stdin stream gets closed. + // executing a command with the last one missing a newline still should work the same, as the stdin stream + // gets closed. std::list args; std::list inputs = {"henlo wurld\n", "1,2,3,4"}; std::vector outputs; - int retval = subprocess::execute("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end(), [&](std::string s) { outputs.push_back(s); }); + int retval = subprocess::execute("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end(), + [&](std::string s) { outputs.push_back(s); }); REQUIRE(retval == 0); REQUIRE(outputs.size() == 2); @@ -180,16 +191,14 @@ TEST_CASE("[iterator] stdin execute simple cat no trailing newline for last inpu REQUIRE(outputs.at(1) == "1,2,3,4"); } - - - - - TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { // execute bc and pass it some equations std::list inputs = {"1+1\n", "2^333\n", "32-32\n"}; - // this one is interesting, there's more than one line of stdout for each input (bc line breaks after a certain number of characters) - std::vector output_expected = {"2\n", "17498005798264095394980017816940970922825355447145699491406164851279\\\n", "623993595007385788105416184430592\n", "0\n"}; + // this one is interesting, there's more than one line of stdout for each input (bc line breaks after a + // certain number of characters) + std::vector output_expected = {"2\n", + "17498005798264095394980017816940970922825355447145699491406164851279\\\n", + "623993595007385788105416184430592\n", "0\n"}; std::vector out = subprocess::check_output("/usr/bin/bc", {}, inputs); REQUIRE(out.size() == output_expected.size()); @@ -197,24 +206,24 @@ TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { } /* tests pending API update */ -//TEST_CASE("asynchronous is actually asynchronous", "[subprocess::async]") { +// TEST_CASE("asynchronous is actually asynchronous", "[subprocess::async]") { // std::list inputs; // std::vector outputs; // // std::atomic isDone(false); -// std::future retval = subprocess::async("/usr/bin/time", {"sleep", "3"}, inputs, [&](std::string s) { isDone.store(true); outputs.push_back(s); }); -// // reaching here after the async starts, means that we prrroooooobbbaabbbly (unless the user has a very, very slow computer) won't be finished -// REQUIRE(isDone.load() == false); -// REQUIRE(retval.get() == 0); +// std::future retval = subprocess::async("/usr/bin/time", {"sleep", "3"}, inputs, [&](std::string s) +// { isDone.store(true); outputs.push_back(s); }); +// // reaching here after the async starts, means that we prrroooooobbbaabbbly (unless the user has a very, +// very slow computer) won't be finished REQUIRE(isDone.load() == false); REQUIRE(retval.get() == 0); // -// // time has different outputs for different OSes, pluuus they will take different times to complete. all we need is some stdout. -// REQUIRE(outputs.size() > 0); +// // time has different outputs for different OSes, pluuus they will take different times to complete. all +// we need is some stdout. REQUIRE(outputs.size() > 0); //} // -//TEST_CASE("output iterator contains everything", "[subprocess::ProcessStream]") { +// TEST_CASE("output iterator contains everything", "[subprocess::ProcessStream]") { // // stream output from a process -// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; -// subprocess::ProcessStream ps("/bin/grep", {"-i", "^Hello, world$"}, inputs); +// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, +// world!\n"}; subprocess::ProcessStream ps("/bin/grep", {"-i", "^Hello, world$"}, inputs); // std::vector expectedOutput = {"hello, world\n", "Hello, world\n"}; // std::vector outputs; // for (std::string out : ps) { @@ -224,9 +233,9 @@ TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { // REQUIRE(outputs == expectedOutput); //} // -//TEST_CASE("output iterator handles empty output", "[subprocess::ProcessStream]") { -// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; -// subprocess::ProcessStream ps("/bin/grep", {"-i", "^bingo bango bongo$"}, inputs); +// TEST_CASE("output iterator handles empty output", "[subprocess::ProcessStream]") { +// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, +// world!\n"}; subprocess::ProcessStream ps("/bin/grep", {"-i", "^bingo bango bongo$"}, inputs); // std::vector expectedOutput = {}; // std::vector outputs; // for (std::string out : ps) { @@ -237,10 +246,10 @@ TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { // REQUIRE(outputs == expectedOutput); //} // -//TEST_CASE("output iterator all operator overload testing", "[subprocess::ProcessStream]") { +// TEST_CASE("output iterator all operator overload testing", "[subprocess::ProcessStream]") { // // stream output from a process -// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; -// subprocess::ProcessStream ps("/bin/grep", {"-i", "Hello, world"}, inputs); +// std::list inputs = {"12232\n", "hello, world\n", "Hello, world\n", "line: Hello, +// world!\n"}; subprocess::ProcessStream ps("/bin/grep", {"-i", "Hello, world"}, inputs); // std::list expectedOutput = {"hello, world\n", "Hello, world\n", "line: Hello, world!\n"}; // // auto beg = ps.begin(); From 1d5f1cc650cc41073d4fada088f2568ff3ded0b7 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Thu, 22 Nov 2018 00:22:47 +1100 Subject: [PATCH 04/17] more tests testing the new interface --- Makefile | 4 +-- test.cpp | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index ec1f3c6..1832542 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,9 @@ -CXX=g++ +CXX=clang++ CXXFLAGS=-g -std=c++11 -Wall -pedantic LIBS=-lpthread -.PHONY: all clean +.PHONY: all clean check all: demo check clean: diff --git a/test.cpp b/test.cpp index 9f02ac7..35d893d 100644 --- a/test.cpp +++ b/test.cpp @@ -9,7 +9,6 @@ #include "subprocess.hpp" -/* TODO: test all permuations of arguments */ TEST_CASE("[iterable] basic echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; @@ -140,7 +139,6 @@ TEST_CASE("[iterator] basic echo execution", "[subprocess::execute]") { REQUIRE(status == 0); } -/* TODO: make the rest iterators */ TEST_CASE("[iterator] no trailing output newline echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; @@ -191,7 +189,7 @@ TEST_CASE("[iterator] stdin execute simple cat no trailing newline for last inpu REQUIRE(outputs.at(1) == "1,2,3,4"); } -TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { +TEST_CASE("[iterable] check_output simple case bc", "[subprocess::check_output]") { // execute bc and pass it some equations std::list inputs = {"1+1\n", "2^333\n", "32-32\n"}; // this one is interesting, there's more than one line of stdout for each input (bc line breaks after a @@ -205,6 +203,88 @@ TEST_CASE("checkOutput simple case cat", "[subprocess::checkOutput]") { REQUIRE(out == output_expected); } +TEST_CASE("[iterable] check_output permutations (varargs)", "[subprocess::check_output]") { + // execute echo over a series of lines + std::list inputs = {"line1\n", "line2\n", "line3\n"}; + std::list env = {"LOL=lol"}; + std::vector output_expected = {"line1\n", "line2\n", "line3\n"}; + std::vector out; + + out.clear(); + out = subprocess::check_output("/bin/cat", {}, inputs, env); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); + + out.clear(); + out = subprocess::check_output("/bin/cat", {}, inputs); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); + + // now do the same with echo (as we're limited to args only at this point) + out.clear(); + std::vector args = {"value"}; + output_expected = {"value\n"}; + out = subprocess::check_output("/bin/echo", args); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); + + // and echo without any args just outputs a blank line + out.clear(); + output_expected = {"\n"}; + out = subprocess::check_output("/bin/echo"); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); +} + +TEST_CASE("[iterator] check_output simple case bc", "[subprocess::check_output]") { + std::vector args; + // execute bc and pass it some equations + std::list inputs = {"1+1\n", "2^333\n", "32-32\n"}; + // this one is interesting, there's more than one line of stdout for each input (bc line breaks after a + // certain number of characters) + std::vector output_expected = {"2\n", + "17498005798264095394980017816940970922825355447145699491406164851279\\\n", + "623993595007385788105416184430592\n", "0\n"}; + std::vector out = subprocess::check_output("/usr/bin/bc", args.begin(), args.end(), inputs.begin(), inputs.end()); + + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); +} + +TEST_CASE("[iterator] check_output permutations (varargs)", "[subprocess::check_output]") { + // execute echo over a series of lines + std::deque args; + std::list inputs = {"line1\n", "line2\n", "line3\n"}; + std::list env = {"LOL=lol"}; + std::vector output_expected = {"line1\n", "line2\n", "line3\n"}; + std::vector out; + + out.clear(); + out = subprocess::check_output("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end(), env.begin(), env.end()); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); + + out.clear(); + out = subprocess::check_output("/bin/cat", args.begin(), args.end(), inputs.begin(), inputs.end()); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); + + // now do the same with echo (as we're limited to args only at this point) + out.clear(); + args = {"value"}; + output_expected = {"value\n"}; + out = subprocess::check_output("/bin/echo", args.begin(), args.end()); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); + + // and echo without any args just outputs a blank line + out.clear(); + output_expected = {"\n"}; + out = subprocess::check_output("/bin/echo"); + REQUIRE(out.size() == output_expected.size()); + REQUIRE(out == output_expected); +} + /* tests pending API update */ // TEST_CASE("asynchronous is actually asynchronous", "[subprocess::async]") { // std::list inputs; From 52aef265a613e8d9353c558ef650a4f7d9edcefc Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Thu, 22 Nov 2018 15:25:20 +1100 Subject: [PATCH 05/17] added skeleton for the ProcessStream class, and added some test TODOs --- subprocess.hpp | 174 ++++++++++++++++++------------------------------- test.cpp | 11 +++- 2 files changed, 72 insertions(+), 113 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index bbbbb40..cf67edf 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -422,6 +422,11 @@ int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, internal::Process childProcess(commandPath, firstArg, lastArg, envBegin, envEnd); childProcess.start(); + // TODO: fix this so we can't block the input/output pipe. it should only require + // reading from the process so-as to unclog their pipe. Pipes only have finite space! (~65k) + // but remember, we may need to read more than one line per for loop (if a process outputs a lot of lines + // per line read in, perhaps..?) + // write our input to the processes stdin pipe for (auto it = stdinBegin; it != stdinEnd; ++it) { childProcess.write(*it); @@ -505,117 +510,62 @@ std::vector check_output(const std::string& commandPath, const ArgI stdinInput.end(), env.begin(), env.end()); } -//// TODO: what if the process terminates? consider error handling potentials... -// class ProcessStream { -// public: -// ProcessStream(const std::string& commandPath, const std::vector& commandArgs); -// -// // write a line to the subprocess's stdin -// void write(const std::string& inputLine); -// // read a line and block until received (or until timeout reached) -// template -// std::string read(std::chrono::duration timeout=-1); -// // if there is a line for reading -// template -// bool ready(std::chrono::duration timeout=0); -// -// ProcessStream& operator<<(const std::string& inputLine); -// ProcessStream& operator>>(std::string& outputLine); -//}; - -/* spawn the process in the background asynchronously, and return a future of the status code */ -// std::future async(const std::string commandPath, const std::vector commandArgs, -// std::list stringInput, std::function lambda) { -// // spawn the function async - we must pass the args by value into the async lambda -// // otherwise they may destruct before the execute fn executes! -// // whew, that was an annoying bug to find... -// return std::async(std::launch::async, -// [&](const std::string cp, const std::vector ca, std::list si, -// std::function l) { return execute(cp, ca, si, l); }, -// commandPath, commandArgs, stringInput, lambda); -//} - -/* TODO: refactor up this function so that there isn't duplicated code - most of this is identical to the - * execute fn execute a program and stream the output after each line input this function calls select to - * check if outputs needs to be pumped after each line input. This means that if the line takes too long to - * output, it may be not input into the functor until another line is fed in. You may modify the delay to try - * and wait longer until moving on. This delay must exist, as several programs may not output a line for each - * line input. Consider grep - it will not output a line if no match is made for that input. */ -// class ProcessStream { -// Process childProcess; -// -// public: -// ProcessStream(const std::string& commandPath, const std::vector& commandArgs, -// std::list& stringInput) { -// childProcess.start(commandPath, commandArgs); -// -// // while our string queue is working, -// while (!stringInput.empty()) { -// // write our input to the -// // process's stdin pipe -// std::string newInput = stringInput.front(); -// stringInput.pop_front(); -// childProcess.write(newInput); -// } -// // now we finished chucking in the string, send -// // an EOF -// childProcess.sendEOF(); -// } -// -// ~ProcessStream() { -// childProcess.waitUntilFinished(); -// } -// -// struct iterator { -// ProcessStream* ps; -// bool isFinished = false; -// // current read line of the process -// std::string cline; -// -// iterator(ProcessStream* ps) : ps(ps) { -// // increment this ptr, because nothing exists initially -// ++(*this); -// } -// // ctor for end() -// iterator(ProcessStream* ps, bool) : ps(ps), isFinished(true) {} -// -// const std::string& operator*() const { -// return cline; -// } -// -// /* preincrement */ -// iterator& operator++() { -// // iterate over each line output by the child's stdout, and call the functor -// cline = ps->childProcess.readLine(); -// if (cline.empty()) { -// isFinished = true; -// } -// return *this; -// } -// -// /* post increment */ -// iterator operator++(int) { -// iterator old(*this); -// ++(*this); -// return old; -// } -// -// bool operator==(const iterator& other) const { -// return other.ps == this->ps && this->isFinished == other.isFinished; -// } -// -// bool operator!=(const iterator& other) const { -// return !((*this) == other); -// } -// }; -// -// iterator begin() { -// return iterator(this); -// } -// -// iterator end() { -// return iterator(this, true); -// } -//}; +// TODO: what if the process terminates? consider error handling potentials... +class ProcessStream { + // need some list of processes this process is supposed to pipe to + // shouldn't need to store the lambda, as the internal::Process object will store that + // std::vector nextProcesses; + + public: + template> + ProcessStream(const std::string& commandPath, const std::vector& commandArgs, const Functor& func = [](std::string){}); + + ~ProcessStream(); + + // start the process and prevent any more pipes from being established. + // may throw an exception? + bool start(); + + // write a line to the subprocess's stdin + void write(const std::string& inputLine); + // read a line and block until received (or until timeout reached) + template + std::string read(std::chrono::duration timeout=-1); + // if there is a line for reading (optionally + template + bool ready(std::chrono::duration timeout=0); + + // pipe some data to the receiver process, and return the receiver process + // we do this so we can have: process1.pipe_to(process2).pipe_to(process3)...etc + // if pipes are set up there are some restrictions on using the << and >> operators. + // if a process is receiving from another process, then they cannot use operator<< anymore + // hmm: what about if its done before .start()? + // if a process is outputting to another, they cannot use operator>> + ProcessStream& pipe_to(ProcessStream& receiver); + // ditto + ProcessStream& operator>>(ProcessStream& receiver); + + // read a line into this process (so it acts as another line of stdin) + // instead of string, probably should be typename Stringable, and use stringstream and stuff. + ProcessStream& operator<<(const std::string& inputLine); + // retrieve a line of stdout from this process + ProcessStream& operator>>(std::string& outputLine); + // write all stdout to file? + ProcessStream& operator>>(std::ofstream& outfile); + + // some other functions which maybe useful (perhaps take a timeout?) + // returns whether it could terminate + bool terminate(); + // a more...extreme way + bool kill(); + // send arbitrary signals to the subprocess + void signal(int signum); + + class iterator; + + // provide an iterator to iterate over the stdout produced + iterator begin(); + iterator end(); +}; } // end namespace subprocess diff --git a/test.cpp b/test.cpp index 35d893d..76d7b97 100644 --- a/test.cpp +++ b/test.cpp @@ -9,6 +9,14 @@ #include "subprocess.hpp" +/* + * Some additional tests I'd like/things to improve: + * - TODO: replace these system binaries with hand written ones in test_programs, + * so we can ensure these are all available across all systems. + * - TODO: add some more tests about lots of output generated for little inputs + * - TODO: add some tests which will fill up the pipe fully (https://unix.stackexchange.com/questions/11946/how-big-is-the-pipe-buffer) + */ + TEST_CASE("[iterable] basic echo execution", "[subprocess::execute]") { std::list inputs; std::vector outputs; @@ -34,7 +42,8 @@ TEST_CASE("[iterable] basic echo execution varargs", "[subprocess::execute]") { REQUIRE(outputs.size() == 1); REQUIRE(status == 0); - // XXX: this causes a linker error..? + // XXX: this causes a linker error..? well.. it used to. + // I think I need to fix the template for the Iterator one, to be more strict in what it accepts (i think it might actually be able to cast some Iterables to Iterators) outputs.clear(); status = subprocess::execute("/bin/echo", {"hello"}, inputs); REQUIRE(status == 0); From 20047cd82d366f66e70fe52fa5cd562742e297ef Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Thu, 22 Nov 2018 21:53:44 +1100 Subject: [PATCH 06/17] whoops, didn't build recursively --- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1832542..2f3578b 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ CXX=clang++ CXXFLAGS=-g -std=c++11 -Wall -pedantic LIBS=-lpthread -.PHONY: all clean check +.PHONY: all clean check test_programs all: demo check clean: @@ -21,3 +21,6 @@ test: test.cpp subprocess.hpp coverage: test.cpp subprocess.hpp $(CXX) $(CXXFLAGS) -fprofile-arcs -ftest-coverage test.cpp -o coverage $(LIBS) .codecov/run_coverage.sh + +test_programs: + $(MAKE) -C test_programs/ From f686ec3c714d878e95a274e4a008be30761aefd1 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Fri, 23 Nov 2018 15:15:19 +1100 Subject: [PATCH 07/17] lol, forgot to actually call make on the subdir, you nonce --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2f3578b..45a411f 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ clean: demo: demo.cpp subprocess.hpp $(CXX) $(CXXFLAGS) demo.cpp -o demo $(LIBS) -check: test +check: test test_programs valgrind ./test test: test.cpp subprocess.hpp From 923520162cfdd2c5fe36da3a6ef45a0d208d3485 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Wed, 5 Dec 2018 23:20:37 +1100 Subject: [PATCH 08/17] barebones Process added It can pipe to new processes, but not sure if that's working... --- subprocess.hpp | 152 ++++++++++++++++++++++++++++++++++++++++++----- testingstuff.cpp | 12 ++++ 2 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 testingstuff.cpp diff --git a/subprocess.hpp b/subprocess.hpp index cf67edf..012ae23 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -13,6 +13,9 @@ #include #include #include +#include +#include +#include // unix process stuff #include @@ -22,6 +25,7 @@ #include #include + namespace subprocess { namespace internal { /** @@ -430,11 +434,16 @@ int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, // write our input to the processes stdin pipe for (auto it = stdinBegin; it != stdinEnd; ++it) { childProcess.write(*it); + + // propagate output as we need to ensure the output pipe isn't clogged + while (childProcess.isReady()) { + lambda(childProcess.readLine()); + } } // close the stdin for the process childProcess.sendEOF(); - // iterate over each line output by the child's stdout, and call the functor + // iterate over each line of remaining output by the child's stdout, and call the functor std::string processOutput; while ((processOutput = childProcess.readLine()).size() > 0) { lambda(processOutput); @@ -511,23 +520,130 @@ std::vector check_output(const std::string& commandPath, const ArgI } // TODO: what if the process terminates? consider error handling potentials... -class ProcessStream { - // need some list of processes this process is supposed to pipe to - // shouldn't need to store the lambda, as the internal::Process object will store that - // std::vector nextProcesses; +class Process { + // need some list of processes this process is supposed to pipe to (if any) + // XXX: what if the child processes are moved? should we keep a reference to the parent(s) then update us within their vector..? + std::vector successor_processes; + std::vector feedin_files; + std::vector feedout_files; + bool started = false; + + internal::Process owned_proc; + + std::deque stdinput_queue; + protected: + + void pump_input() { + assert(started && "error: input propagated for inactive process"); + + // write any queued stdinput + while (!stdinput_queue.empty()) { + this->write(stdinput_queue.front()); + stdinput_queue.pop_front(); + + pump_output(); + } + + // each of the input files to this process has to be pumped too + for (std::ifstream* ifile : feedin_files) { + // write as many lines from the input file until we run out + for (std::string line; std::getline(*ifile, line); ) { + // we gotta append a newline, getline omits it. + this->write(line + "\n"); + pump_output(); + } + } + } + + void pump_output() { + assert(started && "error: input propagated for inactive process"); + + while (owned_proc.isReady()) { + std::string out = owned_proc.readLine(); + + std::cout << "read line " << out << std::endl; + + this->write_next(out); + } + } + + void write_next(const std::string& out) { + + std::cout << "writing line:" << out << std::endl; + + // TODO: call functor + + for (Process* succ_process : successor_processes) { + succ_process->write(out); + } + for (std::ofstream* succ_file : feedout_files) { + std::cout << "writing to file:" << out; + (*succ_file) << out << std::flush; + } + } + public: template> - ProcessStream(const std::string& commandPath, const std::vector& commandArgs, const Functor& func = [](std::string){}); + Process(const std::string& commandPath, const std::vector& commandArgs, const Functor& func = [](std::string){}) : + owned_proc(commandPath, commandArgs.begin(), commandArgs.end(), internal::dummyVec.begin(), internal::dummyVec.end()) { + } + + ~Process() { + pump_input(); + pump_output(); + owned_proc.sendEOF(); + pump_output(); + + // iterate over each line of remaining output by the child's stdout, and call the functor + std::string processOutput; + while ((processOutput = owned_proc.readLine()).size() > 0) { + this->write_next(processOutput); + //lambda(processOutput); + } + + int retval = owned_proc.waitUntilFinished(); + std::cout << "process retval:" << retval; + + // TODO: is this right? + for (Process* succ_process : successor_processes) { + succ_process->owned_proc.waitUntilFinished(); + } + for (std::ofstream* succ_file : feedout_files) { + succ_file->close(); + } - ~ProcessStream(); + // TODO: invoke exit for successor processes? + } // start the process and prevent any more pipes from being established. // may throw an exception? - bool start(); + void start() { + if (started) throw std::runtime_error("cannot start an already running process"); + owned_proc.start(); + started = true; + + // recursively start all successor processes + for (auto successor_proc : successor_processes) { + successor_proc->start(); + } + + pump_input(); + + // close the stdin for the process + // XXX: do we want to do this, or only when the process is finished - i.e. dtor'd or .close()'d, etc. + // owned_proc.sendEOF(); + + // propagate output some more + pump_output(); + } + + bool is_started() { return started; } // write a line to the subprocess's stdin - void write(const std::string& inputLine); + void write(const std::string& inputLine) { + owned_proc.write(inputLine); + } // read a line and block until received (or until timeout reached) template std::string read(std::chrono::duration timeout=-1); @@ -541,17 +657,25 @@ class ProcessStream { // if a process is receiving from another process, then they cannot use operator<< anymore // hmm: what about if its done before .start()? // if a process is outputting to another, they cannot use operator>> - ProcessStream& pipe_to(ProcessStream& receiver); + Process& pipe_to(Process& receiver) { + successor_processes.push_back(&receiver); + return receiver; + } // ditto - ProcessStream& operator>>(ProcessStream& receiver); + Process& operator>>(Process& receiver) { return this->pipe_to(receiver); } + // for files + std::ofstream& pipe_to(std::ofstream& receiver) { + feedout_files.push_back(&receiver); + return receiver; + } // read a line into this process (so it acts as another line of stdin) // instead of string, probably should be typename Stringable, and use stringstream and stuff. - ProcessStream& operator<<(const std::string& inputLine); + Process& operator<<(const std::string& inputLine); // retrieve a line of stdout from this process - ProcessStream& operator>>(std::string& outputLine); + Process& operator>>(std::string& outputLine); // write all stdout to file? - ProcessStream& operator>>(std::ofstream& outfile); + Process& operator>>(std::ofstream& outfile); // some other functions which maybe useful (perhaps take a timeout?) // returns whether it could terminate diff --git a/testingstuff.cpp b/testingstuff.cpp new file mode 100644 index 0000000..41c1265 --- /dev/null +++ b/testingstuff.cpp @@ -0,0 +1,12 @@ +#include "subprocess.hpp" + +#include + +int main() { + subprocess::Process p("/bin/echo", {"lol"}); + std::ofstream outfile("cool.out"); + + p.pipe_to(outfile); + + p.start(); +} From 7422cddb9f24697cc48ae795415607470c474879 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Fri, 7 Dec 2018 20:33:13 +1100 Subject: [PATCH 09/17] replaced piping to file with outputting to string - complicated lifetime problems otherwise --- subprocess.hpp | 77 +++++++++++++++++++++++++++++++++--------------- testingstuff.cpp | 7 ++--- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 012ae23..b8bb758 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -524,8 +524,9 @@ class Process { // need some list of processes this process is supposed to pipe to (if any) // XXX: what if the child processes are moved? should we keep a reference to the parent(s) then update us within their vector..? std::vector successor_processes; - std::vector feedin_files; - std::vector feedout_files; + // TODO: how should we handle this..? + // std::vector feedin_files; + std::vector feedout_files; bool started = false; internal::Process owned_proc; @@ -545,14 +546,14 @@ class Process { } // each of the input files to this process has to be pumped too - for (std::ifstream* ifile : feedin_files) { - // write as many lines from the input file until we run out - for (std::string line; std::getline(*ifile, line); ) { - // we gotta append a newline, getline omits it. - this->write(line + "\n"); - pump_output(); - } - } + // for (std::ifstream* ifile : feedin_files) { + // // write as many lines from the input file until we run out + // for (std::string line; std::getline(*ifile, line); ) { + // // we gotta append a newline, getline omits it. + // this->write(line + "\n"); + // pump_output(); + // } + // } } void pump_output() { @@ -576,16 +577,23 @@ class Process { for (Process* succ_process : successor_processes) { succ_process->write(out); } - for (std::ofstream* succ_file : feedout_files) { + for (std::ofstream& succ_file : feedout_files) { std::cout << "writing to file:" << out; - (*succ_file) << out << std::flush; + if (!succ_file) std::cout << "uh, can't write to file..?\n"; + std::cout << strerror(errno) << '\n'; + succ_file << out << std::flush; + std::cout << "isbad" << succ_file.bad() << std::endl; + std::cout << "isfail" << succ_file.fail() << std::endl; + std::cout << "iseof" << succ_file.eof() << std::endl; + std::cout << strerror(errno) << '\n'; + if (!succ_file) std::cout << "uh, can't write to file..? 2\n"; + std::cout << strerror(errno) << '\n'; } } - public: - template> - Process(const std::string& commandPath, const std::vector& commandArgs, const Functor& func = [](std::string){}) : + template, typename Functor = std::function> + Process(const std::string& commandPath, const ArgIterable& commandArgs, const Functor& func = [](std::string){}) : owned_proc(commandPath, commandArgs.begin(), commandArgs.end(), internal::dummyVec.begin(), internal::dummyVec.end()) { } @@ -599,7 +607,6 @@ class Process { std::string processOutput; while ((processOutput = owned_proc.readLine()).size() > 0) { this->write_next(processOutput); - //lambda(processOutput); } int retval = owned_proc.waitUntilFinished(); @@ -609,8 +616,9 @@ class Process { for (Process* succ_process : successor_processes) { succ_process->owned_proc.waitUntilFinished(); } - for (std::ofstream* succ_file : feedout_files) { - succ_file->close(); + for (std::ofstream& succ_file : feedout_files) { + if (!succ_file) std::cout << "some error with file..?\n"; + succ_file.close(); } // TODO: invoke exit for successor processes? @@ -638,6 +646,22 @@ class Process { pump_output(); } + int finish () { + pump_input(); + pump_output(); + pump_output(); + + // iterate over each line of remaining output by the child's stdout, and call the functor + std::string processOutput; + while ((processOutput = owned_proc.readLine()).size() > 0) { + this->write_next(processOutput); + } + + int retval = owned_proc.waitUntilFinished(); + + return retval; + } + bool is_started() { return started; } // write a line to the subprocess's stdin @@ -663,19 +687,24 @@ class Process { } // ditto Process& operator>>(Process& receiver) { return this->pipe_to(receiver); } + // XXX: removed this because the dtor wasn't handled well // for files - std::ofstream& pipe_to(std::ofstream& receiver) { - feedout_files.push_back(&receiver); - return receiver; + // std::ofstream& pipe_to(std::ofstream& receiver) { + // feedout_files.push_back(&receiver); + // return receiver; + // } + void output_to_file(const std::string& filename) { + feedout_files.push_back(std::ofstream(filename)); + if (!feedout_files.back().good()) throw std::runtime_error("error: file " + filename + " failed to open"); } // read a line into this process (so it acts as another line of stdin) // instead of string, probably should be typename Stringable, and use stringstream and stuff. - Process& operator<<(const std::string& inputLine); + //Process& operator<<(const std::string& inputLine); // retrieve a line of stdout from this process - Process& operator>>(std::string& outputLine); + //Process& operator>>(std::string& outputLine); // write all stdout to file? - Process& operator>>(std::ofstream& outfile); + // Process& operator>>(std::ofstream& outfile); // some other functions which maybe useful (perhaps take a timeout?) // returns whether it could terminate diff --git a/testingstuff.cpp b/testingstuff.cpp index 41c1265..ffb857e 100644 --- a/testingstuff.cpp +++ b/testingstuff.cpp @@ -3,10 +3,7 @@ #include int main() { - subprocess::Process p("/bin/echo", {"lol"}); - std::ofstream outfile("cool.out"); - - p.pipe_to(outfile); - + subprocess::Process p("/bin/echo", {"asjdlksaj"}); + p.output_to_file("cool.out"); p.start(); } From d16a2298ea981ef8dfd1c2521fe1df2554a96668 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Fri, 7 Dec 2018 22:14:26 +1100 Subject: [PATCH 10/17] piping to processes *seems* to work, but propagated output seems borked" --- subprocess.hpp | 79 +++++++++++++++++++++++------------------------- testingstuff.cpp | 16 ++++++++++ 2 files changed, 54 insertions(+), 41 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index b8bb758..0bcf7d0 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -524,10 +524,15 @@ class Process { // need some list of processes this process is supposed to pipe to (if any) // XXX: what if the child processes are moved? should we keep a reference to the parent(s) then update us within their vector..? std::vector successor_processes; + std::vector predecessor_processes; // TODO: how should we handle this..? // std::vector feedin_files; std::vector feedout_files; + // the function to call every time a line is output + // Functor func; bool started = false; + bool finished = false; + int retval; internal::Process owned_proc; @@ -562,66 +567,54 @@ class Process { while (owned_proc.isReady()) { std::string out = owned_proc.readLine(); - std::cout << "read line " << out << std::endl; - this->write_next(out); } } void write_next(const std::string& out) { - std::cout << "writing line:" << out << std::endl; - - // TODO: call functor + // call functor + // func(out); for (Process* succ_process : successor_processes) { succ_process->write(out); } + // TODO: should I throw if cannot write to file..? for (std::ofstream& succ_file : feedout_files) { - std::cout << "writing to file:" << out; - if (!succ_file) std::cout << "uh, can't write to file..?\n"; - std::cout << strerror(errno) << '\n'; succ_file << out << std::flush; - std::cout << "isbad" << succ_file.bad() << std::endl; - std::cout << "isfail" << succ_file.fail() << std::endl; - std::cout << "iseof" << succ_file.eof() << std::endl; - std::cout << strerror(errno) << '\n'; - if (!succ_file) std::cout << "uh, can't write to file..? 2\n"; - std::cout << strerror(errno) << '\n'; } } public: - template, typename Functor = std::function> - Process(const std::string& commandPath, const ArgIterable& commandArgs, const Functor& func = [](std::string){}) : + template, class Functor = std::function> + Process(const std::string& commandPath, const ArgIterable& commandArgs, Functor func = [](std::string){}) : owned_proc(commandPath, commandArgs.begin(), commandArgs.end(), internal::dummyVec.begin(), internal::dummyVec.end()) { } ~Process() { - pump_input(); - pump_output(); - owned_proc.sendEOF(); - pump_output(); - - // iterate over each line of remaining output by the child's stdout, and call the functor - std::string processOutput; - while ((processOutput = owned_proc.readLine()).size() > 0) { - this->write_next(processOutput); + // err need to close all predecessors (if any) + // if there is a cycle in the processes, this doesn't cause an infinite loop + // as if they're already closed, they're a no-op. + for (Process* pred_process : predecessor_processes) { + pred_process->finish(); + } - int retval = owned_proc.waitUntilFinished(); - std::cout << "process retval:" << retval; + this->owned_proc.sendEOF(); + // process any remaining input/output + finish(); + // TODO: is this right? for (Process* succ_process : successor_processes) { - succ_process->owned_proc.waitUntilFinished(); - } - for (std::ofstream& succ_file : feedout_files) { - if (!succ_file) std::cout << "some error with file..?\n"; - succ_file.close(); + succ_process->finish(); } - // TODO: invoke exit for successor processes? + // according to docs, this is not necessary, this'll happen in the dtor + // for (std::ofstream& succ_file : feedout_files) { + // if (!succ_file) std::cout << "some error with file..?\n"; + // succ_file.close(); + // } } // start the process and prevent any more pipes from being established. @@ -636,17 +629,14 @@ class Process { successor_proc->start(); } + // push out any pending input pump_input(); - - // close the stdin for the process - // XXX: do we want to do this, or only when the process is finished - i.e. dtor'd or .close()'d, etc. - // owned_proc.sendEOF(); - // propagate output some more pump_output(); } - int finish () { + int finish() { + if (finished) return this->retval; pump_input(); pump_output(); pump_output(); @@ -657,9 +647,10 @@ class Process { this->write_next(processOutput); } - int retval = owned_proc.waitUntilFinished(); + this->retval = owned_proc.waitUntilFinished(); + finished = true; - return retval; + return this->retval; } bool is_started() { return started; } @@ -683,6 +674,7 @@ class Process { // if a process is outputting to another, they cannot use operator>> Process& pipe_to(Process& receiver) { successor_processes.push_back(&receiver); + receiver.predecessor_processes.push_back(this); return receiver; } // ditto @@ -698,6 +690,11 @@ class Process { if (!feedout_files.back().good()) throw std::runtime_error("error: file " + filename + " failed to open"); } + void output_to_file(std::ofstream&& file) { + feedout_files.push_back(std::move(file)); + if (!feedout_files.back().good()) throw std::runtime_error("error: file is invalid"); + } + // read a line into this process (so it acts as another line of stdin) // instead of string, probably should be typename Stringable, and use stringstream and stuff. //Process& operator<<(const std::string& inputLine); diff --git a/testingstuff.cpp b/testingstuff.cpp index ffb857e..cc83940 100644 --- a/testingstuff.cpp +++ b/testingstuff.cpp @@ -5,5 +5,21 @@ int main() { subprocess::Process p("/bin/echo", {"asjdlksaj"}); p.output_to_file("cool.out"); + subprocess::Process p2("/bin/grep", {"asj"}); + p2.output_to_file("nekcool.out"); + p.pipe_to(p2); p.start(); } + + + + +// v1 - pipe_to(Process&& proc); +//p.pipe_to(Process("/bin/grep", {"-i", "cool"})); +//// equivalent to +//p.pipe_to({"/bin/grep", {"-i", "cool"}}); +// +//// v2 - pipe_to(Process& proc); +//Process p3("/bin/grep", {"-i", "cool"}); +//p.pipe_to(p3); + From 2e5a030cd275b22ba690fbc9045c223d3276b30c Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Fri, 7 Dec 2018 22:44:05 +1100 Subject: [PATCH 11/17] fixed borked output bug the process args were populated via temporary stack variables (intialiser lists), so having two processes created caused process arguments to overwrite each other. --- subprocess.hpp | 13 +++++++++---- testingstuff.cpp | 3 +-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 0bcf7d0..efa47ee 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -276,7 +276,8 @@ class Process { // the process name must be first for execv // charArrayPlex.push_back(const_cast(input.c_str())); for (auto it = begin; it != end; ++it) { - charArrayPlex.push_back(const_cast((*it).c_str())); + charArrayPlex.push_back(strdup((*it).c_str())); + //charArrayPlex.push_back(const_cast((*it).c_str())); } // must be terminated with a nullptr for execv charArrayPlex.push_back(nullptr); @@ -294,12 +295,18 @@ class Process { // generate a vector that is able to be passed into exec for the process arguments processArgs = toNullTerminatedCharIterable(argBegin, argEnd); // process args must start with the processes name - processArgs.insert(processArgs.begin(), const_cast(commandPath.c_str())); + processArgs.insert(processArgs.begin(), strdup(commandPath.c_str())); // ditto for the env variables envVariables = toNullTerminatedCharIterable(envBegin, envEnd); } + ~Process() { + // clean these up because they're created via strdup + for (char* c : processArgs) free(c); + for (char* c : envVariables) free(c); + } + /** * Starts a seperate process with the provided command and arguments * @return TODO return errno returned by child call of execv @@ -597,14 +604,12 @@ class Process { // as if they're already closed, they're a no-op. for (Process* pred_process : predecessor_processes) { pred_process->finish(); - } this->owned_proc.sendEOF(); // process any remaining input/output finish(); - // TODO: is this right? for (Process* succ_process : successor_processes) { succ_process->finish(); diff --git a/testingstuff.cpp b/testingstuff.cpp index cc83940..f127ae4 100644 --- a/testingstuff.cpp +++ b/testingstuff.cpp @@ -5,7 +5,7 @@ int main() { subprocess::Process p("/bin/echo", {"asjdlksaj"}); p.output_to_file("cool.out"); - subprocess::Process p2("/bin/grep", {"asj"}); + subprocess::Process p2("/bin/grep", {"-o", "asj"}); p2.output_to_file("nekcool.out"); p.pipe_to(p2); p.start(); @@ -13,7 +13,6 @@ int main() { - // v1 - pipe_to(Process&& proc); //p.pipe_to(Process("/bin/grep", {"-i", "cool"})); //// equivalent to From 339243de217cdb785ad2235369047266516ee1a6 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Sat, 8 Dec 2018 18:13:59 +1100 Subject: [PATCH 12/17] basic recursion test, it doesn't seem to terminate, uh oh --- next_prime.cpp | 23 ++++++++++ subprocess.hpp | 74 ++++++++++++++++++++----------- test_programs/.gitignore | 2 + test_programs/Makefile | 10 ++++- test_programs/increment.cpp | 16 +++++++ test_programs/tee_if_nonprime.cpp | 38 ++++++++++++++++ testingstuff.cpp | 1 - 7 files changed, 135 insertions(+), 29 deletions(-) create mode 100644 next_prime.cpp create mode 100644 test_programs/increment.cpp create mode 100644 test_programs/tee_if_nonprime.cpp diff --git a/next_prime.cpp b/next_prime.cpp new file mode 100644 index 0000000..4e98295 --- /dev/null +++ b/next_prime.cpp @@ -0,0 +1,23 @@ +/** + * Demonstration of recursive subprocess pipes + * + * This program find the next prime for an input number. + */ + + +#include "subprocess.hpp" + +int main() { + subprocess::Process incrementer("./test_programs/increment", {}, [&](std::string s) { std::cout << s << std::endl; }); + subprocess::Process prime_checker("./test_programs/tee_if_nonprime"); + + incrementer.pipe_to(prime_checker); + prime_checker.pipe_to(incrementer); + prime_checker.output_to_file("prime.out"); + + incrementer.start(); + incrementer << "33\n"; + + prime_checker.finish(); +} + diff --git a/subprocess.hpp b/subprocess.hpp index efa47ee..23175f4 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -433,11 +433,6 @@ int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, internal::Process childProcess(commandPath, firstArg, lastArg, envBegin, envEnd); childProcess.start(); - // TODO: fix this so we can't block the input/output pipe. it should only require - // reading from the process so-as to unclog their pipe. Pipes only have finite space! (~65k) - // but remember, we may need to read more than one line per for loop (if a process outputs a lot of lines - // per line read in, perhaps..?) - // write our input to the processes stdin pipe for (auto it = stdinBegin; it != stdinEnd; ++it) { childProcess.write(*it); @@ -447,6 +442,7 @@ int execute(const std::string& commandPath, ArgIt firstArg, ArgIt lastArg, lambda(childProcess.readLine()); } } + // close the stdin for the process childProcess.sendEOF(); @@ -501,6 +497,7 @@ std::vector check_output(const std::string& commandPath, ArgIt firs StdinIt stdinBegin = internal::dummyVec.begin(), StdinIt stdinEnd = internal::dummyVec.end(), EnvIt envBegin = internal::dummyVec.begin(), EnvIt envEnd = internal::dummyVec.end()) { std::vector retVec; + // XXX: what's a good way to return the return value, do we throw on non-zero return? // int status = execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, [&](std::string s) { // retVec.push_back(std::move(s)); }, envBegin, envEnd); execute(commandPath, firstArg, lastArg, stdinBegin, stdinEnd, @@ -536,10 +533,15 @@ class Process { // std::vector feedin_files; std::vector feedout_files; // the function to call every time a line is output - // Functor func; + // TODO: somehow make it use the ctor template type + std::function func; + bool started = false; bool finished = false; int retval; + + mutable size_t lines_written = 0; + mutable size_t lines_received = 0; internal::Process owned_proc; @@ -553,7 +555,6 @@ class Process { while (!stdinput_queue.empty()) { this->write(stdinput_queue.front()); stdinput_queue.pop_front(); - pump_output(); } @@ -572,6 +573,7 @@ class Process { assert(started && "error: input propagated for inactive process"); while (owned_proc.isReady()) { + lines_written++; std::string out = owned_proc.readLine(); this->write_next(out); @@ -579,9 +581,10 @@ class Process { } void write_next(const std::string& out) { + assert(started && "error: input propagated for inactive process"); // call functor - // func(out); + func(out); for (Process* succ_process : successor_processes) { succ_process->write(out); @@ -593,13 +596,14 @@ class Process { } public: - template, class Functor = std::function> - Process(const std::string& commandPath, const ArgIterable& commandArgs, Functor func = [](std::string){}) : + template> + Process(const std::string& commandPath, const ArgIterable& commandArgs = internal::dummyVec, Functor func = [](std::string){}) : + func(func), owned_proc(commandPath, commandArgs.begin(), commandArgs.end(), internal::dummyVec.begin(), internal::dummyVec.end()) { } ~Process() { - // err need to close all predecessors (if any) + // err, need to close all predecessors (if any) // if there is a cycle in the processes, this doesn't cause an infinite loop // as if they're already closed, they're a no-op. for (Process* pred_process : predecessor_processes) { @@ -610,25 +614,26 @@ class Process { // process any remaining input/output finish(); - // TODO: is this right? + // do the same for outputting processes for (Process* succ_process : successor_processes) { succ_process->finish(); } - - // according to docs, this is not necessary, this'll happen in the dtor - // for (std::ofstream& succ_file : feedout_files) { - // if (!succ_file) std::cout << "some error with file..?\n"; - // succ_file.close(); - // } } // start the process and prevent any more pipes from being established. // may throw an exception? void start() { - if (started) throw std::runtime_error("cannot start an already running process"); + // ignore an already started process + if (started) return; owned_proc.start(); started = true; + // recursively start all predecessor processes + // do this to ensure that + for (auto pred_process : predecessor_processes) { + pred_process->start(); + } + // recursively start all successor processes for (auto successor_proc : successor_processes) { successor_proc->start(); @@ -644,12 +649,12 @@ class Process { if (finished) return this->retval; pump_input(); pump_output(); - pump_output(); // iterate over each line of remaining output by the child's stdout, and call the functor std::string processOutput; while ((processOutput = owned_proc.readLine()).size() > 0) { this->write_next(processOutput); + lines_written++; } this->retval = owned_proc.waitUntilFinished(); @@ -662,8 +667,21 @@ class Process { // write a line to the subprocess's stdin void write(const std::string& inputLine) { - owned_proc.write(inputLine); + if (finished) throw std::runtime_error("cannot write to a finished process"); + + if (is_started()) { + this->lines_written++; + + owned_proc.write(inputLine); + + pump_output(); + + // if it hasn't been started, then we queue up the input for later + } else { + stdinput_queue.push_front(inputLine); + } } + // read a line and block until received (or until timeout reached) template std::string read(std::chrono::duration timeout=-1); @@ -695,14 +713,18 @@ class Process { if (!feedout_files.back().good()) throw std::runtime_error("error: file " + filename + " failed to open"); } - void output_to_file(std::ofstream&& file) { - feedout_files.push_back(std::move(file)); - if (!feedout_files.back().good()) throw std::runtime_error("error: file is invalid"); - } + // can't seem to get this one working..? + //void output_to_file(std::ofstream&& file) { + // feedout_files.push_back(std::move(file)); + // if (!feedout_files.back().good()) throw std::runtime_error("error: file is invalid"); + //} // read a line into this process (so it acts as another line of stdin) // instead of string, probably should be typename Stringable, and use stringstream and stuff. - //Process& operator<<(const std::string& inputLine); + Process& operator<<(const std::string& inputLine) { + this->write(inputLine); + return *this; + } // retrieve a line of stdout from this process //Process& operator>>(std::string& outputLine); // write all stdout to file? diff --git a/test_programs/.gitignore b/test_programs/.gitignore index bdec747..d2ed88d 100644 --- a/test_programs/.gitignore +++ b/test_programs/.gitignore @@ -1 +1,3 @@ print_env +tee_if_nonprime +increment diff --git a/test_programs/Makefile b/test_programs/Makefile index 0b83418..55cac1a 100644 --- a/test_programs/Makefile +++ b/test_programs/Makefile @@ -4,11 +4,17 @@ LIBS=-lpthread .PHONY: all clean -all: print_env +all: print_env tee_if_nonprime increment print_env: print_env.cpp $(CXX) $(CXXFLAGS) print_env.cpp -o print_env $(LIBS) +tee_if_nonprime: tee_if_nonprime.cpp + $(CXX) $(CXXFLAGS) tee_if_nonprime.cpp -o tee_if_nonprime $(LIBS) + +increment: increment.cpp + $(CXX) $(CXXFLAGS) increment.cpp -o increment $(LIBS) + clean: - rm -rvf print_env + rm -rvf print_env tee_if_nonprime increment diff --git a/test_programs/increment.cpp b/test_programs/increment.cpp new file mode 100644 index 0000000..8643d75 --- /dev/null +++ b/test_programs/increment.cpp @@ -0,0 +1,16 @@ + +/** + * Demo program to demonstrate recursive process piping + * All this does is increment the number provided. + */ + +#include + +int main() { + std::string line; + while (std::getline(std::cin, line)) { + std::cout << std::stoi(line) + 1 << std::endl; + } + + return 0; +} diff --git a/test_programs/tee_if_nonprime.cpp b/test_programs/tee_if_nonprime.cpp new file mode 100644 index 0000000..daff4f2 --- /dev/null +++ b/test_programs/tee_if_nonprime.cpp @@ -0,0 +1,38 @@ +/** + * Demo program to demonstrate recursive process piping + * All this does is output the number if it's non-prime, + * otherwise close program if it's prime. + * That should trigger the process hierarchy to collapse, + * and the result can be harvested. + */ + +#include +#include + +bool is_prime(int input) { + if (input <= 1) return false; + + // basic primality test, just check if any of the + // numbers up to sqrt(n) divide n + + int int_sqrt = (int) sqrt((double) input); + for (int dividor = 2; dividor <= int_sqrt; ++dividor) { + if (input % dividor == 0) return false; + } + + // reached here? + return true; +} + +int main() { + std::string line; + while (std::getline(std::cin, line)) { + if (!is_prime(std::stoi(line))) { + std::cout << line << std::endl; + } else { + break; + } + } + + return 0; +} diff --git a/testingstuff.cpp b/testingstuff.cpp index f127ae4..c992b15 100644 --- a/testingstuff.cpp +++ b/testingstuff.cpp @@ -8,7 +8,6 @@ int main() { subprocess::Process p2("/bin/grep", {"-o", "asj"}); p2.output_to_file("nekcool.out"); p.pipe_to(p2); - p.start(); } From 5998eed83aa1cfcffff4325f201e669510bc9c7f Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Sat, 8 Dec 2018 23:15:56 +1100 Subject: [PATCH 13/17] Wrote some tests for Process/minor internal mods One currently doesn't pass - it doesn't fail, mind you, but the test never terminates. A read that is happening in one of the tests is blocking, but it is reliant on the input of the previous process. Perhaps, I modify it that if there isn't a line immediately, it recursively requests a line from the parent process; this isn't neat and is prone to lockup either way. I also added a buffer to the stdout too, as some might build up in a process that is piped to, and not read frequently. Unfortunately, with that addition comes another problem - if the line isn't read explicitly (currently only via >>), the functors DO get called, but the buffer is never empties and is increasingly filled up. I also wrote a new testing program - it simply out puts a lot of data, so that we can test the pipe congestion better. I just wrote this one because we're gonna start writing the processes that are invoked from the testing program from scratch - enabling the tests to be run cross-platform. --- next_prime.cpp | 2 - subprocess.hpp | 57 +++++++-- test.cpp | 186 ++++++++++++++++++++++++++++++ test_programs/Makefile | 7 +- test_programs/large_outputter.cpp | 109 +++++++++++++++++ 5 files changed, 348 insertions(+), 13 deletions(-) create mode 100644 test_programs/large_outputter.cpp diff --git a/next_prime.cpp b/next_prime.cpp index 4e98295..786d80a 100644 --- a/next_prime.cpp +++ b/next_prime.cpp @@ -17,7 +17,5 @@ int main() { incrementer.start(); incrementer << "33\n"; - - prime_checker.finish(); } diff --git a/subprocess.hpp b/subprocess.hpp index 23175f4..1810ee2 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -545,16 +545,17 @@ class Process { internal::Process owned_proc; - std::deque stdinput_queue; + std::deque stdin_queue; + std::deque stdout_queue; protected: void pump_input() { assert(started && "error: input propagated for inactive process"); // write any queued stdinput - while (!stdinput_queue.empty()) { - this->write(stdinput_queue.front()); - stdinput_queue.pop_front(); + while (!stdin_queue.empty()) { + this->write(stdin_queue.front()); + stdin_queue.pop_front(); pump_output(); } @@ -582,13 +583,19 @@ class Process { void write_next(const std::string& out) { assert(started && "error: input propagated for inactive process"); - + // call functor func(out); + // TODO: check this, but I save it for later reads. + if (successor_processes.empty() && feedout_files.empty()) { + stdout_queue.push_back(out); + } + for (Process* succ_process : successor_processes) { succ_process->write(out); } + // TODO: should I throw if cannot write to file..? for (std::ofstream& succ_file : feedout_files) { succ_file << out << std::flush; @@ -678,7 +685,7 @@ class Process { // if it hasn't been started, then we queue up the input for later } else { - stdinput_queue.push_front(inputLine); + stdin_queue.push_front(inputLine); } } @@ -709,7 +716,7 @@ class Process { // return receiver; // } void output_to_file(const std::string& filename) { - feedout_files.push_back(std::ofstream(filename)); + feedout_files.push_back(std::move(std::ofstream(filename))); if (!feedout_files.back().good()) throw std::runtime_error("error: file " + filename + " failed to open"); } @@ -725,8 +732,40 @@ class Process { this->write(inputLine); return *this; } - // retrieve a line of stdout from this process - //Process& operator>>(std::string& outputLine); + + // retrieve a line of stdout from this process (blocking) + Process& operator>>(std::string& outputLine) { + if (!started || finished) { + throw std::runtime_error("cannot read line from inactive process"); + } + + if (successor_processes.size() > 0 || feedout_files.size() > 0) { + throw std::runtime_error("manually reading line from process that is piped from is prohibited"); + } + + lines_written++; + + if (!stdout_queue.empty()) { + outputLine = stdout_queue.front(); + stdout_queue.pop_front(); + } else { + outputLine = owned_proc.readLine(); + } + + // TODO: i think its possible we miss some lines, if a process pipes to this one, this fn isn't called + // We might need to save lines from those instances, and yield from them when necessary.? + // We have to save them rather than leaving them in the pipe as otherwise blockages may occur + + // call functor XXX: this will get called twice for processes that get piped to... + // so.. where do we call this..? + // hmm... perhaps the write fn should instead call this operator...or vice versa and shuffle code. + func(outputLine); + + // no need to output to the successors (they can't exist when using this fn) + + return *this; + } + // write all stdout to file? // Process& operator>>(std::ofstream& outfile); diff --git a/test.cpp b/test.cpp index 76d7b97..f5caf1e 100644 --- a/test.cpp +++ b/test.cpp @@ -15,6 +15,7 @@ * so we can ensure these are all available across all systems. * - TODO: add some more tests about lots of output generated for little inputs * - TODO: add some tests which will fill up the pipe fully (https://unix.stackexchange.com/questions/11946/how-big-is-the-pipe-buffer) + * - TODO: should a process be started in the dtor if it hasn't already been..? */ TEST_CASE("[iterable] basic echo execution", "[subprocess::execute]") { @@ -294,6 +295,191 @@ TEST_CASE("[iterator] check_output permutations (varargs)", "[subprocess::check_ REQUIRE(out == output_expected); } + +// TODO: make all these have timeouts! it's possible that they never terminate +// TODO: somehow ensure that if we try and retrieve more output it fails..? idk, seems annoying +// perhaps we just use the timeouts, with some reasonable duration..? + +TEST_CASE("basic process instantiation", "[subprocess::Process]") { + subprocess::Process p("/bin/echo", {"henlo world"}); + + p.start(); + std::string line; + p >> line; + + REQUIRE(line == "henlo world\n"); +} + +// handy deferrable functions (executed on dtor) +template +struct Deferrable { + Functor func; + Deferrable(Functor f) : func(f) {} + ~Deferrable() { + func(); + } +}; + +TEST_CASE("process functor", "[subprocess::Process]") { + + // requirement from the dead + // just ensure that even after the dtor, the functor isn't invoked again! + std::string line; + size_t func_count = 0; + auto deferred_assertion = Deferrable>([&]() { + REQUIRE(func_count == 1); + }); + + subprocess::Process p("/bin/echo", {"henlo world"}, [&](std::string s) { + func_count += 1; + REQUIRE(s == "henlo world\n"); + }); + + p.start(); + p >> line; + + REQUIRE(line == "henlo world\n"); + REQUIRE(func_count == 1); +} + +TEST_CASE("pre-emptive process input", "[subprocess::Process]") { + subprocess::Process p("/bin/cat"); + + p << "henlo world\n"; + p.start(); + + std::string line; + p >> line; + + REQUIRE(line == "henlo world\n"); +} + +TEST_CASE("post process start input", "[subprocess::Process]") { + subprocess::Process p("/bin/cat"); + + p.start(); + + p << "henlo world\n"; + + std::string line; + p >> line; + + REQUIRE(line == "henlo world\n"); +} + +TEST_CASE("reading from process that itself is a successor proc", "[subprocess::Process]") { + // TODO: add timeout + subprocess::Process p1("/bin/echo", {"high to roam"}); + subprocess::Process p2("/bin/grep", {"-o", "hi"}); + + p1.pipe_to(p2); + + p1.start(); + + std::string line; + // XXX: this line is currently making the test hang. The reason is simple, but the implications are complex. + // When reading from p2, there isn't a line instantly available, as it requires input from p1 + // - but we don't currently call readline in the predecessor. If we do, what if the current + // process *will* output, but is taking a while..? + p2 >> line; + + REQUIRE(line == "hi\n"); +} + +TEST_CASE("malordered process RAII", "[subprocess::Process]") { + bool func_called = false; + auto deferred_assertion = Deferrable>([&]() { + REQUIRE(func_called == true); + }); + // test that initialising the processes in the reverse stack order won't bork them + subprocess::Process p2("/bin/grep", {"-o", "hi"}, [&](std::string s) { + REQUIRE(s == "hi\n"); + func_called = true; + }); + subprocess::Process p1("/bin/echo", {"high to roam"}); + + p1.pipe_to(p2); + + p1.start(); +} + +TEST_CASE("RAII doesn't start non-started process", "[subprocess:Process]") { + subprocess::Process p1("/bin/echo", {"die bart die"}, [&](std::string) { + FAIL("process output when shouldn't have"); + }); +} + +TEST_CASE("superfluous input", "[subprocess::Process]") { + // provide input to a process that won't use it. +} + +TEST_CASE("", "[subprocess::Process]") { + +} + +TEST_CASE("reading from succesor plus functor", "[subprocess::Process]") { + +} + +TEST_CASE("multi-line output", "[subprocess::Process]") { + // test a process that outputs lots of output for each line of stdin + +} + +TEST_CASE("post-start manual process input", "[subprocess::Process]") { + +} + +TEST_CASE("simple process piping", "[subprocess::Process]") { + +} + +TEST_CASE("piping to file", "[subprocess::Process]") { + +} + +TEST_CASE("bifurcated file outputting", "[subprocess::Process]") { + +} + +TEST_CASE("long pipe chain", "[subprocess::Process]") { + +} + +TEST_CASE("complex process piping", "[subprocess::Process]") { + +} + +TEST_CASE("cyclic process piping", "[subprocess::Process]") { + // TODO: fail if this takes too long, that indicates there's a problem + // probably will be the next prime implementation +} + +TEST_CASE("test infinite output cropped via head unix pipe", "[subprocess::Process]") { + +} + +TEST_CASE("", "[subprocess::Process]") { + +} + +TEST_CASE("", "[subprocess::Process]") { + +} + +TEST_CASE("", "[subprocess::Process]") { + +} + +TEST_CASE("test output iterator", "[subprocess::Process]") { + +} + +TEST_CASE("test ctor vargs", "[subprocess::Process]") { + +} + + /* tests pending API update */ // TEST_CASE("asynchronous is actually asynchronous", "[subprocess::async]") { // std::list inputs; diff --git a/test_programs/Makefile b/test_programs/Makefile index 55cac1a..9c374fe 100644 --- a/test_programs/Makefile +++ b/test_programs/Makefile @@ -4,7 +4,7 @@ LIBS=-lpthread .PHONY: all clean -all: print_env tee_if_nonprime increment +all: print_env tee_if_nonprime increment large_outputter print_env: print_env.cpp $(CXX) $(CXXFLAGS) print_env.cpp -o print_env $(LIBS) @@ -15,6 +15,9 @@ tee_if_nonprime: tee_if_nonprime.cpp increment: increment.cpp $(CXX) $(CXXFLAGS) increment.cpp -o increment $(LIBS) +large_outputter: large_outputter.cpp + $(CXX) $(CXXFLAGS) large_outputter.cpp -o large_outputter $(LIBS) + clean: - rm -rvf print_env tee_if_nonprime increment + rm -rvf print_env tee_if_nonprime increment large_outputter diff --git a/test_programs/large_outputter.cpp b/test_programs/large_outputter.cpp new file mode 100644 index 0000000..2d0f407 --- /dev/null +++ b/test_programs/large_outputter.cpp @@ -0,0 +1,109 @@ +/** + * Demo program to output a large number of characters for a series of switches + * @author Patrick Nappa + * + * The reason for needing this program is to ensure that the pipes are flushed + * timely within the process. The unix pipes only support ~65kB of data within + * so we output several times that amount just to be sure. + * This data must be split up via lines, as output is buffered via lines, + * and it's not really expected that a line of input is larger than 65kB + * We believe this is a limitation that we can't avoid. + * -> https://unix.stackexchange.com/questions/11946/how-big-is-the-pipe-buffer + */ + +#include +#include + +// 1024 length string +constexpr ssize_t line_length = 1024; +const std::string basic_line = std::string(line_length-1, 'A'); + +void output_line(ssize_t& amount) { + // duh, nothing to output! + if (amount <= 0) return; + + if (amount < line_length) { + std::cout << std::string(amount - 1, 'A') << std::endl; + } else { + std::cout << basic_line << std::endl; + } + + amount -= line_length; +} + +void prefixed_churn(const ssize_t amount) { + assert(amount > 0 && "amount must be non-zero and positive"); + std::string line; + + ssize_t c_amount = amount; + + do { + while (c_amount > 0) { + output_line(c_amount); + } + + c_amount = amount; + } while (std::getline(std::cin, line)); +} + +void postfix_churn(const ssize_t amount) { + assert(amount > 0 && "amount must be non-zero and positive"); + std::string line; + ssize_t c_amount = amount; + + while (std::getline(std::cin, line)) { + while (c_amount > 0) { + output_line(c_amount); + } + c_amount = amount; + } +} + +void infinite_churn(const ssize_t amount) { + assert(amount > 0 && "amount must be non-zero and positive"); + ssize_t c_amount = amount; + while (true) { + while (c_amount > 0) { + output_line(c_amount); + } + + c_amount = amount; + } +} + +int main(int argc, char* argv[]) { + if (argc < 2) { + std::cerr << "Usage: " + std::string(argv[0]) + " TYPE [amount]" << std::endl; + std::cerr << "Where TYPE is either PRE, FOREACH, INFINITE" << std::endl; + std::cerr << "PRE means lines with be output before each line of stdin is read" << std::endl; + std::cerr << "FOREACH means output will be emitted after each line of stdin is processed" << std::endl; + std::cerr << "INFINITE means an infinite stream of data will be emitted" << std::endl; + std::cerr << "By default amount is 2^17 bytes, i.e. 131072 characters. This is split up into 1024 character lines (1023 plus newline)." << std::endl; + std::cerr << "Cheers cunt" << std::endl; + + return EXIT_FAILURE; + } + + ssize_t num_bytes = 1 << 17; + if (argc >= 3) { + num_bytes = std::stoi(std::string(argv[2])); + if (num_bytes < 0) { + std::cerr << "Amount of bytes emitted must be positive" << std::endl; + return EXIT_FAILURE; + } + } + + std::string execution_type(argv[1]); + if (execution_type == "PRE") { + prefixed_churn(num_bytes); + } else if (execution_type == "FOREACH") { + postfix_churn(num_bytes); + } else if (execution_type == "INFINITE") { + infinite_churn(num_bytes); + } else { + std::cerr << "please provide a valid type of execution" << std::endl; + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; +} From 8b9068c1e91718a13a6e498614fc99205a45133a Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Sun, 9 Dec 2018 00:38:57 +1100 Subject: [PATCH 14/17] Procesess with functors can no longer be read from Using operator>> or .read is prohibited for processes that have processes/files to output to, or if they have a functor to execute. The reason is to avoid massive memory increases for those who don't need the output, and only wish to use a functor. I need to fix the tests so they compile however. Tomorrow's job! --- subprocess.hpp | 104 +++++++++++++++++++++++++------------------------ test.cpp | 1 + 2 files changed, 55 insertions(+), 50 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 1810ee2..6d1d9dc 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -85,7 +85,7 @@ class TwoWayPipe { * */ short inPipeState(long wait_ms) { // file descriptor struct to check if pollin bit will be set - struct pollfd fds = {.fd = input_pipe_file_descriptor[0], .events = POLLIN}; + struct pollfd fds = {input_pipe_file_descriptor[0], POLLIN, 0}; // poll with no wait time int res = poll(&fds, 1, wait_ms); @@ -530,11 +530,13 @@ class Process { std::vector successor_processes; std::vector predecessor_processes; // TODO: how should we handle this..? + // The reason why its not trivial is that we may want to have a file pipe to multiple processes, and it feels + // like a waste to read from the file N times to output to N processes. // std::vector feedin_files; std::vector feedout_files; // the function to call every time a line is output // TODO: somehow make it use the ctor template type - std::function func; + std::function* func = nullptr; bool started = false; bool finished = false; @@ -571,7 +573,7 @@ class Process { } void pump_output() { - assert(started && "error: input propagated for inactive process"); + assert(started && "error: output propagated for inactive process"); while (owned_proc.isReady()) { lines_written++; @@ -583,30 +585,31 @@ class Process { void write_next(const std::string& out) { assert(started && "error: input propagated for inactive process"); - - // call functor - func(out); - // TODO: check this, but I save it for later reads. - if (successor_processes.empty() && feedout_files.empty()) { + // hold onto stdout if we don't have any successors or lambdas to execute + if (successor_processes.empty() && feedout_files.empty() && func == nullptr) { stdout_queue.push_back(out); - } + } else { + // call functor + (*func)(out); - for (Process* succ_process : successor_processes) { - succ_process->write(out); - } + for (Process* succ_process : successor_processes) { + succ_process->write(out); + } - // TODO: should I throw if cannot write to file..? - for (std::ofstream& succ_file : feedout_files) { - succ_file << out << std::flush; + // TODO: should I throw if cannot write to file..? + for (std::ofstream& succ_file : feedout_files) { + succ_file << out << std::flush; + } } } public: template> Process(const std::string& commandPath, const ArgIterable& commandArgs = internal::dummyVec, Functor func = [](std::string){}) : - func(func), owned_proc(commandPath, commandArgs.begin(), commandArgs.end(), internal::dummyVec.begin(), internal::dummyVec.end()) { + // TODO: change back to new Functor(func), when I'm able to set the member variables type in the ctor. + this->func = new std::function(func); } ~Process() { @@ -625,6 +628,9 @@ class Process { for (Process* succ_process : successor_processes) { succ_process->finish(); } + + // our function is dynamically allocated (how else do we test for null functors..?) + delete func; } // start the process and prevent any more pipes from being established. @@ -632,6 +638,7 @@ class Process { void start() { // ignore an already started process if (started) return; + owned_proc.start(); started = true; @@ -670,7 +677,7 @@ class Process { return this->retval; } - bool is_started() { return started; } + bool is_started() const { return started; } // write a line to the subprocess's stdin void write(const std::string& inputLine) { @@ -690,11 +697,35 @@ class Process { } // read a line and block until received (or until timeout reached) - template - std::string read(std::chrono::duration timeout=-1); + template + std::string read(std::chrono::duration timeout = std::chrono::duration(-1)){ + std::string outputLine; + + if (!started || finished) { + throw std::runtime_error("cannot read line from inactive process"); + } + + if (successor_processes.size() > 0 || feedout_files.size() > 0 || func != nullptr) { + throw std::runtime_error("manually reading line from process that is piped from/has a functor is prohibited"); + } + + lines_written++; + + // we may have lines of output to "use" from earlier + if (!stdout_queue.empty()) { + outputLine = stdout_queue.front(); + stdout_queue.pop_front(); + } else { + outputLine = owned_proc.readLine(timeout); + } + + return outputLine; + } // if there is a line for reading (optionally - template - bool ready(std::chrono::duration timeout=0); + template + bool ready(std::chrono::duration timeout=0) { + return owned_proc.isReady(timeout); + } // pipe some data to the receiver process, and return the receiver process // we do this so we can have: process1.pipe_to(process2).pipe_to(process3)...etc @@ -733,36 +764,9 @@ class Process { return *this; } - // retrieve a line of stdout from this process (blocking) + // retrieve a line of stdout from this process (blocking) into the string Process& operator>>(std::string& outputLine) { - if (!started || finished) { - throw std::runtime_error("cannot read line from inactive process"); - } - - if (successor_processes.size() > 0 || feedout_files.size() > 0) { - throw std::runtime_error("manually reading line from process that is piped from is prohibited"); - } - - lines_written++; - - if (!stdout_queue.empty()) { - outputLine = stdout_queue.front(); - stdout_queue.pop_front(); - } else { - outputLine = owned_proc.readLine(); - } - - // TODO: i think its possible we miss some lines, if a process pipes to this one, this fn isn't called - // We might need to save lines from those instances, and yield from them when necessary.? - // We have to save them rather than leaving them in the pipe as otherwise blockages may occur - - // call functor XXX: this will get called twice for processes that get piped to... - // so.. where do we call this..? - // hmm... perhaps the write fn should instead call this operator...or vice versa and shuffle code. - func(outputLine); - - // no need to output to the successors (they can't exist when using this fn) - + outputLine = read(); return *this; } diff --git a/test.cpp b/test.cpp index f5caf1e..b8ba308 100644 --- a/test.cpp +++ b/test.cpp @@ -299,6 +299,7 @@ TEST_CASE("[iterator] check_output permutations (varargs)", "[subprocess::check_ // TODO: make all these have timeouts! it's possible that they never terminate // TODO: somehow ensure that if we try and retrieve more output it fails..? idk, seems annoying // perhaps we just use the timeouts, with some reasonable duration..? +// TODO: replace these tests as I made having the functor AND being able to extract stdout illegal TEST_CASE("basic process instantiation", "[subprocess::Process]") { subprocess::Process p("/bin/echo", {"henlo world"}); From 9673a6ca99333a06d4dbb02085a4aabdadb8ac3a Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Fri, 14 Dec 2018 18:03:12 +1100 Subject: [PATCH 15/17] Added process topology graph gen, general procwork --- next_prime.cpp | 9 +- subprocess.hpp | 179 ++++++++++++++++++++++++++---- test.cpp | 5 +- test_programs/large_outputter.cpp | 1 + 4 files changed, 169 insertions(+), 25 deletions(-) diff --git a/next_prime.cpp b/next_prime.cpp index 786d80a..ff2a5ac 100644 --- a/next_prime.cpp +++ b/next_prime.cpp @@ -8,7 +8,7 @@ #include "subprocess.hpp" int main() { - subprocess::Process incrementer("./test_programs/increment", {}, [&](std::string s) { std::cout << s << std::endl; }); + subprocess::Process incrementer("./test_programs/increment", {}, [&](std::string s) { std::cout << s; }); subprocess::Process prime_checker("./test_programs/tee_if_nonprime"); incrementer.pipe_to(prime_checker); @@ -17,5 +17,12 @@ int main() { incrementer.start(); incrementer << "33\n"; + + incrementer.force_output(); + prime_checker.force_output(); + incrementer.force_output(); + prime_checker.force_output(); + incrementer.force_output(); + prime_checker.force_output(); } diff --git a/subprocess.hpp b/subprocess.hpp index 6d1d9dc..9cca9ee 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -16,6 +16,7 @@ #include #include #include +#include // unix process stuff #include @@ -27,6 +28,8 @@ namespace subprocess { + + class Process; namespace internal { /** * A TwoWayPipe that allows reading and writing between two processes @@ -256,6 +259,8 @@ class TwoWayPipe { * connection * */ class Process { + friend class subprocess::Process; + pid_t pid; TwoWayPipe pipe; @@ -524,11 +529,17 @@ std::vector check_output(const std::string& commandPath, const ArgI } // TODO: what if the process terminates? consider error handling potentials... +/** + * A representation of a process. A process may be piped to one or more other processes or files. + * Currently, this version does not support cyclic pipes, use AsyncProcess for that. + */ class Process { // need some list of processes this process is supposed to pipe to (if any) // XXX: what if the child processes are moved? should we keep a reference to the parent(s) then update us within their vector..? std::vector successor_processes; std::vector predecessor_processes; + static_assert(std::is_same::value, "processes must be stored in same container type"); + // TODO: how should we handle this..? // The reason why its not trivial is that we may want to have a file pipe to multiple processes, and it feels // like a waste to read from the file N times to output to N processes. @@ -545,11 +556,20 @@ class Process { mutable size_t lines_written = 0; mutable size_t lines_received = 0; + static size_t process_id_counter; + internal::Process owned_proc; + size_t identifier = process_id_counter++; std::deque stdin_queue; std::deque stdout_queue; + protected: + std::string get_identifier() { + std::stringstream ret; + ret << owned_proc.processArgs[0] << process_id_counter; + return ret.str(); + } void pump_input() { assert(started && "error: input propagated for inactive process"); @@ -573,10 +593,10 @@ class Process { } void pump_output() { + if (finished) return; assert(started && "error: output propagated for inactive process"); while (owned_proc.isReady()) { - lines_written++; std::string out = owned_proc.readLine(); this->write_next(out); @@ -604,30 +624,127 @@ class Process { } } + /** + * Read from this process (and all predecessors) until their and this process is finished. + */ + void read_until_completion() { + // XXX: should we colour this to check that it isn't a cyclic network, and throw an exception? + if (finished) { + return; + } + + // we need to do block and read for the preds first (they must complete before this one can) + for (Process* pred_process : predecessor_processes) { + pred_process->read_until_completion(); + } + + // then block read for us & forward output + std::string processOutput; + while ((processOutput = owned_proc.readLine()).size() > 0) { + this->write_next(processOutput); + finished = true; + } + } + + /** build a graphvis string of predecessors recursively */ + std::string build_pred_topology() { + std::stringstream ret; + + for (Process* p : predecessor_processes) { + ret << p->get_identifier() << "-->" << this->get_identifier() << "\n"; + ret << p->build_pred_topology(); + } + + return ret.str(); + } + + std::string build_succ_topology() { + std::stringstream ret; + + for (Process* p : successor_processes) { + ret << this->get_identifier() << "-->" << p->get_identifier() << "\n"; + ret << p->build_succ_topology(); + } + + return ret.str(); + } + public: template> - Process(const std::string& commandPath, const ArgIterable& commandArgs = internal::dummyVec, Functor func = [](std::string){}) : + Process(const std::string& commandPath, const ArgIterable& commandArgs = internal::dummyVec) : + owned_proc(commandPath, commandArgs.begin(), commandArgs.end(), internal::dummyVec.begin(), internal::dummyVec.end()) { + } + template> + Process(const std::string& commandPath, const ArgIterable& commandArgs, Functor func) : owned_proc(commandPath, commandArgs.begin(), commandArgs.end(), internal::dummyVec.begin(), internal::dummyVec.end()) { // TODO: change back to new Functor(func), when I'm able to set the member variables type in the ctor. this->func = new std::function(func); } - ~Process() { - // err, need to close all predecessors (if any) - // if there is a cycle in the processes, this doesn't cause an infinite loop - // as if they're already closed, they're a no-op. + /** + * Get a graphvis compatible representation of the process network (DOT format) + * first_call is a bool determining whether this is the first level of recursion or not. + * TODO: less hacky way ^^ + * + */ + std::string get_network_topology(bool first_call=true) { + std::stringstream ret; + + if (first_call) { + ret << "digraph G {\n"; + } + + auto append_topology = [&](decltype(predecessor_processes)& processes) { + for (Process* proc: processes) { + ret << proc->get_network_topology(false); + } + }; + + enum ORDER_TYPE { PRED, SUCC }; + auto append_edges = [&](decltype(predecessor_processes)& processes, + enum ORDER_TYPE type) { + for (Process* p : processes) { + // edge direction based on type + if (type == PRED) { + ret << p->get_identifier() << "-->" << this->get_identifier() << "\n"; + } else if (type == SUCC) { + ret << this->get_identifier() << "-->" << p->get_identifier() << "\n"; + } + } + }; + + append_topology(predecessor_processes); + + // insert this node, and edges (identifier is process name plus incrementing integer) + append_edges(predecessor_processes, ORDER_TYPE::PRED); + append_edges(successor_processes, ORDER_TYPE::SUCC); + + append_topology(successor_processes); + + if (first_call) { + ret << "}\n"; + } + + return ret.str(); + } + + virtual ~Process() { + // what needs to be done here is that output from predecessors needs to be forced until completion + + // need to close all predecessors (if any) for (Process* pred_process : predecessor_processes) { pred_process->finish(); } - this->owned_proc.sendEOF(); + // this->owned_proc.sendEOF(); // process any remaining input/output finish(); // do the same for outputting processes - for (Process* succ_process : successor_processes) { - succ_process->finish(); - } + // XXX: i don't think I need/should do this, right? + // for (Process* succ_process : successor_processes) { + // succ_process->finish(); + // } // our function is dynamically allocated (how else do we test for null functors..?) delete func; @@ -643,7 +760,7 @@ class Process { started = true; // recursively start all predecessor processes - // do this to ensure that + // do this to ensure that if this process relies on a predecessor's input, then it will terminate. for (auto pred_process : predecessor_processes) { pred_process->start(); } @@ -661,20 +778,12 @@ class Process { int finish() { if (finished) return this->retval; + pump_input(); + read_until_completion(); pump_output(); - // iterate over each line of remaining output by the child's stdout, and call the functor - std::string processOutput; - while ((processOutput = owned_proc.readLine()).size() > 0) { - this->write_next(processOutput); - lines_written++; - } - - this->retval = owned_proc.waitUntilFinished(); - finished = true; - - return this->retval; + return owned_proc.waitUntilFinished(); } bool is_started() const { return started; } @@ -747,6 +856,7 @@ class Process { // return receiver; // } void output_to_file(const std::string& filename) { + // XXX: in some compilers this causes a warning, whilst in others omitting it causes an error. feedout_files.push_back(std::move(std::ofstream(filename))); if (!feedout_files.back().good()) throw std::runtime_error("error: file " + filename + " failed to open"); } @@ -788,4 +898,29 @@ class Process { iterator end(); }; +// initialise the id counter, dumb c++ standard doesn't allow it +size_t Process::process_id_counter = 0; + +/** + * An async equivalent of Process + * It constantly blocks for input to allow cyclic process flows + */ +class AsyncProcess : Process { + + std::future retval; + + public: + template> + AsyncProcess(const std::string& commandPath, const ArgIterable& commandArgs = internal::dummyVec, Functor func = [](std::string){}) : + Process(commandPath, commandArgs, func) { } + + ~AsyncProcess() { + + } + + void start() { + + } +}; + } // end namespace subprocess diff --git a/test.cpp b/test.cpp index b8ba308..17c36ec 100644 --- a/test.cpp +++ b/test.cpp @@ -327,6 +327,8 @@ TEST_CASE("process functor", "[subprocess::Process]") { // just ensure that even after the dtor, the functor isn't invoked again! std::string line; size_t func_count = 0; + // as the dtors are invoked in a stack like matter, this fn will be checked after the + // process' termination auto deferred_assertion = Deferrable>([&]() { REQUIRE(func_count == 1); }); @@ -337,9 +339,8 @@ TEST_CASE("process functor", "[subprocess::Process]") { }); p.start(); - p >> line; + p.finish(); - REQUIRE(line == "henlo world\n"); REQUIRE(func_count == 1); } diff --git a/test_programs/large_outputter.cpp b/test_programs/large_outputter.cpp index 2d0f407..7e7bd54 100644 --- a/test_programs/large_outputter.cpp +++ b/test_programs/large_outputter.cpp @@ -84,6 +84,7 @@ int main(int argc, char* argv[]) { return EXIT_FAILURE; } + // default output length ssize_t num_bytes = 1 << 17; if (argc >= 3) { num_bytes = std::stoi(std::string(argv[2])); From abe7eea9b8c43346fc0dded65ca6840b04726d80 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Fri, 14 Dec 2018 18:20:48 +1100 Subject: [PATCH 16/17] graphs work better, but not perfect --- subprocess.hpp | 48 ++++++++++-------------------------------------- 1 file changed, 10 insertions(+), 38 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 9cca9ee..976967a 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -566,9 +566,7 @@ class Process { protected: std::string get_identifier() { - std::stringstream ret; - ret << owned_proc.processArgs[0] << process_id_counter; - return ret.str(); + return std::to_string(identifier); } void pump_input() { @@ -650,8 +648,9 @@ class Process { std::string build_pred_topology() { std::stringstream ret; + ret << this->get_identifier() << " [label=\"" << this->owned_proc.processArgs[0] << "\"];\n"; for (Process* p : predecessor_processes) { - ret << p->get_identifier() << "-->" << this->get_identifier() << "\n"; + ret << p->get_identifier() << "->" << this->get_identifier() << ";\n"; ret << p->build_pred_topology(); } @@ -660,9 +659,10 @@ class Process { std::string build_succ_topology() { std::stringstream ret; + ret << this->get_identifier() << " [label=\"" << this->owned_proc.processArgs[0] << "\"];\n"; for (Process* p : successor_processes) { - ret << this->get_identifier() << "-->" << p->get_identifier() << "\n"; + ret << this->get_identifier() << "->" << p->get_identifier() << ";\n"; ret << p->build_succ_topology(); } @@ -687,43 +687,15 @@ class Process { * TODO: less hacky way ^^ * */ - std::string get_network_topology(bool first_call=true) { + std::string get_network_topology() { std::stringstream ret; - if (first_call) { - ret << "digraph G {\n"; - } - - auto append_topology = [&](decltype(predecessor_processes)& processes) { - for (Process* proc: processes) { - ret << proc->get_network_topology(false); - } - }; - - enum ORDER_TYPE { PRED, SUCC }; - auto append_edges = [&](decltype(predecessor_processes)& processes, - enum ORDER_TYPE type) { - for (Process* p : processes) { - // edge direction based on type - if (type == PRED) { - ret << p->get_identifier() << "-->" << this->get_identifier() << "\n"; - } else if (type == SUCC) { - ret << this->get_identifier() << "-->" << p->get_identifier() << "\n"; - } - } - }; - - append_topology(predecessor_processes); - - // insert this node, and edges (identifier is process name plus incrementing integer) - append_edges(predecessor_processes, ORDER_TYPE::PRED); - append_edges(successor_processes, ORDER_TYPE::SUCC); + ret << "digraph G {\n"; - append_topology(successor_processes); + ret << build_pred_topology(); + ret << build_succ_topology(); - if (first_call) { - ret << "}\n"; - } + ret << "}\n"; return ret.str(); } From 58e1dccc5230930d015d3f66f81bdf5647ffa6f6 Mon Sep 17 00:00:00 2001 From: Patrick Nappa Date: Fri, 14 Dec 2018 21:07:34 +1100 Subject: [PATCH 17/17] network topology seems to work --- subprocess.hpp | 60 +++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/subprocess.hpp b/subprocess.hpp index 976967a..18dd347 100644 --- a/subprocess.hpp +++ b/subprocess.hpp @@ -2,6 +2,8 @@ #include #include +#include +#include #include #include #include @@ -644,31 +646,6 @@ class Process { } } - /** build a graphvis string of predecessors recursively */ - std::string build_pred_topology() { - std::stringstream ret; - - ret << this->get_identifier() << " [label=\"" << this->owned_proc.processArgs[0] << "\"];\n"; - for (Process* p : predecessor_processes) { - ret << p->get_identifier() << "->" << this->get_identifier() << ";\n"; - ret << p->build_pred_topology(); - } - - return ret.str(); - } - - std::string build_succ_topology() { - std::stringstream ret; - ret << this->get_identifier() << " [label=\"" << this->owned_proc.processArgs[0] << "\"];\n"; - - for (Process* p : successor_processes) { - ret << this->get_identifier() << "->" << p->get_identifier() << ";\n"; - ret << p->build_succ_topology(); - } - - return ret.str(); - } - public: template> Process(const std::string& commandPath, const ArgIterable& commandArgs = internal::dummyVec) : @@ -683,17 +660,40 @@ class Process { /** * Get a graphvis compatible representation of the process network (DOT format) - * first_call is a bool determining whether this is the first level of recursion or not. - * TODO: less hacky way ^^ - * */ std::string get_network_topology() { std::stringstream ret; ret << "digraph G {\n"; - ret << build_pred_topology(); - ret << build_succ_topology(); + std::set visited_processes; + std::stack to_visit; + + to_visit.emplace(this); + + while (!to_visit.empty()) { + Process* top = to_visit.top(); + to_visit.pop(); + // ignore the already visited + if (visited_processes.count(top)) continue; + + visited_processes.emplace(top); + + // add the label for this process + ret << top->get_identifier() << " [label=\"" << top->owned_proc.processArgs[0] << "\"];\n"; + + // add edges for each of the parents and children, then queue them up to be visited + // as predecessor_procs and successor_procs are symmetric, we only need to add one + for (Process* proc : top->predecessor_processes) { + ret << proc->get_identifier() << "->" << top->get_identifier() << ";\n"; + to_visit.emplace(proc); + } + + for (Process* proc : top->successor_processes) { + to_visit.emplace(proc); + } + + } ret << "}\n";