[filesystem_utils] Fix a bug in MakeAbsolutePathRelativeIfPossible
Before this change, this function will return true when it is called
with "/some/dir-sufix/a" and "/some/dir" and rebase it into
"//-sufix/a". This change fixed this bug.
Change-Id: Ibb40213497e1c4ee5b874ad2eca88a7704caa2d9
Bug: crbug.com/gn/58
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/5680
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/tools/gn/filesystem_utils.cc b/tools/gn/filesystem_utils.cc
index cc121b3..c378233 100644
--- a/tools/gn/filesystem_utils.cc
+++ b/tools/gn/filesystem_utils.cc
@@ -198,6 +198,18 @@
}
}
+size_t AbsPathLenWithNoTrailingSlash(const base::StringPiece& path) {
+ size_t len = path.size();
+#if defined(OS_WIN)
+ size_t min_len = 3;
+#else
+ // On posix system. The minimal abs path is "/".
+ size_t min_len = 1;
+#endif
+ for (; len > min_len && IsSlash(path[len - 1]); len--)
+ ;
+ return len;
+}
} // namespace
std::string FilePathToUTF8(const base::FilePath::StringType& str) {
@@ -353,9 +365,13 @@
dest->clear();
- if (source_root.size() > path.size())
- return false; // The source root is longer: the path can never be inside.
+ // There is no specification of how many slashes may be at the end of
+ // source_root or path. Trim them off for easier string manipulation.
+ size_t path_len = AbsPathLenWithNoTrailingSlash(path);
+ size_t source_root_len = AbsPathLenWithNoTrailingSlash(source_root);
+ if (source_root_len > path_len)
+ return false; // The source root is longer: the path can never be inside.
#if defined(OS_WIN)
// Source root should be canonical on Windows. Note that the initial slash
// must be forward slash, but that the other ones can be either forward or
@@ -366,19 +382,29 @@
size_t after_common_index = std::string::npos;
if (DoesBeginWindowsDriveLetter(path)) {
// Handle "C:\foo"
- if (AreAbsoluteWindowsPathsEqual(source_root,
- path.substr(0, source_root.size())))
- after_common_index = source_root.size();
- else
+ if (AreAbsoluteWindowsPathsEqual(source_root.substr(0, source_root_len),
+ path.substr(0, source_root_len))) {
+ after_common_index = source_root_len;
+ if (path_len == source_root_len) {
+ *dest = "//";
+ return true;
+ }
+ } else {
return false;
- } else if (path[0] == '/' && source_root.size() <= path.size() - 1 &&
+ }
+ } else if (path[0] == '/' && source_root_len <= path_len - 1 &&
DoesBeginWindowsDriveLetter(path.substr(1))) {
// Handle "/C:/foo"
- if (AreAbsoluteWindowsPathsEqual(source_root,
- path.substr(1, source_root.size())))
- after_common_index = source_root.size() + 1;
- else
+ if (AreAbsoluteWindowsPathsEqual(source_root.substr(0, source_root_len),
+ path.substr(1, source_root_len))) {
+ after_common_index = source_root_len + 1;
+ if (path_len + 1 == source_root_len) {
+ *dest = "//";
+ return true;
+ }
+ } else {
return false;
+ }
} else {
return false;
}
@@ -386,12 +412,15 @@
// If we get here, there's a match and after_common_index identifies the
// part after it.
- // The base may or may not have a trailing slash, so skip all slashes from
- // the path after our prefix match.
- size_t first_after_slash = after_common_index;
- while (first_after_slash < path.size() && IsSlash(path[first_after_slash]))
+ if (!IsSlash(path[after_common_index])) {
+ // path is ${source-root}SUFIX/...
+ return false;
+ }
+ // A source-root relative path, The input may have an unknown number of
+ // slashes after the previous match. Skip over them.
+ size_t first_after_slash = after_common_index + 1;
+ while (first_after_slash < path_len && IsSlash(path[first_after_slash]))
first_after_slash++;
-
dest->assign("//"); // Result is source root relative.
dest->append(&path.data()[first_after_slash],
path.size() - first_after_slash);
@@ -401,11 +430,21 @@
// On non-Windows this is easy. Since we know both are absolute, just do a
// prefix check.
- if (path.substr(0, source_root.size()) == source_root) {
- // The base may or may not have a trailing slash, so skip all slashes from
- // the path after our prefix match.
- size_t first_after_slash = source_root.size();
- while (first_after_slash < path.size() && IsSlash(path[first_after_slash]))
+
+ if (path.substr(0, source_root_len) ==
+ source_root.substr(0, source_root_len)) {
+ if (path_len == source_root_len) {
+ // path is equivalent to source_root.
+ *dest = "//";
+ return true;
+ } else if (!IsSlash(path[source_root_len])) {
+ // path is ${source-root}SUFIX/...
+ return false;
+ }
+ // A source-root relative path, The input may have an unknown number of
+ // slashes after the previous match. Skip over them.
+ size_t first_after_slash = source_root_len + 1;
+ while (first_after_slash < path_len && IsSlash(path[first_after_slash]))
first_after_slash++;
dest->assign("//"); // Result is source root relative.
diff --git a/tools/gn/filesystem_utils_unittest.cc b/tools/gn/filesystem_utils_unittest.cc
index dcaa099..433ea36 100644
--- a/tools/gn/filesystem_utils_unittest.cc
+++ b/tools/gn/filesystem_utils_unittest.cc
@@ -848,3 +848,21 @@
EXPECT_EQ("obj/",
GetBuildDirAsOutputFile(context, BuildDirType::OBJ).value());
}
+
+TEST(FilesystemUtils, ResolveRelativeTest) {
+ std::string result;
+#ifndef OS_WIN
+ EXPECT_TRUE(
+ MakeAbsolutePathRelativeIfPossible("/some/dir", "/some/dir/a", &result));
+ EXPECT_EQ(result, "//a");
+
+ EXPECT_FALSE(MakeAbsolutePathRelativeIfPossible(
+ "/some/dir", "/some/dir-sufix/a", &result));
+#else
+ EXPECT_TRUE(MakeAbsolutePathRelativeIfPossible("C:/some/dir",
+ "/C:/some/dir/a", &result));
+ EXPECT_EQ(result, "//a");
+ EXPECT_FALSE(MakeAbsolutePathRelativeIfPossible(
+ "C:/some/dir", "C:/some/dir-sufix/a", &result));
+#endif
+}
diff --git a/tools/gn/function_rebase_path_unittest.cc b/tools/gn/function_rebase_path_unittest.cc
index 718ecb1..fdd09dc 100644
--- a/tools/gn/function_rebase_path_unittest.cc
+++ b/tools/gn/function_rebase_path_unittest.cc
@@ -121,6 +121,9 @@
RebaseOne(scope, "foo/", "C:/ssd/out/Debug", "//"));
#else
setup.build_settings()->SetBuildDir(SourceDir("/ssd/out/Debug"));
+ setup.build_settings()->SetRootPath(base::FilePath("/ssd/out/Debug"));
+ EXPECT_EQ("../Debug-suffix/a", RebaseOne(scope, "/ssd/out/Debug-suffix/a",
+ "/ssd/out/Debug", "/ssd/out/Debug"));
setup.build_settings()->SetRootPath(base::FilePath("/hdd/src"));
// Test system absolute to-dir.
