From 7c4f5e6464a7d9a9c48b4d6773fbb348624cc32e Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 6 Jun 2017 14:05:38 +0300 Subject: Support passing environment variables to child process --- libbutl/curl.txx | 1 - libbutl/openssl.hxx | 6 +- libbutl/openssl.ixx | 5 +- libbutl/openssl.txx | 23 ++++---- libbutl/process-run.cxx | 8 ++- libbutl/process-run.txx | 104 ++++++++++---------------------- libbutl/process.cxx | 46 +++++++++++---- libbutl/process.hxx | 138 ++++++++++++++++++++++++++++++++----------- libbutl/process.ixx | 32 ++++------ libbutl/sendmail.cxx | 2 +- libbutl/sendmail.ixx | 1 - tests/process-run/driver.cxx | 8 +-- tests/process/driver.cxx | 73 +++++++++++++++++++---- 13 files changed, 269 insertions(+), 178 deletions(-) diff --git a/libbutl/curl.txx b/libbutl/curl.txx index f6f0e96..fb48418 100644 --- a/libbutl/curl.txx +++ b/libbutl/curl.txx @@ -84,7 +84,6 @@ namespace butl map_in (std::forward (in), mp, in_data), map_out (std::forward (out), mp, out_data), std::forward (err), - dir_path (), "curl", "-s", // Silent. "-S", // But do show diagnostics. diff --git a/libbutl/openssl.hxx b/libbutl/openssl.hxx index 2f54f93..422e033 100644 --- a/libbutl/openssl.hxx +++ b/libbutl/openssl.hxx @@ -88,12 +88,11 @@ namespace butl template openssl (I&& in, O&& out, E&& err, - const P& program, + const process_env&, const std::string& command, A&&... options); @@ -103,13 +102,12 @@ namespace butl typename I, typename O, typename E, - typename P, typename... A> openssl (const C&, I&& in, O&& out, E&& err, - const P& program, + const process_env&, const std::string& command, A&&... options); diff --git a/libbutl/openssl.ixx b/libbutl/openssl.ixx index 2af5029..2976659 100644 --- a/libbutl/openssl.ixx +++ b/libbutl/openssl.ixx @@ -10,20 +10,19 @@ namespace butl template inline openssl:: openssl (I&& in, O&& out, E&& err, - const P& program, + const process_env& env, const std::string& command, A&&... options) : openssl ([] (const char* [], std::size_t) {}, std::forward (in), std::forward (out), std::forward (err), - program, + env, command, std::forward (options)...) { diff --git a/libbutl/openssl.txx b/libbutl/openssl.txx index aaaa239..0a1788f 100644 --- a/libbutl/openssl.txx +++ b/libbutl/openssl.txx @@ -24,14 +24,13 @@ namespace butl typename I, typename O, typename E, - typename P, typename... A> openssl:: openssl (const C& cmdc, I&& in, O&& out, E&& err, - const P& program, + const process_env& env, const std::string& command, A&&... options) { @@ -39,17 +38,15 @@ namespace butl io_data out_data; process& p (*this); - p = process_start ( - cmdc, - map_in (std::forward (in), in_data), - map_out (std::forward (out), out_data), - std::forward (err), - dir_path (), - program, - command, - in_data.options, - out_data.options, - std::forward (options)...); + p = process_start (cmdc, + map_in (std::forward (in), in_data), + map_out (std::forward (out), out_data), + std::forward (err), + env, + command, + in_data.options, + out_data.options, + std::forward (options)...); // Note: leaving this scope closes any open ends of the pipes in io_data. } diff --git a/libbutl/process-run.cxx b/libbutl/process-run.cxx index ce5ab20..9c857d0 100644 --- a/libbutl/process-run.cxx +++ b/libbutl/process-run.cxx @@ -12,16 +12,20 @@ using namespace std; namespace butl { process - process_start (const dir_path& cwd, + process_start (const dir_path* cwd, const process_path& pp, const char* cmd[], + const char* const* envvars, int in, int out, int err) { try { - return process (cwd.string ().c_str (), pp, cmd, in, out, err); + return process (pp, cmd, + in, out, err, + cwd != nullptr ? cwd->string ().c_str () : nullptr, + envvars); } catch (const process_child_error& e) { diff --git a/libbutl/process-run.txx b/libbutl/process-run.txx index 28c44cf..d8b4847 100644 --- a/libbutl/process-run.txx +++ b/libbutl/process-run.txx @@ -3,10 +3,23 @@ // license : MIT; see accompanying LICENSE file #include -#include // move(), forward(), index_sequence +#include // forward(), index_sequence namespace butl { + template + process_env:: + process_env (const process_path& p, const V& v) + : process_env (p) + { + if (!v.empty ()) + { + process_args_as (vars_, v, storage_); + vars_.push_back (nullptr); + vars = vars_.data (); + } + } + inline int process_stdin (int v) {assert (v >= 0); return v;} inline int process_stdout (int v) {assert (v >= 0); return v;} inline int process_stderr (int v) {assert (v >= 0); return v;} @@ -21,9 +34,10 @@ namespace butl process_stderr (const auto_fd& v) {assert (v.get () >= 0); return v.get ();} LIBBUTL_EXPORT process - process_start (const dir_path& cwd, + process_start (const dir_path* cwd, const process_path& pp, const char* cmd[], + const char* const* envvars, int in, int out, int err); @@ -48,8 +62,7 @@ namespace butl I&& in, O&& out, E&& err, - const dir_path& cwd, - const process_path& pp, + const process_env& env, A&&... args) { // Map stdin/stdout/stderr arguments to their integer values, as expected @@ -64,7 +77,9 @@ namespace butl const std::size_t args_size (sizeof... (args)); small_vector cmd; - cmd.push_back (pp.recall_string ()); + + assert (env.path != nullptr); + cmd.push_back (env.path->recall_string ()); std::string storage[args_size != 0 ? args_size : 1]; @@ -78,7 +93,10 @@ namespace butl // @@ Do we need to make sure certain fd's are closed before calling // wait()? Is this only the case with pipes? Needs thinking. - return process_start (cwd, pp, cmd.data (), in_i, out_i, err_i); + return process_start (env.cwd, + *env.path, cmd.data (), + env.vars, + in_i, out_i, err_i); } template (), @@ -100,54 +117,26 @@ namespace butl std::forward (in), std::forward (out), std::forward (err), - cwd, - pp, + env, std::forward (args)...); } template inline process process_start (I&& in, O&& out, E&& err, - const dir_path& cwd, - const P& p, + const process_env& env, A&&... args) { return process_start ([] (const char* [], std::size_t) {}, std::forward (in), std::forward (out), std::forward (err), - cwd, - process::path_search (p, true), - std::forward (args)...); - } - - template - inline process - process_start (const C& cmdc, - I&& in, - O&& out, - E&& err, - const dir_path& cwd, - const P& p, - A&&... args) - { - return process_start (cmdc, - std::forward (in), - std::forward (out), - std::forward (err), - cwd, - process::path_search (p, true), + env, std::forward (args)...); } @@ -161,8 +150,7 @@ namespace butl I&& in, O&& out, E&& err, - const dir_path& cwd, - const process_path& pp, + const process_env& env, A&&... args) { process pr ( @@ -170,8 +158,7 @@ namespace butl std::forward (in), std::forward (out), std::forward (err), - cwd, - pp, + env, std::forward (args)...)); pr.wait (); @@ -181,46 +168,19 @@ namespace butl template inline process_exit process_run (I&& in, O&& out, E&& err, - const dir_path& cwd, - const P& p, + const process_env& env, A&&... args) { return process_run ([] (const char* [], std::size_t) {}, std::forward (in), std::forward (out), std::forward (err), - cwd, - process::path_search (p, true), - std::forward (args)...); - } - - template - inline process_exit - process_run (const C& cmdc, - I&& in, - O&& out, - E&& err, - const dir_path& cwd, - const P& p, - A&&... args) - { - return process_run (cmdc, - std::forward (in), - std::forward (out), - std::forward (err), - cwd, - process::path_search (p, true), + env, std::forward (args)...); } } diff --git a/libbutl/process.cxx b/libbutl/process.cxx index 265c775..6f4fd30 100644 --- a/libbutl/process.cxx +++ b/libbutl/process.cxx @@ -5,6 +5,7 @@ #include #ifndef _WIN32 +# include // setenv(), unsetenv() # include // SIG* # include // execvp, fork, dup2, pipe, chdir, *_FILENO, getpid # include // waitpid @@ -134,10 +135,11 @@ namespace butl } process:: - process (const char* cwd, - const process_path& pp, const char* args[], - process& in, int out, int err) - : process (cwd, pp, args, in.in_ofd.get (), out, err) + process (const process_path& pp, const char* args[], + process& in, int out, int err, + const char* cwd, + const char* const* envvars) + : process (pp, args, in.in_ofd.get (), out, err, cwd, envvars) { assert (in.in_ofd.get () != -1); // Should be a pipe. in.in_ofd.reset (); // Close it on our side. @@ -264,9 +266,10 @@ namespace butl } process:: - process (const char* cwd, - const process_path& pp, const char* args[], - int in, int out, int err) + process (const process_path& pp, const char* args[], + int in, int out, int err, + const char* cwd, + const char* const* envvars) { fdpipe out_fd; fdpipe in_ofd; @@ -396,6 +399,23 @@ namespace butl if (cwd != nullptr && *cwd != '\0' && chdir (cwd) != 0) fail (true); + // Set/unset environment variables if requested. + // + if (envvars != nullptr) + { + while (const char* ev = *envvars++) + { + const char* v (strchr (ev, '=')); + + int r (v != nullptr + ? setenv (string (ev, v - ev).c_str (), v + 1, 1) + : unsetenv (ev)); + + if (r == -1) + fail (true); + } + } + if (execv (pp.effect_string (), const_cast (&args[0])) == -1) fail (true); } @@ -919,10 +939,16 @@ namespace butl static map detect_msys_cache_; process:: - process (const char* cwd, - const process_path& pp, const char* args[], - int in, int out, int err) + process (const process_path& pp, const char* args[], + int in, int out, int err, + const char* cwd, + const char* const* envvars) { + // Currently we don't support (un)setting environment variables for the + // child process. + // + assert (envvars == nullptr); + // Figure out if this is a batch file since running them requires starting // cmd.exe and passing the batch file as an argument (see CreateProcess() // for deails). diff --git a/libbutl/process.hxx b/libbutl/process.hxx index 1f82af0..983cb32 100644 --- a/libbutl/process.hxx +++ b/libbutl/process.hxx @@ -215,6 +215,16 @@ namespace butl // // process p (..., 0, 2); // + // The cwd argument allows to change the current working directory of the + // child process. NULL and empty arguments are ignored. + // + // The envvars argument allows to set and unset environment variables in + // the child process. If not NULL, it must contain strings in the + // "name=value" (set) or "name" (unset) forms and be terminated with + // NULL. Note that all other variables are inherited from the parent + // process. Also note that currently is not supported on Windows so must be + // NULL. + // // Throw process_error if anything goes wrong. Note that some of the // exceptions (e.g., if exec() failed) can be thrown in the child // version of us (as process_child_error). @@ -222,10 +232,15 @@ namespace butl // Note that the versions without the the process_path argument may // temporarily change args[0] (see path_search() for details). // - process (const char* args[], int in = 0, int out = 1, int err = 2); + process (const char* [], + int = 0, int = 1, int = 2, + const char* cwd = nullptr, + const char* const* envvars = nullptr); - process (const process_path&, const char* args[], - int in = 0, int out = 1, int err = 2); + process (const process_path&, const char* [], + int = 0, int = 1, int = 2, + const char* cwd = nullptr, + const char* const* envvars = nullptr); // The "piping" constructor, for example: // @@ -235,26 +250,15 @@ namespace butl // rhs.wait (); // Wait for last first. // lhs.wait (); // - process (const char* args[], process& in, int out = 1, int err = 2); - - process (const process_path&, const char* args[], - process& in, int out = 1, int err = 2); - - // Versions of the above constructors that allow us to change the - // current working directory of the child process. NULL and empty - // cwd arguments are ignored. - // - process (const char* cwd, const char* [], int = 0, int = 1, int = 2); - - process (const char* cwd, - const process_path&, const char* [], - int = 0, int = 1, int = 2); - - process (const char* cwd, const char* [], process&, int = 1, int = 2); + process (const char* [], + process&, int = 1, int = 2, + const char* cwd = nullptr, + const char* const* envvars = nullptr); - process (const char* cwd, - const process_path&, const char* [], - process&, int = 1, int = 2); + process (const process_path&, const char* [], + process&, int = 1, int = 2, + const char* cwd = nullptr, + const char* const* envvars = nullptr); // Wait for the process to terminate. Return true if the process // terminated normally and with the zero exit code. Unless ignore_error @@ -405,17 +409,89 @@ namespace butl // char*, std::string, path/dir_path, (as well as [small_]vector[_view] of // these), and numeric types. // + struct process_env + { + const process_path* path; + const dir_path* cwd = nullptr; + const char* const* vars = nullptr; + + process_env (const process_path& p, + const dir_path& c = dir_path (), + const char* const* v = nullptr) + : path (&p), + + // Note that this is not just an optimization. It is required when + // the ctor is called with the default arguments (not to keep the + // temporary object pointer). + // + cwd (!c.empty () ? &c : nullptr), + + vars (v) {} + + process_env (const process_path& p, const char* const* v) + : path (&p), cwd (nullptr), vars (v) {} + + template + process_env (const process_path& p, const dir_path& c, const V& v) + : process_env (p, v) {cwd = &c;} + + template + process_env (const process_path& p, const V& v); + + process_env (const char* p, + const dir_path& c = dir_path (), + const char* const* v = nullptr) + : process_env (path_, c, v) {path_ = process::path_search (p, true);} + + process_env (const std::string& p, + const dir_path& c = dir_path (), + const char* const* v = nullptr) + : process_env (p.c_str (), c, v) {} + + process_env (const butl::path& p, + const dir_path& c = dir_path (), + const char* const* v = nullptr) + : process_env (p.string (), c, v) {} + + template + process_env (const char* p, const dir_path& c, const V& v) + : process_env (path_, c, v) {path_ = process::path_search (p, true);} + + template + process_env (const std::string& p, const dir_path& c, const V& v) + : process_env (p.c_str (), c, v) {} + + template + process_env (const butl::path& p, const dir_path& c, const V& v) + : process_env (p.string (), c, v) {} + + template + process_env (const char* p, const V& v) + : process_env (path_, v) {path_ = process::path_search (p, true);} + + template + process_env (const std::string& p, const V& v) + : process_env (p.c_str (), v) {} + + template + process_env (const butl::path& p, const V& v) + : process_env (p.string (), v) {} + + private: + process_path path_; + small_vector vars_; + std::string storage_; + }; + template process_exit process_run (I&& in, O&& out, E&& err, - const dir_path& cwd, - const P&, + const process_env&, A&&... args); // The version with the command callback that can be used for printing the @@ -428,15 +504,13 @@ namespace butl typename I, typename O, typename E, - typename P, typename... A> process_exit process_run (const C&, I&& in, O&& out, E&& err, - const dir_path& cwd, - const P&, + const process_env&, A&&... args); // Versions that start the process without waiting. @@ -444,29 +518,25 @@ namespace butl template process process_start (I&& in, O&& out, E&& err, - const dir_path& cwd, - const P&, + const process_env&, A&&... args); template process process_start (const C&, I&& in, O&& out, E&& err, - const dir_path& cwd, - const P&, + const process_env&, A&&... args); // Conversion of types to their C string representations. Can be overloaded diff --git a/libbutl/process.ixx b/libbutl/process.ixx index 9a09487..4bdd438 100644 --- a/libbutl/process.ixx +++ b/libbutl/process.ixx @@ -138,30 +138,18 @@ namespace butl } inline process:: - process (const char* args[], int in, int out, int err) - : process (nullptr, path_search (args[0]), args, in, out, err) {} + process (const char* args[], + int in, int out, int err, + const char* cwd, + const char* const* envvars) + : process (path_search (args[0]), args, in, out, err, cwd, envvars) {} inline process:: - process (const process_path& pp, const char* args[], - int in, int out, int err) - : process (nullptr, pp, args, in, out, err) {} - - inline process:: - process (const char* args[], process& in, int out, int err) - : process (nullptr, path_search (args[0]), args, in, out, err) {} - - inline process:: - process (const process_path& pp, const char* args[], - process& in, int out, int err) - : process (nullptr, pp, args, in, out, err) {} - - inline process:: - process (const char* cwd, const char* args[], int in, int out, int err) - : process (cwd, path_search (args[0]), args, in, out, err) {} - - inline process:: - process (const char* cwd, const char* args[], process& in, int out, int err) - : process (cwd, path_search (args[0]), args, in, out, err) {} + process (const char* args[], + process& in, int out, int err, + const char* cwd, + const char* const* envvars) + : process (path_search (args[0]), args, in, out, err, cwd, envvars) {} inline process:: process (process&& p) diff --git a/libbutl/sendmail.cxx b/libbutl/sendmail.cxx index 5b1c863..abdea32 100644 --- a/libbutl/sendmail.cxx +++ b/libbutl/sendmail.cxx @@ -18,7 +18,7 @@ namespace butl if (!from.empty ()) out << "From: " << from << endl; - auto rcp =[this] (const char* h, const recipients_type& rs) + auto rcp = [this] (const char* h, const recipients_type& rs) { if (!rs.empty ()) { diff --git a/libbutl/sendmail.ixx b/libbutl/sendmail.ixx index 9746a8b..72d3d45 100644 --- a/libbutl/sendmail.ixx +++ b/libbutl/sendmail.ixx @@ -66,7 +66,6 @@ namespace butl pipe.in, 2, // No output expected so redirect to stderr. std::forward (err), - dir_path (), "sendmail", "-i", // Don't treat '.' as the end of input. "-t", // Read recipients from headers. diff --git a/tests/process-run/driver.cxx b/tests/process-run/driver.cxx index ed35d1e..ff9dd25 100644 --- a/tests/process-run/driver.cxx +++ b/tests/process-run/driver.cxx @@ -7,6 +7,7 @@ #include #include #include +#include using namespace std; using namespace butl; @@ -14,20 +15,18 @@ using namespace butl; template process_exit run (I&& in, O&& out, E&& err, - const P& p, + const process_env& env, A&&... args) { return process_run (forward (in), forward (out), forward (err), - dir_path (), - p, + env, forward (args)...); } @@ -72,7 +71,6 @@ main (int argc, const char* argv[]) cout << endl; }, 0, 1, 2, - dir_path (), p, "-c"); diff --git a/tests/process/driver.cxx b/tests/process/driver.cxx index 4694d5b..cd79d71 100644 --- a/tests/process/driver.cxx +++ b/tests/process/driver.cxx @@ -19,6 +19,8 @@ using namespace std; using namespace butl; +static const char* envvars[] = {"ABC=1", "DEF", nullptr}; + static bool exec (const path& p, vector in = vector (), @@ -26,7 +28,8 @@ exec (const path& p, bool err = false, bool pipeline = false, bool bin = true, // Set binary mode for file descriptors. - dir_path wd = dir_path ()) // Set the working dir for the child process. + dir_path wd = dir_path (), // Set the working dir for the child process. + bool env = false) // Set environment variables for the child. { using cstrings = vector; @@ -40,6 +43,25 @@ exec (const path& p, if (bin) args.push_back ("-b"); + if (env) + { + args.push_back ("-e"); + + // Here we set the environment variables for the current process to make + // sure that the child process will not see the variable that is requested + // to be unset, and will see the other one unaffected. + // + // Note that we don't support (un)setting environment variables for the + // child process on Windows. + // +#ifndef _WIN32 + assert (setenv ("DEF", "2", 1) == 0); + assert (setenv ("XYZ", "3", 1) == 0); +#else + assert (false); +#endif + } + if (cwd != nullptr) args.push_back (cwd); @@ -52,11 +74,12 @@ exec (const path& p, // If both o and e are true, then redirect STDERR to STDOUT, so both can be // read from the same stream. // - process pr (cwd, - args.data (), + process pr (args.data (), !in.empty () ? -1 : -2, out ? -1 : -2, - err ? (out ? 1 : -1) : -2); + err ? (out ? 1 : -1) : -2, + cwd, + env ? envvars : nullptr); try { @@ -88,10 +111,15 @@ exec (const path& p, // input fd as an output for another one (pr2.out = pr3.in). The // overall pipeline looks like 'os -> pr -> pr2 -> pr3 -> is'. // - process pr3 (cwd, args.data (), -1, -1, -2); + process pr3 (args.data (), + -1, -1, -2, + cwd, + env ? envvars : nullptr); - process pr2 ( - cwd, args.data (), pr, bin_mode (move (pr3.out_fd)).get (), -2); + process pr2 (args.data (), + pr, bin_mode (move (pr3.out_fd)).get (), -2, + cwd, + env ? envvars : nullptr); ifdstream is (bin_mode (move (pr3.in_ofd))); o = is.read_binary (); @@ -152,10 +180,11 @@ exec (const path& p, bool o = false, bool e = false, bool pipeline = false, - dir_path wd = dir_path ()) + dir_path wd = dir_path (), + bool env = false) { return exec ( - p, vector (i.begin (), i.end ()), o, e, pipeline, false, wd); + p, vector (i.begin (), i.end ()), o, e, pipeline, false, wd, env); } int @@ -163,7 +192,8 @@ main (int argc, const char* argv[]) { bool child (false); bool bin (false); - dir_path wd; // Working directory. + dir_path wd; // Working directory. + bool env (false); // Check the environment variables. assert (argc > 0); @@ -176,6 +206,8 @@ main (int argc, const char* argv[]) child = true; else if (v == "-b") bin = true; + else if (v == "-e") + env = true; else { if (!wd.empty ()) @@ -224,6 +256,18 @@ main (int argc, const char* argv[]) if (!wd.empty () && wd.realize () != dir_path::current_directory ()) return 1; + if (env) + { + // Check that the ABC variable is set, the DEF is unset and the XYZ is + // left unchanged. + // + const char* v; + if ((v = getenv ("ABC")) == nullptr || string ("1") != v || + getenv ("DEF") != nullptr || + (v = getenv ("XYZ")) == nullptr || string ("3") != v) + return 1; + } + try { if (bin) @@ -282,6 +326,15 @@ main (int argc, const char* argv[]) assert (exec (p, s, true, true)); assert (exec (p, s, true, true, true)); // Same but with piping. + // Passing environment variables to the child process. + // + // Note that we don't support (un)setting environment variables for the + // child process on Windows. + // +#ifndef _WIN32 + assert (exec (p, string (), false, false, false, dir_path (), true)); +#endif + // Transmit large binary data through the child. // vector v; -- cgit v1.1