diff options
author | 2018-04-26 03:24:58 -0700 | |
---|---|---|
committer | 2018-04-26 03:26:11 -0700 | |
commit | 9f3825f97dffbbaa5d0a1f78c51cea87c2eb577c (patch) | |
tree | 5e11e20ade9524585d5445e388479f62ebf1a286 | |
parent | 1210ad94fef7abb9fb8caec26395d07e29579a1b (diff) |
windows,client: no longer support Unix-style paths
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 https://github.com/bazelbuild/bazel/issues/4319
Change-Id: I39d81843015c5a4014dd5953bac2e1c29dcd5bed
PiperOrigin-RevId: 194372504
-rw-r--r-- | src/main/cpp/util/file_windows.cc | 115 | ||||
-rw-r--r-- | src/test/cpp/blaze_util_windows_test.cc | 3 | ||||
-rw-r--r-- | src/test/cpp/util/file_windows_test.cc | 84 |
3 files changed, 23 insertions, 179 deletions
diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index e51161ca09..124ae7ed17 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -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 @@ -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(); @@ -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. @@ -512,8 +423,9 @@ 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. @@ -521,11 +433,8 @@ bool AsWindowsPath(const string& path, string* result, string* error) { // 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)) { diff --git a/src/test/cpp/blaze_util_windows_test.cc b/src/test/cpp/blaze_util_windows_test.cc index 5412019efa..2ac596957d 100644 --- a/src/test/cpp/blaze_util_windows_test.cc +++ b/src/test/cpp/blaze_util_windows_test.cc @@ -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")); diff --git a/src/test/cpp/util/file_windows_test.cc b/src/test/cpp/util/file_windows_test.cc index 43224043fc..87987b4d89 100644 --- a/src/test/cpp/util/file_windows_test.cc +++ b/src/test/cpp/util/file_windows_test.cc @@ -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 { @@ -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. @@ -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)); @@ -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)); @@ -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; @@ -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) { @@ -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) { @@ -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); @@ -410,7 +364,6 @@ 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. @@ -418,13 +371,6 @@ TEST_F(FileWindowsTest, TestIsDirectory) { 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); @@ -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)); |