Fix escaping of interpreter, script, & source dir
The wrong escaping mode is used in a few contexts, which results in
failed builds. This is happening in particular to folks whose sources
are inside a directory with a space in it, and whose builds are not
contained within that directory.
Such builds encode the surrounding filesystem structure into the ninja
files, which is unfortunate but unavoidable in some environments. Fix
the escaping mode in this case.
Bug: 325
Change-Id: Iab53514a3c5cbdb6e7a1d995469e2a75e44924dd
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/15080
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/ninja_action_target_writer.cc b/src/gn/ninja_action_target_writer.cc
index 60d4204..c33fc6e 100644
--- a/src/gn/ninja_action_target_writer.cc
+++ b/src/gn/ninja_action_target_writer.cc
@@ -141,10 +141,16 @@
out_ << std::endl;
}
+ // The command line requires shell escaping to properly handle filenames
+ // with spaces.
+ PathOutput command_output(path_output_.current_dir(),
+ settings_->build_settings()->root_path_utf8(),
+ ESCAPE_NINJA_COMMAND);
+
out_ << " command = ";
- path_output_.WriteFile(out_, settings_->build_settings()->python_path());
+ command_output.WriteFile(out_, settings_->build_settings()->python_path());
out_ << " ";
- path_output_.WriteFile(out_, target_->action_values().script());
+ command_output.WriteFile(out_, target_->action_values().script());
for (const auto& arg : args.list()) {
out_ << " ";
SubstitutionWriter::WriteWithNinjaVariables(arg, args_escape_options, out_);
diff --git a/src/gn/ninja_action_target_writer_unittest.cc b/src/gn/ninja_action_target_writer_unittest.cc
index d57f149..8fcddb6 100644
--- a/src/gn/ninja_action_target_writer_unittest.cc
+++ b/src/gn/ninja_action_target_writer_unittest.cc
@@ -547,3 +547,46 @@
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
}
}
+
+// Check for proper escaping of actions with spaces in python & script.
+TEST(NinjaActionTargetWriter, ActionWithSpaces) {
+ Err err;
+ TestWithScope setup;
+
+ Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+ target.set_output_type(Target::ACTION);
+
+ target.action_values().set_script(SourceFile("//foo/my script.py"));
+ target.action_values().args() = SubstitutionList::MakeForTest("my argument");
+ target.config_values().inputs().push_back(SourceFile("//foo/input file.txt"));
+
+ target.action_values().outputs() =
+ SubstitutionList::MakeForTest("//out/Debug/foo.out");
+
+ target.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target.OnResolved(&err));
+
+ setup.build_settings()->set_python_path(
+ base::FilePath(FILE_PATH_LITERAL("/Program Files/python")));
+
+ std::ostringstream out;
+ NinjaActionTargetWriter writer(&target, out);
+ writer.Run();
+
+ const char expected[] =
+ R"(rule __foo_bar___rule)" "\n"
+// Escaping is different between Windows and Posix.
+#if defined(OS_WIN)
+ R"( command = "/Program$ Files/python" "../../foo/my$ script.py" "my$ argument")" "\n"
+#else
+ R"( command = /Program\$ Files/python ../../foo/my\$ script.py my\$ argument)" "\n"
+#endif
+ R"( description = ACTION //foo:bar()
+ restat = 1
+
+build foo.out: __foo_bar___rule | ../../foo/my$ script.py ../../foo/input$ file.txt
+
+build obj/foo/bar.stamp: stamp foo.out
+)";
+ EXPECT_EQ(expected, out.str()) << expected << "--" << out.str();
+}
diff --git a/src/gn/path_output.cc b/src/gn/path_output.cc
index d2886d7..86c92e6 100644
--- a/src/gn/path_output.cc
+++ b/src/gn/path_output.cc
@@ -145,7 +145,7 @@
} else {
// Ninja (and none) escaping can avoid the intermediate string and
// reprocessing of the inverse_current_dir_.
- out << inverse_current_dir_;
+ EscapeStringToStream(out, inverse_current_dir_, options_);
EscapeStringToStream(out, str, options_);
}
}
