From 99847a1febb01be2fa35a4aa54dcf99873deb65c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Csomor?= Date: Wed, 22 Mar 2017 16:16:31 +0000 Subject: [PATCH] Bazel client, Windows: fix MSYS root computation Make it more robust: it now works with Cygwin too, e.g. BAZEL_SH=c:/cygwin64/bin/bash.exe See https://github.com/bazelbuild/bazel/issues/2725 Related to https://github.com/bazelbuild/bazel/issues/2447 -- Change-Id: I911f09acd3e39c7cd0fe0750774fa0a900ffd844 Reviewed-on: https://cr.bazel.build/9510 PiperOrigin-RevId: 150885982 MOS_MIGRATED_REVID=150885982 --- src/main/cpp/util/file_windows.cc | 26 ++++++++------- src/test/cpp/util/file_windows_test.cc | 44 ++++++++++++++++++-------- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index 66c0197d478701..c2e0fd79ee7651 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -407,19 +407,21 @@ bool MsysRoot::Get(string* path) { result = value2; } - ToLower(&result); - - // BAZEL_SH is usually "c:\tools\msys64\usr\bin\bash.exe", we need to return - // "c:\tools\msys64". Look for the rightmost msys-looking component. - while (!IsRootDirectory(result) && - Basename(result).find("msys") == string::npos) { - result = Dirname(result); - } - if (IsRootDirectory(result)) { - return false; + // 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 parent(SplitPath(result)); + pair 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; } - *path = result; - return true; + return false; } void MsysRoot::InitIfNecessary() { diff --git a/src/test/cpp/util/file_windows_test.cc b/src/test/cpp/util/file_windows_test.cc index 45e95384aed8dc..340a46acd46d6a 100644 --- a/src/test/cpp/util/file_windows_test.cc +++ b/src/test/cpp/util/file_windows_test.cc @@ -173,7 +173,7 @@ TEST_F(FileWindowsTest, TestIsRootDirectory) { } TEST_F(FileWindowsTest, TestAsWindowsPath) { - SetEnvironmentVariableA("BAZEL_SH", "c:\\msys\\some\\long\\path\\bash.exe"); + SetEnvironmentVariableA("BAZEL_SH", "c:\\some\\long/path\\bin\\bash.exe"); ResetMsysRootForTesting(); wstring actual; @@ -220,7 +220,7 @@ TEST_F(FileWindowsTest, TestAsWindowsPath) { ASSERT_EQ(wstring(L"d:\\progra~1\\micros~1"), actual); ASSERT_TRUE(AsWindowsPath("/foo", &actual)); - ASSERT_EQ(wstring(L"c:\\msys\\foo"), actual); + ASSERT_EQ(wstring(L"c:\\some\\long\\path\\foo"), actual); wstring wlongpath(L"\\dummy_long_path"); string longpath("dummy_long_path/"); @@ -287,15 +287,33 @@ TEST_F(FileWindowsTest, TestAsShortWindowsPath) { TEST_F(FileWindowsTest, TestMsysRootRetrieval) { wstring actual; - SetEnvironmentVariableA("BAZEL_SH", "c:/foo/msys/bar/qux.exe"); + // We just need "bin/" or "usr/bin/". + // Forward slashes are converted to backslashes. + SetEnvironmentVariableA("BAZEL_SH", "c:/foo\\bin/some_bash.exe"); ResetMsysRootForTesting(); ASSERT_TRUE(AsWindowsPath("/blah", &actual)); - ASSERT_EQ(wstring(L"c:\\foo\\msys\\blah"), actual); + ASSERT_EQ(wstring(L"c:\\foo\\blah"), actual); - SetEnvironmentVariableA("BAZEL_SH", "c:/foo/MSYS64/bar/qux.exe"); + SetEnvironmentVariableA("BAZEL_SH", "c:\\foo/MSYS64/usr\\bin/dummy.exe"); ResetMsysRootForTesting(); ASSERT_TRUE(AsWindowsPath("/blah", &actual)); - ASSERT_EQ(wstring(L"c:\\foo\\msys64\\blah"), actual); + ASSERT_EQ(wstring(L"c:\\foo\\MSYS64\\blah"), actual); + + // We just need "bin/" or "usr/bin/". + SetEnvironmentVariableA("BAZEL_SH", "c:/bin/kitty.exe"); + ResetMsysRootForTesting(); + ASSERT_TRUE(AsWindowsPath("/blah", &actual)); + 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(); + ASSERT_FALSE(AsWindowsPath("/blah", &actual)); + + // We need "bin/" or "usr/bin/", not "usr/". + SetEnvironmentVariableA("BAZEL_SH", "c:/msys/usr/bash.exe"); + ResetMsysRootForTesting(); + ASSERT_FALSE(AsWindowsPath("/blah", &actual)); SetEnvironmentVariableA("BAZEL_SH", "c:/qux.exe"); ResetMsysRootForTesting(); @@ -316,14 +334,14 @@ TEST_F(FileWindowsTest, TestPathExistsWindows) { GET_TEST_TMPDIR(tmpdir); ASSERT_TRUE(PathExists(tmpdir)); - // Create a fake msys root. We'll also use it as a junction target. - string fake_msys_root(tmpdir + "/fake_msys"); + // Create junction target that'll double as the fake msys root. + string fake_msys_root(tmpdir + "/blah"); ASSERT_EQ(0, mkdir(fake_msys_root.c_str())); ASSERT_TRUE(PathExists(fake_msys_root)); // Set the BAZEL_SH root so we can resolve MSYS paths. SetEnvironmentVariableA("BAZEL_SH", - (fake_msys_root + "/fake_bash.exe").c_str()); + (fake_msys_root + "/bin/fake_bash.exe").c_str()); ResetMsysRootForTesting(); // Assert existence check for MSYS paths. @@ -355,14 +373,14 @@ TEST_F(FileWindowsTest, TestIsDirectory) { ASSERT_FALSE(IsDirectory("non.existent")); // Create a directory under `tempdir`, verify that IsDirectory reports true. - // Call it msys_dir1 so we can also use it as a mock msys root. - string dir1(JoinPath(tmpdir, "msys_dir1")); + 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, "bash.exe").c_str()); + SetEnvironmentVariableA("BAZEL_SH", + JoinPath(dir1, "usr\\bin\\bash.exe").c_str()); ResetMsysRootForTesting(); ASSERT_TRUE(IsDirectory("/")); @@ -412,7 +430,7 @@ TEST_F(FileWindowsTest, TestMakeDirectories) { ASSERT_LT(0, tmpdir.size()); SetEnvironmentVariableA( - "BAZEL_SH", (JoinPath(tmpdir, "fake_msys/fake_bash.exe")).c_str()); + "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")));