https://reviews.apache.org/r/29809/

gistfile1.txt

diff --git a/src/launcher/fetcher.cpp b/src/launcher/fetcher.cpp
index 4bda91c..bcd90ae 100644
--- a/src/launcher/fetcher.cpp
+++ b/src/launcher/fetcher.cpp
@@ -89,8 +89,7 @@ Try<bool> extract(const string& filename, const string& directory)
 // Attempt to get the uri using the hadoop client.
 Try<string> downloadWithHadoopClient(
     const string& uri,
-    const string& directory,
-    const string& basename)
+    const string& destPath)
 {
   HDFS hdfs;
   Try<bool> available = hdfs.available();
@@ -101,13 +100,10 @@ Try<string> downloadWithHadoopClient(
     return Error("Hadoop Client unavailable");
   }
 
-  LOG(INFO) << "Fetching URI '" << uri << "' using Hadoop Client";
+  LOG(INFO) << "Downloading resource with Hadoop client from '" << uri
+            << "' to '" << destPath << "'";
 
-  string path = path::join(directory, basename);
-
-  LOG(INFO) << "Downloading resource from '" << uri  << "' to '" << path << "'";
-
-  Try<Nothing> result = hdfs.copyToLocal(uri, path);
+  Try<Nothing> result = hdfs.copyToLocal(uri, destPath);
   if (result.isError()) {
     LOG(ERROR) << "HDFS copyToLocal failed: " << result.error();
     return Error(result.error());
@@ -119,21 +115,19 @@ Try<string> downloadWithHadoopClient(
 
 Try<string> downloadWithNet(
     const string& uri,
-    const string& directory,
-    const string& basename)
+    const string& destPath)
 {
   LOG(INFO) << "Fetching URI '" << uri << "' with os::net";
 
   string path = uri.substr(uri.find("://") + 3);
   if (path.find("/") == string::npos ||
       path.size() <= path.find("/") + 1) {
-    LOG(ERROR) << "Malformed URL (missing path)";
+    LOG(ERROR) << "Malformed URL (missing path): " << uri;
     return Error("Malformed URI");
   }
 
-  path = path::join(directory, basename);
-  LOG(INFO) << "Downloading '" << uri << "' to '" << path << "'";
-  Try<int> code = net::download(uri, path);
+  LOG(INFO) << "Downloading '" << uri << "' to '" << destPath << "'";
+  Try<int> code = net::download(uri, destPath);
   if (code.isError()) {
     LOG(ERROR) << "Error downloading resource: " << code.error().c_str();
     return Error("Fetch of URI failed (" + code.error() + ")");
@@ -149,8 +143,7 @@ Try<string> downloadWithNet(
 
 Try<string> downloadWithLocalCopy(
     const string& uri,
-    const string& directory,
-    const string& basename)
+    const string& destPath)
 {
     string local = uri;
     bool fileUri = false;
@@ -171,40 +164,40 @@ Try<string> downloadWithLocalCopy(
         if (os::hasenv("MESOS_FRAMEWORKS_HOME")) {
             local = path::join(os::getenv("MESOS_FRAMEWORKS_HOME"), local);
             LOG(INFO) << "Prepended environment variable "
-            << "MESOS_FRAMEWORKS_HOME to relative path, "
-            << "making it: '" << local << "'";
+                      << "MESOS_FRAMEWORKS_HOME to relative path, "
+                      << "making it: '" << local << "'";
         } else {
             LOG(ERROR) << "A relative path was passed for the resource but the "
-            << "environment variable MESOS_FRAMEWORKS_HOME is not set. "
-            << "Please either specify this config option "
-            << "or avoid using a relative path";
+                       << "environment variable MESOS_FRAMEWORKS_HOME is not set. "
+                       << "Please either specify this config option "
+                       << "or avoid using a relative path";
             return Error("Could not resolve relative URI");
         }
     }
 
     // Copy the resource to the directory.
-    string path = path::join(directory, basename);
     std::ostringstream command;
-    command << "cp '" << local << "' '" << path << "'";
+    command << "cp '" << local << "' '" << destPath << "'";
+
     LOG(INFO) << "Copying resource from '" << local
-    << "' to '" << directory << "'";
+              << "' to '" << directory << "'";
 
     int status = os::system(command.str());
     if (status != 0) {
         LOG(ERROR) << "Failed to copy '" << local
-        << "' : Exit status " << status;
+                   << "' : Exit status " << status;
+
         return Error("Local copy failed");
     }
 
-    return path;
+    return destPath;
 }
 
 
 // Fetch URI into directory.
 Try<string> download(
     const string& uri,
-    const string& directory,
-    const string& basename)
+    const string& destPath)
 {
     LOG(INFO) << "Fetching URI '" << uri << "'";
     // Some checks to make sure using the URI value in shell commands
@@ -221,7 +214,7 @@ Try<string> download(
     // We consider file:// or no scheme uri are considered local uri.
     if (strings::startsWith(uri, "file://") ||
         uri.find("://") == string::npos) {
-      return downloadWithLocalCopy(uri, directory, basename);
+      return downloadWithLocalCopy(uri, destPath);
     }
 
     // 2. Try to fetch URI using os::net / libcurl implementation.
@@ -230,7 +223,7 @@ Try<string> download(
                strings::startsWith(uri, "https://") ||
                strings::startsWith(uri, "ftp://") ||
                strings::startsWith(uri, "ftps://")) {
-      return downloadWithNet(uri, directory, basename);
+      return downloadWithNet(uri, destPath);
     }
 
     // 3. Try to fetch the URI using hadoop client.
@@ -245,7 +238,7 @@ Try<string> download(
     // hadoop_home or the hadoop client setup but in case we reach
     // this part and don't have one configured, the fetch would fail
     // and log an appropriate error.
-    return downloadWithHadoopClient(uri, directory, basename);
+    return downloadWithHadoopClient(uri, destPath);
 }
 
 
@@ -262,6 +255,9 @@ static Try<Nothing> chmodExecutable(const string& filePath)
 }
 
 
+static Try<Nothing>
+
+
 static Try<Nothing> fetch(
     const CommandInfo::URI& uri,
     const FetcherInfo::Action& action,
@@ -277,14 +273,13 @@ static Try<Nothing> fetch(
 
       Try<string> basename = Fetcher::basename(uri.value());
       if (basename.isError()) {
-        LOG(ERROR) << basename.error();
         return Error(basename.error());
       }
 
       Try<string> downloaded =
-        download(uri.value(), workDirectory, basename.get());
+        download(uri.value(), path::join(workDirectory, basename.get()));
+
       if (downloaded.isError()) {
-        LOG(ERROR) << downloaded.error();
         return Error(downloaded.error());
       }
 
@@ -292,13 +287,15 @@ static Try<Nothing> fetch(
         return chmodExecutable(downloaded.get());
       } else if (uri.extract()) {
         Try<bool> extracted = extract(downloaded.get(), workDirectory);
+
         if (extracted.isError()) {
-          LOG(ERROR) << extracted.error();
           return Error(extracted.error());
         }
       }
+
       break;
     }
+
     case FetcherInfo::DOWNLOAD_AND_INSTALL: {
       LOG(INFO) << "fetcher action DOWNLOAD_AND_INSTALL '" << uri.value();
 
@@ -306,14 +303,14 @@ static Try<Nothing> fetch(
       if (mkdir.isError()) {
         string message = "Failed to create fetcher cache directory '" +
                          cacheDirectory + "': " + mkdir.error();
-        LOG(ERROR) << message;
+
         return Error(message);
       }
 
       Try<string> downloaded =
-        download(uri.value(), cacheDirectory, cacheFilename);
+        download(uri.value(), path::join(cacheDirectory, cacheFilename));
+
       if (downloaded.isError()) {
-        LOG(ERROR) << downloaded.error();
         return Error(downloaded.error());
       }
 
@@ -324,28 +321,20 @@ static Try<Nothing> fetch(
           cacheDirectory,
           workDirectory);
     }
+
     case FetcherInfo::INSTALL: {
       LOG(INFO) << "fetcher action INSTALL '" << uri.value() << "'";
 
       Try<string> basename = Fetcher::basename(uri.value());
       if (basename.isError()) {
-        LOG(ERROR) << basename.error();
         return Error(basename.error());
       }
 
       string sourcePath = path::join(cacheDirectory, cacheFilename);
-      if (uri.executable()) {
-        Try<string> workCopy =
-          download(sourcePath, workDirectory, basename.get());
-        if (workCopy.isError()) {
-          LOG(ERROR) << workCopy.error();
-          return Error(workCopy.error());
-        }
-        return chmodExecutable(workCopy.get());
-      } else if (uri.extract()) {
+
+      if (uri.extract()) {
         Try<bool> extracted = extract(sourcePath, workDirectory);
         if (extracted.isError()) {
-          LOG(ERROR) << extracted.error();
           return Error(extracted.error());
         } else if (extracted.get()) {
           return Nothing();
@@ -353,19 +342,27 @@ static Try<Nothing> fetch(
       }
 
       Try<string> downloaded =
-        download(sourcePath, workDirectory, basename.get());
+        download(sourcePath, path::join(workDirectory, basename.get()));
+
       if (downloaded.isError()) {
-        LOG(ERROR) << downloaded.error();
         return Error(downloaded.error());
       }
+
+      if (uri.executable()) {
+        return chmodExecutable(downloaded.get());
+      }
+
       break;
     }
+
     default: {
       string message = "Unexpected fetcher action selector";
       LOG(FATAL) << message;
+
       return Error(message);
     }
   }
+
   return Nothing();
 }