fix: Fix Windows MakeRelativePath.
On Windows, it is invalid to make a relative path span across different drives. This fix:
1. Instead of prepending slash, we remove slash for both paths if there's one.
2. Return input's absolute path if the drive letter is different between input and dest.
Fixes: https://bugs.chromium.org/p/gn/issues/detail?id=317
Change-Id: I3ee2a46d412f663b0739f95fe3683d892b8fe6b5
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/16620
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Reviewed-by: David Turner <digit@google.com>
Commit-Queue: David Turner <digit@google.com>
diff --git a/src/gn/filesystem_utils.cc b/src/gn/filesystem_utils.cc
index d37750b..d747289 100644
--- a/src/gn/filesystem_utils.cc
+++ b/src/gn/filesystem_utils.cc
@@ -17,6 +17,7 @@
#include "util/build_config.h"
#if defined(OS_WIN)
+#include <direct.h>
#include <windows.h>
#endif
@@ -639,33 +640,75 @@
#endif
}
-std::string MakeRelativePath(const std::string& input,
- const std::string& dest) {
#if defined(OS_WIN)
- // Make sure that absolute |input| path starts with a slash if |dest| path
- // does. Otherwise skipping common prefixes won't work properly. Ensure the
- // same for |dest| path too.
- if (IsPathAbsolute(input) && !IsSlash(input[0]) && IsSlash(dest[0])) {
- std::string corrected_input(1, dest[0]);
- corrected_input.append(input);
- return MakeRelativePath(corrected_input, dest);
- }
- if (IsPathAbsolute(dest) && !IsSlash(dest[0]) && IsSlash(input[0])) {
- std::string corrected_dest(1, input[0]);
- corrected_dest.append(dest);
- return MakeRelativePath(input, corrected_dest);
+std::string GetPathWithDriveLetter(std::string_view path) {
+ if (!IsPathAbsolute(path) || !IsSlash(path[0]))
+ return std::string(path);
+
+ int drive = _getdrive();
+ DCHECK(drive > 0 && drive <= 26);
+
+ std::string ret;
+ ret.reserve(2 + path.size());
+ ret.push_back('A' + drive - 1);
+ ret.push_back(':');
+ ret += path;
+ return ret;
+}
+
+// Regulate path if it is an absolute path.
+std::string RegulatePathIfAbsolute(std::string_view path) {
+ CHECK(!path.empty());
+ bool is_start_slash = IsSlash(path[0]);
+
+ // 1. /C:/ -> C:/
+ if (path.size() > 3 && is_start_slash &&
+ base::IsAsciiAlpha(path[1]) && path[2] == ':') {
+ return RegulatePathIfAbsolute(path.substr(1));
}
- // Make sure that both absolute paths use the same drive letter case.
+ bool is_path_absolute = IsPathAbsolute(path);
+
+ // 2. /Path -> ($PWD's Drive):/Path
+ if (is_path_absolute && is_start_slash) {
+ return GetPathWithDriveLetter(path);
+ }
+
+ // 3. c:/ -> C:/
+ std::string ret(path);
+ if (is_path_absolute && !is_start_slash) {
+ ret[0] = base::ToUpperASCII(path[0]);
+ }
+
+ return ret;
+}
+#endif
+
+std::string MakeRelativePath(std::string_view input,
+ std::string_view dest) {
+#if defined(OS_WIN)
+ // Regulate the paths.
+ std::string input_regulated = RegulatePathIfAbsolute(input);
+ std::string dest_regulated = RegulatePathIfAbsolute(dest);
+
+ input = input_regulated;
+ dest = dest_regulated;
+
+ // On Windows, it is invalid to make a relative path across different
+ // drive letters. A relative path cannot span over different drives.
+ // For example:
+ // Input : D:/Path/Any/Where
+ // Dest : C:/Path/In/Another/Drive
+ // Invalid Result : ../../../../../D:/Path/Any/Where
+ // Correct Result : D:/Path/Any/Where
+ // It will at least make ninja fail.
+ // See: https://bugs.chromium.org/p/gn/issues/detail?id=317
if (IsPathAbsolute(input) && IsPathAbsolute(dest) && input.size() > 1 &&
dest.size() > 1) {
- int letter_pos = base::IsAsciiAlpha(input[0]) ? 0 : 1;
- if (input[letter_pos] != dest[letter_pos] &&
- base::ToUpperASCII(input[letter_pos]) ==
- base::ToUpperASCII(dest[letter_pos])) {
- std::string corrected_input = input;
- corrected_input[letter_pos] = dest[letter_pos];
- return MakeRelativePath(corrected_input, dest);
+ if (input[0] != dest[0]) {
+ // If the drive letters are differnet, we have no choice but use
+ // the absolute path of input for correctness.
+ return input_regulated;
}
}
#endif
diff --git a/src/gn/filesystem_utils_unittest.cc b/src/gn/filesystem_utils_unittest.cc
index ef25324..6f5baa3 100644
--- a/src/gn/filesystem_utils_unittest.cc
+++ b/src/gn/filesystem_utils_unittest.cc
@@ -543,6 +543,24 @@
EXPECT_EQ("../../../../path/to/foo",
RebasePath("/c:/path/to/foo", SourceDir("//a/b"),
std::string_view("C:/source/root")));
+
+ // Should be absolute path of destination if the drive letters are
+ // different between two paths.
+ EXPECT_EQ("C:/path/to/foo",
+ RebasePath("C:/path/to/foo", SourceDir("//a/b"),
+ std::string_view("D:/source/root")));
+ EXPECT_EQ("D:/path/to/foo",
+ RebasePath("D:/path/to/foo", SourceDir("//a/b"),
+ std::string_view("C:/source/root")));
+ EXPECT_EQ("E:/path/to/foo",
+ RebasePath("/E:/path/to/foo", SourceDir("//a/b"),
+ std::string_view("/c:/source/root")));
+ EXPECT_EQ("E:/path/to/foo",
+ RebasePath("/e:/path/to/foo", SourceDir("//a/b"),
+ std::string_view("c:/source/root")));
+ EXPECT_EQ("C:/path/to/foo",
+ RebasePath("/c:/path/to/foo", SourceDir("//a/b"),
+ std::string_view("D:/source/root")));
#endif
}
