[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.