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>

src/gn/filesystem_utils.cc

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

src/gn/filesystem_utils_unittest.cc

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
 }