You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2014/04/03 00:09:11 UTC
[2/2] git commit: Added optional environment map to subprocess.
Added optional environment map to subprocess.
Review: https://reviews.apache.org/r/19162
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/653462cc
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/653462cc
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/653462cc
Branch: refs/heads/master
Commit: 653462ccdf771cac0ee41c5377fb64e005a9fc19
Parents: 8cfea6f
Author: Dominic Hamon <dh...@twopensource.com>
Authored: Wed Apr 2 15:06:05 2014 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Wed Apr 2 15:08:48 2014 -0700
----------------------------------------------------------------------
.../libprocess/include/process/subprocess.hpp | 13 +-
3rdparty/libprocess/src/subprocess.cpp | 69 ++++++++++-
.../libprocess/src/tests/subprocess_tests.cpp | 121 +++++++++++++++++--
3 files changed, 188 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/653462cc/3rdparty/libprocess/include/process/subprocess.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/subprocess.hpp b/3rdparty/libprocess/include/process/subprocess.hpp
index 9e60e08..6c51c0b 100644
--- a/3rdparty/libprocess/include/process/subprocess.hpp
+++ b/3rdparty/libprocess/include/process/subprocess.hpp
@@ -3,6 +3,7 @@
#include <sys/types.h>
+#include <map>
#include <string>
#include <process/future.hpp>
@@ -36,7 +37,9 @@ struct Subprocess
private:
Subprocess() : data(new Data()) {}
- friend Try<Subprocess> subprocess(const std::string&);
+ friend Try<Subprocess> subprocess(
+ const std::string& command,
+ const std::map<std::string, std::string>& environment);
struct Data
{
@@ -62,7 +65,13 @@ private:
};
-Try<Subprocess> subprocess(const std::string& command);
+// Environment is combined with the OS environment and overrides where
+// necessary.
+// TODO(dhamon): Add an option to not combine the two environments.
+Try<Subprocess> subprocess(
+ const std::string& command,
+ const std::map<std::string, std::string>& environment =
+ std::map<std::string, std::string>());
} // namespace process {
http://git-wip-us.apache.org/repos/asf/mesos/blob/653462cc/3rdparty/libprocess/src/subprocess.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/subprocess.cpp b/3rdparty/libprocess/src/subprocess.cpp
index 703ecbf..e09c808 100644
--- a/3rdparty/libprocess/src/subprocess.cpp
+++ b/3rdparty/libprocess/src/subprocess.cpp
@@ -15,8 +15,10 @@
#include <stout/lambda.hpp>
#include <stout/foreach.hpp>
#include <stout/option.hpp>
+#include <stout/os.hpp>
#include <stout/try.hpp>
+using std::map;
using std::string;
namespace process {
@@ -40,11 +42,70 @@ void cleanup(
delete promise;
}
+// Used to build the environment passed to the subproces.
+class Envp
+{
+public:
+ explicit Envp(const map<string, string>& environment);
+ ~Envp();
+
+ char** operator () () const { return envp; }
+
+private:
+ // Not default constructable, not copyable, not assignable.
+ Envp();
+ Envp(const Envp&);
+ Envp& operator = (const Envp&);
+
+ char** envp;
+ size_t size;
+};
+
+
+Envp::Envp(const map<string, string>& _environment)
+ : envp(NULL),
+ size(0)
+{
+ // Merge passed environment with OS environment, overriding where necessary.
+ hashmap<string, string> environment = os::environment();
+
+ foreachpair (const string& key, const string& value, _environment) {
+ environment[key] = value;
+ }
+
+ size = environment.size();
+
+ // Convert environment to internal representation.
+ // Add 1 to the size for a NULL terminator.
+ envp = new char*[size + 1];
+ int index = 0;
+ foreachpair (const string& key, const string& value, environment) {
+ string entry = key + "=" + value;
+ envp[index] = new char[entry.size() + 1];
+ strncpy(envp[index], entry.c_str(), entry.size() + 1);
+ ++index;
+ }
+
+ envp[index] = NULL;
+}
+
+
+Envp::~Envp()
+{
+ for (size_t i = 0; i < size; ++i) {
+ delete[] envp[i];
+ }
+ delete[] envp;
+ envp = NULL;
+}
+
} // namespace internal {
// Runs the provided command in a subprocess.
-Try<Subprocess> subprocess(const string& command)
+Try<Subprocess> subprocess(
+ const string& command,
+ const map<string, string>& environment)
{
// Create pipes for stdin, stdout, stderr.
// Index 0 is for reading, and index 1 is for writing.
@@ -66,6 +127,8 @@ Try<Subprocess> subprocess(const string& command)
return ErrnoError("Failed to create pipe");
}
+ internal::Envp envp(environment);
+
pid_t pid;
if ((pid = fork()) == -1) {
os::close(stdinPipe[0]);
@@ -97,9 +160,9 @@ Try<Subprocess> subprocess(const string& command)
os::close(stdoutPipe[1]);
os::close(stderrPipe[1]);
- execl("/bin/sh", "sh", "-c", command.c_str(), (char*) NULL);
+ execle("/bin/sh", "sh", "-c", command.c_str(), (char*) NULL, envp());
- ABORT("Failed to execl '/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/653462cc/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 d15d4d1..9413ecc 100644
--- a/3rdparty/libprocess/src/tests/subprocess_tests.cpp
+++ b/3rdparty/libprocess/src/tests/subprocess_tests.cpp
@@ -4,6 +4,7 @@
#include <sys/types.h>
+#include <map>
#include <string>
#include <process/gmock.hpp>
@@ -18,6 +19,7 @@
using namespace process;
+using std::map;
using std::string;
@@ -105,7 +107,6 @@ TEST(Subprocess, status)
}
-
TEST(Subprocess, output)
{
Clock::pause();
@@ -233,18 +234,22 @@ TEST(Subprocess, splice)
}
-TEST(Subprocess, env)
+TEST(Subprocess, environment)
{
Clock::pause();
- // Simple envvar
- Try<Subprocess> s =
- subprocess("/usr/bin/env MESSAGE=hello bash -c 'echo $MESSAGE'");
+ // Simple value.
+ map<string, string> environment;
+ environment["MESSAGE"] = "hello";
+ Try<Subprocess> s = subprocess("echo $MESSAGE", environment);
+
ASSERT_SOME(s);
+
ASSERT_SOME(os::nonblock(s.get().out()));
+
AWAIT_EXPECT_EQ("hello\n", io::read(s.get().out()));
- // Advance time until the reaper reaps the subprocess.
+ // Advance time until the internal reaper reaps the subprocess.
while (s.get().status().isPending()) {
Clock::advance(Seconds(1));
Clock::settle();
@@ -257,15 +262,19 @@ TEST(Subprocess, env)
ASSERT_TRUE(WIFEXITED(status));
ASSERT_EQ(0, WEXITSTATUS(status));
+ // Multiple key-value pairs.
+ environment.clear();
+ environment["MESSAGE0"] = "hello";
+ environment["MESSAGE1"] = "world";
+ s = subprocess("echo $MESSAGE0 $MESSAGE1", environment);
- // Spaces and quotes
- s = subprocess(
- "/usr/bin/env MESSAGE=\"hello world\" bash -c 'echo $MESSAGE'");
ASSERT_SOME(s);
+
ASSERT_SOME(os::nonblock(s.get().out()));
+
AWAIT_EXPECT_EQ("hello world\n", io::read(s.get().out()));
- // Advance time until the reaper reaps the subprocess.
+ // Advance time until the internal reaper reaps the subprocess.
while (s.get().status().isPending()) {
Clock::advance(Seconds(1));
Clock::settle();
@@ -278,3 +287,95 @@ TEST(Subprocess, env)
ASSERT_TRUE(WIFEXITED(status));
ASSERT_EQ(0, WEXITSTATUS(status));
}
+
+
+TEST(Subprocess, environmentWithSpaces)
+{
+ Clock::pause();
+
+ // Spaces in value.
+ map<string, string> environment;
+ environment["MESSAGE"] = "hello world";
+ Try<Subprocess> s = subprocess("echo $MESSAGE", environment);
+
+ ASSERT_SOME(s);
+
+ ASSERT_SOME(os::nonblock(s.get().out()));
+
+ AWAIT_EXPECT_EQ("hello world\n", io::read(s.get().out()));
+
+ // 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();
+
+ ASSERT_TRUE(WIFEXITED(status));
+ ASSERT_EQ(0, WEXITSTATUS(status));
+}
+
+
+TEST(Subprocess, environmentWithSpacesAndQuotes)
+{
+ Clock::pause();
+
+ // Spaces and quotes in value.
+ map<string, string> environment;
+ environment["MESSAGE"] = "\"hello world\"";
+ Try<Subprocess> s = subprocess("echo $MESSAGE", environment);
+
+ ASSERT_SOME(s);
+
+ ASSERT_SOME(os::nonblock(s.get().out()));
+
+ AWAIT_EXPECT_EQ("\"hello world\"\n", io::read(s.get().out()));
+
+ // 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();
+
+ ASSERT_TRUE(WIFEXITED(status));
+ ASSERT_EQ(0, WEXITSTATUS(status));
+}
+
+
+TEST(Subprocess, environmentOverride)
+{
+ Clock::pause();
+
+ // Ensure we override an existing environment variable.
+ os::setenv("MESSAGE", "hello");
+
+ map<string, string> environment;
+ environment["MESSAGE"] = "goodbye";
+ Try<Subprocess> s = subprocess("echo $MESSAGE", environment);
+
+ ASSERT_SOME(s);
+
+ ASSERT_SOME(os::nonblock(s.get().out()));
+
+ AWAIT_EXPECT_EQ("goodbye\n", io::read(s.get().out()));
+
+ // 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();
+
+ ASSERT_TRUE(WIFEXITED(status));
+ ASSERT_EQ(0, WEXITSTATUS(status));
+}