windows,client: fix error reporting

Fix error reporting in the path conversion methods
of the Bazel client. Previously the error
reporting logic used GetLastErrorString in places
where it was not appropriate (i.e. it was not a
failed Windows API call that caused an error).

This cleanup prepares removing the concept of the
MSYS root from the Bazel client, since MSYS paths
are no longer supported and we want to cut Bazel's
dependency on Bash (thus MSYS) completely.

See bazelbuild#4319

Change-Id: Ie50a20e0ee0c572592f637340a2f2948c7f53088

src/main/cpp/blaze_util_windows.cc

@@ -276,10 +276,11 @@ static void CreateCommandLine(CmdLine* result, const string& exe,
                               const std::vector<string>& args_vector) {
   std::ostringstream cmdline;
   string short_exe;
-  if (!blaze_util::AsShortWindowsPath(exe, &short_exe)) {
+  string error;
+  if (!blaze_util::AsShortWindowsPath(exe, &short_exe, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "CreateCommandLine: AsShortWindowsPath(" << exe
-        << "): " << GetLastErrorString();
+        << "): " << error;
   }
   bool first = true;
   for (const auto& s : args_vector) {
@@ -440,10 +441,11 @@ int ExecuteDaemon(const string& exe,
                   const string& server_dir,
                   BlazeServerStartup** server_startup) {
   wstring wdaemon_output;
-  if (!blaze_util::AsAbsoluteWindowsPath(daemon_output, &wdaemon_output)) {
+  string error;
+  if (!blaze_util::AsAbsoluteWindowsPath(daemon_output, &wdaemon_output, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "ExecuteDaemon(" << exe << "): AsAbsoluteWindowsPath("
-        << daemon_output << ") failed: " << GetLastErrorString();
+        << daemon_output << ") failed: " << error;
   }
 
   SECURITY_ATTRIBUTES sa;
@@ -653,10 +655,11 @@ const char kListSeparator = ';';
 
 string PathAsJvmFlag(const string& path) {
   string spath;
-  if (!blaze_util::AsShortWindowsPath(path, &spath)) {
+  string error;
+  if (!blaze_util::AsShortWindowsPath(path, &spath, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "PathAsJvmFlag(" << path
-        << "): AsShortWindowsPath failed: " << GetLastErrorString();
+        << "): AsShortWindowsPath failed: " << error;
   }
   // Convert backslashes to forward slashes, in order to avoid the JVM parsing
   // Windows paths as if they contained escaped characters.
@@ -668,10 +671,11 @@ string PathAsJvmFlag(const string& path) {
 string ConvertPath(const string& path) {
   // The path may not be Windows-style and may not be normalized, so convert it.
   wstring wpath;
-  if (!blaze_util::AsAbsoluteWindowsPath(path, &wpath)) {
+  string error;
+  if (!blaze_util::AsAbsoluteWindowsPath(path, &wpath, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "ConvertPath(" << path
-        << "): AsAbsoluteWindowsPath failed: " << GetLastErrorString();
+        << "): AsAbsoluteWindowsPath failed: " << error;
   }
   std::transform(wpath.begin(), wpath.end(), wpath.begin(), ::towlower);
   return string(blaze_util::WstringToCstring(
@@ -682,18 +686,19 @@ string ConvertPath(const string& path) {
 bool SymlinkDirectories(const string &posix_target, const string &posix_name) {
   wstring name;
   wstring target;
-  if (!blaze_util::AsAbsoluteWindowsPath(posix_name, &name)) {
+  string error;
+  if (!blaze_util::AsAbsoluteWindowsPath(posix_name, &name, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "SymlinkDirectories(" << posix_target << ", " << posix_name
         << "): AsAbsoluteWindowsPath(" << posix_target
-        << ") failed: " << GetLastErrorString();
+        << ") failed: " << error;
     return false;
   }
-  if (!blaze_util::AsAbsoluteWindowsPath(posix_target, &target)) {
+  if (!blaze_util::AsAbsoluteWindowsPath(posix_target, &target, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "SymlinkDirectories(" << posix_target << ", " << posix_name
         << "): AsAbsoluteWindowsPath(" << posix_name
-        << ") failed: " << GetLastErrorString();
+        << ") failed: " << error;
     return false;
   }
   wstring werror(CreateJunction(name, target));
@@ -961,10 +966,11 @@ uint64_t AcquireLock(const string& output_base, bool batch_mode, bool block,
                      BlazeLock* blaze_lock) {
   string lockfile = blaze_util::JoinPath(output_base, "lock");
   wstring wlockfile;
-  if (!blaze_util::AsAbsoluteWindowsPath(lockfile, &wlockfile)) {
+  string error;
+  if (!blaze_util::AsAbsoluteWindowsPath(lockfile, &wlockfile, &error)) {
     BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
         << "AcquireLock(" << output_base << "): AsAbsoluteWindowsPath("
-        << lockfile << ") failed: " << GetLastErrorString();
+        << lockfile << ") failed: " << error;
   }
 
   blaze_lock->handle = INVALID_HANDLE_VALUE;

src/main/cpp/util/file_platform.h

@@ -213,15 +213,18 @@ void ForEachDirectoryEntry(const std::string &path,
 #if defined(COMPILER_MSVC) || defined(__CYGWIN__)
 const wchar_t *RemoveUncPrefixMaybe(const wchar_t *ptr);
 
-bool AsWindowsPath(const std::string &path, std::string *result);
+bool AsWindowsPath(const std::string &path, std::string *result,
+                   std::string* error);
 
-bool AsAbsoluteWindowsPath(const std::string &path, std::wstring *wpath);
+bool AsAbsoluteWindowsPath(const std::string &path, std::wstring *wpath,
+                           std::string* error);
 
 // Same as `AsWindowsPath`, but returns a lowercase 8dot3 style shortened path.
 // Result will never have a UNC prefix, nor a trailing "/" or "\".
 // Works also for non-existent paths; shortens as much of them as it can.
 // Also works for non-existent drives.
-bool AsShortWindowsPath(const std::string &path, std::string *result);
+bool AsShortWindowsPath(const std::string &path, std::string *result,
+                        std::string* error);
 #endif  // defined(COMPILER_MSVC) || defined(__CYGWIN__)
 
 }  // namespace blaze_util