Commit b4bcc1a7 authored by Alexey Spiridonov's avatar Alexey Spiridonov Committed by Facebook Github Bot

Replace Subprocess::pipe* syntax sugar with Subprocess::Options().pipe*

Summary:
This is a bit too magical -- it's not clear that the thing produces an Options object. If you do know that you can chain further option setters off this thing, it's nice, but otherwise, the first impression is "what just happened?".

So, let's have one good way for doing things.

Reviewed By: yfeldblum

Differential Revision: D4863947

fbshipit-source-id: 3dfe83cfc077d47f604f47dcb21149fbaa2d2243
parent 0e5ec48b
...@@ -33,12 +33,12 @@ ...@@ -33,12 +33,12 @@
* to complete, returning the exit status. * to complete, returning the exit status.
* *
* A thread-safe [1] version of popen() (type="r", to read from the child): * A thread-safe [1] version of popen() (type="r", to read from the child):
* Subprocess proc(cmd, Subprocess::pipeStdout()); * Subprocess proc(cmd, Subprocess::Options().pipeStdout());
* // read from proc.stdoutFd() * // read from proc.stdoutFd()
* proc.wait(); * proc.wait();
* *
* A thread-safe [1] version of popen() (type="w", to write to the child): * A thread-safe [1] version of popen() (type="w", to write to the child):
* Subprocess proc(cmd, Subprocess::pipeStdin()); * Subprocess proc(cmd, Subprocess::Options().pipeStdin());
* // write to proc.stdinFd() * // write to proc.stdinFd()
* proc.wait(); * proc.wait();
* *
...@@ -439,10 +439,6 @@ class Subprocess { ...@@ -439,10 +439,6 @@ class Subprocess {
#endif #endif
}; };
static Options pipeStdin() { return Options().stdinFd(PIPE); }
static Options pipeStdout() { return Options().stdoutFd(PIPE); }
static Options pipeStderr() { return Options().stderrFd(PIPE); }
// Non-copiable, but movable // Non-copiable, but movable
Subprocess(const Subprocess&) = delete; Subprocess(const Subprocess&) = delete;
Subprocess& operator=(const Subprocess&) = delete; Subprocess& operator=(const Subprocess&) = delete;
......
...@@ -55,7 +55,7 @@ std::string callHelper(ProgramOptionsStyle style, ...@@ -55,7 +55,7 @@ std::string callHelper(ProgramOptionsStyle style,
break; break;
} }
Subprocess proc(allArgs, Subprocess::pipeStdout(), nullptr, &env); Subprocess proc(allArgs, Subprocess::Options().pipeStdout(), nullptr, &env);
auto p = proc.communicate(); auto p = proc.communicate();
EXPECT_EQ(0, proc.wait().exitStatus()); EXPECT_EQ(0, proc.wait().exitStatus());
......
...@@ -222,8 +222,9 @@ TEST(SimpleSubprocessTest, FdLeakTest) { ...@@ -222,8 +222,9 @@ TEST(SimpleSubprocessTest, FdLeakTest) {
// Test where the exec call fails() with pipes // Test where the exec call fails() with pipes
checkFdLeak([] { checkFdLeak([] {
try { try {
Subprocess proc(std::vector<std::string>({"/no/such/file"}), Subprocess proc(
Subprocess::pipeStdout().stderrFd(Subprocess::PIPE)); std::vector<std::string>({"/no/such/file"}),
Subprocess::Options().pipeStdout().stderrFd(Subprocess::PIPE));
ADD_FAILURE() << "expected an error when running /no/such/file"; ADD_FAILURE() << "expected an error when running /no/such/file";
} catch (const SubprocessSpawnError& ex) { } catch (const SubprocessSpawnError& ex) {
EXPECT_EQ(ENOENT, ex.errnoValue()); EXPECT_EQ(ENOENT, ex.errnoValue());
...@@ -258,7 +259,7 @@ TEST(ParentDeathSubprocessTest, ParentDeathSignal) { ...@@ -258,7 +259,7 @@ TEST(ParentDeathSubprocessTest, ParentDeathSignal) {
} }
TEST(PopenSubprocessTest, PopenRead) { TEST(PopenSubprocessTest, PopenRead) {
Subprocess proc("ls /", Subprocess::pipeStdout()); Subprocess proc("ls /", Subprocess::Options().pipeStdout());
int found = 0; int found = 0;
gen::byLine(File(proc.stdoutFd())) | gen::byLine(File(proc.stdoutFd())) |
[&] (StringPiece line) { [&] (StringPiece line) {
...@@ -317,8 +318,9 @@ TEST(AfterForkCallbackSubprocessTest, TestAfterForkCallbackError) { ...@@ -317,8 +318,9 @@ TEST(AfterForkCallbackSubprocessTest, TestAfterForkCallbackError) {
} }
TEST(CommunicateSubprocessTest, SimpleRead) { TEST(CommunicateSubprocessTest, SimpleRead) {
Subprocess proc(std::vector<std::string>{ "/bin/echo", "-n", "foo", "bar"}, Subprocess proc(
Subprocess::pipeStdout()); std::vector<std::string>{"/bin/echo", "-n", "foo", "bar"},
Subprocess::Options().pipeStdout());
auto p = proc.communicate(); auto p = proc.communicate();
EXPECT_EQ("foo bar", p.first); EXPECT_EQ("foo bar", p.first);
proc.waitChecked(); proc.waitChecked();
...@@ -375,7 +377,8 @@ TEST(CommunicateSubprocessTest, Duplex2) { ...@@ -375,7 +377,8 @@ TEST(CommunicateSubprocessTest, Duplex2) {
"-e", "s/a test/a successful test/", "-e", "s/a test/a successful test/",
"-e", "/^another line/w/dev/stderr", "-e", "/^another line/w/dev/stderr",
}); });
auto options = Subprocess::pipeStdin().pipeStdout().pipeStderr().usePath(); auto options =
Subprocess::Options().pipeStdin().pipeStdout().pipeStderr().usePath();
Subprocess proc(cmd, options); Subprocess proc(cmd, options);
auto out = proc.communicateIOBuf(std::move(input)); auto out = proc.communicateIOBuf(std::move(input));
proc.waitChecked(); proc.waitChecked();
...@@ -463,7 +466,8 @@ TEST(CommunicateSubprocessTest, Chatty) { ...@@ -463,7 +466,8 @@ TEST(CommunicateSubprocessTest, Chatty) {
int wcount = 0; int wcount = 0;
int rcount = 0; int rcount = 0;
auto options = Subprocess::pipeStdin().pipeStdout().pipeStderr().usePath(); auto options =
Subprocess::Options().pipeStdin().pipeStdout().pipeStderr().usePath();
std::vector<std::string> cmd { std::vector<std::string> cmd {
"sed", "sed",
"-u", "-u",
......
...@@ -98,7 +98,8 @@ static std::string getNoteRawContent(const std::string& fileName) { ...@@ -98,7 +98,8 @@ static std::string getNoteRawContent(const std::string& fileName) {
"section", "section",
".note." + kUSDTSubsectionName, ".note." + kUSDTSubsectionName,
fileName); fileName);
auto subProc = folly::Subprocess(args, folly::Subprocess::pipeStdout()); auto subProc =
folly::Subprocess(args, folly::Subprocess::Options().pipeStdout());
auto output = subProc.communicate(); auto output = subProc.communicate();
auto retCode = subProc.wait(); auto retCode = subProc.wait();
CHECK(retCode.exited()); CHECK(retCode.exited());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment