Bazel client: wrap some POSIX functions

This change:
- starts using blaze_util::CanAccess and
  blaze_util::PathExists instead of access(2)
- implements and starts using blaze_util::GetCwd
  instead of getcwd(2)
- implements and starts using
  blaze_util::ChangeDirectory instead of chdir(2)
- adds tests for the new wrapper methods

--
MOS_MIGRATED_REVID=138750297

12 files changed, 199 insertions, 41 deletions

diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index 39215a51be..b69586012a 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -75,6 +75,7 @@
 #include "src/main/cpp/util/errors.h"
 #include "src/main/cpp/util/exit_code.h"
 #include "src/main/cpp/util/file.h"
+#include "src/main/cpp/util/file_platform.h"
 #include "src/main/cpp/util/md5.h"
 #include "src/main/cpp/util/numbers.h"
 #include "src/main/cpp/util/port.h"
@@ -614,9 +615,9 @@ static string GetArgumentString(const vector<string>& argument_array) {
 // Do a chdir into the workspace, and die if it fails.
 static void GoToWorkspace() {
   if (WorkspaceLayout::InWorkspace(globals->workspace) &&
-      chdir(globals->workspace.c_str()) != 0) {
+      !blaze_util::ChangeDirectory(globals->workspace)) {
     pdie(blaze_exit_code::INTERNAL_ERROR,
-         "chdir() into %s failed", globals->workspace.c_str());
+         "changing directory into %s failed", globals->workspace.c_str());
   }
 }
 
@@ -1334,11 +1335,7 @@ static string MakeCanonical(const char *path) {
 
 // Compute the globals globals->cwd and globals->workspace.
 static void ComputeWorkspace() {
-  char cwdbuf[PATH_MAX];
-  if (getcwd(cwdbuf, sizeof cwdbuf) == NULL) {
-    pdie(blaze_exit_code::INTERNAL_ERROR, "getcwd() failed");
-  }
-  globals->cwd = MakeCanonical(cwdbuf);
+  globals->cwd = MakeCanonical(blaze_util::GetCwd().c_str());
   globals->workspace = WorkspaceLayout::GetWorkspace(globals->cwd);
 }
 
@@ -1392,7 +1389,7 @@ static void ComputeBaseDirectories(const string &self_path) {
           output_base);
     }
   }
-  if (access(output_base, R_OK | W_OK | X_OK) != 0) {
+  if (!blaze_util::CanAccess(globals->options->output_base, true, true, true)) {
     die(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
         "Error: Output base directory '%s' must be readable and writable.",
         output_base);
diff --git a/src/main/cpp/blaze_util.cc b/src/main/cpp/blaze_util.cc
index b93c699b4d..fe2bb72931 100644
--- a/src/main/cpp/blaze_util.cc
+++ b/src/main/cpp/blaze_util.cc
@@ -16,7 +16,6 @@
 
 #include <errno.h>
 #include <fcntl.h>
-#include <limits.h>
 #include <pwd.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -35,6 +34,7 @@
 #include "src/main/cpp/util/errors.h"
 #include "src/main/cpp/util/exit_code.h"
 #include "src/main/cpp/util/file.h"
+#include "src/main/cpp/util/file_platform.h"
 #include "src/main/cpp/util/numbers.h"
 #include "src/main/cpp/util/strings.h"
 #include "src/main/cpp/util/port.h"
@@ -68,14 +68,11 @@ string MakeAbsolute(const string &path) {
     return path;
   }
 
-  char cwdbuf[PATH_MAX];
-  if (getcwd(cwdbuf, sizeof cwdbuf) == NULL) {
-    pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, "getcwd() failed");
-  }
+  string cwd = blaze_util::GetCwd();
 
   // Determine whether the cwd ends with "/" or not.
-  string separator = (cwdbuf[strlen(cwdbuf) - 1] == '/') ? "" : "/";
-  return cwdbuf + separator + path;
+  string separator = cwd.back() == '/' ? "" : "/";
+  return cwd + separator + path;
 }
 
 // Runs "stat" on `path`. Returns -1 and sets errno if stat fails or
diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc
index 7b4718ad90..e67876144a 100644
--- a/src/main/cpp/blaze_util_posix.cc
+++ b/src/main/cpp/blaze_util_posix.cc
@@ -14,7 +14,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
-#include <limits.h>
+#include <limits.h>  // PATH_MAX
 #include <signal.h>
 #include <stdio.h>
 #include <sys/types.h>
