Escape prerequisites filenames in build.ninja.d depfile
Currently gn can't handle BUILD.gn files that are placed in some folders
with space. This happens because files written into build.ninja.d aren't
escaped.
Unfortunately ninja escape rules are not applicable to depfile syntax.
Actually none of the existing escaping modes can handle depfile syntax.
So new mode was created for this specific case.
Bug: none
Change-Id: I6d95e018ecce8d7a341b4293a57e19cb8a93a1d5
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/5101
Commit-Queue: Brett Wilson <brettw@google.com>
Reviewed-by: Brett Wilson <brettw@google.com>
diff --git a/tools/gn/escape.cc b/tools/gn/escape.cc
index aa6b967..d5e33c0 100644
--- a/tools/gn/escape.cc
+++ b/tools/gn/escape.cc
@@ -75,6 +75,24 @@
return i;
}
+size_t EscapeStringToString_Depfile(const base::StringPiece& str,
+ const EscapeOptions& options,
+ char* dest,
+ bool* needed_quoting) {
+ size_t i = 0;
+ for (const auto& elem : str) {
+ // Escape all characters that ninja depfile parser can recognize as escaped,
+ // even if some of them can work without escaping.
+ if (elem == ' ' || elem == '\\' || elem == '#' || elem == '*' ||
+ elem == '[' || elem == '|' || elem == ']')
+ dest[i++] = '\\';
+ else if (elem == '$') // Extra rule for $$
+ dest[i++] = '$';
+ dest[i++] = elem;
+ }
+ return i;
+}
+
size_t EscapeStringToString_NinjaPreformatted(const base::StringPiece& str,
char* dest) {
// Only Ninja-escape $.
@@ -188,6 +206,8 @@
return str.size();
case ESCAPE_NINJA:
return EscapeStringToString_Ninja(str, options, dest, needed_quoting);
+ case ESCAPE_DEPFILE:
+ return EscapeStringToString_Depfile(str, options, dest, needed_quoting);
case ESCAPE_NINJA_COMMAND:
switch (options.platform) {
case ESCAPE_PLATFORM_CURRENT:
diff --git a/tools/gn/escape.h b/tools/gn/escape.h
index bed2646..888d1b3 100644
--- a/tools/gn/escape.h
+++ b/tools/gn/escape.h
@@ -16,6 +16,9 @@
// Ninja string escaping.
ESCAPE_NINJA,
+ // Ninja/makefile depfile string escaping.
+ ESCAPE_DEPFILE,
+
// For writing commands to ninja files. This assumes the output is "one
// thing" like a filename, so will escape or quote spaces as necessary for
// both Ninja and the shell to keep that thing together.
diff --git a/tools/gn/escape_unittest.cc b/tools/gn/escape_unittest.cc
index 0aa8adc..d7ff23f 100644
--- a/tools/gn/escape_unittest.cc
+++ b/tools/gn/escape_unittest.cc
@@ -12,6 +12,13 @@
EXPECT_EQ("asdf$:$ \"$$\\bar", result);
}
+TEST(Escape, Depfile) {
+ EscapeOptions opts;
+ opts.mode = ESCAPE_DEPFILE;
+ std::string result = EscapeString("asdf:$ \\#*[|]bar", opts, nullptr);
+ EXPECT_EQ("asdf:$$\\ \\\\\\#\\*\\[\\|\\]bar", result);
+}
+
TEST(Escape, WindowsCommand) {
EscapeOptions opts;
opts.mode = ESCAPE_NINJA_COMMAND;
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc
index c1df3bc..3127c9a 100644
--- a/tools/gn/ninja_build_writer.cc
+++ b/tools/gn/ninja_build_writer.cc
@@ -306,10 +306,16 @@
const base::FilePath build_path =
build_settings_->build_dir().Resolve(build_settings_->root_path());
+
+ EscapeOptions depfile_escape;
+ depfile_escape.mode = ESCAPE_DEPFILE;
for (const auto& other_file : fileset) {
const base::FilePath file =
MakeAbsoluteFilePathRelativeIfPossible(build_path, other_file);
- dep_out_ << " " << FilePathToUTF8(file.NormalizePathSeparatorsTo('/'));
+ dep_out_ << " ";
+ EscapeStringToStream(dep_out_,
+ FilePathToUTF8(file.NormalizePathSeparatorsTo('/')),
+ depfile_escape);
}
out_ << std::endl;
diff --git a/tools/gn/ninja_build_writer_unittest.cc b/tools/gn/ninja_build_writer_unittest.cc
index 2c73cd8..7b5e82a 100644
--- a/tools/gn/ninja_build_writer_unittest.cc
+++ b/tools/gn/ninja_build_writer_unittest.cc
@@ -184,6 +184,33 @@
EXPECT_EQ(std::string::npos, out_str.find("pool console"));
}
+TEST_F(NinjaBuildWriterTest, SpaceInDepfile) {
+ TestWithScope setup;
+ Err err;
+
+ // Setup sets the default root dir to ".".
+ base::FilePath root(FILE_PATH_LITERAL("."));
+ base::FilePath root_realpath = base::MakeAbsoluteFilePath(root);
+ setup.build_settings()->SetRootPath(root_realpath);
+
+ // Cannot use MakeAbsoluteFilePath for non-existed paths
+ base::FilePath dependency =
+ root_realpath.Append(FILE_PATH_LITERAL("path with space/BUILD.gn"));
+ g_scheduler->AddGenDependency(dependency);
+
+ std::unordered_map<const Settings*, const Toolchain*> used_toolchains;
+ used_toolchains[setup.settings()] = setup.toolchain();
+ std::vector<const Target*> targets;
+ std::ostringstream ninja_out;
+ std::ostringstream depfile_out;
+ NinjaBuildWriter writer(setup.build_settings(), used_toolchains, targets,
+ setup.toolchain(), targets, ninja_out, depfile_out);
+ ASSERT_TRUE(writer.Run(&err));
+
+ EXPECT_EQ(depfile_out.str(),
+ "build.ninja: ../../path\\ with\\ space/BUILD.gn");
+}
+
TEST_F(NinjaBuildWriterTest, DuplicateOutputs) {
TestWithScope setup;
Err err;
diff --git a/tools/gn/scheduler.h b/tools/gn/scheduler.h
index 160921e..0e2203c 100644
--- a/tools/gn/scheduler.h
+++ b/tools/gn/scheduler.h
@@ -52,6 +52,8 @@
// Declares that the given file was read and affected the build output.
//
+ // Some consumers expect provided path to be absolute.kk
+ //
// TODO(brettw) this is global rather than per-BuildSettings. If we
// start using >1 build settings, then we probably want this to take a
// BuildSettings object so we know the depdency on a per-build basis.
