Skip to content

Commit

Permalink
windows,client: no longer support Unix-style paths
Browse files Browse the repository at this point in the history 
The Bazel client no longer supports MSYS paths.
The only exception is "/dev/null" which the client
treats as "NUL".

After this change you can no longer pass MSYS
paths as Bazel flag values on Windows.

See bazelbuild#4319

Change-Id: I39d81843015c5a4014dd5953bac2e1c29dcd5bed
PiperOrigin-RevId: 194372504
  • Loading branch information
@laszlocsomor
laszlocsomor authored and Copybara-Service committed Apr 26, 2018
1 parent 1210ad9 commit 9f3825f
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 179 deletions.
115 changes: 12 additions & 103 deletions src/main/cpp/util/file_windows.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ static bool CanReadFileW(const wstring& path);
// Returns a normalized form of the input `path`.
//
// `path` must be a relative or absolute Windows path, it may use "/" instead of
// "\" but must not be an absolute MSYS path.
// "\" but must not be a Unix-style (MSYS) path.
// The result won't have a UNC prefix, even if `path` did.
//
// Normalization means removing "." references, resolving ".." references, and
Expand Down Expand Up @@ -371,78 +371,6 @@ pair<wstring, wstring> SplitPathW(const wstring& path) {
return SplitPathImpl(path);
}

class MsysRoot {
public:
static bool IsValid();
static const string& GetPath();
static void ResetForTesting() { instance_.initialized_ = false; }

private:
bool initialized_;
bool valid_;
string path_;
static MsysRoot instance_;

static bool Get(string* path);

MsysRoot() : initialized_(false) {}
void InitIfNecessary();
};

MsysRoot MsysRoot::instance_;

void ResetMsysRootForTesting() { MsysRoot::ResetForTesting(); }

bool MsysRoot::IsValid() {
instance_.InitIfNecessary();
return instance_.valid_;
}

const string& MsysRoot::GetPath() {
instance_.InitIfNecessary();
return instance_.path_;
}

bool MsysRoot::Get(string* path) {
string result;
char value[MAX_PATH];
DWORD len = GetEnvironmentVariableA("BAZEL_SH", value, MAX_PATH);
if (len > 0) {
result = value;
} else {
const char* value2 = getenv("BAZEL_SH");
if (value2 == nullptr || value2[0] == '\0') {
BAZEL_LOG(ERROR) << "BAZEL_SH environment variable is not defined, "
"cannot convert MSYS paths to Windows paths";
return false;
}
result = value2;
}

// BAZEL_SH is usually "c:\tools\msys64\usr\bin\bash.exe" but could also be
// "c:\cygwin64\bin\bash.exe", and may have forward slashes instead of
// backslashes. Either way, we just need to remove the "usr/bin/bash.exe" or
// "bin/bash.exe" suffix (we don't care about the basename being "bash.exe").
result = Dirname(result);
pair<string, string> parent(SplitPath(result));
pair<string, string> grandparent(SplitPath(parent.first));
if (AsLower(grandparent.second) == "usr" && AsLower(parent.second) == "bin") {
*path = grandparent.first;
return true;
} else if (AsLower(parent.second) == "bin") {
*path = parent.first;
return true;
}
return false;
}

void MsysRoot::InitIfNecessary() {
if (!initialized_) {
valid_ = Get(&path_);
initialized_ = true;
}
}