@@ -26,6 +26,7 @@
 #include "src/main/cpp/util/errors.h"
 #include "src/main/cpp/util/exit_code.h"
 #include "src/main/cpp/util/file.h"
+#include "src/main/cpp/util/file_platform.h"
 
 namespace blaze {
 
@@ -43,13 +44,9 @@ void ExecuteProgram(const string &exe, const vector<string> &args_vector) {
       dbg.append(" ");
     }
 
-    char cwd[PATH_MAX] = {};
-    if (getcwd(cwd, sizeof(cwd)) == NULL) {
-      pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, "getcwd() failed");
-    }
-
-    fprintf(stderr, "Invoking binary %s in %s:\n  %s\n", exe.c_str(), cwd,
-            dbg.c_str());
+    string cwd = blaze_util::GetCwd();
+    fprintf(stderr, "Invoking binary %s in %s:\n  %s\n", exe.c_str(),
+            cwd.c_str(), dbg.c_str());
   }
 
   // Copy to a char* array for execv:
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 1cd1d81f5b..29f59cf8fe 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -17,7 +17,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
 #include <algorithm>
 #include <cassert>
 #include <set>
@@ -26,6 +25,7 @@
 #include "src/main/cpp/blaze_util.h"
 #include "src/main/cpp/blaze_util_platform.h"
 #include "src/main/cpp/util/file.h"
+#include "src/main/cpp/util/file_platform.h"
 #include "src/main/cpp/util/strings.h"
 #include "src/main/cpp/workspace_layout.h"
 
