Windows, JNI: move around sources

Move the Windows JNI C++ sources to a separate package and separate namespace. This no-op refactoring allows other build rules than Bazel's client library to depend on file I/O and/or JNI functionality. A follow-up commit will split the //src/main/native/windows:processes library into :jni-processes and :jni-file. Change-Id: I33c5f8ebd8961cc440db3b4a95ff78024d7c1d74 PiperOrigin-RevId: 160404298
bazelbuild · Jun 29, 2017 · f070234 · f070234
1 parent 1e0628d
commit f070234
Show file tree

Hide file tree

Showing 16 changed files with 195 additions and 158 deletions.
diff --git a/src/main/cpp/BUILD b/src/main/cpp/BUILD
@@ -67,9 +67,9 @@ cc_library(
         "//src/main/cpp/util",
         "//src/main/cpp/util:blaze_exit_code",
     ] + select({
-        "//src:windows": ["//src/main/native:windows_jni_lib"],
-        "//src:windows_msys": ["//src/main/native:windows_jni_lib"],
-        "//src:windows_msvc": ["//src/main/native:windows_jni_lib"],
+        "//src:windows": ["//src/main/native/windows:lib-file"],
+        "//src:windows_msys": ["//src/main/native/windows:lib-file"],
+        "//src:windows_msvc": ["//src/main/native/windows:lib-file"],
         "//conditions:default": [],
     }),
 )

diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
@@ -56,8 +56,8 @@
 #include "src/main/cpp/util/md5.h"
 #include "src/main/cpp/util/numbers.h"
 #include "src/main/cpp/util/strings.h"
-#include "src/main/native/windows_file_operations.h"
-#include "src/main/native/windows_util.h"
+#include "src/main/native/windows/file.h"
+#include "src/main/native/windows/util.h"
 
 namespace blaze {
 
@@ -70,6 +70,9 @@ static_assert(sizeof(wchar_t) == sizeof(WCHAR),
 // Add 4 characters for potential UNC prefix and a couple more for safety.
 static const size_t kWindowsPathBufferSize = 0x8010;
 
+using bazel::windows::AutoHandle;
+using bazel::windows::CreateJunction;
+
 // TODO(bazel-team): get rid of die/pdie, handle errors on the caller side.
 // die/pdie are exit points in the code and they make it difficult to follow the
 // control flow, plus it's not clear whether they call destructors on local
@@ -669,7 +672,7 @@ class ProcessHandleBlazeServerStartup : public BlazeServerStartup {
   }
 
  private:
-  windows_util::AutoHandle proc;
+  AutoHandle proc;
 };
 
 #else  // COMPILER_MSVC
@@ -711,16 +714,14 @@ void ExecuteDaemon(const string& exe, const std::vector<string>& args_vector,
   sa.bInheritHandle = TRUE;
   sa.lpSecurityDescriptor = NULL;
 
-  windows_util::AutoHandle devnull(
-      ::CreateFileA("NUL", GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
-                    FILE_ATTRIBUTE_NORMAL, NULL));
+  AutoHandle devnull(::CreateFileA("NUL", GENERIC_READ, FILE_SHARE_READ, NULL,
+                                   OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL));
   if (!devnull.IsValid()) {
     pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
          "ExecuteDaemon: Could not open NUL device");
   }
 
-  windows_util::AutoHandle stdout_file(
-      CreateJvmOutputFile(wdaemon_output.c_str(), &sa));
+  AutoHandle stdout_file(CreateJvmOutputFile(wdaemon_output.c_str(), &sa));
   if (!stdout_file.IsValid()) {
     pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
          "ExecuteDaemon: CreateJvmOutputFile %ls", wdaemon_output.c_str());
@@ -741,7 +742,7 @@ void ExecuteDaemon(const string& exe, const std::vector<string>& args_vector,
     pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR,
          "ExecuteDaemon: DuplicateHandle %ls", wdaemon_output.c_str());
   }
-  windows_util::AutoHandle stderr_file(stderr_handle);
+  AutoHandle stderr_file(stderr_handle);
 
   PROCESS_INFORMATION processInfo = {0};
   STARTUPINFOA startupInfo = {0};
@@ -997,7 +998,7 @@ bool SymlinkDirectories(const string &posix_target, const string &posix_name) {
                posix_target + ")");
     return false;
   }
