Windows,JNI,logging: improve error messages In this commit: - introduce the MakeErrorMessage function, which creates a structured error message with file and line information of the error's origin - update all error messages in the Windows JNI library - simplify GetLastErrorMessage to just convert an error code to string, without prepending a cause Change-Id: Ia8162bfdaee37d4b7ccb3a46d6c8a861b0a1bd94 PiperOrigin-RevId: 173402968
diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc index 7c4f599..b2c809e 100644 --- a/src/main/native/windows/file-jni.cc +++ b/src/main/native/windows/file-jni.cc
@@ -17,6 +17,7 @@ #include <windows.h> #include <memory> +#include <sstream> #include <string> #include "src/main/native/jni.h" @@ -24,26 +25,30 @@ #include "src/main/native/windows/jni-util.h" #include "src/main/native/windows/util.h" -static void MaybeReportLastError(const std::wstring& reason, JNIEnv* env, - jobjectArray error_msg_holder) { - if (error_msg_holder != nullptr && - env->GetArrayLength(error_msg_holder) > 0) { - std::wstring error_str = bazel::windows::GetLastErrorString(reason); - jstring error_msg = env->NewString( - reinterpret_cast<const jchar*>(error_str.c_str()), error_str.size()); - env->SetObjectArrayElement(error_msg_holder, 0, error_msg); - } +static bool CanReportError(JNIEnv* env, jobjectArray error_msg_holder) { + return error_msg_holder != nullptr && + env->GetArrayLength(error_msg_holder) > 0; +} + +static void ReportLastError(const std::wstring& error_str, JNIEnv* env, + jobjectArray error_msg_holder) { + jstring error_msg = env->NewString( + reinterpret_cast<const jchar*>(error_str.c_str()), error_str.size()); + env->SetObjectArrayElement(error_msg_holder, 0, error_msg); } extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeIsJunction( JNIEnv* env, jclass clazz, jstring path, jobjectArray error_msg_holder) { - int result = bazel::windows::IsJunctionOrDirectorySymlink( - bazel::windows::GetJavaWstring(env, path).c_str()); - if (result == bazel::windows::IS_JUNCTION_ERROR) { - MaybeReportLastError(std::wstring(L"GetFileAttributes(") + - bazel::windows::GetJavaWstring(env, path) + L")", - env, error_msg_holder); + std::wstring wpath(bazel::windows::GetJavaWstring(env, path)); + int result = bazel::windows::IsJunctionOrDirectorySymlink(wpath.c_str()); + if (result == bazel::windows::IS_JUNCTION_ERROR && + CanReportError(env, error_msg_holder)) { + DWORD err_code = GetLastError(); + ReportLastError( + bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"nativeIsJunction", wpath, err_code), + env, error_msg_holder); } return result; } @@ -53,13 +58,13 @@ JNIEnv* env, jclass clazz, jstring path, jobjectArray result_holder, jobjectArray error_msg_holder) { std::unique_ptr<WCHAR[]> result; - std::wstring err_msg(bazel::windows::GetLongPath( - bazel::windows::GetJavaWstring(env, path).c_str(), &result)); - if (!err_msg.empty()) { - MaybeReportLastError(std::wstring(L"GetLongPathName(") + - bazel::windows::GetJavaWstring(env, path) + - L"): " + err_msg, - env, error_msg_holder); + std::wstring wpath(bazel::windows::GetJavaWstring(env, path)); + std::wstring error(bazel::windows::GetLongPath(wpath.c_str(), &result)); + if (!error.empty() && CanReportError(env, error_msg_holder)) { + ReportLastError( + bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"nativeGetLongPath", wpath, error), + env, error_msg_holder); return JNI_FALSE; } env->SetObjectArrayElement( @@ -73,11 +78,14 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeCreateJunction( JNIEnv* env, jclass clazz, jstring name, jstring target, jobjectArray error_msg_holder) { - std::wstring error(bazel::windows::CreateJunction( - bazel::windows::GetJavaWstring(env, name), - bazel::windows::GetJavaWstring(env, target))); - if (!error.empty()) { - MaybeReportLastError(error, env, error_msg_holder); + std::wstring wname(bazel::windows::GetJavaWstring(env, name)); + std::wstring wtarget(bazel::windows::GetJavaWstring(env, target)); + std::wstring error(bazel::windows::CreateJunction(wname, wtarget)); + if (!error.empty() && CanReportError(env, error_msg_holder)) { + ReportLastError(bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateJunction", + wname + L", " + wtarget, error), + env, error_msg_holder); return JNI_FALSE; } return JNI_TRUE;
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc index d694c3c..e9d7c18 100644 --- a/src/main/native/windows/file.cc +++ b/src/main/native/windows/file.cc
@@ -45,7 +45,9 @@ wstring GetLongPath(const WCHAR* path, unique_ptr<WCHAR[]>* result) { DWORD size = ::GetLongPathNameW(path, NULL, 0); if (size == 0) { - return GetLastErrorString(L"GetLongPathNameW"); + DWORD err_code = GetLastError(); + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetLongPathNameW", path, + err_code); } result->reset(new WCHAR[size]); ::GetLongPathNameW(path, result->get(), size); @@ -113,10 +115,8 @@ /* two copies of the string are stored */ 2) / sizeof(WCHAR); if (target.size() > kMaxJunctionTargetLen) { - std::wstringstream error; - error << L"junction target is too long (" << target.size() - << L" characters, limit: " << kMaxJunctionTargetLen << L")"; - return error.str(); + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateJunction", target, + L"target is too long"); } const wstring name = HasUncPrefix(junction_name.c_str()) ? junction_name @@ -124,12 +124,16 @@ // Junctions are directories, so create one if (!::CreateDirectoryW(name.c_str(), NULL)) { - return wstring(L"CreateDirectoryW failed"); + DWORD err_code = GetLastError(); + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateJunction", name, + err_code); } AutoHandle handle(OpenDirectory(name.c_str(), true)); if (!handle.IsValid()) { - return wstring(L"OpenDirectory failed"); + DWORD err_code = GetLastError(); + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"OpenDirectory", name, + err_code); } uint8_t reparse_buffer_bytes[MAXIMUM_REPARSE_DATA_BUFFER_SIZE]; @@ -177,7 +181,9 @@ reparse_buffer->header.ReparseDataLength + sizeof(JunctionDescription::Header), NULL, 0, &bytes_returned, NULL)) { - return wstring(L"DeviceIoControl(FSCTL_SET_REPARSE_POINT) failed"); + DWORD err_code = GetLastError(); + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"DeviceIoControl", L"", + err_code); } return L""; }
diff --git a/src/main/native/windows/processes-jni.cc b/src/main/native/windows/processes-jni.cc index 78ba8d2..27d8b45 100644 --- a/src/main/native/windows/processes-jni.cc +++ b/src/main/native/windows/processes-jni.cc
@@ -18,6 +18,7 @@ #include <atomic> #include <memory> +#include <sstream> #include <string> #include <type_traits> // static_assert @@ -25,6 +26,13 @@ #include "src/main/native/windows/jni-util.h" #include "src/main/native/windows/util.h" +template <typename T> +static std::wstring ToString(const T& e) { + std::wstringstream s; + s << e; + return s.str(); +} + extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetpid( JNIEnv* env, jclass clazz) { @@ -125,12 +133,6 @@ static jlong PtrAsJlong(void* p) { return reinterpret_cast<jlong>(p); } -static std::wstring AsExecutableForCreateProcess(JNIEnv* env, jstring path, - std::wstring* result) { - return bazel::windows::AsExecutablePathForCreateProcess( - bazel::windows::GetJavaWstring(env, path), result); -} - // The following CreateProcessWithExplicitHandles function is from oldnewthing. // https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873 // We need it to prevent unintended handles being inherited by child process, @@ -206,9 +208,12 @@ NativeProcess* result = new NativeProcess(); std::wstring argv0; - std::wstring error_msg(AsExecutableForCreateProcess(env, java_argv0, &argv0)); + std::wstring wpath(bazel::windows::GetJavaWstring(env, java_argv0)); + std::wstring error_msg( + bazel::windows::AsExecutablePathForCreateProcess(wpath, &argv0)); if (!error_msg.empty()) { - result->error_ = error_msg; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); return PtrAsJlong(result); } @@ -219,10 +224,11 @@ std::wstring stderr_redirect = AddUncPrefixMaybe( bazel::windows::GetJavaWstring(env, java_stderr_redirect)); std::wstring cwd; - error_msg = bazel::windows::AsShortPath( - bazel::windows::GetJavaWstring(env, java_cwd), &cwd); + std::wstring wcwd(bazel::windows::GetJavaWstring(env, java_cwd)); + error_msg = bazel::windows::AsShortPath(wcwd, &cwd); if (!error_msg.empty()) { - result->error_ = error_msg; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); return PtrAsJlong(result); } @@ -250,18 +256,24 @@ JavaByteArray env_map(env, java_env); if (env_map.ptr() != nullptr) { if (env_map.size() < 2) { - result->error_ = L"The environment must be at least two bytes long"; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, + L"the environment must be at least two bytes long"); return PtrAsJlong(result); } else if (env_map.ptr()[env_map.size() - 1] != 0 || env_map.ptr()[env_map.size() - 2] != 0) { - result->error_ = L"Environment array must end with two null bytes"; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, + L"environment array must end with two null bytes"); return PtrAsJlong(result); } } HANDLE temp_stdin_handle = INVALID_HANDLE_VALUE; if (!CreatePipe(&temp_stdin_handle, &result->stdin_, &sa, 0)) { - result->error_ = bazel::windows::GetLastErrorString(L"CreatePipe(stdin)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); CloseHandle(temp_stdin_handle); return PtrAsJlong(result); } @@ -280,15 +292,17 @@ /* hTemplateFile */ NULL); if (!stdout_process.IsValid()) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreateFile(stdout)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } } else { HANDLE temp_stdout_handle = INVALID_HANDLE_VALUE; if (!CreatePipe(&result->stdout_.handle_, &temp_stdout_handle, &sa, 0)) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreatePipe(stdout)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); CloseHandle(temp_stdout_handle); return PtrAsJlong(result); } @@ -326,8 +340,9 @@ /* hTemplateFile */ NULL); if (stderr_handle == INVALID_HANDLE_VALUE) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreateFile(stderr)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } // stderr_process != stdout_process, so set its handle, so the AutoHandle @@ -336,8 +351,9 @@ } } else { if (!CreatePipe(&result->stderr_.handle_, &stderr_handle, &sa, 0)) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreatePipe(stderr)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } stderr_process = stderr_handle; @@ -347,7 +363,9 @@ // allowed. Thus, we don't need to do any more setup here. HANDLE job = CreateJobObject(NULL, NULL); if (job == NULL) { - result->error_ = bazel::windows::GetLastErrorString(L"CreateJobObject()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -357,8 +375,9 @@ JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation, &job_info, sizeof(job_info))) { - result->error_ = - bazel::windows::GetLastErrorString(L"SetInformationJobObject()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -386,7 +405,9 @@ /* rgHandlesToInherit */ handlesToInherit); if (!ok) { - result->error_ = bazel::windows::GetLastErrorString(L"CreateProcess()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -405,15 +426,18 @@ CloseHandle(result->job_); result->job_ = INVALID_HANDLE_VALUE; } else { - result->error_ = - bazel::windows::GetLastErrorString(L"AssignProcessToJobObject()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); 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_ = bazel::windows::GetLastErrorString(L"ResumeThread()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -429,7 +453,9 @@ JavaByteArray bytes(env, java_bytes); if (offset < 0 || length <= 0 || offset > bytes.size() - length) { - process->error_ = L"Array index out of bounds"; + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_), + L"Array index out of bounds"); return -1; } @@ -437,7 +463,10 @@ if (!::WriteFile(process->stdin_, bytes.ptr() + offset, length, &bytes_written, NULL)) { - process->error_ = bazel::windows::GetLastErrorString(L"WriteFile()"); + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_), + err_code); bytes_written = -1; } @@ -468,7 +497,9 @@ JavaByteArray bytes(env, java_bytes); if (offset < 0 || length <= 0 || offset > bytes.size() - length) { - stream->error_ = L"Array index out of bounds"; + stream->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", + L"Array index out of bounds"); return -1; } @@ -488,7 +519,9 @@ stream->error_ = L""; bytes_read = 0; } else { - stream->error_ = bazel::windows::GetLastErrorString(L"ReadFile()"); + DWORD err_code = GetLastError(); + stream->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", err_code); bytes_read = -1; } } else { @@ -504,8 +537,10 @@ NativeProcess* process = reinterpret_cast<NativeProcess*>(process_long); DWORD exit_code; if (!GetExitCodeProcess(process->process_, &exit_code)) { - process->error_ = - bazel::windows::GetLastErrorString(L"GetExitCodeProcess()"); + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeGetExitCode", ToString(process->pid_), + err_code); return -1; } @@ -537,8 +572,10 @@ break; default: - process->error_ = L"WaitForMultipleObjects() returned unknown result"; - result = 2; + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeWaitFor", ToString(process->pid_), + err_code); break; } @@ -566,14 +603,18 @@ if (process->job_ != INVALID_HANDLE_VALUE) { if (!TerminateJobObject(process->job_, exit_code)) { - process->error_ = - bazel::windows::GetLastErrorString(L"TerminateJobObject()"); + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeTerminate", ToString(process->pid_), + err_code); return JNI_FALSE; } } else if (process->process_ != INVALID_HANDLE_VALUE) { if (!TerminateProcess(process->process_, exit_code)) { - process->error_ = - bazel::windows::GetLastErrorString(L"TerminateProcess()"); + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeTerminate", ToString(process->pid_), + err_code); return JNI_FALSE; } }
diff --git a/src/main/native/windows/util.cc b/src/main/native/windows/util.cc index 156b2e3..9ed930d 100644 --- a/src/main/native/windows/util.cc +++ b/src/main/native/windows/util.cc
@@ -28,11 +28,23 @@ using std::wstring; using std::wstringstream; -wstring GetLastErrorString(const wstring& cause) { - return GetLastErrorString(cause, GetLastError()); +wstring MakeErrorMessage(const wchar_t* file, int line, + const wchar_t* failed_func, const wstring& func_arg, + const wstring& message) { + wstringstream result; + result << L"ERROR: " << file << L"(" << line << L"): " << failed_func << L"(" + << func_arg << L"): " << message; + return result.str(); } -wstring GetLastErrorString(const wstring& cause, DWORD error_code) { +wstring MakeErrorMessage(const wchar_t* file, int line, + const wchar_t* failed_func, const wstring& func_arg, + DWORD error_code) { + return MakeErrorMessage(file, line, failed_func, func_arg, + GetLastErrorString(error_code)); +} + +wstring GetLastErrorString(DWORD error_code) { if (error_code == 0) { return L""; } @@ -46,12 +58,13 @@ if (size == 0) { wstringstream err; DWORD format_message_error = GetLastError(); - err << cause << L": Error " << error_code - << L"; cannot format message due to error " << format_message_error; + err << L"Error code " << error_code + << L"; cannot format message due to error code " + << format_message_error; return err.str(); } - wstring result(cause + L": " + message); + wstring result(message); HeapFree(GetProcessHeap(), LMEM_FIXED, message); return result; } @@ -77,21 +90,26 @@ return L""; } if (path[0] == '"') { - return wstring(L"path should not be quoted"); + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path, + L"path should not be quoted"); } if (IsSeparator(path[0])) { - return wstring(L"path='") + path + L"' is absolute"; + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path, + L"path is absolute without a drive letter"); } if (Contains(path, L"/./") || Contains(path, L"\\.\\") || Contains(path, L"/..") || Contains(path, L"\\..")) { - return wstring(L"path='") + path + L"' is not normalized"; + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path, + L"path is not normalized"); } if (path.size() >= MAX_PATH && !HasSeparator(path)) { - return wstring(L"path='") + path + L"' is just a file name but too long"; + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path, + L"path is just a file name but too long"); } if (HasSeparator(path) && !(isalpha(path[0]) && path[1] == L':' && IsSeparator(path[2]))) { - return wstring(L"path='") + path + L"' is not an absolute path"; + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"AsShortPath", path, + L"path is not absolute"); } // At this point we know the path is either just a file name (shorter than // MAX_PATH), or an absolute, normalized, Windows-style path (of any length). @@ -120,14 +138,15 @@ WCHAR wshort[kMaxShortPath]; DWORD wshort_size = ::GetShortPathNameW(wlong.c_str(), NULL, 0); if (wshort_size == 0) { - return GetLastErrorString(wstring(L"GetShortPathName failed (path=") + - path + L")"); + DWORD err_code = GetLastError(); + wstring res = MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"GetShortPathNameW", wlong, err_code); + return res; } if (wshort_size >= kMaxShortPath) { - return wstring( - L"GetShortPathName would not shorten the path enough (path=") + - path + L")"; + return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetShortPathNameW", + wlong, L"cannot shorten the path enough"); } GetShortPathNameW(wlong.c_str(), wshort, kMaxShortPath); result->assign(wshort + 4); @@ -136,16 +155,20 @@ wstring AsExecutablePathForCreateProcess(const wstring& path, wstring* result) { if (path.empty()) { - return L"path should not be empty"; + return MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"AsExecutablePathForCreateProcess", path, + L"path should not be empty"); } wstring error = AsShortPath(path, result); - if (error.empty()) { - // Quote the path in case it's something like "c:\foo\app name.exe". - // Do this unconditionally, there's no harm in quoting. Quotes are not - // allowed inside paths so we don't need to escape quotes. - QuotePath(*result, result); + if (!error.empty()) { + return MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"AsExecutablePathForCreateProcess", path, error); } - return error; + // Quote the path in case it's something like "c:\foo\app name.exe". + // Do this unconditionally, there's no harm in quoting. Quotes are not + // allowed inside paths so we don't need to escape quotes. + QuotePath(*result, result); + return L""; } } // namespace windows
diff --git a/src/main/native/windows/util.h b/src/main/native/windows/util.h index 0069eeb..730a1d4 100644 --- a/src/main/native/windows/util.h +++ b/src/main/native/windows/util.h
@@ -49,8 +49,16 @@ HANDLE handle_; }; -wstring GetLastErrorString(const wstring& cause); -wstring GetLastErrorString(const wstring& cause, DWORD error_code); +#define WSTR1(x) L##x +#define WSTR(x) WSTR1(x) + +wstring MakeErrorMessage(const wchar_t* file, int line, + const wchar_t* failed_func, const wstring& func_arg, + const wstring& message); +wstring MakeErrorMessage(const wchar_t* file, int line, + const wchar_t* failed_func, const wstring& func_arg, + DWORD error_code); +wstring GetLastErrorString(DWORD error_code); // Same as `AsExecutablePathForCreateProcess` except it won't quote the result. wstring AsShortPath(wstring path, wstring* result);
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java index 383b5c8..2f7483a 100644 --- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java +++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java
@@ -115,7 +115,7 @@ WindowsFileOperations.isJunction(root + "/non-existent"); fail("expected to throw"); } catch (IOException e) { - assertThat(e.getMessage()).contains("GetFileAttributes"); + assertThat(e.getMessage()).contains("nativeIsJunction"); } assertThat(Arrays.asList(new File(root + "/shrtpath/a").list())).containsExactly("file1.txt"); assertThat(Arrays.asList(new File(root + "/shrtpath/b").list())).containsExactly("file2.txt");
diff --git a/src/test/native/windows/util_test.cc b/src/test/native/windows/util_test.cc index 83177b9..2a131e7 100644 --- a/src/test/native/windows/util_test.cc +++ b/src/test/native/windows/util_test.cc
@@ -234,8 +234,8 @@ /* const WCHAR* */ error_msg) \ { \ wstring actual; \ - ASSERT_CONTAINS(AsExecutablePathForCreateProcess(input, &actual), \ - error_msg); \ + wstring result(AsExecutablePathForCreateProcess(input, &actual)); \ + ASSERT_CONTAINS(result, error_msg); \ } // This is a macro so the assertions will have the correct line number. @@ -249,13 +249,12 @@ TEST(WindowsUtilTest, TestAsExecutablePathForCreateProcessBadInputs) { ASSERT_SHORTENING_FAILS(L"", L"should not be empty"); - ASSERT_SHORTENING_FAILS(L"\"cmd.exe\"", L"should not be quoted"); - ASSERT_SHORTENING_FAILS(L"/dev/null", L"path='/dev/null' is absolute"); - ASSERT_SHORTENING_FAILS(L"/usr/bin/bash", - L"path='/usr/bin/bash' is absolute"); - ASSERT_SHORTENING_FAILS(L"foo\\bar.exe", L"absolute"); - ASSERT_SHORTENING_FAILS(L"foo\\..\\bar.exe", L"normalized"); - ASSERT_SHORTENING_FAILS(L"\\bar.exe", L"path='\\bar.exe' is absolute"); + ASSERT_SHORTENING_FAILS(L"\"cmd.exe\"", L"path should not be quoted"); + ASSERT_SHORTENING_FAILS(L"/dev/null", L"path is absolute without a drive"); + ASSERT_SHORTENING_FAILS(L"/usr/bin/bash", L"path is absolute without a"); + ASSERT_SHORTENING_FAILS(L"foo\\bar.exe", L"path is not absolute"); + ASSERT_SHORTENING_FAILS(L"foo\\..\\bar.exe", L"path is not normalized"); + ASSERT_SHORTENING_FAILS(L"\\bar.exe", L"path is absolute"); wstring dummy = L"hello"; while (dummy.size() < MAX_PATH) { @@ -295,7 +294,7 @@ if (i > 0) { ASSERT_EQ(::GetFileAttributesW(wfilename.c_str()), INVALID_FILE_ATTRIBUTES); - ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"GetShortPathName failed"); + ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"GetShortPathNameW"); } // Create the file, now we should be able to shorten it when i=0, but not @@ -309,7 +308,7 @@ // The wfilename was too long to begin with, and it was impossible to // shorten any of the segments (since we deliberately created them that // way), so shortening failed. - ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"would not shorten"); + ASSERT_SHORTENING_FAILS(wfilename.c_str(), L"cannot shorten the path"); } DELETE_FILE(wfilename); } @@ -327,7 +326,7 @@ ASSERT_GT(wshortenable.size(), MAX_PATH); // Attempt to shorten. It will fail because the file doesn't exist yet. - ASSERT_SHORTENING_FAILS(wshortenable, L"GetShortPathName failed"); + ASSERT_SHORTENING_FAILS(wshortenable, L"GetShortPathNameW"); // Create the file so shortening will succeed. CREATE_FILE(wshortenable);