You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2017/01/18 02:02:08 UTC

[2/5] mesos git commit: Stop using os::system to extract fetcher archives.

Stop using os::system to extract fetcher archives.

Review: https://reviews.apache.org/r/55239/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1b0c61d3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1b0c61d3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1b0c61d3

Branch: refs/heads/master
Commit: 1b0c61d30b4fe4dfbc301c0dbfeb3697a056b2ef
Parents: 6a8c6e9
Author: James Peach <jp...@apache.org>
Authored: Tue Jan 17 17:52:28 2017 -0800
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Tue Jan 17 17:52:28 2017 -0800

----------------------------------------------------------------------
 src/launcher/fetcher.cpp | 49 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1b0c61d3/src/launcher/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/fetcher.cpp b/src/launcher/fetcher.cpp
index 4456c28..06b8620 100644
--- a/src/launcher/fetcher.cpp
+++ b/src/launcher/fetcher.cpp
@@ -15,8 +15,10 @@
 // limitations under the License.
 
 #include <string>
+#include <vector>
 
 #include <process/owned.hpp>
+#include <process/subprocess.hpp>
 
 #include <stout/json.hpp>
 #include <stout/net.hpp>
@@ -30,6 +32,8 @@
 
 #include <mesos/fetcher/fetcher.hpp>
 
+#include "common/status_utils.hpp"
+
 #include "hdfs/hdfs.hpp"
 
 #include "logging/flags.hpp"
@@ -45,6 +49,7 @@ using namespace mesos;
 using namespace mesos::internal;
 
 using std::string;
+using std::vector;
 
 using mesos::fetcher::FetcherInfo;
 
@@ -59,7 +64,12 @@ static Try<bool> extract(
     const string& sourcePath,
     const string& destinationDirectory)
 {
-  string command;
+  Try<Nothing> result = Nothing();
+
+  Option<Subprocess::IO> in = None();
+  Option<Subprocess::IO> out = None();
+  vector<string> command;
+
   // Extract any .tar, .tgz, tar.gz, tar.bz2 or zip files.
   if (strings::endsWith(sourcePath, ".tar") ||
       strings::endsWith(sourcePath, ".tgz") ||
@@ -68,25 +78,44 @@ static Try<bool> extract(
       strings::endsWith(sourcePath, ".tar.bz2") ||
       strings::endsWith(sourcePath, ".txz") ||
       strings::endsWith(sourcePath, ".tar.xz")) {
-    command = "tar -C '" + destinationDirectory + "' -xf";
+    command = {"tar", "-C", destinationDirectory, "-xf", sourcePath};
   } else if (strings::endsWith(sourcePath, ".gz")) {
     string pathWithoutExtension = sourcePath.substr(0, sourcePath.length() - 3);
     string filename = Path(pathWithoutExtension).basename();
-    command = "gzip -dc > '" + destinationDirectory + "/" + filename + "' <";
+    string destinationPath = path::join(destinationDirectory, filename);
+
+    command = {"gunzip", "-d", "-c"};
+    in = Subprocess::PATH(sourcePath);
+    out = Subprocess::PATH(destinationPath);
   } else if (strings::endsWith(sourcePath, ".zip")) {
-    command = "unzip -o -d '" + destinationDirectory + "'";
+    command = {"unzip", "-o", "-d", destinationDirectory, sourcePath};
   } else {
     return false;
   }
 
-  command += " '" + sourcePath + "'";
+  CHECK_GT(command.size(), 0u);
 
-  LOG(INFO) << "Extracting with command: " << command;
+  Try<Subprocess> extractProcess = subprocess(
+      command[0],
+      command,
+      in.getOrElse(Subprocess::PATH("/dev/null")),
+      out.getOrElse(Subprocess::FD(STDOUT_FILENO)),
+      Subprocess::FD(STDERR_FILENO));
 
-  int status = os::system(command);
-  if (status != 0) {
-    return Error("Failed to extract: command " + command +
-                 " exited with status: " + stringify(status));
+  if (extractProcess.isError()) {
+    return Error(
+        "Failed to extract '" + sourcePath + "': '" +
+        strings::join(" ", command) + "' failed: " +
+        extractProcess.error());
+  }
+
+  // `status()` never fails or gets discarded.
+  int status = extractProcess->status()->get();
+  if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
+    return Error(
+        "Failed to extract '" + sourcePath + "': '" +
+        strings::join(" ", command) + "' failed: " +
+        WSTRINGIFY(status));
   }
 
   LOG(INFO) << "Extracted '" << sourcePath << "' into '"