Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow longpath executables #22532

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 100 additions & 1 deletion src/main/native/windows/process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
#include <memory>
#include <sstream>

#include <codecvt>
#include <locale.h>
#include <string_view>

namespace bazel {
namespace windows {

Expand All @@ -47,6 +51,81 @@ static bool DupeHandle(HANDLE h, AutoHandle* out, std::wstring* error) {
return true;
}

static std::wstring CreateProcessWrapper(const std::wstring argv0,
const std::wstring &commandline,
std::wstring &out_path) {
// Safety first, ensure the out_path is empty before we do any work
out_path = L"";

/*
Save on possible collisions by using the argv0 name as a base for the
wrapper file name.
Argv0 is likely quoted, so we can use a string_view to cheaply manipulate
it and get rid of the quote if needed.
*/
auto const view = std::wstring_view(argv0);
auto const pos = view.find_last_of('\\');
auto stem = view.substr(pos == std::wstring::npos ? 0 : (pos + 1));
if (stem.find(L"\"") != std::wstring::npos) {
/*
After removing the initial quote, there's no other place for the quote to
be than at the end, so we can simply remove it and move on.
*/
stem.remove_suffix(1);
}

std::wstring wrapper_file_name = std::wstring(stem) + L".wrapper.bat";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be risk of conflict with existing file or lack of write permission? Can we find a safer place to create the wrapper?

HANDLE handle = ::CreateFileW(
/* lpFileName */ wrapper_file_name.c_str(),
/* dwDesiredAccess */ GENERIC_WRITE,
/* dwShareMode */ FILE_SHARE_READ,
/* lpSecurityAttributes */ nullptr,
/* dwCreationDisposition */ CREATE_ALWAYS,
/* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL,
/* hTemplateFile */ nullptr);

if (handle == INVALID_HANDLE_VALUE) {
return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateProcessWrapper",
wrapper_file_name,
L"Unable to create file wrapper");
}

/*
Simply call the original command line
Different powershells seem to struggle with widechars, so convert
to normal char instead.
*/
std::string contents =
"@ECHO OFF\n\n" +
std::wstring_convert<std::codecvt_utf8<wchar_t>>().to_bytes(commandline);

DWORD bytesWritten;
if (!::WriteFile(handle, contents.data(), contents.length(), &bytesWritten,
nullptr)) {

DWORD err_code = GetLastError();
return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateProcessWrapper",
wrapper_file_name, err_code);
}

if (bytesWritten != contents.length()) {
std::wstring res = MakeErrorMessage(
WSTR(__FILE__), __LINE__, L"CreateProcessWrapper", wrapper_file_name,
L"Unable to write all data to file wrapper");
return res;
}

if (!::CloseHandle(handle)) {
DWORD err_code = GetLastError();
return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"CreateProcessWrapper",
wrapper_file_name, err_code);
}
handle = INVALID_HANDLE_VALUE;

out_path = wrapper_file_name;
return L"";
}

bool WaitableProcess::Create(const std::wstring& argv0,
const std::wstring& argv_rest, void* env,
const std::wstring& wcwd, std::wstring* error) {
Expand Down Expand Up @@ -90,15 +169,35 @@ bool WaitableProcess::Create(const std::wstring& argv0,
}

std::wstring argv0short;
error_msg = AsExecutablePathForCreateProcess(argv0, &argv0short);
error_msg = AsExecutablePathForCreateProcess(argv0, &argv0short, false);
if (!error_msg.empty()) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"WaitableProcess::Create", argv0, error_msg);
return false;
}

/*
If the short form of the executable is still well above MAX_PATH, then we
cannot use it for CreateProcessW in any form.
Instead, output a wrapper file into a known location that ends up calling
the executable instead. This should side-step the MAX_PATH limitations.
*/
std::wstring commandline =
argv_rest.empty() ? argv0short : (argv0short + L" " + argv_rest);
if (argv0short.length() >= MAX_PATH) {
std::wstring wrapper_file_path;
std::wstring error_msg(
CreateProcessWrapper(argv0short, commandline, wrapper_file_path));
if (!error_msg.empty()) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"WaitableProcess::Create", argv0, error_msg);
return false;
}

