From a4d0ea406e8622e305fc3253075cfee60da3d3d2 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Mon, 19 Dec 2016 14:30:52 +0000 Subject: Bazel client: SplitPath works with Windows paths This allows correct behavior of Dirname and Basename on Windows. See https://github.com/bazelbuild/bazel/issues/2107 -- PiperOrigin-RevId: 142441234 MOS_MIGRATED_REVID=142441234 --- src/main/cpp/util/file.cc | 12 ------ src/main/cpp/util/file_platform.h | 3 ++ src/main/cpp/util/file_posix.cc | 12 ++++++ src/main/cpp/util/file_windows.cc | 51 +++++++++++++++++++++++++ src/test/cpp/util/BUILD | 5 +++ src/test/cpp/util/file_posix_test.cc | 47 +++++++++++++++++++++++ src/test/cpp/util/file_windows_test.cc | 68 ++++++++++++++++++++++++++++++++++ 7 files changed, 186 insertions(+), 12 deletions(-) create mode 100644 src/test/cpp/util/file_windows_test.cc diff --git a/src/main/cpp/util/file.cc b/src/main/cpp/util/file.cc index 2f6e6c66f2..392af372af 100644 --- a/src/main/cpp/util/file.cc +++ b/src/main/cpp/util/file.cc @@ -69,18 +69,6 @@ bool WriteFile(const std::string &content, const std::string &filename) { return WriteFile(content.c_str(), content.size(), filename); } -pair SplitPath(const string &path) { - size_t pos = path.rfind('/'); - - // Handle the case with no '/' in 'path'. - if (pos == string::npos) return std::make_pair("", path); - - // Handle the case with a single leading '/' in 'path'. - if (pos == 0) return std::make_pair(string(path, 0, 1), string(path, 1)); - - return std::make_pair(string(path, 0, pos), string(path, pos + 1)); -} - string Dirname(const string &path) { return SplitPath(path).first; } diff --git a/src/main/cpp/util/file_platform.h b/src/main/cpp/util/file_platform.h index 1db579a494..78322dfeb8 100644 --- a/src/main/cpp/util/file_platform.h +++ b/src/main/cpp/util/file_platform.h @@ -26,6 +26,9 @@ class IPipe; IPipe* CreatePipe(); +// Split a path to dirname and basename parts. +std::pair SplitPath(const std::string &path); + // Replaces 'content' with contents of file 'filename'. // If `max_size` is positive, the method reads at most that many bytes; // otherwise the method reads the whole file. diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc index e197a09453..9e835d759e 100644 --- a/src/main/cpp/util/file_posix.cc +++ b/src/main/cpp/util/file_posix.cc @@ -169,6 +169,18 @@ IPipe* CreatePipe() { } #endif // __CYGWIN__ +pair SplitPath(const string &path) { + size_t pos = path.rfind('/'); + + // Handle the case with no '/' in 'path'. + if (pos == string::npos) return std::make_pair("", path); + + // Handle the case with a single leading '/' in 'path'. + if (pos == 0) return std::make_pair(string(path, 0, 1), string(path, 1)); + + return std::make_pair(string(path, 0, pos), string(path, pos + 1)); +} + bool ReadFile(const string &filename, string *content, int max_size) { int fd = open(filename.c_str(), O_RDONLY); if (fd == -1) return false; diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index 29153d1812..1d00269002 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -21,6 +21,7 @@ namespace blaze_util { +using std::pair; using std::string; class WindowsPipe : public IPipe { @@ -71,6 +72,56 @@ IPipe* CreatePipe() { return new WindowsPipe(read_handle, write_handle); } +static bool IsRootDirectory(const string& path) { + // Return true if path is "/", "\", "c:/", "c:\", "\\?\c:\", or "\??\c:\". + // + // It is unclear whether the UNC prefix is just "\\?\" or is "\??\" also + // valid (in some cases it seems to be, though MSDN doesn't mention it). + return + // path is "/" or "\" + (path.size() == 1 && (path[0] == '/' || path[0] == '\\')) || + // path is "c:/" or "c:\" + (path.size() == 3 && isalpha(path[0]) && path[1] == ':' && + (path[2] == '/' || path[2] == '\\')) || + // path is "\\?\c:\" or "\??\c:\" + (path.size() == 7 && path[0] == '\\' && + (path[1] == '\\' || path[1] == '?') && path[2] == '?' && + path[3] == '\\' && isalpha(path[4]) && path[5] == ':' && + path[6] == '\\'); +} + +pair SplitPath(const string& path) { + if (path.empty()) { + return std::make_pair("", ""); + } + + size_t pos = path.size() - 1; + for (auto it = path.crbegin(); it != path.crend(); ++it, --pos) { + if (*it == '/' || *it == '\\') { + if ((pos == 2 || pos == 6) && IsRootDirectory(path.substr(0, pos + 1))) { + // Windows path, top-level directory, e.g. "c:\foo", + // result is ("c:\", "foo"). + // Or UNC path, top-level directory, e.g. "\\?\c:\foo" + // result is ("\\?\c:\", "foo"). + return std::make_pair( + // Include the "/" or "\" in the drive specifier. + path.substr(0, pos + 1), path.substr(pos + 1)); + } else { + // Unix path, or relative path. + return std::make_pair( + // If the only "/" is the leading one, then that shall be the first + // pair element, otherwise the substring up to the rightmost "/". + pos == 0 ? path.substr(0, 1) : path.substr(0, pos), + // If the rightmost "/" is the tail, then the second pair element + // should be empty. + pos == path.size() - 1 ? "" : path.substr(pos + 1)); + } + } + } + // Handle the case with no '/' or '\' in `path`. + return std::make_pair("", path); +} + #ifdef COMPILER_MSVC bool ReadFile(const string& filename, string* content, int max_size) { // TODO(bazel-team): implement this. diff --git a/src/test/cpp/util/BUILD b/src/test/cpp/util/BUILD index e16f14f4ce..80e69b7267 100644 --- a/src/test/cpp/util/BUILD +++ b/src/test/cpp/util/BUILD @@ -21,7 +21,12 @@ cc_test( cc_test( name = "file_test", srcs = ["file_test.cc"] + select({ + "//src:windows": [ + "file_posix_test.cc", + "file_windows_test.cc", + ], "//src:windows_msvc": [ + "file_windows_test.cc", ], "//conditions:default": [ "file_posix_test.cc", diff --git a/src/test/cpp/util/file_posix_test.cc b/src/test/cpp/util/file_posix_test.cc index bd91343672..0acd392bf6 100644 --- a/src/test/cpp/util/file_posix_test.cc +++ b/src/test/cpp/util/file_posix_test.cc @@ -44,6 +44,53 @@ static bool CreateEmptyFile(const string& path) { return close(fd) == 0; } +TEST(FileTest, TestDirname) { + // The Posix version of SplitPath (thus Dirname too, which is implemented on + // top of it) is not aware of Windows paths. + ASSERT_EQ("", Dirname("")); + ASSERT_EQ("/", Dirname("/")); + ASSERT_EQ("", Dirname("foo")); + ASSERT_EQ("/", Dirname("/foo")); + ASSERT_EQ("/foo", Dirname("/foo/")); + ASSERT_EQ("foo", Dirname("foo/bar")); + ASSERT_EQ("foo/bar", Dirname("foo/bar/baz")); + ASSERT_EQ("", Dirname("\\foo")); + ASSERT_EQ("", Dirname("\\foo\\")); + ASSERT_EQ("", Dirname("foo\\bar")); + ASSERT_EQ("", Dirname("foo\\bar\\baz")); + ASSERT_EQ("foo\\bar", Dirname("foo\\bar/baz\\qux")); + ASSERT_EQ("c:", Dirname("c:/")); + ASSERT_EQ("", Dirname("c:\\")); + ASSERT_EQ("c:", Dirname("c:/foo")); + ASSERT_EQ("", Dirname("c:\\foo")); + ASSERT_EQ("", Dirname("\\\\?\\c:\\")); + ASSERT_EQ("", Dirname("\\\\?\\c:\\foo")); +} + +TEST(FileTest, TestBasename) { + // The Posix version of SplitPath (thus Basename too, which is implemented on + // top of it) is not aware of Windows paths. + ASSERT_EQ("", Basename("")); + ASSERT_EQ("", Basename("/")); + ASSERT_EQ("foo", Basename("foo")); + ASSERT_EQ("foo", Basename("/foo")); + ASSERT_EQ("", Basename("/foo/")); + ASSERT_EQ("bar", Basename("foo/bar")); + ASSERT_EQ("baz", Basename("foo/bar/baz")); + ASSERT_EQ("\\foo", Basename("\\foo")); + ASSERT_EQ("\\foo\\", Basename("\\foo\\")); + ASSERT_EQ("foo\\bar", Basename("foo\\bar")); + ASSERT_EQ("foo\\bar\\baz", Basename("foo\\bar\\baz")); + ASSERT_EQ("baz\\qux", Basename("foo\\bar/baz\\qux")); + ASSERT_EQ("qux", Basename("qux")); + ASSERT_EQ("", Basename("c:/")); + ASSERT_EQ("c:\\", Basename("c:\\")); + ASSERT_EQ("foo", Basename("c:/foo")); + ASSERT_EQ("c:\\foo", Basename("c:\\foo")); + ASSERT_EQ("\\\\?\\c:\\", Basename("\\\\?\\c:\\")); + ASSERT_EQ("\\\\?\\c:\\foo", Basename("\\\\?\\c:\\foo")); +} + TEST(FileTest, JoinPath) { std::string path = JoinPath("", ""); ASSERT_EQ("", path); diff --git a/src/test/cpp/util/file_windows_test.cc b/src/test/cpp/util/file_windows_test.cc new file mode 100644 index 0000000000..85cc7a491e --- /dev/null +++ b/src/test/cpp/util/file_windows_test.cc @@ -0,0 +1,68 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include + +#include "src/main/cpp/util/file.h" +#include "src/main/cpp/util/file_platform.h" +#include "gtest/gtest.h" + +#if !defined(COMPILER_MSVC) && !defined(__CYGWIN__) +#error("This test should only be run on Windows") +#endif // !defined(COMPILER_MSVC) && !defined(__CYGWIN__) + +namespace blaze_util { + +TEST(FileTest, TestDirname) { + ASSERT_EQ("", Dirname("")); + ASSERT_EQ("/", Dirname("/")); + ASSERT_EQ("", Dirname("foo")); + ASSERT_EQ("/", Dirname("/foo")); + ASSERT_EQ("/foo", Dirname("/foo/")); + ASSERT_EQ("foo", Dirname("foo/bar")); + ASSERT_EQ("foo/bar", Dirname("foo/bar/baz")); + ASSERT_EQ("\\", Dirname("\\foo")); + ASSERT_EQ("\\foo", Dirname("\\foo\\")); + ASSERT_EQ("foo", Dirname("foo\\bar")); + ASSERT_EQ("foo\\bar", Dirname("foo\\bar\\baz")); + ASSERT_EQ("foo\\bar/baz", Dirname("foo\\bar/baz\\qux")); + ASSERT_EQ("c:/", Dirname("c:/")); + ASSERT_EQ("c:\\", Dirname("c:\\")); + ASSERT_EQ("c:/", Dirname("c:/foo")); + ASSERT_EQ("c:\\", Dirname("c:\\foo")); + ASSERT_EQ("\\\\?\\c:\\", Dirname("\\\\?\\c:\\")); + ASSERT_EQ("\\\\?\\c:\\", Dirname("\\\\?\\c:\\foo")); +} + +TEST(FileTest, TestBasename) { + ASSERT_EQ("", Basename("")); + ASSERT_EQ("", Basename("/")); + ASSERT_EQ("foo", Basename("foo")); + ASSERT_EQ("foo", Basename("/foo")); + ASSERT_EQ("", Basename("/foo/")); + ASSERT_EQ("bar", Basename("foo/bar")); + ASSERT_EQ("baz", Basename("foo/bar/baz")); + ASSERT_EQ("foo", Basename("\\foo")); + ASSERT_EQ("", Basename("\\foo\\")); + ASSERT_EQ("bar", Basename("foo\\bar")); + ASSERT_EQ("baz", Basename("foo\\bar\\baz")); + ASSERT_EQ("qux", Basename("foo\\bar/baz\\qux")); + ASSERT_EQ("", Basename("c:/")); + ASSERT_EQ("", Basename("c:\\")); + ASSERT_EQ("foo", Basename("c:/foo")); + ASSERT_EQ("foo", Basename("c:\\foo")); + ASSERT_EQ("", Basename("\\\\?\\c:\\")); + ASSERT_EQ("foo", Basename("\\\\?\\c:\\foo")); +} + +} // namespace blaze_util -- cgit v1.2.3