From 4cc0d59eb0ddcc8f46f7c0e13c807ee75fb31852 Mon Sep 17 00:00:00 2001 From: ccalvarin Date: Tue, 19 Jun 2018 08:52:08 -0700 Subject: [PATCH] Add a utility function to accept windows-style envvars in paths. For paths passed to Bazel on the command line, the shell expands these variables, but for hardcoded defaults, we must make the library call ourselves. We do not add similar support in Posix systems, where it is less common to rely on standard path-related variables. Prerequisit for issue #4502. RELNOTES: None. PiperOrigin-RevId: 201183214 --- src/main/cpp/util/path_platform.h | 10 +++++++++ src/main/cpp/util/path_posix.cc | 5 +++++ src/main/cpp/util/path_windows.cc | 27 ++++++++++++++++++++++++ src/test/cpp/util/BUILD | 1 + src/test/cpp/util/path_posix_test.cc | 11 ++++++++++ src/test/cpp/util/path_windows_test.cc | 29 +++++++++++++++++++++++++- 6 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/main/cpp/util/path_platform.h b/src/main/cpp/util/path_platform.h index d16e41b6f10747..b0fb88de51c0ad 100644 --- a/src/main/cpp/util/path_platform.h +++ b/src/main/cpp/util/path_platform.h @@ -54,6 +54,16 @@ bool IsAbsolute(const std::string &path); // MakeAbsolute("C:/foo") ---> "C:/foo" std::string MakeAbsolute(const std::string &path); +// Returns the given path in absolute form, taking into account a possible +// starting environment variable on the windows platform, so that we can +// accept standard path variables like %USERPROFILE%. We do not support +// unix-style envvars here: recreating that logic is error-prone and not +// worthwhile, since they are less critical to standard paths as in Windows. +// +// MakeAbsolute("foo") in wd "/bar" --> "/bar/foo" +// MakeAbsolute("%USERPROFILE%/foo") --> "C:\Users\bazel-user\foo" +std::string MakeAbsoluteAndResolveWindowsEnvvars(const std::string &path); + // TODO(bazel-team) consider changing the path(_platform) header split to be a // path.h and path_windows.h split, which would make it clearer what functions // are included by an import statement. The downside to this gain in clarity diff --git a/src/main/cpp/util/path_posix.cc b/src/main/cpp/util/path_posix.cc index 267dc2293172f4..9c0b440d11306f 100644 --- a/src/main/cpp/util/path_posix.cc +++ b/src/main/cpp/util/path_posix.cc @@ -66,4 +66,9 @@ std::string MakeAbsolute(const std::string &path) { return JoinPath(blaze_util::GetCwd(), path); } + +std::string MakeAbsoluteAndResolveWindowsEnvvars(const std::string &path) { + return MakeAbsolute(path); +} + } // namespace blaze_util diff --git a/src/main/cpp/util/path_windows.cc b/src/main/cpp/util/path_windows.cc index 051783dcfb5541..c7afe4021d3578 100644 --- a/src/main/cpp/util/path_windows.cc +++ b/src/main/cpp/util/path_windows.cc @@ -14,6 +14,7 @@ #include "src/main/cpp/util/path_platform.h" +#include #include // wcslen #include @@ -87,6 +88,32 @@ std::string MakeAbsolute(const std::string& path) { WstringToCstring(RemoveUncPrefixMaybe(wpath.c_str())).get()); } +std::string MakeAbsoluteAndResolveWindowsEnvvars(const std::string& path) { + // Get the size of the expanded string, so we know how big of a buffer to + // provide. The returned size includes the null terminator. + std::unique_ptr resolved(new CHAR[MAX_PATH]); + DWORD size = + ::ExpandEnvironmentStrings(path.c_str(), resolved.get(), MAX_PATH); + if (size == 0) { + BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) + << "MakeAbsoluteAndResolveWindowsEnvvars(" << path + << "): ExpandEnvironmentStrings failed: " << GetLastErrorString(); + } else if (size > MAX_PATH) { + // Try again with a buffer bigger than MAX_PATH. + resolved.reset(new CHAR[size]); + DWORD second_size = + ::ExpandEnvironmentStrings(path.c_str(), resolved.get(), size); + if (second_size == 0) { + BAZEL_DIE(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR) + << "MakeAbsoluteAndResolveWindowsEnvvars(" << path + << "): ExpandEnvironmentStrings failed with second buffer: " + << GetLastErrorString(); + } + assert(second_size <= size); + } + return MakeAbsolute(std::string(resolved.get())); +} + bool CompareAbsolutePaths(const std::string& a, const std::string& b) { return ConvertPath(a) == ConvertPath(b); } diff --git a/src/test/cpp/util/BUILD b/src/test/cpp/util/BUILD index 455994f3d9be5c..1a86b0d0c69bd6 100644 --- a/src/test/cpp/util/BUILD +++ b/src/test/cpp/util/BUILD @@ -60,6 +60,7 @@ cc_test( ] + select({ "//src/conditions:windows": [ ":windows_test_util", + "//src/main/cpp:blaze_util", "//src/main/native/windows:lib-file", ], "//conditions:default": [], diff --git a/src/test/cpp/util/path_posix_test.cc b/src/test/cpp/util/path_posix_test.cc index dd22f9bacea70e..7824fd17b48638 100644 --- a/src/test/cpp/util/path_posix_test.cc +++ b/src/test/cpp/util/path_posix_test.cc @@ -161,4 +161,15 @@ TEST(PathPosixTest, MakeAbsolute) { EXPECT_EQ(MakeAbsolute(""), ""); } +TEST(PathPosixTest, MakeAbsoluteAndResolveWindowsEnvvars) { + // Check that Unix-style envvars are not resolved. + EXPECT_EQ(MakeAbsoluteAndResolveWindowsEnvvars("$PATH"), + JoinPath(GetCwd(), "$PATH")); + EXPECT_EQ(MakeAbsoluteAndResolveWindowsEnvvars("${PATH}"), + JoinPath(GetCwd(), "${PATH}")); + // Check that Windows-style envvars are not resolved when not on Windows. + EXPECT_EQ(MakeAbsoluteAndResolveWindowsEnvvars("%PATH%"), + JoinPath(GetCwd(), "%PATH%")); +} + } // namespace blaze_util diff --git a/src/test/cpp/util/path_windows_test.cc b/src/test/cpp/util/path_windows_test.cc index daeba056de7ab9..965d885c081416 100644 --- a/src/test/cpp/util/path_windows_test.cc +++ b/src/test/cpp/util/path_windows_test.cc @@ -20,6 +20,7 @@ #include #include "gtest/gtest.h" +#include "src/main/cpp/blaze_util_platform.h" #include "src/main/cpp/util/file_platform.h" #include "src/main/cpp/util/path.h" #include "src/main/cpp/util/path_platform.h" @@ -318,7 +319,7 @@ TEST(PathWindowsTest, ConvertPathTest) { EXPECT_EQ("c:\\foo\\bar", ConvertPath("c:/../foo\\BAR\\.\\")); } -TEST(PathWindowsTest, TestMakeAbsolute) { +TEST(PathWindowsTest, MakeAbsolute) { EXPECT_EQ("c:\\foo\\bar", MakeAbsolute("C:\\foo\\BAR")); EXPECT_EQ("c:\\foo\\bar", MakeAbsolute("C:/foo/bar")); EXPECT_EQ("c:\\foo\\bar", MakeAbsolute("C:\\foo\\bar\\")); @@ -334,4 +335,30 @@ TEST(PathWindowsTest, TestMakeAbsolute) { EXPECT_EQ("", MakeAbsolute("")); } +TEST(PathWindowsTest, MakeAbsoluteAndResolveWindowsEnvvars_WithTmpdir) { + // We cannot test the system-default paths like %ProgramData% because these + // are wiped from the test environment. TestTmpdir is set by Bazel though, + // so serves as a fine substitute. + char buf[MAX_PATH] = {0}; + DWORD len = ::GetEnvironmentVariableA("TEST_TMPDIR", buf, MAX_PATH); + const std::string tmpdir = buf; + const std::string expected_tmpdir_bar = ConvertPath(tmpdir + "\\bar"); + + EXPECT_EQ(expected_tmpdir_bar, + MakeAbsoluteAndResolveWindowsEnvvars("%TEST_TMPDIR%\\bar")); + EXPECT_EQ(expected_tmpdir_bar, + MakeAbsoluteAndResolveWindowsEnvvars("%Test_Tmpdir%\\bar")); + EXPECT_EQ(expected_tmpdir_bar, + MakeAbsoluteAndResolveWindowsEnvvars("%test_tmpdir%\\bar")); + EXPECT_EQ(expected_tmpdir_bar, + MakeAbsoluteAndResolveWindowsEnvvars("%test_tmpdir%/bar")); +} + +TEST(PathWindowsTest, MakeAbsoluteAndResolveWindowsEnvvars_LongPaths) { + const std::string long_path = "c:\\" + std::string(MAX_PATH, 'a'); + blaze::SetEnv("long", long_path); + + EXPECT_EQ(long_path, MakeAbsoluteAndResolveWindowsEnvvars("%long%")); +} + } // namespace blaze_util