Fix how the regeneration command writes gn's own paths

GetSelfInvocationCommandLine() escapes the switches it copies from the
current command line, but not the gn executable, --root or --dotfile,
which it writes itself. A space or "$" in any of those paths broke
regeneration, since Ninja expanded "$" and the shell split on spaces.
Escape them like every other switch.

Also only write the gn executable and --dotfile relative to the build
directory if they're inside the source root or the build directory,
since those are the places where relative paths keep working even if
they're moved.

For that, FilePath::IsParent() has to match roots written with different
separators on Windows: the source root always uses '/', but the paths
compared with it may not, and "C:/src" wasn't considered a parent of
"C:\src\list.txt". Port the root comparison that
https://crrev.com/c/8674581 added to AppendRelativePath(), which treats
roots made of the same number of separators as equal.

This is a precursor to https://gn-review.googlesource.com/c/gn/+/18220,
which handles --runtime-deps-list-file the same way.

Change-Id: I107aabe2d8a9f1b67c62792bb3ff2ae92973d5a8
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/28400
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@google.com>
Commit-Queue: Andrew Grieve <agrieve@google.com>

src/base/files/file_path.cc

diff --git a/src/base/files/file_path.cc b/src/base/files/file_path.cc
index be045b0..a3029ba 100644
--- a/src/base/files/file_path.cc
+++ b/src/base/files/file_path.cc
@@ -278,6 +278,18 @@
   }
 #endif  // defined(FILE_PATH_USES_DRIVE_LETTERS)
 