-  string error(windows_util::CreateJunction(name, target));
+  string error(CreateJunction(name, target));
   if (!error.empty()) {
     PrintError("SymlinkDirectories(name=" + posix_name +
                ", target=" + posix_target + "): " + error);
@@ -1019,7 +1020,7 @@ bool CompareAbsolutePaths(const string& a, const string& b) {
 // processes than there are PIDs available within a single jiffy.
 bool VerifyServerProcess(
     int pid, const string& output_base, const string& install_base) {
-  windows_util::AutoHandle process(
+  AutoHandle process(
       ::OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid));
   if (!process.IsValid()) {
     // Cannot find the server process. Can happen if the PID file is stale.
@@ -1046,7 +1047,7 @@ bool VerifyServerProcess(
 }
 
 bool KillServerProcess(int pid) {
-  windows_util::AutoHandle process(::OpenProcess(
+  AutoHandle process(::OpenProcess(
       PROCESS_TERMINATE | PROCESS_QUERY_LIMITED_INFORMATION, FALSE, pid));
   DWORD exitcode = 0;
   if (!process.IsValid() || !::GetExitCodeProcess(process, &exitcode) ||

diff --git a/src/main/cpp/util/BUILD b/src/main/cpp/util/BUILD
@@ -59,9 +59,9 @@ cc_library(
         ":errors",
         ":strings",
     ] + select({
-        "//src:windows": ["//src/main/native:windows_jni_lib"],
-        "//src:windows_msys": ["//src/main/native:windows_jni_lib"],
-        "//src:windows_msvc": ["//src/main/native:windows_jni_lib"],
+        "//src:windows": ["//src/main/native/windows:lib-file"],
+        "//src:windows_msys": ["//src/main/native/windows:lib-file"],
+        "//src:windows_msvc": ["//src/main/native/windows:lib-file"],
         "//conditions:default": [],
     }),
 )

diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc
@@ -24,8 +24,8 @@
 #include "src/main/cpp/util/exit_code.h"
 #include "src/main/cpp/util/file.h"
 #include "src/main/cpp/util/strings.h"
-#include "src/main/native/windows_file_operations.h"
-#include "src/main/native/windows_util.h"
+#include "src/main/native/windows/file.h"
+#include "src/main/native/windows/util.h"
 
 namespace blaze_util {
 
@@ -34,7 +34,10 @@ using std::pair;
 using std::string;
 using std::unique_ptr;
 using std::wstring;
-using windows_util::HasUncPrefix;
+using bazel::windows::AutoHandle;
+using bazel::windows::GetLongPath;
+using bazel::windows::HasUncPrefix;
+using bazel::windows::OpenDirectory;
 
 // Returns the current working directory as a Windows path.
 // The result may have a UNC prefix.
@@ -108,7 +111,7 @@ static void AddUncPrefixMaybe(wstring* path, size_t max_path = MAX_PATH) {
 }
 
 static const wchar_t* RemoveUncPrefixMaybe(const wchar_t* ptr) {
-  return ptr + (windows_util::HasUncPrefix(ptr) ? 4 : 0);
+  return ptr + (HasUncPrefix(ptr) ? 4 : 0);
 }
 
 class WindowsPipe : public IPipe {
@@ -135,8 +138,8 @@ class WindowsPipe : public IPipe {
   }
 
  private:
-  windows_util::AutoHandle _read_handle;
-  windows_util::AutoHandle _write_handle;
+  AutoHandle _read_handle;
+  AutoHandle _write_handle;
 };
 
 IPipe* CreatePipe() {
@@ -184,13 +187,14 @@ bool WindowsFileMtime::GetIfInDistantFuture(const string& path, bool* result) {
     return false;
   }
 
-  windows_util::AutoHandle handle(::CreateFileW(
+  AutoHandle handle(::CreateFileW(
       /* lpFileName */ wpath.c_str(),
       /* dwDesiredAccess */ GENERIC_READ,
       /* dwShareMode */ FILE_SHARE_READ,
       /* lpSecurityAttributes */ NULL,
       /* dwCreationDisposition */ OPEN_EXISTING,
-      /* dwFlagsAndAttributes */ IsDirectoryW(wpath)
+      /* dwFlagsAndAttributes */
+      IsDirectoryW(wpath)
           ? (FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS)
           : FILE_ATTRIBUTE_NORMAL,
       /* hTemplateFile */ NULL));
@@ -233,13 +237,14 @@ bool WindowsFileMtime::Set(const string& path, const FILETIME& time) {
     return false;
   }
 
-  windows_util::AutoHandle handle(::CreateFileW(
+  AutoHandle handle(::CreateFileW(
       /* lpFileName */ wpath.c_str(),
       /* dwDesiredAccess */ FILE_WRITE_ATTRIBUTES,
       /* dwShareMode */ FILE_SHARE_READ,
       /* lpSecurityAttributes */ NULL,
       /* dwCreationDisposition */ OPEN_EXISTING,
-      /* dwFlagsAndAttributes */ IsDirectoryW(wpath)
+      /* dwFlagsAndAttributes */
+      IsDirectoryW(wpath)
           ? (FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS)
           : FILE_ATTRIBUTE_NORMAL,
       /* hTemplateFile */ NULL));
@@ -611,7 +616,7 @@ bool ReadFile(const string& filename, string* content, int max_size) {
     return false;
   }
 
-  windows_util::AutoHandle autohandle(handle);
+  AutoHandle autohandle(handle);
   if (!autohandle.IsValid()) {
     return false;
   }
@@ -629,7 +634,7 @@ bool ReadFile(const string& filename, void* data, size_t size) {
     return false;
   }
 
-  windows_util::AutoHandle autohandle(handle);
+  AutoHandle autohandle(handle);
   if (!autohandle.IsValid()) {
     return false;
   }
@@ -647,7 +652,7 @@ bool WriteFile(const void* data, size_t size, const string& filename,
   }
 
   UnlinkPathW(wpath);  // We don't care about the success of this.
-  windows_util::AutoHandle handle(::CreateFileW(
+  AutoHandle handle(::CreateFileW(
       /* lpFileName */ wpath.c_str(),
       /* dwDesiredAccess */ GENERIC_WRITE,
       /* dwShareMode */ FILE_SHARE_READ,
@@ -827,8 +832,7 @@ bool JunctionResolver::Resolve(const WCHAR* path, unique_ptr<WCHAR[]>* result,
         return false;
       }
       // Get a handle to the directory.
-      windows_util::AutoHandle handle(
-          windows_util::OpenDirectory(path, /* read_write */ false));
+      AutoHandle handle(OpenDirectory(path, /* read_write */ false));
       if (!handle.IsValid()) {
         // Opening the junction failed for whatever reason. For all intents and
         // purposes we can treat this file as if it didn't exist.
@@ -950,7 +954,7 @@ string MakeCanonical(const char* path) {
     builder << *segment;
   }
   wstring realpath(builder.str());
-  if (windows_util::HasUncPrefix(realpath.c_str())) {
+  if (HasUncPrefix(realpath.c_str())) {
     // `realpath` has an UNC prefix if `path` did, or if `path` contained
     // junctions.
     // In the first case, the UNC prefix is the usual "\\?\", but in the second
@@ -966,13 +970,13 @@ string MakeCanonical(const char* path) {
   // Resolve all 8dot3 style segments of the path, if any. The input path may
   // have had some. Junctions may also refer to 8dot3 names.
   unique_ptr<WCHAR[]> long_realpath;
-  if (!windows_util::GetLongPath(realpath.c_str(), &long_realpath)) {
+  if (!GetLongPath(realpath.c_str(), &long_realpath)) {
     return "";
   }
 
   // Convert the path to lower-case.
-  size_t size = wcslen(long_realpath.get()) -
-                (windows_util::HasUncPrefix(long_realpath.get()) ? 4 : 0);
+  size_t size =
+      wcslen(long_realpath.get()) - (HasUncPrefix(long_realpath.get()) ? 4 : 0);
   unique_ptr<WCHAR[]> lcase_realpath(new WCHAR[size + 1]);
   const WCHAR* p_from = RemoveUncPrefixMaybe(long_realpath.get());
   WCHAR* p_to = lcase_realpath.get();
@@ -992,7 +996,7 @@ static bool CanReadFileW(const wstring& path) {
   }
   // The only easy way to find out if a file is readable is to attempt to open
   // it for reading.
-  windows_util::AutoHandle handle(::CreateFileW(
+  AutoHandle handle(::CreateFileW(
       /* lpFileName */ path.c_str(),
       /* dwDesiredAccess */ GENERIC_READ,
       /* dwShareMode */ FILE_SHARE_READ,
@@ -1188,7 +1192,7 @@ void ForEachDirectoryEntry(const string &path,
   static const wstring kDot(L".");
   static const wstring kDotDot(L"..");
   // Always add an UNC prefix to ensure we can work with long paths.
-  if (!windows_util::HasUncPrefix(wpath.c_str())) {
+  if (!HasUncPrefix(wpath.c_str())) {
     wpath = kUncPrefix + wpath;
   }
   // Unconditionally add a trailing backslash. We know `wpath` has no trailing

diff --git a/src/main/native/BUILD b/src/main/native/BUILD
@@ -62,29 +62,9 @@ cc_binary(
     ],
 )
 
-cc_library(
-    name = "windows_jni_lib",
-    srcs = [
-        "windows_file_operations.cc",
-        "windows_util.cc",
-    ],
-    hdrs = [
-        "windows_file_operations.h",
-        "windows_util.h",
-    ],
-    visibility = [
-        "//src/main/cpp:__subpackages__",
-        "//src/test/cpp:__subpackages__",
-        "//src/test/native:__pkg__",
-    ],
-)
-
 genrule(
     name = "windows_jni",
-    srcs = glob([
-        "windows_*.cc",
-        "windows_*.h",
-    ]),
+    srcs = ["//src/main/native/windows:raw-sources"],
     outs = ["windows_jni.dll"],
     cmd = "$(location build_windows_jni.sh) $@ $(SRCS)",
     output_to_bindir = 1,
@@ -105,7 +85,9 @@ genrule(
 
 filegroup(
     name = "srcs",
-    srcs = glob(["**"]),
+    srcs = glob(["**"]) + [
+        "//src/main/native/windows:srcs",
+    ],
     visibility = ["//src:__pkg__"],
 )
 
@@ -114,6 +96,9 @@ filegroup(
 # embedded tools folder.
 filegroup(
     name = "embedded_tools",
-    srcs = glob(["*.cc"]) + glob(["*.h"]) + ["BUILD"],
+    srcs = glob(["*.cc"]) + glob(["*.h"]) + [
+        "BUILD",
+        "//src/main/native/windows:embedded_tools",
+    ],
     visibility = ["//visibility:public"],
 )
diff --git a/src/main/native/windows/BUILD b/src/main/native/windows/BUILD
@@ -0,0 +1,43 @@
+package(default_visibility = ["//visibility:private"])
+
+filegroup(
+    name = "srcs",
+    srcs = glob(["**"]),
+    visibility = ["//src/main/native:__pkg__"],
+)
+
+# TODO(xingao): verify that this filegroup contains exactly what it has to wrt.
+# where it is used (//src/main/native:embedded_tools).
+# Context: https://github.com/bazelbuild/bazel/commit/33d05f6b551cf2fdb257cb536a5e864d095144a1
+filegroup(
+    name = "embedded_tools",
+    srcs = [":srcs"],
+    visibility = ["//src/main/native:__pkg__"],
+)
+
+filegroup(
+    name = "raw-sources",
+    srcs = glob([
+        "*.cc",
+        "*.h",
+    ]),
+    visibility = ["//src/main/native:__pkg__"],
+)
+
+cc_library(
+    name = "lib-file",
+    srcs = ["file.cc"],
+    hdrs = ["file.h"],
+    visibility = [
+        "//src/main/cpp:__subpackages__",
+        "//src/test/cpp:__subpackages__",
+        "//src/test/native:__subpackages__",
+    ],
+    deps = [":lib-util"],
+)
+
+cc_library(
+    name = "lib-util",
+    srcs = ["util.cc"],
+    hdrs = ["util.h"],
+)
diff --git a/src/main/native/windows_file_operations.cc → src/main/native/windows/file.cc b/src/main/native/windows_file_operations.cc → src/main/native/windows/file.cc
@@ -18,10 +18,11 @@
 #include <sstream>
 #include <string>
 
-#include "src/main/native/windows_file_operations.h"
-#include "src/main/native/windows_util.h"
+#include "src/main/native/windows/file.h"
+#include "src/main/native/windows/util.h"
 
-namespace windows_util {
+namespace bazel {
+namespace windows {
 
 using std::string;
 using std::unique_ptr;
@@ -181,4 +182,5 @@ string CreateJunction(const wstring& junction_name,
   return "";
 }
 
-}  // namespace windows_util
+}  // namespace windows
+}  // namespace bazel