// Redirect command line to call the wrapper instead
commandline = L"cmd /c " + wrapper_file_path;
}

std::unique_ptr<WCHAR[]> mutable_commandline(
new WCHAR[commandline.size() + 1]);
wcsncpy(mutable_commandline.get(), commandline.c_str(),
Expand Down
23 changes: 16 additions & 7 deletions src/main/native/windows/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ static bool Contains(const wstring& s, const WCHAR* substr) {
return s.find(substr) != wstring::npos;
}

wstring AsShortPath(wstring path, wstring* result) {
wstring AsShortPath(wstring path, wstring *result, bool ensure_short) {
// Using `MAX_PATH` - 4 (256) instead of `MAX_PATH` to fix
// https://github.com/bazelbuild/bazel/issues/12310
static const size_t kMaxPath = MAX_PATH - 4;
Expand Down Expand Up @@ -249,7 +249,6 @@ wstring AsShortPath(wstring path, wstring* result) {
// it and which we'll omit from `result`, plus a null terminator.
static const size_t kMaxShortPath = kMaxPath + 4;

WCHAR wshort[kMaxShortPath];
DWORD wshort_size = ::GetShortPathNameW(wlong.c_str(), nullptr, 0);
if (wshort_size == 0) {
DWORD err_code = GetLastError();
Expand All @@ -258,16 +257,26 @@ wstring AsShortPath(wstring path, wstring* result) {
return res;
}

if (wshort_size >= kMaxShortPath) {
if (ensure_short && (wshort_size >= kMaxShortPath)) {
return MakeErrorMessage(WSTR(__FILE__), __LINE__, L"GetShortPathNameW",
wlong, L"cannot shorten the path enough");
}
GetShortPathNameW(wlong.c_str(), wshort, kMaxShortPath);
result->assign(wshort + 4);

std::unique_ptr<WCHAR[]> wshort(new WCHAR[wshort_size]);
if (wshort_size - 1 !=
::GetShortPathNameW(wlong.c_str(), wshort.get(), wshort_size)) {
DWORD err_code = GetLastError();
wstring res = MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"GetShortPathNameW", wlong, err_code);
return res;
}

result->assign(wshort.get() + 4);
return L"";
}

wstring AsExecutablePathForCreateProcess(wstring path, wstring* result) {
wstring AsExecutablePathForCreateProcess(wstring path, wstring *result,
bool ensure_short) {
if (path.empty()) {
return MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"AsExecutablePathForCreateProcess", path,
Expand All @@ -287,7 +296,7 @@ wstring AsExecutablePathForCreateProcess(wstring path, wstring* result) {
}
path = cwd + L"\\" + path;
}
wstring error = AsShortPath(path, result);
wstring error = AsShortPath(path, result, ensure_short);
if (!error.empty()) {
return MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"AsExecutablePathForCreateProcess", path, error);
Expand Down
10 changes: 7 additions & 3 deletions src/main/native/windows/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ wstring MakeErrorMessage(const wchar_t* file, int line,
wstring GetLastErrorString(DWORD error_code);

// Same as `AsExecutablePathForCreateProcess` except it won't quote the result.
wstring AsShortPath(wstring path, wstring* result);
wstring AsShortPath(wstring path, wstring* result, bool ensure_short = true);

// Computes a path suitable as the executable part in CreateProcessA's cmdline.
//
Expand All @@ -158,8 +158,12 @@ wstring AsShortPath(wstring path, wstring* result);
// Otherwise this method attempts to compute an 8dot3 style short name for
// `path`, and if that succeeds and the result is at most MAX_PATH - 1 long (not
// including null terminator), then that will be the result (plus quotes).
// Otherwise this function fails and returns an error message.
wstring AsExecutablePathForCreateProcess(wstring path, wstring* result);

// If ensure_short is set to true, this function fails and returns an error message when
// the path is still too long after 8dot3 shortening.
// If ensure_short is set to false, the input path is simply returned and the caller must check
// the length of the path.
wstring AsExecutablePathForCreateProcess(wstring path, wstring* result, bool ensure_short = true);

} // namespace windows
} // namespace bazel
Expand Down
Loading