Skip to content

Commit

Permalink
windows,client: fix error reporting
Browse files Browse the repository at this point in the history
Fix error reporting in the path conversion methods
of the Bazel client. Previously the error
reporting logic used GetLastErrorString in places
where it was not appropriate (i.e. it was not a
failed Windows API call that caused an error).

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

See bazelbuild#4319

Change-Id: Ie50a20e0ee0c572592f637340a2f2948c7f53088
  • Loading branch information
laszlocsomor committed Apr 23, 2018
1 parent 538c3eb commit 88b957a
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 135 deletions.
34 changes: 20 additions & 14 deletions src/main/cpp/blaze_util_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,11 @@ static void CreateCommandLine(CmdLine* result, const string& exe,
const std::vector<string>& args_vector) {
std::ostringstream cmdline;
string short_exe;
if (!blaze_util::AsShortWindowsPath(exe, &short_exe)) {
string error;
if (!blaze_util::AsShortWindowsPath(exe, &short_exe, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "CreateCommandLine: AsShortWindowsPath(" << exe
<< "): " << GetLastErrorString();
<< "): " << error;
}
bool first = true;
for (const auto& s : args_vector) {
Expand Down Expand Up @@ -440,10 +441,11 @@ int ExecuteDaemon(const string& exe,
const string& server_dir,
BlazeServerStartup** server_startup) {
wstring wdaemon_output;
if (!blaze_util::AsAbsoluteWindowsPath(daemon_output, &wdaemon_output)) {
string error;
if (!blaze_util::AsAbsoluteWindowsPath(daemon_output, &wdaemon_output, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "ExecuteDaemon(" << exe << "): AsAbsoluteWindowsPath("
<< daemon_output << ") failed: " << GetLastErrorString();
<< daemon_output << ") failed: " << error;
}

SECURITY_ATTRIBUTES sa;
Expand Down Expand Up @@ -653,10 +655,11 @@ const char kListSeparator = ';';

string PathAsJvmFlag(const string& path) {
string spath;
if (!blaze_util::AsShortWindowsPath(path, &spath)) {
string error;
if (!blaze_util::AsShortWindowsPath(path, &spath, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "PathAsJvmFlag(" << path
<< "): AsShortWindowsPath failed: " << GetLastErrorString();
<< "): AsShortWindowsPath failed: " << error;
}
// Convert backslashes to forward slashes, in order to avoid the JVM parsing
// Windows paths as if they contained escaped characters.
Expand All @@ -668,10 +671,11 @@ string PathAsJvmFlag(const string& path) {
string ConvertPath(const string& path) {
// The path may not be Windows-style and may not be normalized, so convert it.
wstring wpath;
if (!blaze_util::AsAbsoluteWindowsPath(path, &wpath)) {
string error;
if (!blaze_util::AsAbsoluteWindowsPath(path, &wpath, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "ConvertPath(" << path
<< "): AsAbsoluteWindowsPath failed: " << GetLastErrorString();
<< "): AsAbsoluteWindowsPath failed: " << error;
}
std::transform(wpath.begin(), wpath.end(), wpath.begin(), ::towlower);
return string(blaze_util::WstringToCstring(
Expand All @@ -682,18 +686,19 @@ string ConvertPath(const string& path) {
bool SymlinkDirectories(const string &posix_target, const string &posix_name) {
wstring name;
wstring target;
if (!blaze_util::AsAbsoluteWindowsPath(posix_name, &name)) {
string error;
if (!blaze_util::AsAbsoluteWindowsPath(posix_name, &name, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "SymlinkDirectories(" << posix_target << ", " << posix_name
<< "): AsAbsoluteWindowsPath(" << posix_target
<< ") failed: " << GetLastErrorString();
<< ") failed: " << error;
return false;
}
if (!blaze_util::AsAbsoluteWindowsPath(posix_target, &target)) {
if (!blaze_util::AsAbsoluteWindowsPath(posix_target, &target, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "SymlinkDirectories(" << posix_target << ", " << posix_name
<< "): AsAbsoluteWindowsPath(" << posix_name
<< ") failed: " << GetLastErrorString();
<< ") failed: " << error;
return false;
}
wstring werror(CreateJunction(name, target));
Expand Down Expand Up @@ -961,10 +966,11 @@ uint64_t AcquireLock(const string& output_base, bool batch_mode, bool block,
BlazeLock* blaze_lock) {
string lockfile = blaze_util::JoinPath(output_base, "lock");
wstring wlockfile;
if (!blaze_util::AsAbsoluteWindowsPath(lockfile, &wlockfile)) {
string error;
if (!blaze_util::AsAbsoluteWindowsPath(lockfile, &wlockfile, &error)) {
BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR)
<< "AcquireLock(" << output_base << "): AsAbsoluteWindowsPath("
<< lockfile << ") failed: " << GetLastErrorString();
<< lockfile << ") failed: " << error;
}

blaze_lock->handle = INVALID_HANDLE_VALUE;
Expand Down
9 changes: 6 additions & 3 deletions src/main/cpp/util/file_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,15 +213,18 @@ void ForEachDirectoryEntry(const std::string &path,
#if defined(COMPILER_MSVC) || defined(__CYGWIN__)
const wchar_t *RemoveUncPrefixMaybe(const wchar_t *ptr);

bool AsWindowsPath(const std::string &path, std::string *result);
bool AsWindowsPath(const std::string &path, std::string *result,
std::string* error);

bool AsAbsoluteWindowsPath(const std::string &path, std::wstring *wpath);
bool AsAbsoluteWindowsPath(const std::string &path, std::wstring *wpath,
std::string* error);

// Same as `AsWindowsPath`, but returns a lowercase 8dot3 style shortened path.
// Result will never have a UNC prefix, nor a trailing "/" or "\".
// Works also for non-existent paths; shortens as much of them as it can.
// Also works for non-existent drives.
bool AsShortWindowsPath(const std::string &path, std::string *result);
bool AsShortWindowsPath(const std::string &path, std::string *result,
std::string* error);
#endif // defined(COMPILER_MSVC) || defined(__CYGWIN__)

} // namespace blaze_util
Expand Down
Loading

0 comments on commit 88b957a

Please sign in to comment.