[dsymutil] Improve error reporting when we cannot create output file.

Before this patch we were returning an empty string in case we couldn't
create the output file. Now we return an expected string so we can
return and print the proper issue. We now return errors instead of bools
and defer printing to the call site.

llvm-svn: 344983

llvm/tools/dsymutil/dsymutil.cpp

@@ -7,9 +7,8 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This program is a utility that aims to be a dropin replacement for
+// This program is a utility that aims to be a dropin replacement for Darwin's
-// Darwin's dsymutil.
+// dsymutil.
-//
 //===----------------------------------------------------------------------===//
 
 #include "dsymutil.h"
@@ -165,20 +164,18 @@ static opt<bool>
                        desc("Embed warnings in the linked DWARF debug info."),
                        cat(DsymCategory));
 
-static bool createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) {
+static Error createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) {
   if (NoOutput)
-    return true;
+    return Error::success();
 
   // Create plist file to write to.
   llvm::SmallString<128> InfoPlist(BundleRoot);
   llvm::sys::path::append(InfoPlist, "Contents/Info.plist");
   std::error_code EC;
   llvm::raw_fd_ostream PL(InfoPlist, EC, llvm::sys::fs::F_Text);
-  if (EC) {
+  if (EC)
-    WithColor::error() << "cannot create plist file " << InfoPlist << ": "
+    return make_error<StringError>(
-                       << EC.message() << '\n';
+        "cannot create Plist: " + toString(errorCodeToError(EC)), EC);
-    return false;
-  }
 
   CFBundleInfo BI = getBundleInfo(Bin);
 
@@ -230,22 +227,21 @@ static bool createPlistFile(llvm::StringRef Bin, llvm::StringRef BundleRoot) {
      << "</plist>\n";
 
   PL.close();
-  return true;
+  return Error::success();
 }
 
-static bool createBundleDir(llvm::StringRef BundleBase) {
+static Error createBundleDir(llvm::StringRef BundleBase) {
   if (NoOutput)
-    return true;
+    return Error::success();
 
   llvm::SmallString<128> Bundle(BundleBase);
   llvm::sys::path::append(Bundle, "Contents", "Resources", "DWARF");
-  if (std::error_code EC = create_directories(Bundle.str(), true,
+  if (std::error_code EC =
-                                              llvm::sys::fs::perms::all_all)) {
+          create_directories(Bundle.str(), true, llvm::sys::fs::perms::all_all))
-    WithColor::error() << "cannot create directory " << Bundle << ": "
+    return make_error<StringError>(
-                       << EC.message() << "\n";
+        "cannot create bundle: " + toString(errorCodeToError(EC)), EC);
-    return false;
+
-  }
+  return Error::success();
-  return true;
 }
 
 static bool verify(llvm::StringRef OutputFile, llvm::StringRef Arch) {
@@ -257,7 +253,7 @@ static bool verify(llvm::StringRef OutputFile, llvm::StringRef Arch) {
 
   Expected<OwningBinary<Binary>> BinOrErr = createBinary(OutputFile);
   if (!BinOrErr) {
-    errs() << OutputFile << ": " << toString(BinOrErr.takeError());
+    WithColor::error() << OutputFile << ": " << toString(BinOrErr.takeError());
     return false;
   }
 
@@ -276,7 +272,7 @@ static bool verify(llvm::StringRef OutputFile, llvm::StringRef Arch) {
   return false;
 }
 
-static std::string getOutputFileName(llvm::StringRef InputFile) {
+static Expected<std::string> getOutputFileName(llvm::StringRef InputFile) {
   // When updating, do in place replacement.
   if (OutputFileOpt.empty() && Update)
     return InputFile;
@@ -305,8 +301,10 @@ static std::string getOutputFileName(llvm::StringRef InputFile) {
   llvm::SmallString<128> BundleDir(OutputFileOpt);
   if (BundleDir.empty())
     BundleDir = DwarfFile + ".dSYM";
-  if (!createBundleDir(BundleDir) || !createPlistFile(DwarfFile, BundleDir))
+  if (auto E = createBundleDir(BundleDir))
-    return "";
+    return std::move(E);
+  if (auto E = createPlistFile(DwarfFile, BundleDir))
+    return std::move(E);
 
   llvm::sys::path::append(BundleDir, "Contents", "Resources", "DWARF",
                           llvm::sys::path::filename(DwarfFile));
@@ -521,13 +519,20 @@ int main(int argc, char **argv) {
       // Using a std::shared_ptr rather than std::unique_ptr because move-only
       // types don't work with std::bind in the ThreadPool implementation.
       std::shared_ptr<raw_fd_ostream> OS;
-      std::string OutputFile = getOutputFileName(InputFile);
+
+      Expected<std::string> OutputFileOrErr = getOutputFileName(InputFile);
+      if (!OutputFileOrErr) {
+        WithColor::error() << toString(OutputFileOrErr.takeError());
+        return 1;
+      }
+
+      std::string OutputFile = *OutputFileOrErr;
       if (NeedsTempFiles) {
         TempFiles.emplace_back(Map->getTriple().getArchName().str());
 
         auto E = TempFiles.back().createTempFile();
         if (E) {
-          errs() << toString(std::move(E));
+          WithColor::error() << toString(std::move(E));
           return 1;
         }
 
@@ -540,7 +545,7 @@ int main(int argc, char **argv) {
         OS = std::make_shared<raw_fd_ostream>(NoOutput ? "-" : OutputFile, EC,
                                               sys::fs::F_None);
         if (EC) {
-          errs() << OutputFile << ": " << EC.message();
+          WithColor::error() << OutputFile << ": " << EC.message();
           return 1;
         }
       }
@@ -567,10 +572,16 @@ int main(int argc, char **argv) {
     if (!AllOK)
       return 1;
 
-    if (NeedsTempFiles &&
+    if (NeedsTempFiles) {
-        !MachOUtils::generateUniversalBinary(
+      Expected<std::string> OutputFileOrErr = getOutputFileName(InputFile);
-            TempFiles, getOutputFileName(InputFile), *OptionsOrErr, SDKPath))
+      if (!OutputFileOrErr) {
-      return 1;
+        WithColor::error() << toString(OutputFileOrErr.takeError());
+        return 1;
+      }
+      if (!MachOUtils::generateUniversalBinary(TempFiles, *OutputFileOrErr,
+                                               *OptionsOrErr, SDKPath))
+        return 1;
+    }
   }
 
   return 0;