bool AsWindowsPath(const string& path, string* result, string* error) {
if (path.empty()) {
result->clear();
Expand Down Expand Up @@ -476,30 +404,13 @@ bool AsWindowsPath(const string& path, string* result, string* error) {

string mutable_path = path;
if (path[0] == '/') {
// This is an absolute MSYS path.
if (path.size() == 2 || (path.size() > 2 && path[2] == '/')) {
// The path is either "/x" or "/x/" or "/x/something". In all three cases
// "x" is the drive letter.
// TODO(laszlocsomor): use GetLogicalDrives to retrieve the list of drives
// and only apply this heuristic for the valid drives. It's possible that
// the user has a directory "/a" but no "A:\" drive, so in that case we
// should prepend the MSYS root.
mutable_path = path.substr(1, 1) + ":\\";
if (path.size() > 2) {
mutable_path += path.substr(3);
}
} else {
// The path is a normal MSYS path e.g. "/usr". Prefix it with the MSYS
// root.
if (!MsysRoot::IsValid()) {
if (error) {
*error = "MSYS root is invalid";
}
return false;
}
mutable_path = JoinPath(MsysRoot::GetPath(), path);
if (error) {
*error = "Unix-style paths are unsupported";
}
} else if (path[0] == '\\') {
return false;
}

if (path[0] == '\\') {
// This is an absolute Windows path on the current drive, e.g. "\foo\bar".
mutable_path = string(1, GetCurrentDrive()) + ":" + path;
} // otherwise this is a relative path, or absolute Windows path.
Expand All @@ -512,20 +423,18 @@ bool AsWindowsPath(const string& path, string* result, string* error) {
//
// Returns true if conversion succeeded and sets the contents of `result` to it.
//
// The `path` may be absolute or relative, and may be a Windows or MSYS path.
// In every case, the output is normalized (see NormalizeWindowsPath).
// The input `path` may be an absolute or relative Windows path.
//
// The returned path is normalized (see NormalizeWindowsPath).
//
// If `path` had a "\\?\" prefix then the function assumes it's already Windows
// style and converts it to wstring without any alterations.
// Otherwise `path` is normalized and converted to a Windows path and the result
// won't have a "\\?\" prefix even if it's longer than MAX_PATH (adding the
// prefix is the caller's responsibility).
//
// The function recognizes the drive letter in MSYS paths, so e.g. "/c/windows"
// becomes "c:\windows". Prepends the MSYS root (computed from the BAZEL_SH
// envvar) to absolute MSYS paths, so e.g. "/usr" becomes "c:\tools\msys64\usr".
// Recognizes current-drive-relative Windows paths ("\foo") turning them into
// absolute paths ("c:\foo").
// The method recognizes current-drive-relative Windows paths ("\foo") turning
// them into absolute paths ("c:\foo").
bool AsWindowsPath(const string& path, wstring* result, string* error) {
string normalized_win_path;
if (!AsWindowsPath(path, &normalized_win_path, error)) {
Expand Down
3 changes: 0 additions & 3 deletions src/test/cpp/blaze_util_windows_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,6 @@ TEST(BlazeUtilWindowsTest, TestUnsetEnv) {

TEST(BlazeUtilWindowsTest, ConvertPathTest) {
EXPECT_EQ("c:\\foo", ConvertPath("C:\\FOO"));
EXPECT_EQ("c:\\blah", ConvertPath("/c/Blah"));
EXPECT_EQ("c:\\", ConvertPath("/c"));
EXPECT_EQ("c:\\", ConvertPath("/c/"));
EXPECT_EQ("c:\\", ConvertPath("c:/"));
EXPECT_EQ("c:\\foo\\bar", ConvertPath("c:/../foo\\BAR\\.\\"));
EXPECT_EQ("nul", MakeAbsolute("NUL"));
Expand Down
84 changes: 11 additions & 73 deletions src/test/cpp/util/file_windows_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ using std::wstring;

// Methods defined in file_windows.cc that are only visible for testing.
bool AsWindowsPath(const string& path, wstring* result, string* error);
void ResetMsysRootForTesting();
string NormalizeWindowsPath(string path);

class FileWindowsTest : public ::testing::Test {
Expand Down Expand Up @@ -177,7 +176,6 @@ TEST_F(FileWindowsTest, TestIsRootDirectory) {

TEST_F(FileWindowsTest, TestAsWindowsPath) {
SetEnvironmentVariableA("BAZEL_SH", "c:\\some\\long/path\\bin\\bash.exe");
ResetMsysRootForTesting();
wstring actual;

// Null and empty input produces empty result.
Expand Down Expand Up @@ -220,18 +218,10 @@ TEST_F(FileWindowsTest, TestAsWindowsPath) {
ASSERT_EQ(wstring(L"NUL"), actual);

// MSYS path with drive letter.
ASSERT_TRUE(AsWindowsPath("/c", &actual, nullptr));
ASSERT_EQ(wstring(L"c:\\"), actual);
ASSERT_TRUE(AsWindowsPath("/c/", &actual, nullptr));
ASSERT_EQ(wstring(L"c:\\"), actual);
ASSERT_TRUE(AsWindowsPath("/c/blah", &actual, nullptr));
ASSERT_EQ(wstring(L"c:\\blah"), actual);
ASSERT_TRUE(AsWindowsPath("/d/progra~1/micros~1", &actual, nullptr));
ASSERT_EQ(wstring(L"d:\\progra~1\\micros~1"), actual);

// Absolute MSYS path without drive letter is relative to MSYS root.
ASSERT_TRUE(AsWindowsPath("/foo", &actual, nullptr));
ASSERT_EQ(wstring(L"c:\\some\\long\\path\\foo"), actual);
ASSERT_FALSE(AsWindowsPath("/c", &actual, &error));
EXPECT_TRUE(error.find("Unix-style") != string::npos);
ASSERT_FALSE(AsWindowsPath("/c/", &actual, &error));
EXPECT_TRUE(error.find("Unix-style") != string::npos);

// Absolute-on-current-drive path gets a drive letter.
ASSERT_TRUE(AsWindowsPath("\\foo", &actual, nullptr));
Expand All @@ -252,7 +242,6 @@ TEST_F(FileWindowsTest, TestAsWindowsPath) {

TEST_F(FileWindowsTest, TestAsAbsoluteWindowsPath) {
SetEnvironmentVariableA("BAZEL_SH", "c:\\some\\long/path\\bin\\bash.exe");
ResetMsysRootForTesting();
wstring actual;

ASSERT_TRUE(AsAbsoluteWindowsPath("c:/", &actual, nullptr));
Expand Down Expand Up @@ -280,15 +269,15 @@ TEST_F(FileWindowsTest, TestAsShortWindowsPath) {

ASSERT_TRUE(AsShortWindowsPath("C://", &actual, nullptr));
ASSERT_EQ(string("c:\\"), actual);
ASSERT_TRUE(AsShortWindowsPath("/C//", &actual, nullptr));
ASSERT_EQ(string("c:\\"), actual);

string error;
ASSERT_FALSE(AsShortWindowsPath("/C//", &actual, &error));
EXPECT_TRUE(error.find("Unix-style") != string::npos);

// The A drive usually doesn't exist but AsShortWindowsPath should still work.
// Here we even have multiple trailing slashes, that should be handled too.
ASSERT_TRUE(AsShortWindowsPath("A://", &actual, nullptr));
ASSERT_EQ(string("a:\\"), actual);
ASSERT_TRUE(AsShortWindowsPath("/A//", &actual, nullptr));
ASSERT_EQ(string("a:\\"), actual);

// Assert that we can shorten the TEST_TMPDIR.
string tmpdir;
Expand Down Expand Up @@ -316,9 +305,6 @@ TEST_F(FileWindowsTest, TestAsShortWindowsPath) {
ASSERT_TRUE(AsShortWindowsPath(JoinPath(tmpdir, "NonExistent/FOO"), &actual,
nullptr));
ASSERT_EQ(short_tmpdir + "\\nonexistent\\foo", actual);
// Assert shortening non-existent root paths.
ASSERT_TRUE(AsShortWindowsPath("/c/NonExistent/FOO", &actual, nullptr));
ASSERT_EQ("c:\\nonexistent\\foo", actual);
}

TEST_F(FileWindowsTest, TestMsysRootRetrieval) {
Expand All @@ -327,41 +313,14 @@ TEST_F(FileWindowsTest, TestMsysRootRetrieval) {
// We just need "bin/<something>" or "usr/bin/<something>".
// Forward slashes are converted to backslashes.
SetEnvironmentVariableA("BAZEL_SH", "c:/foo\\bin/some_bash.exe");
ResetMsysRootForTesting();
ASSERT_TRUE(AsWindowsPath("/blah", &actual, nullptr));
ASSERT_EQ(wstring(L"c:\\foo\\blah"), actual);

SetEnvironmentVariableA("BAZEL_SH", "c:\\foo/MSYS64/usr\\bin/dummy.exe");
ResetMsysRootForTesting();
ASSERT_TRUE(AsWindowsPath("/blah", &actual, nullptr));
ASSERT_EQ(wstring(L"c:\\foo\\MSYS64\\blah"), actual);

// We just need "bin/<something>" or "usr/bin/<something>".
SetEnvironmentVariableA("BAZEL_SH", "c:/bin/kitty.exe");
ResetMsysRootForTesting();
ASSERT_TRUE(AsWindowsPath("/blah", &actual, nullptr));
ASSERT_EQ(wstring(L"c:\\blah"), actual);

// Just having "msys" in the path isn't enough.
SetEnvironmentVariableA("BAZEL_SH", "c:/msys/foo/bash.exe");
ResetMsysRootForTesting();
string error;
ASSERT_FALSE(AsWindowsPath("/blah", &actual, &error));
EXPECT_TRUE(error.find("MSYS root is invalid") != string::npos);

// We need "bin/<something>" or "usr/bin/<something>", not "usr/<something>".
SetEnvironmentVariableA("BAZEL_SH", "c:/msys/usr/bash.exe");
ResetMsysRootForTesting();
ASSERT_FALSE(AsWindowsPath("/blah", &actual, &error));
EXPECT_TRUE(error.find("MSYS root is invalid") != string::npos);
EXPECT_TRUE(error.find("Unix-style") != string::npos);

SetEnvironmentVariableA("BAZEL_SH", "c:/qux.exe");
ResetMsysRootForTesting();
SetEnvironmentVariableA("BAZEL_SH", "c:/tools/msys64/usr/bin/bash.exe");
ASSERT_FALSE(AsWindowsPath("/blah", &actual, &error));
EXPECT_TRUE(error.find("MSYS root is invalid") != string::npos);

SetEnvironmentVariableA("BAZEL_SH", nullptr);
ResetMsysRootForTesting();
EXPECT_TRUE(error.find("Unix-style") != string::npos);
}

TEST_F(FileWindowsTest, TestPathExistsWindows) {
Expand All @@ -383,11 +342,6 @@ TEST_F(FileWindowsTest, TestPathExistsWindows) {
// Set the BAZEL_SH root so we can resolve MSYS paths.
SetEnvironmentVariableA("BAZEL_SH",
(fake_msys_root + "/bin/fake_bash.exe").c_str());
ResetMsysRootForTesting();

// Assert existence check for MSYS paths.
ASSERT_FALSE(PathExists("/this/should/not/exist/mkay"));
ASSERT_TRUE(PathExists("/"));

// Create a junction pointing to an existing directory.
CREATE_JUNCTION(tmpdir + "/junc1", fake_msys_root);
Expand All @@ -410,21 +364,13 @@ TEST_F(FileWindowsTest, TestIsDirectory) {
ASSERT_TRUE(IsDirectory(tmpdir));
ASSERT_TRUE(IsDirectory("C:\\"));
ASSERT_TRUE(IsDirectory("C:/"));
ASSERT_TRUE(IsDirectory("/c"));

ASSERT_FALSE(IsDirectory("non.existent"));
// Create a directory under `tempdir`, verify that IsDirectory reports true.
string dir1(JoinPath(tmpdir, "dir1"));
ASSERT_EQ(0, mkdir(dir1.c_str()));
ASSERT_TRUE(IsDirectory(dir1));

// Use dir1 as the mock msys root, verify that IsDirectory works for a MSYS
// path.
SetEnvironmentVariableA("BAZEL_SH",
JoinPath(dir1, "usr\\bin\\bash.exe").c_str());
ResetMsysRootForTesting();
ASSERT_TRUE(IsDirectory("/"));

// Verify that IsDirectory works for a junction.
string junc1(JoinPath(tmpdir, "junc1"));
CREATE_JUNCTION(junc1, dir1);
Expand Down Expand Up @@ -470,19 +416,11 @@ TEST_F(FileWindowsTest, TestMakeDirectories) {
GET_TEST_TMPDIR(tmpdir);
ASSERT_LT(0, tmpdir.size());

SetEnvironmentVariableA(
"BAZEL_SH", (JoinPath(tmpdir, "fake_msys/bin/fake_bash.exe")).c_str());
ResetMsysRootForTesting();
ASSERT_EQ(0, mkdir(JoinPath(tmpdir, "fake_msys").c_str()));
ASSERT_TRUE(IsDirectory(JoinPath(tmpdir, "fake_msys")));

// Test that we can create come directories, can't create others.
ASSERT_FALSE(MakeDirectories("", 0777));
ASSERT_FALSE(MakeDirectories("/dev/null", 0777));
ASSERT_TRUE(MakeDirectories("c:/", 0777));
ASSERT_TRUE(MakeDirectories("c:\\", 0777));
ASSERT_TRUE(MakeDirectories("/", 0777));
ASSERT_TRUE(MakeDirectories("/foo", 0777));
ASSERT_TRUE(MakeDirectories(".", 0777));
ASSERT_TRUE(MakeDirectories(tmpdir, 0777));
ASSERT_TRUE(MakeDirectories(JoinPath(tmpdir, "dir1/dir2/dir3"), 0777));
Expand Down

0 comments on commit 9f3825f

Please sign in to comment.