@@ -180,7 +180,7 @@ blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
     string* error) {
   if (cmdLineRcFile != NULL) {
     string rcFile = MakeAbsolute(cmdLineRcFile);
-    if (access(rcFile.c_str(), R_OK)) {
+    if (!blaze_util::CanAccess(rcFile, true, false, false)) {
       blaze_util::StringPrintf(error,
           "Error: Unable to read .blazerc file '%s'.", rcFile.c_str());
       return blaze_exit_code::BAD_ARGV;
@@ -190,7 +190,7 @@ blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
   }
 
   string workspaceRcFile = blaze_util::JoinPath(workspace, rc_basename);
-  if (!access(workspaceRcFile.c_str(), R_OK)) {
+  if (blaze_util::CanAccess(workspaceRcFile, true, false, false)) {
     *blaze_rc_file = workspaceRcFile;
     return blaze_exit_code::SUCCESS;
   }
@@ -202,7 +202,7 @@ blaze_exit_code::ExitCode OptionProcessor::FindUserBlazerc(
   }
 
   string userRcFile = blaze_util::JoinPath(home, rc_basename);
-  if (!access(userRcFile.c_str(), R_OK)) {
+  if (blaze_util::CanAccess(userRcFile, true, false, false)) {
     *blaze_rc_file = userRcFile;
     return blaze_exit_code::SUCCESS;
   }
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index 9b048d9014..c4590b9336 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -16,7 +16,6 @@
 #include <assert.h>
 #include <errno.h>  // errno, ENOENT
 #include <string.h>  // strerror
-#include <unistd.h>  // access
 
 #include <cstdio>
 #include <cstdlib>
@@ -25,6 +24,7 @@
 #include "src/main/cpp/blaze_util.h"
 #include "src/main/cpp/util/exit_code.h"
 #include "src/main/cpp/util/file.h"
+#include "src/main/cpp/util/file_platform.h"
 #include "src/main/cpp/util/numbers.h"
 #include "src/main/cpp/util/strings.h"
 #include "src/main/cpp/workspace_layout.h"
@@ -331,8 +331,8 @@ string StartupOptions::GetHostJavabase() {
 
 string StartupOptions::GetJvm() {
   string java_program = GetHostJavabase() + "/bin/java";
-  if (access(java_program.c_str(), X_OK) == -1) {
-    if (errno == ENOENT) {
+  if (!blaze_util::CanAccess(java_program, false, false, true)) {
+    if (!blaze_util::PathExists(java_program)) {
       fprintf(stderr, "Couldn't find java at '%s'.\n", java_program.c_str());
     } else {
       fprintf(stderr, "Couldn't access %s: %s\n", java_program.c_str(),
@@ -344,8 +344,8 @@ string StartupOptions::GetJvm() {
   string jdk_rt_jar = GetHostJavabase() + "/jre/lib/rt.jar";
   // If just the JRE is installed
   string jre_rt_jar = GetHostJavabase() + "/lib/rt.jar";
-  if ((access(jdk_rt_jar.c_str(), R_OK) == 0)
-      || (access(jre_rt_jar.c_str(), R_OK) == 0)) {
+  if (blaze_util::CanAccess(jdk_rt_jar, true, false, false)
+      || blaze_util::CanAccess(jre_rt_jar, true, false, false)) {
     return java_program;
   }
   fprintf(stderr, "Problem with java installation: "
diff --git a/src/main/cpp/util/BUILD b/src/main/cpp/util/BUILD
index 7537ac594d..600725f1c3 100644
--- a/src/main/cpp/util/BUILD
+++ b/src/main/cpp/util/BUILD
@@ -33,7 +33,10 @@ cc_library(
         ],
     }),
     hdrs = ["file_platform.h"],
-    visibility = ["//src/main/cpp:__pkg__"],
+    visibility = [
+        "//src/main/cpp:__pkg__",
+        "//src/test/cpp:__pkg__",
+    ],
     deps = [
         ":blaze_exit_code",
         ":errors",
@@ -46,7 +49,10 @@ cc_library(
     name = "file",
     srcs = ["file.cc"],
     hdrs = ["file.h"],
-    visibility = ["//src/tools/singlejar:__pkg__"],
+    visibility = [
+        "//src/test/cpp:__pkg__",
+        "//src/tools/singlejar:__pkg__",
+    ],
     deps = [
         ":blaze_exit_code",
         ":errors",
diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h
index 6d00daffff..b0df5ea18d 100644
--- a/src/main/cpp/util/file_platform.h
+++ b/src/main/cpp/util/file_platform.h
@@ -34,6 +34,12 @@ bool PathExists(const std::string& path);
 // openable.
 bool CanAccess(const std::string& path, bool read, bool write, bool exec);
 
+// Returns the current working directory.
+std::string GetCwd();
+
+// Changes the current working directory to `path`, returns true upon success.
+bool ChangeDirectory(const std::string& path);
+
 }  // namespace blaze_util
 
 #endif  // BAZEL_SRC_MAIN_CPP_UTIL_FILE_PLATFORM_H_
diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc
index 11f5d1058b..90355b92ee 100644
--- a/src/main/cpp/util/file_posix.cc
+++ b/src/main/cpp/util/file_posix.cc
@@ -14,6 +14,7 @@
 #include "src/main/cpp/util/file_platform.h"
 
 #include <sys/stat.h>
+#include <limits.h>  // PATH_MAX
 #include <stdlib.h>  // getenv
 #include <unistd.h>  // access
 #include <vector>
@@ -71,4 +72,16 @@ bool CanAccess(const string& path, bool read, bool write, bool exec) {
   return access(path.c_str(), mode) == 0;
 }
 
+string GetCwd() {
+  char cwdbuf[PATH_MAX];
+  if (getcwd(cwdbuf, sizeof cwdbuf) == NULL) {
+    pdie(blaze_exit_code::INTERNAL_ERROR, "getcwd() failed");
+  }
+  return string(cwdbuf);
+}
+
+bool ChangeDirectory(const string& path) {
+  return chdir(path.c_str()) == 0;
+}
+
 }  // namespace blaze_util
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
index 34f8bcf7a2..5cf9987486 100644
--- a/src/main/cpp/util/file_windows.cc
+++ b/src/main/cpp/util/file_windows.cc
@@ -33,4 +33,14 @@ bool CanAccess(const string& path, bool read, bool write, bool exec) {
   pdie(255, "blaze_util::CanAccess is not implemented on Windows");
 }
 
+string GetCwd() {
+  // TODO(bazel-team): implement this.
+  pdie(255, "blaze_util::GetCwd is not implemented on Windows");
+}
+
+bool ChangeDirectory(const string& path) {
+  // TODO(bazel-team): implement this.
+  pdie(255, "blaze_util::ChangeDirectory is not implemented on Windows");
+}
+
 }  // namespace blaze_util
diff --git a/src/main/cpp/workspace_layout.cc b/src/main/cpp/workspace_layout.cc
index d4836b5415..aa8bb09f2a 100644
--- a/src/main/cpp/workspace_layout.cc
+++ b/src/main/cpp/workspace_layout.cc
@@ -14,10 +14,10 @@
 #include "src/main/cpp/workspace_layout.h"
 
 #include <assert.h>
-#include <unistd.h>  // access
 
 #include "src/main/cpp/blaze_util_platform.h"
 #include "src/main/cpp/util/file.h"
+#include "src/main/cpp/util/file_platform.h"
 
 namespace blaze {
 
@@ -31,8 +31,8 @@ string WorkspaceLayout::GetOutputRoot() {
 }
 
 bool WorkspaceLayout::InWorkspace(const string &workspace) {
-  return access(
-      blaze_util::JoinPath(workspace, kWorkspaceMarker).c_str(), F_OK) == 0;
+  return blaze_util::PathExists(
+      blaze_util::JoinPath(workspace, kWorkspaceMarker));
 }
 
 string WorkspaceLayout::GetWorkspace(const string &cwd) {
@@ -59,7 +59,7 @@ static string FindDepotBlazerc(const string& workspace) {
   WorkspaceLayout::WorkspaceRcFileSearchPath(&candidates);
   for (const auto& candidate : candidates) {
     string blazerc = blaze_util::JoinPath(workspace, candidate);
-    if (!access(blazerc.c_str(), R_OK)) {
+    if (blaze_util::CanAccess(blazerc, true, false, false)) {
       return blazerc;
     }
   }
@@ -71,7 +71,7 @@ static string FindAlongsideBinaryBlazerc(const string& cwd,
   string path = arg0[0] == '/' ? arg0 : blaze_util::JoinPath(cwd, arg0);
   string base = blaze_util::Basename(arg0);
   string binary_blazerc_path = path + "." + base + "rc";
-  if (!access(binary_blazerc_path.c_str(), R_OK)) {
+  if (blaze_util::CanAccess(binary_blazerc_path, true, false, false)) {
     return binary_blazerc_path;
   }
   return "";
@@ -79,7 +79,7 @@ static string FindAlongsideBinaryBlazerc(const string& cwd,
 
 static string FindSystemWideBlazerc() {
   string path = "/etc/bazel.bazelrc";
-  if (!access(path.c_str(), R_OK)) {
+  if (blaze_util::CanAccess(path, true, false, false)) {
     return path;
   }
   return "";
diff --git a/src/test/cpp/util/BUILD b/src/test/cpp/util/BUILD
index 0ec570063e..01c2b7c04d 100644
--- a/src/test/cpp/util/BUILD
+++ b/src/test/cpp/util/BUILD
@@ -22,7 +22,17 @@ cc_test(
     name = "file_test",
     srcs = ["file_test.cc"],
     deps = [
-        "//src/main/cpp/util",
+        "//src/main/cpp/util:file",
+        "//third_party:gtest",
+    ],
+)
+
+cc_test(
+    name = "file_platform_test",
+    srcs = ["file_posix_test.cc"],
+    deps = [
+        "//src/main/cpp/util:file",
+        "//src/main/cpp/util:file_platform",
         "//third_party:gtest",
     ],
 )
diff --git a/src/test/cpp/util/file_posix_test.cc b/src/test/cpp/util/file_posix_test.cc
new file mode 100644
index 0000000000..52ddbd6da2
--- /dev/null
+++ b/src/test/cpp/util/file_posix_test.cc
@@ -0,0 +1,122 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "src/main/cpp/util/file_platform.h"
+#include "gtest/gtest.h"
+
+namespace blaze_util {
+
+TEST(FilePosixTest, Which) {
+  ASSERT_EQ("", Which(""));
+  ASSERT_EQ("", Which("foo"));
+  ASSERT_EQ("", Which("/"));
+
+  // /usr/bin/yes exists on Linux, Darwin, and MSYS, but "which yes" does not
+  // always return that (if $PATH is different).
+  string actual = Which("yes");
+  // Assert that it's an absolute path
+  ASSERT_EQ(0, actual.find("/"));
+  // Assert that it ends with /yes, we cannot assume more than that.
+  ASSERT_EQ(actual.size() - string("/yes").size(), actual.rfind("/yes"));
+}
+
+TEST(FilePosixTest, PathExists) {
+  ASSERT_FALSE(PathExists("/this/should/not/exist/mkay"));
+  ASSERT_FALSE(PathExists("non.existent"));
+  ASSERT_FALSE(PathExists(""));
+
+  // /usr/bin/yes exists on Linux, Darwin, and MSYS
+  ASSERT_TRUE(PathExists("/"));
+  ASSERT_TRUE(PathExists("/usr"));
+  ASSERT_TRUE(PathExists("/usr/"));
+  ASSERT_TRUE(PathExists("/usr/bin/yes"));
+}
+
+TEST(FilePosixTest, CanAccess) {
+  for (int i = 0; i < 8; ++i) {
+    ASSERT_FALSE(CanAccess("/this/should/not/exist/mkay", i & 1, i & 2, i & 4));
+    ASSERT_FALSE(CanAccess("non.existent", i & 1, i & 2, i & 4));
+  }
+
+  for (int i = 0; i < 4; ++i) {
+    // /usr/bin/yes exists on Linux, Darwin, and MSYS
+    ASSERT_TRUE(CanAccess("/", i & 1, false, i & 2));
+    ASSERT_TRUE(CanAccess("/usr", i & 1, false, i & 2));
+    ASSERT_TRUE(CanAccess("/usr/", i & 1, false, i & 2));
+    ASSERT_TRUE(CanAccess("/usr/bin/yes", i & 1, false, i & 2));
+  }
+
+  char* tmpdir_cstr = getenv("TEST_TMPDIR");
+  ASSERT_FALSE(tmpdir_cstr == NULL);
+
+  string tmpdir(tmpdir_cstr);
+  ASSERT_NE("", tmpdir);
+
+  string mock_file = tmpdir + (tmpdir.back() == '/' ? "" : "/") +
+                     "FilePosixTest.CanAccess.mock_file";
+  int fd = open(mock_file.c_str(), O_CREAT, 0500);
+  ASSERT_GT(fd, 0);
+  close(fd);
+
+  // Sanity check: assert that we successfully created the file with the given
+  // permissions.
+  ASSERT_EQ(0, access(mock_file.c_str(), R_OK | X_OK));
+  ASSERT_NE(0, access(mock_file.c_str(), R_OK | W_OK | X_OK));
+
+  // Actual assertion
+  for (int i = 0; i < 4; ++i) {
+    ASSERT_TRUE(CanAccess(mock_file, i & 1, false, i & 2));
+    ASSERT_FALSE(CanAccess(mock_file, i & 1, true, i & 2));
+  }
+}
+
+TEST(FilePosixTest, GetCwd) {
+  char cwdbuf[PATH_MAX];
+  ASSERT_EQ(cwdbuf, getcwd(cwdbuf, PATH_MAX));
+
+  // Assert that GetCwd() and getcwd() return the same value.
+  string cwd(cwdbuf);
+  ASSERT_EQ(cwd, blaze_util::GetCwd());
+
+  // Change to a different directory.
+  ASSERT_EQ(0, chdir("/usr"));
+
+  // Assert that GetCwd() returns the new CWD.
+  ASSERT_EQ(string("/usr"), blaze_util::GetCwd());
+
+  ASSERT_EQ(0, chdir(cwd.c_str()));
+  ASSERT_EQ(cwd, blaze_util::GetCwd());
+}
+
+TEST(FilePosixTest, ChangeDirectory) {
+  // Retrieve the current working directory.
+  char old_wd[PATH_MAX];
+  ASSERT_EQ(old_wd, getcwd(old_wd, PATH_MAX));
+
+  // Change to a different directory and assert it was successful.
+  ASSERT_FALSE(blaze_util::ChangeDirectory("/non/existent/path"));
+  ASSERT_TRUE(blaze_util::ChangeDirectory("/usr"));
+  char new_wd[PATH_MAX];
+  ASSERT_EQ(new_wd, getcwd(new_wd, PATH_MAX));
+  ASSERT_EQ(string("/usr"), string(new_wd));
+
+  // Change back to the original CWD.
+  ASSERT_TRUE(blaze_util::ChangeDirectory(old_wd));
+  ASSERT_EQ(new_wd, getcwd(new_wd, PATH_MAX));
+  ASSERT_EQ(string(old_wd), string(new_wd));
+}
+
+}  // namespace blaze_util