+  // The root is the only component made of separators, and it comes right
+  // after the drive letter, if any. Roots made of the same number of separators
+  // are equal, even if written with different separators (e.g. "/" and "\").
+  if (parent_comp != parent_components.end() &&
+      AreAllSeparators(*parent_comp)) {
+    if (parent_comp->size() != child_comp->size() ||
+        !AreAllSeparators(*child_comp))
+      return false;
+    ++parent_comp;
+    ++child_comp;
+  }
+
   while (parent_comp != parent_components.end()) {
     if (*parent_comp != *child_comp)
       return false;

src/gn/ninja_build_writer.cc

diff --git a/src/gn/ninja_build_writer.cc b/src/gn/ninja_build_writer.cc
index ecc32a4..375cfc8 100644
--- a/src/gn/ninja_build_writer.cc
+++ b/src/gn/ninja_build_writer.cc
@@ -55,27 +55,6 @@
   const base::FilePath build_path =
       build_settings->build_dir().Resolve(build_settings->root_path());
 
-  base::FilePath exe_path = GetExePath();
-  if (build_path.IsAbsolute())
-    exe_path = MakeAbsoluteFilePathRelativeIfPossible(build_path, exe_path);
-
-  base::CommandLine cmdline(exe_path.NormalizePathSeparatorsTo('/'));
-
-  // Use "." for the directory to generate. When Ninja runs the command it
-  // will have the build directory as the current one. Coding it explicitly
-  // will cause everything to get confused if the user renames the directory.
-  cmdline.AppendArg("gen");
-  cmdline.AppendArg(".");
-
-  base::FilePath root_path = build_settings->root_path();
-  if (build_path.IsAbsolute())
-    root_path = MakeAbsoluteFilePathRelativeIfPossible(build_path, root_path);
-
-  cmdline.AppendSwitchPath(std::string("--") + switches::kRoot,
-                           root_path.NormalizePathSeparatorsTo('/'));
-  // Successful automatic invocations shouldn't print output.
-  cmdline.AppendSwitch(std::string("-") + switches::kQuiet);
-
   EscapeOptions escape_shell;
   escape_shell.mode = ESCAPE_NINJA_COMMAND;
 #if defined(OS_WIN)
@@ -85,16 +64,37 @@
   escape_shell.inhibit_quoting = true;
 #endif
 
-  // If both --root and --dotfile are passed, make sure the --dotfile is
-  // made relative to the build dir here.
-  base::FilePath dotfile_path = build_settings->dotfile_name();
-  if (!dotfile_path.empty()) {
-    if (build_path.IsAbsolute()) {
-      dotfile_path =
-          MakeAbsoluteFilePathRelativeIfPossible(build_path, dotfile_path);
+  // The source root, and paths inside it or the build dir, are written relative
+  // to the build dir, so that build.ninja doesn't depend on where they live.
+  // Paths use '/', and are escaped since the command is written to build.ninja.
+  auto path_for_command = [build_settings, &build_path,
+                           &escape_shell](const base::FilePath& path) {
+    const base::FilePath& root = build_settings->root_path();
+    base::FilePath written = path;
+    if (build_path.IsAbsolute() &&
+        (path == root || root.IsParent(path) || build_path.IsParent(path))) {
+      written = MakeAbsoluteFilePathRelativeIfPossible(build_path, path);
     }
-    cmdline.AppendSwitchPath(std::string("--") + switches::kDotfile,
-                             dotfile_path.NormalizePathSeparatorsTo('/'));
+    return EscapeString(FilePathToUTF8(written.NormalizePathSeparatorsTo('/')),
+                        escape_shell, nullptr);
+  };
+
+  base::CommandLine cmdline(UTF8ToFilePath(path_for_command(GetExePath())));
+
+  // Use "." for the directory to generate. When Ninja runs the command it
+  // will have the build directory as the current one. Coding it explicitly
+  // will cause everything to get confused if the user renames the directory.
+  cmdline.AppendArg("gen");
+  cmdline.AppendArg(".");
+
+  cmdline.AppendSwitch(switches::kRoot,
+                       path_for_command(build_settings->root_path()));
+  // Successful automatic invocations shouldn't print output.
+  cmdline.AppendSwitch(std::string("-") + switches::kQuiet);
+
+  if (!build_settings->dotfile_name().empty()) {
+    cmdline.AppendSwitch(switches::kDotfile,
+                         path_for_command(build_settings->dotfile_name()));
   }
 
   const base::CommandLine& our_cmdline =

src/gn/ninja_build_writer_unittest.cc

diff --git a/src/gn/ninja_build_writer_unittest.cc b/src/gn/ninja_build_writer_unittest.cc
index 423aa90..c7f1885 100644
--- a/src/gn/ninja_build_writer_unittest.cc
+++ b/src/gn/ninja_build_writer_unittest.cc
@@ -9,6 +9,7 @@
 
 #include "base/command_line.h"
 #include "base/files/file_util.h"
+#include "gn/filesystem_utils.h"
 #include "gn/pool.h"
 #include "gn/scheduler.h"
 #include "gn/string_output_buffer.h"
@@ -16,6 +17,7 @@
 #include "gn/target.h"
 #include "gn/test_with_scheduler.h"
 #include "gn/test_with_scope.h"
+#include "util/build_config.h"
 #include "util/test/test.h"
 
 using NinjaBuildWriterTest = TestWithScheduler;
@@ -67,6 +69,36 @@
   EXPECT_EQ("../..", cmd_out.GetSwitchValueString(switches::kRoot));
   EXPECT_EQ("../../testdot.gn",
             cmd_out.GetSwitchValueString(switches::kDotfile));
+
+  // When outside of the source root, it's kept absolute.
+  setup.build_settings()->set_dotfile_name(
+      base::FilePath(FILE_PATH_LITERAL("/outside/.gn")));
+  cmd_out = GetSelfInvocationCommandLine(setup.build_settings());
+  EXPECT_EQ("/outside/.gn", cmd_out.GetSwitchValueString(switches::kDotfile));
+
+  // Paths are escaped for Ninja and the shell.
+  setup.build_settings()->set_dotfile_name(
+      root_realpath.Append(FILE_PATH_LITERAL("my dot$.gn")));
+  cmd_out = GetSelfInvocationCommandLine(setup.build_settings());
+  // Escaping is different between Windows and Posix.
+#if defined(OS_WIN)
+  EXPECT_EQ("../../my$ dot$$.gn",
+            cmd_out.GetSwitchValueString(switches::kDotfile));
+#else
+  EXPECT_EQ("../../my\\$ dot\\$$.gn",
+            cmd_out.GetSwitchValueString(switches::kDotfile));
+#endif
+
+  // Paths inside the build dir are written relative to it even when it's
+  // outside the source root.
+  base::FilePath outside_build_dir =
+      root_realpath.DirName().Append(FILE_PATH_LITERAL("out"));
+  setup.build_settings()->SetBuildDir(
+      SourceDirForPath(root_realpath, outside_build_dir));
+  setup.build_settings()->set_dotfile_name(
+      outside_build_dir.Append(FILE_PATH_LITERAL("dot.gn")));
+  cmd_out = GetSelfInvocationCommandLine(setup.build_settings());
+  EXPECT_EQ("./dot.gn", cmd_out.GetSwitchValueString(switches::kDotfile));
 }
 
 TEST_F(NinjaBuildWriterTest, TwoTargets) {