From f07023471d261abbefee65adb2ca769b1da0ba42 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Wed, 28 Jun 2017 16:05:23 +0200 Subject: [PATCH] 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 --- src/main/cpp/BUILD | 6 +- src/main/cpp/blaze_util_windows.cc | 25 ++--- src/main/cpp/util/BUILD | 6 +- src/main/cpp/util/file_windows.cc | 46 ++++----- src/main/native/BUILD | 31 ++---- src/main/native/windows/BUILD | 43 +++++++++ .../file.cc} | 10 +- .../file.h} | 12 ++- .../processes.cc} | 94 +++++++++---------- .../{windows_util.cc => windows/util.cc} | 29 +++--- .../native/{windows_util.h => windows/util.h} | 6 +- src/test/cpp/util/BUILD | 6 +- src/test/cpp/util/file_windows_test.cc | 5 +- src/test/native/BUILD | 18 ++-- .../file_test.cc} | 8 +- .../util_test.cc} | 8 +- 16 files changed, 195 insertions(+), 158 deletions(-) create mode 100644 src/main/native/windows/BUILD rename src/main/native/{windows_file_operations.cc => windows/file.cc} (97%) rename src/main/native/{windows_file_operations.h => windows/file.h} (93%) rename src/main/native/{windows_processes.cc => windows/processes.cc} (88%) rename src/main/native/{windows_util.cc => windows/util.cc} (89%) rename src/main/native/{windows_util.h => windows/util.h} (97%) rename src/test/native/{windows_file_operations_test.cc => windows/file_test.cc} (97%) rename src/test/native/{windows_util_test.cc => windows/util_test.cc} (99%) diff --git a/src/main/cpp/BUILD b/src/main/cpp/BUILD index a3abaf482e7ab6..54b8cc2ae03121 100644 --- 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 index c07fc13ef9c304..f3790bbabcbf3f 100644 --- 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& 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& 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 index b595ccb571ed54..cc8db9f34ee47a 100644 --- 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 index af6ed4f3259535..17b4264ebb3beb 100644 --- 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* 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 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 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 index 3799a109cf5707..59eaf02cb6820f 100644 --- 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 new file mode 100644 index 00000000000000..c7dead2469b8f9 --- /dev/null +++ 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 b/src/main/native/windows/file.cc similarity index 97% rename from src/main/native/windows_file_operations.cc rename to src/main/native/windows/file.cc index 0b02fb92382e64..b5dc5974c8966c 100644 --- a/src/main/native/windows_file_operations.cc +++ b/src/main/native/windows/file.cc @@ -18,10 +18,11 @@ #include #include -#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 diff --git a/src/main/native/windows_file_operations.h b/src/main/native/windows/file.h similarity index 93% rename from src/main/native/windows_file_operations.h rename to src/main/native/windows/file.h index 9b2c76837a7dd2..b1a57101ff48c4 100644 --- a/src/main/native/windows_file_operations.h +++ b/src/main/native/windows/file.h @@ -11,15 +11,16 @@ // 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. -#ifndef BAZEL_SRC_MAIN_NATIVE_WINDOWS_FILE_OPERATIONS_H_ -#define BAZEL_SRC_MAIN_NATIVE_WINDOWS_FILE_OPERATIONS_H_ +#ifndef BAZEL_SRC_MAIN_NATIVE_WINDOWS_FILE_H_ +#define BAZEL_SRC_MAIN_NATIVE_WINDOWS_FILE_H_ #include #include #include -namespace windows_util { +namespace bazel { +namespace windows { using std::string; using std::unique_ptr; @@ -79,6 +80,7 @@ HANDLE OpenDirectory(const WCHAR* path, bool read_write); string CreateJunction(const wstring& junction_name, const wstring& junction_target); -} // namespace windows_util +} // namespace windows +} // namespace bazel -#endif // BAZEL_SRC_MAIN_NATIVE_WINDOWS_FILE_OPERATIONS_H_ +#endif // BAZEL_SRC_MAIN_NATIVE_WINDOWS_FILE_H_ diff --git a/src/main/native/windows_processes.cc b/src/main/native/windows/processes.cc similarity index 88% rename from src/main/native/windows_processes.cc rename to src/main/native/windows/processes.cc index cf0b677c938b68..89001733eaa779 100644 --- a/src/main/native/windows_processes.cc +++ b/src/main/native/windows/processes.cc @@ -28,8 +28,8 @@ #include #include // static_assert -#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" // Ensure we can safely cast (const) jchar* to (const) WCHAR* and LP(C)WSTR. // This is true with MSVC but not always with GCC. @@ -47,9 +47,7 @@ struct NativeOutputStream { std::string error_; std::atomic closed_; NativeOutputStream() - : handle_(INVALID_HANDLE_VALUE), - error_(""), - closed_(false) {} + : handle_(INVALID_HANDLE_VALUE), error_(""), closed_(false) {} void close() { closed_.store(true); @@ -117,14 +115,14 @@ static bool NestedJobsSupported() { } return version_info.dwMajorVersion > 6 || - version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 2; + version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 2; } static void MaybeReportLastError(const std::string& reason, JNIEnv* env, jobjectArray error_msg_holder) { if (error_msg_holder != nullptr && env->GetArrayLength(error_msg_holder) > 0) { - std::string error_str = windows_util::GetLastErrorString(reason); + std::string error_str = bazel::windows::GetLastErrorString(reason); jstring error_msg = env->NewStringUTF(error_str.c_str()); env->SetObjectArrayElement(error_msg_holder, 0, error_msg); } @@ -160,7 +158,7 @@ static jlong PtrAsJlong(void* p) { return reinterpret_cast(p); } static std::string AsExecutableForCreateProcess(JNIEnv* env, jstring path, std::string* result) { - return windows_util::AsExecutablePathForCreateProcess( + return bazel::windows::AsExecutablePathForCreateProcess( GetJavaUTFString(env, path), [env, path]() { return GetJavaWstring(env, path); }, result); } @@ -185,10 +183,9 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( std::wstring stderr_redirect = AddUncPrefixMaybe(GetJavaWstring(env, java_stderr_redirect)); std::string cwd; - error_msg = windows_util::AsShortPath( + error_msg = bazel::windows::AsShortPath( GetJavaUTFString(env, java_cwd), - [env, java_cwd]() { return GetJavaWstring(env, java_cwd); }, - &cwd); + [env, java_cwd]() { return GetJavaWstring(env, java_cwd); }, &cwd); if (!error_msg.empty()) { result->error_ = error_msg; return PtrAsJlong(result); @@ -206,10 +203,10 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( // created. If this was not so, operations on these file handles would not // return immediately if the process is terminated. // Therefore we make these handles auto-closing (by using AutoHandle). - windows_util::AutoHandle stdin_process; - windows_util::AutoHandle stdout_process; - windows_util::AutoHandle stderr_process; - windows_util::AutoHandle thread; + bazel::windows::AutoHandle stdin_process; + bazel::windows::AutoHandle stdout_process; + bazel::windows::AutoHandle stderr_process; + bazel::windows::AutoHandle thread; PROCESS_INFORMATION process_info = {0}; STARTUPINFOA startup_info = {0}; JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info = {0}; @@ -227,7 +224,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( } if (!CreatePipe(&stdin_process.handle, &result->stdin_, &sa, 0)) { - result->error_ = windows_util::GetLastErrorString("CreatePipe(stdin)"); + result->error_ = bazel::windows::GetLastErrorString("CreatePipe(stdin)"); return PtrAsJlong(result); } @@ -244,12 +241,12 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( /* hTemplateFile */ NULL); if (!stdout_process.IsValid()) { - result->error_ = windows_util::GetLastErrorString("CreateFile(stdout)"); + result->error_ = bazel::windows::GetLastErrorString("CreateFile(stdout)"); return PtrAsJlong(result); } } else { if (!CreatePipe(&result->stdout_.handle_, &stdout_process.handle, &sa, 0)) { - result->error_ = windows_util::GetLastErrorString("CreatePipe(stdout)"); + result->error_ = bazel::windows::GetLastErrorString("CreatePipe(stdout)"); return PtrAsJlong(result); } } @@ -283,7 +280,8 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( /* hTemplateFile */ NULL); if (stderr_handle == INVALID_HANDLE_VALUE) { - result->error_ = windows_util::GetLastErrorString("CreateFile(stderr)"); + result->error_ = + bazel::windows::GetLastErrorString("CreateFile(stderr)"); return PtrAsJlong(result); } // stderr_process != stdout_process, so set its handle, so the AutoHandle @@ -292,7 +290,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( } } else { if (!CreatePipe(&result->stderr_.handle_, &stderr_handle, &sa, 0)) { - result->error_ = windows_util::GetLastErrorString("CreatePipe(stderr)"); + result->error_ = bazel::windows::GetLastErrorString("CreatePipe(stderr)"); return PtrAsJlong(result); } stderr_process.handle = stderr_handle; @@ -302,7 +300,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( // allowed. Thus, we don't need to do any more setup here. HANDLE job = CreateJobObject(NULL, NULL); if (job == NULL) { - result->error_ = windows_util::GetLastErrorString("CreateJobObject()"); + result->error_ = bazel::windows::GetLastErrorString("CreateJobObject()"); return PtrAsJlong(result); } @@ -310,13 +308,10 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( job_info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - if (!SetInformationJobObject( - job, - JobObjectExtendedLimitInformation, - &job_info, - sizeof(job_info))) { + if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation, + &job_info, sizeof(job_info))) { result->error_ = - windows_util::GetLastErrorString("SetInformationJobObject()"); + bazel::windows::GetLastErrorString("SetInformationJobObject()"); return PtrAsJlong(result); } @@ -340,7 +335,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( /* lpProcessInformation */ &process_info); if (!ok) { - result->error_ = windows_util::GetLastErrorString("CreateProcess()"); + result->error_ = bazel::windows::GetLastErrorString("CreateProcess()"); return PtrAsJlong(result); } @@ -350,9 +345,8 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( if (!AssignProcessToJobObject(result->job_, result->process_)) { BOOL is_in_job = false; - if (IsProcessInJob(result->process_, NULL, &is_in_job) - && is_in_job - && !NestedJobsSupported()) { + if (IsProcessInJob(result->process_, NULL, &is_in_job) && is_in_job && + !NestedJobsSupported()) { // We are on a pre-Windows 8 system and the Bazel is already in a job. // We can't create nested jobs, so just revert to TerminateProcess() and // hope for the best. In batch mode, the launcher puts Bazel in a job so @@ -361,14 +355,14 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( result->job_ = INVALID_HANDLE_VALUE; } else { result->error_ = - windows_util::GetLastErrorString("AssignProcessToJobObject()"); + bazel::windows::GetLastErrorString("AssignProcessToJobObject()"); return PtrAsJlong(result); } } // Now that we put the process in a new job object, we can start executing it if (ResumeThread(thread) == -1) { - result->error_ = windows_util::GetLastErrorString("ResumeThread()"); + result->error_ = bazel::windows::GetLastErrorString("ResumeThread()"); return PtrAsJlong(result); } @@ -378,7 +372,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeWriteStdin( - JNIEnv *env, jclass clazz, jlong process_long, jbyteArray java_bytes, + JNIEnv* env, jclass clazz, jlong process_long, jbyteArray java_bytes, jint offset, jint length) { NativeProcess* process = reinterpret_cast(process_long); @@ -392,7 +386,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeWriteStdin( if (!::WriteFile(process->stdin_, bytes.ptr() + offset, length, &bytes_written, NULL)) { - process->error_ = windows_util::GetLastErrorString("WriteFile()"); + process->error_ = bazel::windows::GetLastErrorString("WriteFile()"); bytes_written = -1; } @@ -443,7 +437,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeReadStream( stream->error_ = ""; bytes_read = 0; } else { - stream->error_ = windows_util::GetLastErrorString("ReadFile()"); + stream->error_ = bazel::windows::GetLastErrorString("ReadFile()"); bytes_read = -1; } } else { @@ -455,11 +449,12 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeReadStream( extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetExitCode( - JNIEnv *env, jclass clazz, jlong process_long) { + JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); DWORD exit_code; if (!GetExitCodeProcess(process->process_, &exit_code)) { - process->error_ = windows_util::GetLastErrorString("GetExitCodeProcess()"); + process->error_ = + bazel::windows::GetLastErrorString("GetExitCodeProcess()"); return -1; } @@ -472,9 +467,9 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetExitCode( // 2: Wait returned with an error extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeWaitFor( - JNIEnv *env, jclass clazz, jlong process_long, jlong java_timeout) { + JNIEnv* env, jclass clazz, jlong process_long, jlong java_timeout) { NativeProcess* process = reinterpret_cast(process_long); - HANDLE handles[1] = { process->process_ }; + HANDLE handles[1] = {process->process_}; DWORD win32_timeout = java_timeout < 0 ? INFINITE : java_timeout; jint result; switch (WaitForMultipleObjects(1, handles, FALSE, win32_timeout)) { @@ -521,7 +516,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeWaitFor( extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetProcessPid( - JNIEnv *env, jclass clazz, jlong process_long) { + JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); process->error_ = ""; return GetProcessId(process->process_); // MSDN says that this cannot fail @@ -529,19 +524,20 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetProcessPid( extern "C" JNIEXPORT jboolean JNICALL Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeTerminate( - JNIEnv *env, jclass clazz, jlong process_long) { + JNIEnv* env, jclass clazz, jlong process_long) { static const UINT exit_code = 130; // 128 + SIGINT, like on Linux NativeProcess* process = reinterpret_cast(process_long); if (process->job_ != INVALID_HANDLE_VALUE) { if (!TerminateJobObject(process->job_, exit_code)) { process->error_ = - windows_util::GetLastErrorString("TerminateJobObject()"); + bazel::windows::GetLastErrorString("TerminateJobObject()"); return JNI_FALSE; } } else if (process->process_ != INVALID_HANDLE_VALUE) { if (!TerminateProcess(process->process_, exit_code)) { - process->error_ = windows_util::GetLastErrorString("TerminateProcess()"); + process->error_ = + bazel::windows::GetLastErrorString("TerminateProcess()"); return JNI_FALSE; } } @@ -603,9 +599,9 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeStreamGetLastE extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeIsJunction( JNIEnv* env, jclass clazz, jstring path, jobjectArray error_msg_holder) { - int result = windows_util::IsJunctionOrDirectorySymlink( + int result = bazel::windows::IsJunctionOrDirectorySymlink( GetJavaWstring(env, path).c_str()); - if (result == windows_util::IS_JUNCTION_ERROR) { + if (result == bazel::windows::IS_JUNCTION_ERROR) { MaybeReportLastError( std::string("GetFileAttributes(") + GetJavaUTFString(env, path) + ")", env, error_msg_holder); @@ -619,7 +615,7 @@ Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeGetLongPa jobjectArray error_msg_holder) { std::unique_ptr result; bool success = - windows_util::GetLongPath(GetJavaWstring(env, path).c_str(), &result); + bazel::windows::GetLongPath(GetJavaWstring(env, path).c_str(), &result); if (!success) { MaybeReportLastError( std::string("GetLongPathName(") + GetJavaUTFString(env, path) + ")", @@ -637,8 +633,8 @@ extern "C" JNIEXPORT jboolean JNICALL Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeCreateJunction( JNIEnv* env, jclass clazz, jstring name, jstring target, jobjectArray error_msg_holder) { - std::string error = windows_util::CreateJunction(GetJavaWstring(env, name), - GetJavaWstring(env, target)); + std::string error = bazel::windows::CreateJunction( + GetJavaWstring(env, name), GetJavaWstring(env, target)); if (!error.empty()) { MaybeReportLastError(error, env, error_msg_holder); return JNI_FALSE; diff --git a/src/main/native/windows_util.cc b/src/main/native/windows/util.cc similarity index 89% rename from src/main/native/windows_util.cc rename to src/main/native/windows/util.cc index 7d8200214d96ba..8f43fe7b74f1bb 100644 --- a/src/main/native/windows_util.cc +++ b/src/main/native/windows/util.cc @@ -21,9 +21,10 @@ #include #include -#include "src/main/native/windows_util.h" +#include "src/main/native/windows/util.h" -namespace windows_util { +namespace bazel { +namespace windows { using std::function; using std::string; @@ -38,21 +39,16 @@ string GetLastErrorString(const string& cause) { LPSTR message; DWORD size = FormatMessageA( - FORMAT_MESSAGE_ALLOCATE_BUFFER - | FORMAT_MESSAGE_FROM_SYSTEM - | FORMAT_MESSAGE_IGNORE_INSERTS, - NULL, - last_error, - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - (LPSTR) &message, - 0, - NULL); + FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, last_error, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPSTR)&message, 0, NULL); if (size == 0) { char buf[256]; snprintf(buf, sizeof(buf), - "%s: Error %d (cannot format message due to error %d)", - cause.c_str(), last_error, GetLastError()); + "%s: Error %d (cannot format message due to error %d)", + cause.c_str(), last_error, GetLastError()); buf[sizeof(buf) - 1] = 0; } @@ -127,8 +123,8 @@ string AsShortPath(string path, function path_as_wstring, WCHAR wshort[kMaxShortPath]; DWORD wshort_size = ::GetShortPathNameW(wlong.c_str(), NULL, 0); if (wshort_size == 0) { - return windows_util::GetLastErrorString( - string("GetShortPathName failed (path=") + path + ")"); + return GetLastErrorString(string("GetShortPathName failed (path=") + path + + ")"); } if (wshort_size >= kMaxShortPath) { @@ -168,4 +164,5 @@ string AsExecutablePathForCreateProcess(const string& path, return error; } -} // namespace windows_util +} // namespace windows +} // namespace bazel diff --git a/src/main/native/windows_util.h b/src/main/native/windows/util.h similarity index 97% rename from src/main/native/windows_util.h rename to src/main/native/windows/util.h index 05644675d56291..1e3c2ceaf1f387 100644 --- a/src/main/native/windows_util.h +++ b/src/main/native/windows/util.h @@ -21,7 +21,8 @@ #include #include -namespace windows_util { +namespace bazel { +namespace windows { using std::function; using std::string; @@ -88,6 +89,7 @@ string AsExecutablePathForCreateProcess(const string& path, function path_as_wstring, string* result); -} // namespace windows_util +} // namespace windows +} // namespace bazel #endif // BAZEL_SRC_MAIN_NATIVE_WINDOWS_UTIL_H__ diff --git a/src/test/cpp/util/BUILD b/src/test/cpp/util/BUILD index 46f4e2ee2ad78a..bc995bfb05b38e 100644 --- a/src/test/cpp/util/BUILD +++ b/src/test/cpp/util/BUILD @@ -43,15 +43,15 @@ cc_test( ] + select({ "//src:windows": [ ":windows_test_util", - "//src/main/native:windows_jni_lib", + "//src/main/native/windows:lib-file", ], "//src:windows_msys": [ ":windows_test_util", - "//src/main/native:windows_jni_lib", + "//src/main/native/windows:lib-file", ], "//src:windows_msvc": [ ":windows_test_util", - "//src/main/native:windows_jni_lib", + "//src/main/native/windows:lib-file", ], "//conditions:default": [], }), diff --git a/src/test/cpp/util/file_windows_test.cc b/src/test/cpp/util/file_windows_test.cc index 340a46acd46d6a..b33a80e5cf9b42 100644 --- a/src/test/cpp/util/file_windows_test.cc +++ b/src/test/cpp/util/file_windows_test.cc @@ -22,7 +22,7 @@ #include "gtest/gtest.h" #include "src/main/cpp/util/file.h" #include "src/main/cpp/util/file_platform.h" -#include "src/main/native/windows_file_operations.h" +#include "src/main/native/windows/file.h" #include "src/test/cpp/util/test_util.h" #include "src/test/cpp/util/windows_test_util.h" @@ -32,6 +32,7 @@ namespace blaze_util { +using bazel::windows::CreateJunction; using std::string; using std::unique_ptr; using std::wstring; @@ -61,7 +62,7 @@ class FileWindowsTest : public ::testing::Test { wstring wtarget; \ EXPECT_TRUE(AsWindowsPath(name, &wname)); \ EXPECT_TRUE(AsWindowsPath(target, &wtarget)); \ - EXPECT_EQ("", windows_util::CreateJunction(wname, wtarget)); \ + EXPECT_EQ("", CreateJunction(wname, wtarget)); \ } // Asserts that dir1 can be created with some content, and dir2 doesn't exist. diff --git a/src/test/native/BUILD b/src/test/native/BUILD index 4dd916fa8b7b9b..cd2278500436be 100644 --- a/src/test/native/BUILD +++ b/src/test/native/BUILD @@ -13,32 +13,32 @@ cc_test( size = "small", srcs = select({ "//src:windows": [ - "windows_util_test.cc", - "windows_file_operations_test.cc", + "windows/util_test.cc", + "windows/file_test.cc", ], "//src:windows_msys": [ - "windows_util_test.cc", - "windows_file_operations_test.cc", + "windows/util_test.cc", + "windows/file_test.cc", ], "//src:windows_msvc": [ - "windows_util_test.cc", - "windows_file_operations_test.cc", + "windows/util_test.cc", + "windows/file_test.cc", ], "//conditions:default": ["dummy_test.cc"], }), deps = select({ "//src:windows": [ - "//src/main/native:windows_jni_lib", + "//src/main/native/windows:lib-file", "//src/test/cpp/util:windows_test_util", "//third_party:gtest", ], "//src:windows_msys": [ - "//src/main/native:windows_jni_lib", + "//src/main/native/windows:lib-file", "//src/test/cpp/util:windows_test_util", "//third_party:gtest", ], "//src:windows_msvc": [ - "//src/main/native:windows_jni_lib", + "//src/main/native/windows:lib-file", "//src/test/cpp/util:windows_test_util", "//third_party:gtest", ], diff --git a/src/test/native/windows_file_operations_test.cc b/src/test/native/windows/file_test.cc similarity index 97% rename from src/test/native/windows_file_operations_test.cc rename to src/test/native/windows/file_test.cc index f3258a9a8611ea..043d4df880d67a 100644 --- a/src/test/native/windows_file_operations_test.cc +++ b/src/test/native/windows/file_test.cc @@ -20,14 +20,15 @@ #include #include "gtest/gtest.h" -#include "src/main/native/windows_file_operations.h" +#include "src/main/native/windows/file.h" #include "src/test/cpp/util/windows_test_util.h" #if !defined(COMPILER_MSVC) && !defined(__CYGWIN__) #error("This test should only be run on Windows") #endif // !defined(COMPILER_MSVC) && !defined(__CYGWIN__) -namespace windows_util { +namespace bazel { +namespace windows { using blaze_util::DeleteAllUnder; using blaze_util::GetTestTmpDirW; @@ -108,4 +109,5 @@ TEST_F(WindowsFileOperationsTest, TestCreateJunction) { ::GetFileAttributesW((name + L"4\\bar").c_str())); } -} // namespace windows_util +} // namespace windows +} // namespace bazel diff --git a/src/test/native/windows_util_test.cc b/src/test/native/windows/util_test.cc similarity index 99% rename from src/test/native/windows_util_test.cc rename to src/test/native/windows/util_test.cc index 9e4b7cccd6afd8..8cbb15a0536228 100644 --- a/src/test/native/windows_util_test.cc +++ b/src/test/native/windows/util_test.cc @@ -20,14 +20,15 @@ #include // unique_ptr #include -#include "src/main/native/windows_util.h" #include "gtest/gtest.h" +#include "src/main/native/windows/util.h" #if !defined(COMPILER_MSVC) && !defined(__CYGWIN__) #error("This test should only be run on Windows") #endif // !defined(COMPILER_MSVC) && !defined(__CYGWIN__) -namespace windows_util { +namespace bazel { +namespace windows { using std::function; using std::string; @@ -363,4 +364,5 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessConversions) { DeleteDirsUnder(tmpdir, short_root); } -} // namespace windows_util +} // namespace windows +} // namespace bazel