Repository: mesos Updated Branches: refs/heads/master 83fcc8ca1 -> 02fa100ea
Allowed launching subprocesses with flags. Review: https://reviews.apache.org/r/22749 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/82eb111a Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/82eb111a Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/82eb111a Branch: refs/heads/master Commit: 82eb111a536c0d5935ca4ff92ba6434a9bbae34b Parents: 83fcc8c Author: Jie Yu <yujie....@gmail.com> Authored: Wed Jun 18 11:43:44 2014 -0700 Committer: Jie Yu <yujie....@gmail.com> Committed: Wed Jun 18 14:56:07 2014 -0700 ---------------------------------------------------------------------- .../libprocess/include/process/subprocess.hpp | 12 +- 3rdparty/libprocess/src/subprocess.cpp | 25 +++- .../libprocess/src/tests/subprocess_tests.cpp | 118 +++++++++++++++++++ 3 files changed, 150 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/82eb111a/3rdparty/libprocess/include/process/subprocess.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/include/process/subprocess.hpp b/3rdparty/libprocess/include/process/subprocess.hpp index e93608a..d6e2c1f 100644 --- a/3rdparty/libprocess/include/process/subprocess.hpp +++ b/3rdparty/libprocess/include/process/subprocess.hpp @@ -10,6 +10,7 @@ #include <process/future.hpp> +#include <stout/flags.hpp> #include <stout/lambda.hpp> #include <stout/memory.hpp> #include <stout/option.hpp> @@ -45,6 +46,7 @@ public: const IO& in, const IO& out, const IO& err, + const Option<flags::FlagsBase>& flags, const Option<std::map<std::string, std::string> >& environment, const Option<lambda::function<int()> >& setup); @@ -94,9 +96,10 @@ public: private: friend Try<Subprocess> subprocess( const std::string& command, - const Subprocess::IO& in, - const Subprocess::IO& out, - const Subprocess::IO& err, + const IO& in, + const IO& out, + const IO& err, + const Option<flags::FlagsBase>& flags, const Option<std::map<std::string, std::string> >& environment, const Option<lambda::function<int()> >& setup); @@ -142,12 +145,14 @@ Try<Subprocess> subprocess( const Subprocess::IO& in, const Subprocess::IO& out, const Subprocess::IO& err, + const Option<flags::FlagsBase>& flags = None(), const Option<std::map<std::string, std::string> >& environment = None(), const Option<lambda::function<int()> >& setup = None()); inline Try<Subprocess> subprocess( const std::string& command, + const Option<flags::FlagsBase>& flags = None(), const Option<std::map<std::string, std::string> >& environment = None(), const Option<lambda::function<int()> >& setup = None()) { @@ -156,6 +161,7 @@ inline Try<Subprocess> subprocess( Subprocess::FD(STDIN_FILENO), Subprocess::FD(STDOUT_FILENO), Subprocess::FD(STDERR_FILENO), + flags, environment, setup); } http://git-wip-us.apache.org/repos/asf/mesos/blob/82eb111a/3rdparty/libprocess/src/subprocess.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp index 78fa1ec..6e2601d 100644 --- a/3rdparty/libprocess/src/subprocess.cpp +++ b/3rdparty/libprocess/src/subprocess.cpp @@ -16,6 +16,7 @@ #include <stout/foreach.hpp> #include <stout/option.hpp> #include <stout/os.hpp> +#include <stout/strings.hpp> #include <stout/try.hpp> #include <stout/unreachable.hpp> @@ -86,10 +87,11 @@ static Try<Nothing> cloexec(int stdinFd[2], int stdoutFd[2], int stderrFd[2]) // Runs the provided command in a subprocess. Try<Subprocess> subprocess( - const string& command, + const string& _command, const Subprocess::IO& in, const Subprocess::IO& out, const Subprocess::IO& err, + const Option<flags::FlagsBase>& flags, const Option<map<string, string> >& environment, const Option<lambda::function<int()> >& setup) { @@ -217,6 +219,22 @@ Try<Subprocess> subprocess( return Error("Failed to cloexec: " + cloexec.error()); } + // Prepare the command to execute. If the user specifies the + // 'flags', we will stringify it and append it to the command. + string command = _command; + + if (flags.isSome()) { + foreachpair (const string& name, const flags::Flag& flag, flags.get()) { + Option<string> value = flag.stringify(flags.get()); + if (value.isSome()) { + // TODO(jieyu): Need a better way to escape quotes. For + // example, what if 'value.get()' contains a single quote? + string argument = "--" + name + "='" + value.get() + "'"; + command = strings::join(" ", command, argument); + } + } + } + // We need to do this construction before doing the fork as it // might not be async-safe. // TODO(tillt): Consider optimizing this to not pass an empty map @@ -281,9 +299,12 @@ Try<Subprocess> subprocess( } } + // TODO(jieyu): Consider providing an optional way to launch the + // subprocess without using the shell (similar to 'shell=False' + // used in python subprocess.Popen). execle("/bin/sh", "sh", "-c", command.c_str(), (char*) NULL, envp()); - ABORT("Failed to execle '/bin sh -c ", command.c_str(), "'\n"); + ABORT("Failed to execle '/bin/sh -c ", command.c_str(), "'\n"); } // Parent. http://git-wip-us.apache.org/repos/asf/mesos/blob/82eb111a/3rdparty/libprocess/src/tests/subprocess_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/subprocess_tests.cpp b/3rdparty/libprocess/src/tests/subprocess_tests.cpp index 1cb1ce3..7dfa384 100644 --- a/3rdparty/libprocess/src/tests/subprocess_tests.cpp +++ b/3rdparty/libprocess/src/tests/subprocess_tests.cpp @@ -6,6 +6,7 @@ #include <map> #include <string> +#include <vector> #include <process/gmock.hpp> #include <process/gtest.hpp> @@ -25,6 +26,7 @@ using namespace process; using std::map; using std::string; +using std::vector; class SubprocessTest: public TemporaryDirectoryTest {}; @@ -500,6 +502,114 @@ TEST_F(SubprocessTest, Default) } +struct Flags : public flags::FlagsBase +{ + Flags() + { + add(&b, "b", "bool"); + add(&i, "i", "int"); + add(&s, "s", "string"); + add(&d, "d", "Duration"); + add(&y, "y", "Bytes"); + add(&j, "j", "JSON::Object"); + } + + Option<bool> b; + Option<int> i; + Option<string> s; + Option<Duration> d; + Option<Bytes> y; + Option<JSON::Object> j; +}; + + +TEST_F(SubprocessTest, Flags) +{ + Clock::pause(); + + Flags flags; + flags.b = true; + flags.i = 42; + flags.s = "hello"; + flags.d = Seconds(10); + flags.y = Bytes(100); + + JSON::Object object; + object.values["strings"] = "string"; + object.values["integer1"] = 1; + object.values["integer2"] = -1; + object.values["double1"] = 1; + object.values["double2"] = -1; + object.values["double3"] = -1.42; + + JSON::Object nested; + nested.values["string"] = "string"; + object.values["nested"] = nested; + + JSON::Array array; + array.values.push_back(nested); + object.values["array"] = array; + + flags.j = object; + + string out = path::join(os::getcwd(), "stdout"); + + Try<Subprocess> s = subprocess( + "echo", + Subprocess::PIPE(), + Subprocess::PATH(out), + Subprocess::PIPE(), + flags); + + ASSERT_SOME(s); + + // Advance time until the internal reaper reaps the subprocess. + while (s.get().status().isPending()) { + Clock::advance(Seconds(1)); + Clock::settle(); + } + + AWAIT_ASSERT_READY(s.get().status()); + ASSERT_SOME(s.get().status().get()); + + int status = s.get().status().get().get(); + EXPECT_TRUE(WIFEXITED(status)); + EXPECT_EQ(0, WEXITSTATUS(status)); + + // Parse the output and make sure that it matches the flags we + // specified in the beginning. + Try<string> read = os::read(out); + ASSERT_SOME(read); + + // TODO(jieyu): Consider testing the case where escaped spaces exist + // in the arguments. + vector<string> split = strings::split(read.get(), " "); + int argc = split.size() + 1; + char** argv = new char*[argc]; + argv[0] = (char*) "command"; + for (int i = 1; i < argc; i++) { + argv[i] = ::strdup(split[i - 1].c_str()); + } + + Flags flags2; + Try<Nothing> load = flags2.load(None(), argc, argv); + ASSERT_SOME(load); + EXPECT_EQ(flags.b, flags2.b); + EXPECT_EQ(flags.i, flags2.i); + EXPECT_EQ(flags.s, flags2.s); + EXPECT_EQ(flags.d, flags2.d); + EXPECT_EQ(flags.y, flags2.y); + EXPECT_EQ(flags.j, flags2.j); + + for (int i = 1; i < argc; i++) { + ::free(argv[i]); + } + delete argv; + + Clock::resume(); +} + + TEST_F(SubprocessTest, Environment) { Clock::pause(); @@ -513,6 +623,7 @@ TEST_F(SubprocessTest, Environment) Subprocess::PIPE(), Subprocess::PIPE(), Subprocess::PIPE(), + None(), environment); ASSERT_SOME(s); @@ -543,6 +654,7 @@ TEST_F(SubprocessTest, Environment) Subprocess::PIPE(), Subprocess::PIPE(), Subprocess::PIPE(), + None(), environment); ASSERT_SOME(s); @@ -580,6 +692,7 @@ TEST_F(SubprocessTest, EnvironmentWithSpaces) Subprocess::PIPE(), Subprocess::PIPE(), Subprocess::PIPE(), + None(), environment); ASSERT_SOME(s); @@ -617,6 +730,7 @@ TEST_F(SubprocessTest, EnvironmentWithSpacesAndQuotes) Subprocess::PIPE(), Subprocess::PIPE(), Subprocess::PIPE(), + None(), environment); ASSERT_SOME(s); @@ -656,6 +770,7 @@ TEST_F(SubprocessTest, EnvironmentOverride) Subprocess::PIPE(), Subprocess::PIPE(), Subprocess::PIPE(), + None(), environment); ASSERT_SOME(s); @@ -705,6 +820,7 @@ TEST_F(SubprocessTest, Setup) Subprocess::PIPE(), Subprocess::PIPE(), None(), + None(), lambda::bind(&setupChdir, directory.get())); ASSERT_SOME(s); @@ -746,6 +862,7 @@ TEST_F(SubprocessTest, SetupStatus) Subprocess::PIPE(), Subprocess::PIPE(), None(), + None(), lambda::bind(&setupStatus, 1)); ASSERT_SOME(s); @@ -772,6 +889,7 @@ TEST_F(SubprocessTest, SetupStatus) Subprocess::PIPE(), Subprocess::PIPE(), None(), + None(), lambda::bind(&setupStatus, 0)); ASSERT_SOME(s);