Rename stdin, etc. in Subprocess to work with MSVC

Summary:
`stdin`, `stdout` and `stderr` are macros that expand to function calls with the MSVC CRT implementation. This is also the case for musl-libc. This means that Subprocess simply cannot be compiled on those platforms without changing the API.
To solve that, we change the API and deprecate the old API.

For more fun, `stdin`, `stdout` and `stderr` are also macros in glibc, they just expand to other identifiers rather than a function call.

Reviewed By: yfeldblum

Differential Revision: D4229544

fbshipit-source-id: 97f1a3b228b83cfdcaffee56d729063ea235e608

diff --git a/folly/Subprocess.h b/folly/Subprocess.h
index a3b9f877682013f588fc7ac3ccf3a11a09e25fd9..35b9a5328043b10ddf12caf59e4a44be1f9f7b81 100644 (file)
--- a/folly/Subprocess.h
+++ b/folly/Subprocess.h
@@ -34,12 +34,12 @@
  *
  * A thread-safe [1] version of popen() (type="r", to read from the child):
  *    Subprocess proc(cmd, Subprocess::pipeStdout());
- *    // read from proc.stdout()
+ *    // read from proc.stdoutFd()
  *    proc.wait();
  *
  * A thread-safe [1] version of popen() (type="w", to write to the child):
  *    Subprocess proc(cmd, Subprocess::pipeStdin());
- *    // write to proc.stdin()
+ *    // write to proc.stdinFd()
  *    proc.wait();
  *
  * If you want to redirect both stdin and stdout to pipes, you can, but note
@@ -298,19 +298,19 @@ class Subprocess {
     /**
      * Shortcut to change the action for standard input.
      */
-    Options& stdin(int action) { return fd(STDIN_FILENO, action); }
+    Options& stdinFd(int action) { return fd(STDIN_FILENO, action); }
 
     /**
      * Shortcut to change the action for standard output.
      */
-    Options& stdout(int action) { return fd(STDOUT_FILENO, action); }
+    Options& stdoutFd(int action) { return fd(STDOUT_FILENO, action); }
 
     /**
      * Shortcut to change the action for standard error.
      * Note that stderr(1) will redirect the standard error to the same
      * file descriptor as standard output; the equivalent of bash's "2>&1"
      */
-    Options& stderr(int action) { return fd(STDERR_FILENO, action); }
+    Options& stderrFd(int action) { return fd(STDERR_FILENO, action); }
 
     Options& pipeStdin() { return fd(STDIN_FILENO, PIPE_IN); }
     Options& pipeStdout() { return fd(STDOUT_FILENO, PIPE_OUT); }
@@ -416,9 +416,9 @@ class Subprocess {
       dangerousPostForkPreExecCallback_{nullptr};
   };
 
-  static Options pipeStdin() { return Options().stdin(PIPE); }
-  static Options pipeStdout() { return Options().stdout(PIPE); }
-  static Options pipeStderr() { return Options().stderr(PIPE); }
+  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
   Subprocess(const Subprocess&) = delete;
@@ -768,9 +768,9 @@ class Subprocess {
   int parentFd(int childFd) const {
     return pipes_[findByChildFd(childFd)].pipe.fd();
   }
-  int stdin() const { return parentFd(0); }
-  int stdout() const { return parentFd(1); }
-  int stderr() const { return parentFd(2); }
+  int stdinFd() const { return parentFd(0); }
+  int stdoutFd() const { return parentFd(1); }
+  int stderrFd() const { return parentFd(2); }
 
   /**
    * The child's pipes are logically separate from the process metadata

diff --git a/folly/experimental/test/NestedCommandLineAppTest.cpp b/folly/experimental/test/NestedCommandLineAppTest.cpp
index 597b15f7e1690bf7cb1bcccf1c0dd19e5fc48afc..4acddd2ecc345167588a59d636c141daa6d2ad86 100644 (file)
--- a/folly/experimental/test/NestedCommandLineAppTest.cpp
+++ b/folly/experimental/test/NestedCommandLineAppTest.cpp
@@ -47,7 +47,7 @@ std::string callHelper(std::initializer_list<std::string> args,
 
   Subprocess::Options options;
   if (stdoutFd != -1) {
-    options.stdout(stdoutFd);
+    options.stdoutFd(stdoutFd);
   } else {
     options.pipeStdout();
   }

diff --git a/folly/test/SingletonTest.cpp b/folly/test/SingletonTest.cpp
index 40e12b982b07d3f3327a1331350f458eaafaeda8..14c4efa621c5f371bdfb7cdf45dd82c937ea7a18 100644 (file)
--- a/folly/test/SingletonTest.cpp
+++ b/folly/test/SingletonTest.cpp
@@ -608,8 +608,8 @@ TEST(Singleton, DoubleRegistrationLogging) {
   auto p = Subprocess(
       std::vector<std::string>{sub.string()},
       Subprocess::Options()
-          .stdin(Subprocess::CLOSE)
-          .stdout(Subprocess::CLOSE)
+          .stdinFd(Subprocess::CLOSE)
+          .stdoutFd(Subprocess::CLOSE)
           .pipeStderr()
           .closeOtherFds());
   auto err = p.communicate("").second;

diff --git a/folly/test/SubprocessTest.cpp b/folly/test/SubprocessTest.cpp
index 3d29c972e2ae5cd1d290c7dde1163efec09ca592..93fd08ce97fbbabdb5023ca7b366d38c74d3bc14 100644 (file)
--- a/folly/test/SubprocessTest.cpp
+++ b/folly/test/SubprocessTest.cpp
@@ -199,7 +199,7 @@ TEST(SimpleSubprocessTest, FdLeakTest) {
   checkFdLeak([] {
     try {
       Subprocess proc(std::vector<std::string>({"/no/such/file"}),
-                      Subprocess::pipeStdout().stderr(Subprocess::PIPE));
+                      Subprocess::pipeStdout().stderrFd(Subprocess::PIPE));
       ADD_FAILURE() << "expected an error when running /no/such/file";
     } catch (const SubprocessSpawnError& ex) {
       EXPECT_EQ(ENOENT, ex.errnoValue());
@@ -236,7 +236,7 @@ TEST(ParentDeathSubprocessTest, ParentDeathSignal) {
 TEST(PopenSubprocessTest, PopenRead) {
   Subprocess proc("ls /", Subprocess::pipeStdout());
   int found = 0;
-  gen::byLine(File(proc.stdout())) |
+  gen::byLine(File(proc.stdoutFd())) |
     [&] (StringPiece line) {
       if (line == "etc" || line == "bin" || line == "usr") {
         ++found;