summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--base/containers/stack_container.h7
-rw-r--r--third_party/mesa/BUILD.gn2
-rw-r--r--tools/gn/escape.cc191
-rw-r--r--tools/gn/escape.h27
-rw-r--r--tools/gn/escape_unittest.cc84
-rw-r--r--tools/gn/file_template.cc6
-rw-r--r--tools/gn/file_template_unittest.cc10
-rw-r--r--tools/gn/ninja_action_target_writer.cc5
-rw-r--r--tools/gn/ninja_action_target_writer_unittest.cc82
-rw-r--r--tools/gn/ninja_binary_target_writer.cc29
-rw-r--r--tools/gn/ninja_build_writer.cc4
-rw-r--r--tools/gn/ninja_target_writer.cc4
-rw-r--r--tools/gn/ninja_toolchain_writer.cc4
-rw-r--r--tools/gn/path_output.cc16
-rw-r--r--tools/gn/path_output.h17
-rw-r--r--tools/gn/path_output_unittest.cc80
16 files changed, 304 insertions, 264 deletions
diff --git a/base/containers/stack_container.h b/base/containers/stack_container.h
index f0106d7..87fa036 100644
--- a/base/containers/stack_container.h
+++ b/base/containers/stack_container.h
@@ -90,6 +90,13 @@ class StackAllocator : public std::allocator<T> {
: source_(NULL) {
}
+ // This constructor must exist. It creates a default allocator that doesn't
+ // actually have a stack buffer. glibc's std::string() will compare the
+ // current allocator against the default-constructed allocator, so this
+ // should be fast.
+ StackAllocator() : source_(NULL) {
+ }
+
explicit StackAllocator(Source* source) : source_(source) {
}
diff --git a/third_party/mesa/BUILD.gn b/third_party/mesa/BUILD.gn
index 7c56f0e..753b46a 100644
--- a/third_party/mesa/BUILD.gn
+++ b/third_party/mesa/BUILD.gn
@@ -716,7 +716,7 @@ static_library("mesa") {
if (is_win) {
# Because we're building as a static library
- defines += [ "_GLAPI_NO_EXPORTS" ]
+ defines = [ "_GLAPI_NO_EXPORTS" ]
}
deps = [
diff --git a/tools/gn/escape.cc b/tools/gn/escape.cc
index 1658a8a..7028a38 100644
--- a/tools/gn/escape.cc
+++ b/tools/gn/escape.cc
@@ -5,10 +5,11 @@
#include "tools/gn/escape.h"
#include "base/containers/stack_container.h"
+#include "base/logging.h"
namespace {
-// A "1" in this lookup table means that char is valid in the shell.
+// A "1" in this lookup table means that char is valid in the Posix shell.
const char kShellValid[0x80] = {
// 00-1f: all are invalid
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
@@ -26,70 +27,150 @@ const char kShellValid[0x80] = {
// p q r s t u v w x y z { | } ~
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 0 };
+// Append one character to the given string, escaping it for Ninja.
+//
+// Ninja's escaping rules are very simple. We always escape colons even
+// though they're OK in many places, in case the resulting string is used on
+// the left-hand-side of a rule.
template<typename DestString>
-void EscapeStringToString(const base::StringPiece& str,
- const EscapeOptions& options,
- DestString* dest,
- bool* needed_quoting) {
- bool used_quotes = false;
+inline void NinjaEscapeChar(char ch, DestString* dest) {
+ if (ch == '$' || ch == ' ' || ch == ':')
+ dest->push_back('$');
+ dest->push_back(ch);
+}
- for (size_t i = 0; i < str.size(); i++) {
- if (str[i] == '$' && (options.mode & ESCAPE_NINJA)) {
- // Escape dollars signs since ninja treats these specially. If we're also
- // escaping for the shell, we need to backslash-escape that again.
- if (options.mode & ESCAPE_SHELL)
- dest->push_back('\\');
- dest->push_back('$');
- dest->push_back('$');
- } else if (str[i] == ' ') {
- if (options.mode & ESCAPE_NINJA) {
- // For Ninja just escape spaces with $.
- dest->push_back('$');
+template<typename DestString>
+void EscapeStringToString_Ninja(const base::StringPiece& str,
+ const EscapeOptions& options,
+ DestString* dest,
+ bool* needed_quoting) {
+ for (size_t i = 0; i < str.size(); i++)
+ NinjaEscapeChar(str[i], dest);
+ if (needed_quoting)
+ *needed_quoting = false;
+}
+
+// Escape for CommandLineToArgvW and additionally escape Ninja characters.
+//
+// The basic algorithm is if the string doesn't contain any parse-affecting
+// characters, don't do anything (other than the Ninja processing). If it does,
+// quote the string, and backslash-escape all quotes and backslashes.
+// See:
+// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+// http://blogs.msdn.com/b/oldnewthing/archive/2010/09/17/10063629.aspx
+template<typename DestString>
+void EscapeStringToString_WindowsNinjaFork(const base::StringPiece& str,
+ const EscapeOptions& options,
+ DestString* dest,
+ bool* needed_quoting) {
+ // We assume we don't have any whitespace chars that aren't spaces.
+ DCHECK(str.find_first_of("\r\n\v\t") == std::string::npos);
+
+ if (str.find_first_of(" \"") == std::string::npos) {
+ // Simple case, don't quote.
+ EscapeStringToString_Ninja(str, options, dest, needed_quoting);
+ } else {
+ if (!options.inhibit_quoting)
+ dest->push_back('"');
+
+ for (size_t i = 0; i < str.size(); i++) {
+ // Count backslashes in case they're followed by a quote.
+ size_t backslash_count = 0;
+ while (i < str.size() && str[i] == '\\') {
+ i++;
+ backslash_count++;
}
- if (options.mode & ESCAPE_SHELL) {
- // For the shell, quote the whole string.
- if (needed_quoting)
- *needed_quoting = true;
- if (!options.inhibit_quoting) {
- if (!used_quotes) {
- used_quotes = true;
- dest->insert(dest->begin(), '"');
- }
- }
+ if (i == str.size()) {
+ // Backslashes at end of string. Backslash-escape all of them since
+ // they'll be followed by a quote.
+ dest->append(backslash_count * 2, '\\');
+ } else if (str[i] == '"') {
+ // 0 or more backslashes followed by a quote. Backslash-escape the
+ // backslashes, then backslash-escape the quote.
+ dest->append(backslash_count * 2 + 1, '\\');
+ dest->push_back('"');
+ } else {
+ // Non-special Windows character, just escape for Ninja. Also, add any
+ // backslashes we read previously, these are literals.
+ dest->append(backslash_count, '\\');
+ NinjaEscapeChar(str[i], dest);
}
- dest->push_back(' ');
- } else if (str[i] == '\'' && (options.mode & ESCAPE_JSON)) {
- dest->push_back('\\');
- dest->push_back('\'');
-#if defined(OS_WIN)
- } else if (str[i] == '/' && options.convert_slashes) {
- // Convert slashes on Windows if requested.
- dest->push_back('\\');
-#else
- } else if (str[i] == '\\' && (options.mode & ESCAPE_SHELL)) {
- // For non-Windows shell, escape backslashes.
- dest->push_back('\\');
- dest->push_back('\\');
-#endif
- } else if (str[i] == '\\' && (options.mode & ESCAPE_JSON)) {
- dest->push_back('\\');
+ }
+
+ if (!options.inhibit_quoting)
+ dest->push_back('"');
+ if (needed_quoting)
+ *needed_quoting = true;
+ }
+}
+
+template<typename DestString>
+void EscapeStringToString_PosixNinjaFork(const base::StringPiece& str,
+ const EscapeOptions& options,
+ DestString* dest,
+ bool* needed_quoting) {
+ for (size_t i = 0; i < str.size(); i++) {
+ if (str[i] == '$' || str[i] == ' ') {
+ // Space and $ are special to both Ninja and the shell. '$' escape for
+ // Ninja, then backslash-escape for the shell.
dest->push_back('\\');
- } else if (str[i] == ':' && (options.mode & ESCAPE_NINJA)) {
+ dest->push_back('$');
+ dest->push_back(str[i]);
+ } else if (str[i] == ':') {
+ // Colon is the only other Ninja special char, which is not special to
+ // the shell.
dest->push_back('$');
dest->push_back(':');
- } else if ((options.mode & ESCAPE_SHELL) &&
- (static_cast<unsigned>(str[i]) >= 0x80 ||
- !kShellValid[static_cast<int>(str[i])])) {
+ } else if (static_cast<unsigned>(str[i]) >= 0x80 ||
+ !kShellValid[static_cast<int>(str[i])]) {
// All other invalid shell chars get backslash-escaped.
dest->push_back('\\');
dest->push_back(str[i]);
} else {
+ // Everything else is a literal.
dest->push_back(str[i]);
}
}
+}
- if (used_quotes)
- dest->push_back('"');
+template<typename DestString>
+void EscapeStringToString(const base::StringPiece& str,
+ const EscapeOptions& options,
+ DestString* dest,
+ bool* needed_quoting) {
+ switch (options.mode) {
+ case ESCAPE_NONE:
+ dest->append(str.data(), str.size());
+ break;
+ case ESCAPE_NINJA:
+ EscapeStringToString_Ninja(str, options, dest, needed_quoting);
+ break;
+ case ESCAPE_NINJA_COMMAND:
+ switch (options.platform) {
+ case ESCAPE_PLATFORM_CURRENT:
+#if defined(OS_WIN)
+ EscapeStringToString_WindowsNinjaFork(str, options, dest,
+ needed_quoting);
+#else
+ EscapeStringToString_PosixNinjaFork(str, options, dest,
+ needed_quoting);
+#endif
+ break;
+ case ESCAPE_PLATFORM_WIN:
+ EscapeStringToString_WindowsNinjaFork(str, options, dest,
+ needed_quoting);
+ break;
+ case ESCAPE_PLATFORM_POSIX:
+ EscapeStringToString_PosixNinjaFork(str, options, dest,
+ needed_quoting);
+ break;
+ default:
+ NOTREACHED();
+ }
+ break;
+ default:
+ NOTREACHED();
+ }
}
} // namespace
@@ -106,10 +187,8 @@ std::string EscapeString(const base::StringPiece& str,
void EscapeStringToStream(std::ostream& out,
const base::StringPiece& str,
const EscapeOptions& options) {
- // Escape to a stack buffer and then write out to the stream.
- base::StackVector<char, 256> result;
- result->reserve(str.size() + 4); // Guess we'll add a couple of extra chars.
- EscapeStringToString(str, options, &result.container(), NULL);
- if (!result->empty())
- out.write(&result[0], result->size());
+ base::StackString<256> escaped;
+ EscapeStringToString(str, options, &escaped.container(), NULL);
+ if (!escaped->empty())
+ out.write(escaped->data(), escaped->size());
}
diff --git a/tools/gn/escape.h b/tools/gn/escape.h
index 1e4fa68..6f8cf9c 100644
--- a/tools/gn/escape.h
+++ b/tools/gn/escape.h
@@ -14,35 +14,42 @@ enum EscapingMode {
ESCAPE_NONE,
// Ninja string escaping.
- ESCAPE_NINJA = 1,
+ ESCAPE_NINJA,
- // Shell string escaping.
- ESCAPE_SHELL = 2,
+ // For writing commands to ninja files.
+ ESCAPE_NINJA_COMMAND,
+};
- // For writing shell commands to ninja files.
- ESCAPE_NINJA_SHELL = ESCAPE_NINJA | ESCAPE_SHELL,
+enum EscapingPlatform {
+ // Do escaping for the current platform.
+ ESCAPE_PLATFORM_CURRENT,
- ESCAPE_JSON = 4,
+ // Force escaping for the given platform.
+ ESCAPE_PLATFORM_POSIX,
+ ESCAPE_PLATFORM_WIN,
};
struct EscapeOptions {
EscapeOptions()
: mode(ESCAPE_NONE),
- convert_slashes(false),
+ platform(ESCAPE_PLATFORM_CURRENT),
inhibit_quoting(false) {
}
EscapingMode mode;
- // When set, converts forward-slashes to system-specific path separators.
- bool convert_slashes;
+ // Controls how "fork" escaping is done. You will generally want to keep the
+ // default "current" platform.
+ EscapingPlatform platform;
// When the escaping mode is ESCAPE_SHELL, the escaper will normally put
// quotes around things with spaces. If this value is set to true, we'll
// disable the quoting feature and just add the spaces.
//
// This mode is for when quoting is done at some higher-level. Defaults to
- // false.
+ // false. Note that Windows has strange behavior where the meaning of the
+ // backslashes changes according to if it is followed by a quote. The
+ // escaping rules assume that a double-quote will be appended to the result.
bool inhibit_quoting;
};
diff --git a/tools/gn/escape_unittest.cc b/tools/gn/escape_unittest.cc
index 9d0cf7a..1a79a78 100644
--- a/tools/gn/escape_unittest.cc
+++ b/tools/gn/escape_unittest.cc
@@ -12,69 +12,41 @@ TEST(Escape, Ninja) {
EXPECT_EQ("asdf$:$ \"$$\\bar", result);
}
-TEST(Escape, Shell) {
+TEST(Escape, WindowsCommand) {
EscapeOptions opts;
- opts.mode = ESCAPE_SHELL;
- std::string result = EscapeString("asdf: \"$\\bar", opts, NULL);
-#if defined(OS_WIN)
- // Windows shell doesn't escape backslashes, but it does backslash-escape
- // quotes.
- EXPECT_EQ("\"asdf: \\\"\\$\\bar\"", result);
-#else
- EXPECT_EQ("\"asdf: \\\"\\$\\\\bar\"", result);
-#endif
-
- // Some more generic shell chars.
- result = EscapeString("a_;<*b", opts, NULL);
- EXPECT_EQ("a_\\;\\<\\*b", result);
-}
+ opts.mode = ESCAPE_NINJA_COMMAND;
+ opts.platform = ESCAPE_PLATFORM_WIN;
-TEST(Escape, NinjaShell) {
- EscapeOptions opts;
- opts.mode = ESCAPE_NINJA_SHELL;
+ // Regular string is passed, even if it has backslashes.
+ EXPECT_EQ("foo\\bar", EscapeString("foo\\bar", opts, NULL));
- // When escaping for Ninja and the shell, we would escape a $, then escape
- // the backslash again.
- std::string result = EscapeString("a$b", opts, NULL);
- EXPECT_EQ("a\\$$b", result);
-}
+ // Spaces means the string is quoted, normal backslahes untouched.
+ bool needs_quoting = false;
+ EXPECT_EQ("\"foo\\$ bar\"", EscapeString("foo\\ bar", opts, &needs_quoting));
+ EXPECT_TRUE(needs_quoting);
-TEST(Escape, UsedQuotes) {
- EscapeOptions shell_options;
- shell_options.mode = ESCAPE_SHELL;
+ // Inhibit quoting.
+ needs_quoting = false;
+ opts.inhibit_quoting = true;
+ EXPECT_EQ("foo\\$ bar", EscapeString("foo\\ bar", opts, &needs_quoting));
+ EXPECT_TRUE(needs_quoting);
+ opts.inhibit_quoting = false;
- EscapeOptions ninja_options;
- ninja_options.mode = ESCAPE_NINJA;
+ // Backslashes at the end of the string get escaped.
+ EXPECT_EQ("\"foo$ bar\\\\\\\\\"", EscapeString("foo bar\\\\", opts, NULL));
- EscapeOptions ninja_shell_options;
- ninja_shell_options.mode = ESCAPE_NINJA_SHELL;
-
- // Shell escaping with quoting inhibited.
- bool used_quotes = false;
- shell_options.inhibit_quoting = true;
- EXPECT_EQ("foo bar", EscapeString("foo bar", shell_options, &used_quotes));
- EXPECT_TRUE(used_quotes);
-
- // Shell escaping with regular quoting.
- used_quotes = false;
- shell_options.inhibit_quoting = false;
- EXPECT_EQ("\"foo bar\"",
- EscapeString("foo bar", shell_options, &used_quotes));
- EXPECT_TRUE(used_quotes);
+ // Backslashes preceeding quotes are escaped, and the quote is escaped.
+ EXPECT_EQ("\"foo\\\\\\\"$ bar\"", EscapeString("foo\\\" bar", opts, NULL));
+}
- // Ninja shell escaping should be the same.
- used_quotes = false;
- EXPECT_EQ("\"foo$ bar\"",
- EscapeString("foo bar", ninja_shell_options, &used_quotes));
- EXPECT_TRUE(used_quotes);
+TEST(Escape, PosixCommand) {
+ EscapeOptions opts;
+ opts.mode = ESCAPE_NINJA_COMMAND;
+ opts.platform = ESCAPE_PLATFORM_POSIX;
- // Ninja escaping shouldn't use quoting.
- used_quotes = false;
- EXPECT_EQ("foo$ bar", EscapeString("foo bar", ninja_options, &used_quotes));
- EXPECT_FALSE(used_quotes);
+ // : and $ ninja escaped with $. Then Shell-escape backslashes and quotes.
+ EXPECT_EQ("a$:\\$ \\\"\\$$\\\\b", EscapeString("a: \"$\\b", opts, NULL));
- // Used quotes not reset if it's already true.
- used_quotes = true;
- EscapeString("foo", ninja_options, &used_quotes);
- EXPECT_TRUE(used_quotes);
+ // Some more generic shell chars.
+ EXPECT_EQ("a_\\;\\<\\*b", EscapeString("a_;<*b", opts, NULL));
}
diff --git a/tools/gn/file_template.cc b/tools/gn/file_template.cc
index b626482..b00eb39 100644
--- a/tools/gn/file_template.cc
+++ b/tools/gn/file_template.cc
@@ -166,7 +166,7 @@ void FileTemplate::ApplyString(const std::string& str,
void FileTemplate::WriteWithNinjaExpansions(std::ostream& out) const {
EscapeOptions escape_options;
- escape_options.mode = ESCAPE_NINJA_SHELL;
+ escape_options.mode = ESCAPE_NINJA_COMMAND;
escape_options.inhibit_quoting = true;
for (size_t template_i = 0;
@@ -184,8 +184,10 @@ void FileTemplate::WriteWithNinjaExpansions(std::ostream& out) const {
for (size_t subrange_i = 0; subrange_i < t.container().size();
subrange_i++) {
if (t[subrange_i].type == Subrange::LITERAL) {
+ bool cur_needs_quoting = false;
item_str.append(EscapeString(t[subrange_i].literal, escape_options,
- &needs_quoting));
+ &cur_needs_quoting));
+ needs_quoting |= cur_needs_quoting;
} else {
// Don't escape this since we need to preserve the $.
item_str.append("${");
diff --git a/tools/gn/file_template_unittest.cc b/tools/gn/file_template_unittest.cc
index cd5611c..8e3bf40 100644
--- a/tools/gn/file_template_unittest.cc
+++ b/tools/gn/file_template_unittest.cc
@@ -63,11 +63,19 @@ TEST(FileTemplate, NinjaExpansions) {
std::ostringstream out;
t.WriteWithNinjaExpansions(out);
+#if defined(OS_WIN)
// The third argument should get quoted since it contains a space, and the
// embedded quotes should get shell escaped.
EXPECT_EQ(
" -i ${source} \"--out=foo$ bar\\\"${source_name_part}\\\".o\" \"\"",
out.str());
+#else
+ // On Posix we don't need to quote the whole thing and can escape all
+ // individual bad chars.
+ EXPECT_EQ(
+ " -i ${source} --out=foo\\$ bar\\\"${source_name_part}\\\".o \"\"",
+ out.str());
+#endif
}
TEST(FileTemplate, NinjaVariables) {
@@ -80,7 +88,7 @@ TEST(FileTemplate, NinjaVariables) {
std::ostringstream out;
EscapeOptions options;
- options.mode = ESCAPE_NINJA_SHELL;
+ options.mode = ESCAPE_NINJA_COMMAND;
t.WriteNinjaVariablesForSubstitution(out, "../../foo/bar.txt", options);
// Just the variables used above should be written.
diff --git a/tools/gn/ninja_action_target_writer.cc b/tools/gn/ninja_action_target_writer.cc
index 1b71fce..27bd8a4 100644
--- a/tools/gn/ninja_action_target_writer.cc
+++ b/tools/gn/ninja_action_target_writer.cc
@@ -16,7 +16,7 @@ NinjaActionTargetWriter::NinjaActionTargetWriter(const Target* target,
: NinjaTargetWriter(target, toolchain, out),
path_output_no_escaping_(
target->settings()->build_settings()->build_dir(),
- ESCAPE_NONE, false) {
+ ESCAPE_NONE) {
}
NinjaActionTargetWriter::~NinjaActionTargetWriter() {
@@ -144,8 +144,7 @@ void NinjaActionTargetWriter::WriteArgsSubstitutions(
path_output_no_escaping_.WriteFile(source_file_stream, source);
EscapeOptions template_escape_options;
- template_escape_options.mode = ESCAPE_NINJA_SHELL;
- template_escape_options.inhibit_quoting = true;
+ template_escape_options.mode = ESCAPE_NINJA_COMMAND;
args_template.WriteNinjaVariablesForSubstitution(
out_, source_file_stream.str(), template_escape_options);
diff --git a/tools/gn/ninja_action_target_writer_unittest.cc b/tools/gn/ninja_action_target_writer_unittest.cc
index 10b5f81..9c6f2f1 100644
--- a/tools/gn/ninja_action_target_writer_unittest.cc
+++ b/tools/gn/ninja_action_target_writer_unittest.cc
@@ -29,11 +29,7 @@ TEST(NinjaActionTargetWriter, WriteOutputFilesForBuildLine) {
std::vector<OutputFile> output_files;
writer.WriteOutputFilesForBuildLine(output_template, source, &output_files);
- std::string out_str = out.str();
-#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
- EXPECT_EQ(" gen/a$ bbar.h gen/bar.cc", out_str);
+ EXPECT_EQ(" gen/a$ bbar.h gen/bar.cc", out.str());
}
TEST(NinjaActionTargetWriter, WriteOutputFilesForBuildLineWithDepfile) {
@@ -57,11 +53,7 @@ TEST(NinjaActionTargetWriter, WriteOutputFilesForBuildLineWithDepfile) {
std::vector<OutputFile> output_files;
writer.WriteOutputFilesForBuildLine(output_template, source, &output_files);
- std::string out_str = out.str();
-#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
- EXPECT_EQ(" gen/bar.d gen/bar.h gen/bar.cc", out_str);
+ EXPECT_EQ(" gen/bar.d gen/bar.h gen/bar.cc", out.str());
}
TEST(NinjaActionTargetWriter, WriteArgsSubstitutions) {
@@ -79,13 +71,15 @@ TEST(NinjaActionTargetWriter, WriteArgsSubstitutions) {
FileTemplate args_template(args);
writer.WriteArgsSubstitutions(SourceFile("//foo/b ar.in"), args_template);
-
- std::string out_str = out.str();
#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
+ EXPECT_EQ(" source = \"../../foo/b$ ar.in\"\n"
+ " source_name_part = \"b$ ar\"\n",
+ out.str());
+#else
+ EXPECT_EQ(" source = ../../foo/b\\$ ar.in\n"
+ " source_name_part = b\\$ ar\n",
+ out.str());
#endif
- EXPECT_EQ(" source = ../../foo/b$ ar.in\n source_name_part = b$ ar\n",
- out_str);
}
// Makes sure that we write sources as input dependencies for actions with
@@ -126,11 +120,7 @@ TEST(NinjaActionTargetWriter, ActionWithSources) {
"build foo.out: __foo_bar___rule | obj/foo/bar.inputdeps.stamp\n"
"\n"
"build obj/foo/bar.stamp: stamp foo.out\n";
- std::string out_str = out.str();
-#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
- EXPECT_EQ(expected_linux, out_str);
+ EXPECT_EQ(expected_linux, out.str());
}
// Windows.
@@ -145,8 +135,6 @@ TEST(NinjaActionTargetWriter, ActionWithSources) {
NinjaActionTargetWriter writer(&target, setup.toolchain(), out);
writer.Run();
- // TODO(brettw) I think we'll need to worry about backslashes here
- // depending if we're on actual Windows or Linux pretending to be Windows.
const char expected_win[] =
"rule __foo_bar___rule\n"
" command = C$:/python/python.exe gyp-win-tool action-wrapper environment.x86 __foo_bar___rule.$unique_name.rsp\n"
@@ -160,11 +148,7 @@ TEST(NinjaActionTargetWriter, ActionWithSources) {
"build foo.out: __foo_bar___rule | obj/foo/bar.inputdeps.stamp\n"
"\n"
"build obj/foo/bar.stamp: stamp foo.out\n";
- std::string out_str = out.str();
-#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
- EXPECT_EQ(expected_win, out_str);
+ EXPECT_EQ(expected_win, out.str());
}
}
@@ -214,7 +198,12 @@ TEST(NinjaActionTargetWriter, ForEach) {
const char expected_linux[] =
"rule __foo_bar___rule\n"
" command = /usr/bin/python ../../foo/script.py -i ${source} "
+ // Escaping is different between Windows and Posix.
+#if defined(OS_WIN)
"\"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+ "--out=foo\\$ bar${source_name_part}.o\n"
+#endif
" description = ACTION //foo:bar()\n"
" restat = 1\n"
"build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
@@ -241,8 +230,6 @@ TEST(NinjaActionTargetWriter, ForEach) {
// Windows.
{
- // Note: we use forward slashes here so that the output will be the same on
- // Linux and Windows.
setup.build_settings()->set_python_path(base::FilePath(FILE_PATH_LITERAL(
"C:/python/python.exe")));
setup.settings()->set_target_os(Settings::WIN);
@@ -251,8 +238,6 @@ TEST(NinjaActionTargetWriter, ForEach) {
NinjaActionTargetWriter writer(&target, setup.toolchain(), out);
writer.Run();
- // TODO(brettw) I think we'll need to worry about backslashes here
- // depending if we're on actual Windows or Linux pretending to be Windows.
const char expected_win[] =
"rule __foo_bar___rule\n"
" command = C$:/python/python.exe gyp-win-tool action-wrapper "
@@ -261,7 +246,11 @@ TEST(NinjaActionTargetWriter, ForEach) {
" restat = 1\n"
" rspfile = __foo_bar___rule.$unique_name.rsp\n"
" rspfile_content = C$:/python/python.exe ../../foo/script.py -i "
+#if defined(OS_WIN)
"${source} \"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+ "${source} --out=foo\\$ bar${source_name_part}.o\n"
+#endif
"build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
"../../foo/included.txt obj/foo/dep.stamp\n"
"\n"
@@ -278,11 +267,7 @@ TEST(NinjaActionTargetWriter, ForEach) {
"\n"
"build obj/foo/bar.stamp: "
"stamp input1.out input2.out obj/foo/datadep.stamp\n";
- std::string out_str = out.str();
-#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
- EXPECT_EQ(expected_win, out_str);
+ EXPECT_EQ(expected_win, out.str());
}
}
@@ -322,7 +307,11 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
const char expected_linux[] =
"rule __foo_bar___rule\n"
" command = /usr/bin/python ../../foo/script.py -i ${source} "
+#if defined(OS_WIN)
"\"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+ "--out=foo\\$ bar${source_name_part}.o\n"
+#endif
" description = ACTION //foo:bar()\n"
" restat = 1\n"
"build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
@@ -340,18 +329,11 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
" depfile = gen/input2.d\n"
"\n"
"build obj/foo/bar.stamp: stamp input1.out input2.out\n";
-
- std::string out_str = out.str();
-#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
- EXPECT_EQ(expected_linux, out_str);
+ EXPECT_EQ(expected_linux, out.str());
}
// Windows.
{
- // Note: we use forward slashes here so that the output will be the same on
- // Linux and Windows.
setup.build_settings()->set_python_path(base::FilePath(FILE_PATH_LITERAL(
"C:/python/python.exe")));
setup.settings()->set_target_os(Settings::WIN);
@@ -360,8 +342,6 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
NinjaActionTargetWriter writer(&target, setup.toolchain(), out);
writer.Run();
- // TODO(brettw) I think we'll need to worry about backslashes here
- // depending if we're on actual Windows or Linux pretending to be Windows.
const char expected_win[] =
"rule __foo_bar___rule\n"
" command = C$:/python/python.exe gyp-win-tool action-wrapper "
@@ -370,7 +350,11 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
" restat = 1\n"
" rspfile = __foo_bar___rule.$unique_name.rsp\n"
" rspfile_content = C$:/python/python.exe ../../foo/script.py -i "
+#if defined(OS_WIN)
"${source} \"--out=foo$ bar${source_name_part}.o\"\n"
+#else
+ "${source} --out=foo\\$ bar${source_name_part}.o\n"
+#endif
"build obj/foo/bar.inputdeps.stamp: stamp ../../foo/script.py "
"../../foo/included.txt\n"
"\n"
@@ -388,10 +372,6 @@ TEST(NinjaActionTargetWriter, ForEachWithDepfile) {
" depfile = gen/input2.d\n"
"\n"
"build obj/foo/bar.stamp: stamp input1.out input2.out\n";
- std::string out_str = out.str();
-#if defined(OS_WIN)
- std::replace(out_str.begin(), out_str.end(), '\\', '/');
-#endif
- EXPECT_EQ(expected_win, out_str);
+ EXPECT_EQ(expected_win, out.str());
}
}
diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc
index b645624..9cfbdfb 100644
--- a/tools/gn/ninja_binary_target_writer.cc
+++ b/tools/gn/ninja_binary_target_writer.cc
@@ -17,7 +17,7 @@ namespace {
// Returns the proper escape options for writing compiler and linker flags.
EscapeOptions GetFlagOptions() {
EscapeOptions opts;
- opts.mode = ESCAPE_NINJA;
+ opts.mode = ESCAPE_NINJA_COMMAND;
// Some flag strings are actually multiple flags that expect to be just
// added to the command line. We assume that quoting is done by the
@@ -29,7 +29,7 @@ EscapeOptions GetFlagOptions() {
struct DefineWriter {
DefineWriter() {
- options.mode = ESCAPE_SHELL;
+ options.mode = ESCAPE_NINJA_COMMAND;
}
void operator()(const std::string& s, std::ostream& out) const {
@@ -41,33 +41,20 @@ struct DefineWriter {
};
struct IncludeWriter {
- IncludeWriter(PathOutput& path_output,
- const NinjaHelper& h)
+ IncludeWriter(PathOutput& path_output, const NinjaHelper& h)
: helper(h),
- path_output_(path_output),
- old_inhibit_quoting_(path_output.inhibit_quoting()) {
- // Inhibit quoting since we'll put quotes around the whole thing ourselves.
- // Since we're writing in NINJA escaping mode, this won't actually do
- // anything, but I think we may need to change to shell-and-then-ninja
- // escaping for this in the future.
- path_output_.set_inhibit_quoting(true);
+ path_output_(path_output) {
}
~IncludeWriter() {
- path_output_.set_inhibit_quoting(old_inhibit_quoting_);
}
void operator()(const SourceDir& d, std::ostream& out) const {
- out << " \"-I";
- // It's important not to include the trailing slash on directories or on
- // Windows it will be a backslash and the compiler might think we're
- // escaping the quote!
+ out << " -I";
path_output_.WriteDir(out, d, PathOutput::DIR_NO_LAST_SLASH);
- out << "\"";
}
const NinjaHelper& helper;
PathOutput& path_output_;
- bool old_inhibit_quoting_; // So we can put the PathOutput back.
};
Toolchain::ToolType GetToolTypeForTarget(const Target* target) {
@@ -268,8 +255,8 @@ void NinjaBinaryTargetWriter::WriteLinkerFlags(
if (!all_lib_dirs.empty()) {
// Since we're passing these on the command line to the linker and not
// to Ninja, we need to do shell escaping.
- PathOutput lib_path_output(
- path_output_.current_dir(), ESCAPE_NINJA_SHELL, false);
+ PathOutput lib_path_output(path_output_.current_dir(),
+ ESCAPE_NINJA_COMMAND);
for (size_t i = 0; i < all_lib_dirs.size(); i++) {
out_ << " " << tool.lib_dir_prefix;
lib_path_output.WriteDir(out_, all_lib_dirs[i],
@@ -291,7 +278,7 @@ void NinjaBinaryTargetWriter::WriteLibs(const Toolchain::Tool& tool) {
// Libraries that have been recursively pushed through the dependency tree.
EscapeOptions lib_escape_opts;
- lib_escape_opts.mode = ESCAPE_NINJA_SHELL;
+ lib_escape_opts.mode = ESCAPE_NINJA_COMMAND;
const OrderedSet<std::string> all_libs = target_->all_libs();
const std::string framework_ending(".framework");
for (size_t i = 0; i < all_libs.size(); i++) {
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc
index f903904..007089d 100644
--- a/tools/gn/ninja_build_writer.cc
+++ b/tools/gn/ninja_build_writer.cc
@@ -39,7 +39,7 @@ std::string GetSelfInvocationCommand(const BuildSettings* build_settings) {
cmdline.AppendSwitch("-q"); // Don't write output.
EscapeOptions escape_shell;
- escape_shell.mode = ESCAPE_SHELL;
+ escape_shell.mode = ESCAPE_NINJA_COMMAND;
#if defined(OS_WIN)
// The command line code quoting varies by platform. We have one string,
// possibly with spaces, that we want to quote. The Windows command line
@@ -82,7 +82,7 @@ NinjaBuildWriter::NinjaBuildWriter(
default_toolchain_targets_(default_toolchain_targets),
out_(out),
dep_out_(dep_out),
- path_output_(build_settings->build_dir(), ESCAPE_NINJA, false),
+ path_output_(build_settings->build_dir(), ESCAPE_NINJA),
helper_(build_settings) {
}
diff --git a/tools/gn/ninja_target_writer.cc b/tools/gn/ninja_target_writer.cc
index 778e983..a67c3fa 100644
--- a/tools/gn/ninja_target_writer.cc
+++ b/tools/gn/ninja_target_writer.cc
@@ -26,9 +26,7 @@ NinjaTargetWriter::NinjaTargetWriter(const Target* target,
target_(target),
toolchain_(toolchain),
out_(out),
- path_output_(settings_->build_settings()->build_dir(),
- ESCAPE_NINJA,
- false),
+ path_output_(settings_->build_settings()->build_dir(), ESCAPE_NINJA),
helper_(settings_->build_settings()) {
}
diff --git a/tools/gn/ninja_toolchain_writer.cc b/tools/gn/ninja_toolchain_writer.cc
index 44d2586..99087c6 100644
--- a/tools/gn/ninja_toolchain_writer.cc
+++ b/tools/gn/ninja_toolchain_writer.cc
@@ -23,9 +23,7 @@ NinjaToolchainWriter::NinjaToolchainWriter(
toolchain_(toolchain),
targets_(targets),
out_(out),
- path_output_(settings_->build_settings()->build_dir(),
- ESCAPE_NINJA,
- false),
+ path_output_(settings_->build_settings()->build_dir(), ESCAPE_NINJA),
helper_(settings->build_settings()) {
}
diff --git a/tools/gn/path_output.cc b/tools/gn/path_output.cc
index 4e71f9b..7e739b6 100644
--- a/tools/gn/path_output.cc
+++ b/tools/gn/path_output.cc
@@ -9,9 +9,7 @@
#include "tools/gn/output_file.h"
#include "tools/gn/string_utils.h"
-PathOutput::PathOutput(const SourceDir& current_dir,
- EscapingMode escaping,
- bool convert_slashes)
+PathOutput::PathOutput(const SourceDir& current_dir, EscapingMode escaping)
: current_dir_(current_dir) {
CHECK(current_dir.is_source_absolute())
<< "Currently this only supports writing to output directories inside "
@@ -20,11 +18,6 @@ PathOutput::PathOutput(const SourceDir& current_dir,
inverse_current_dir_ = InvertDir(current_dir_);
options_.mode = escaping;
- options_.convert_slashes = convert_slashes;
- options_.inhibit_quoting = false;
-
- if (convert_slashes)
- ConvertPathToSystem(&inverse_current_dir_);
}
PathOutput::~PathOutput() {
@@ -91,12 +84,9 @@ void PathOutput::WriteFile(std::ostream& out,
void PathOutput::WriteSourceRelativeString(
std::ostream& out,
const base::StringPiece& str) const {
- if (options_.mode == ESCAPE_SHELL) {
+ if (options_.mode == ESCAPE_NINJA_COMMAND) {
// Shell escaping needs an intermediate string since it may end up
- // quoting the whole thing. On Windows, the slashes may already be
- // converted to backslashes in inverse_current_dir_, but we assume that on
- // Windows the escaper won't try to then escape the preconverted
- // backslashes and will just pass them, so this is fine.
+ // quoting the whole thing.
std::string intermediate;
intermediate.reserve(inverse_current_dir_.size() + str.size());
intermediate.assign(inverse_current_dir_.c_str(),
diff --git a/tools/gn/path_output.h b/tools/gn/path_output.h
index 33227d4..ba4a5f6 100644
--- a/tools/gn/path_output.h
+++ b/tools/gn/path_output.h
@@ -33,9 +33,7 @@ class PathOutput {
DIR_NO_LAST_SLASH,
};
- PathOutput(const SourceDir& current_dir,
- EscapingMode escaping,
- bool convert_slashes);
+ PathOutput(const SourceDir& current_dir, EscapingMode escaping);
~PathOutput();
// Read-only since inverse_current_dir_ is computed depending on this.
@@ -43,19 +41,10 @@ class PathOutput {
const SourceDir& current_dir() const { return current_dir_; }
- // When true, converts slashes to the system-type path separators (on
- // Windows, this is a backslash, this is a NOP otherwise).
- //
- // Read-only since inverse_current_dir_ is computed depending on this.
- bool convert_slashes_to_system() const { return options_.convert_slashes; }
-
- // When the output escaping is ESCAPE_SHELL, the escaper will normally put
- // quotes around suspect things. If this value is set to true, we'll disable
- // the quoting feature. This means that in ESCAPE_SHELL mode, strings with
- // spaces in them qon't be quoted. This mode is for when quoting is done at
- // some higher-level. Defaults to false.
+ // Getter/setters for flags inside the escape options.
bool inhibit_quoting() const { return options_.inhibit_quoting; }
void set_inhibit_quoting(bool iq) { options_.inhibit_quoting = iq; }
+ void set_escape_platform(EscapingPlatform p) { options_.platform = p; }
void WriteFile(std::ostream& out, const SourceFile& file) const;
void WriteFile(std::ostream& out, const OutputFile& file) const;
diff --git a/tools/gn/path_output_unittest.cc b/tools/gn/path_output_unittest.cc
index 49f29c9..389f96a 100644
--- a/tools/gn/path_output_unittest.cc
+++ b/tools/gn/path_output_unittest.cc
@@ -11,7 +11,7 @@
TEST(PathOutput, Basic) {
SourceDir build_dir("//out/Debug/");
- PathOutput writer(build_dir, ESCAPE_NONE, false);
+ PathOutput writer(build_dir, ESCAPE_NONE);
{
// Normal source-root path.
std::ostringstream out;
@@ -52,7 +52,7 @@ TEST(PathOutput, Basic) {
// Same as basic but the output dir is the root.
TEST(PathOutput, BasicInRoot) {
SourceDir build_dir("//");
- PathOutput writer(build_dir, ESCAPE_NONE, false);
+ PathOutput writer(build_dir, ESCAPE_NONE);
{
// Normal source-root path.
std::ostringstream out;
@@ -69,7 +69,7 @@ TEST(PathOutput, BasicInRoot) {
TEST(PathOutput, NinjaEscaping) {
SourceDir build_dir("//out/Debug/");
- PathOutput writer(build_dir, ESCAPE_NINJA, false);
+ PathOutput writer(build_dir, ESCAPE_NINJA);
{
// Spaces and $ in filenames.
std::ostringstream out;
@@ -84,63 +84,85 @@ TEST(PathOutput, NinjaEscaping) {
}
}
-TEST(PathOutput, ShellEscaping) {
+TEST(PathOutput, NinjaForkEscaping) {
SourceDir build_dir("//out/Debug/");
- PathOutput writer(build_dir, ESCAPE_SHELL, false);
+ PathOutput writer(build_dir, ESCAPE_NINJA_COMMAND);
+
+ // Spaces in filenames should get quoted on Windows.
+ writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
{
- // Spaces in filenames should get quoted.
std::ostringstream out;
writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
- EXPECT_EQ("\"../../foo/foo bar.cc\"", out.str());
+ EXPECT_EQ("\"../../foo/foo$ bar.cc\"", out.str());
+ }
+
+ // Spaces in filenames should get escaped on Posix.
+ writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
+ {
+ std::ostringstream out;
+ writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
+ EXPECT_EQ("../../foo/foo\\$ bar.cc", out.str());
+ }
+
+ // Quotes should get blackslash-escaped on Windows and Posix.
+ writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
+ {
+ std::ostringstream out;
+ writer.WriteFile(out, SourceFile("//foo/\"foobar\".cc"));
+ // Our Windows code currently quotes the whole thing in this case for
+ // code simplicity, even though it's strictly unnecessary. This might
+ // change in the future.
+ EXPECT_EQ("\"../../foo/\\\"foobar\\\".cc\"", out.str());
}
+ writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
{
- // Quotes should get blackslash-escaped.
std::ostringstream out;
writer.WriteFile(out, SourceFile("//foo/\"foobar\".cc"));
EXPECT_EQ("../../foo/\\\"foobar\\\".cc", out.str());
}
+
+
+ // Backslashes should get escaped on non-Windows and preserved on Windows.
+ writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
{
- // Backslashes should get escaped on non-Windows and preserved on Windows.
std::ostringstream out;
writer.WriteFile(out, SourceFile("//foo\\bar.cc"));
-#if defined(OS_WIN)
EXPECT_EQ("../../foo\\bar.cc", out.str());
-#else
- EXPECT_EQ("../../foo\\\\bar.cc", out.str());
-#endif
}
-}
-
-TEST(PathOutput, SlashConversion) {
- SourceDir build_dir("//out/Debug/");
- PathOutput writer(build_dir, ESCAPE_NINJA, true);
+ writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
{
std::ostringstream out;
- writer.WriteFile(out, SourceFile("//foo/bar.cc"));
-#if defined(OS_WIN)
- EXPECT_EQ("..\\..\\foo\\bar.cc", out.str());
-#else
- EXPECT_EQ("../../foo/bar.cc", out.str());
-#endif
+ writer.WriteFile(out, SourceFile("//foo\\bar.cc"));
+ EXPECT_EQ("../../foo\\\\bar.cc", out.str());
}
}
TEST(PathOutput, InhibitQuoting) {
SourceDir build_dir("//out/Debug/");
- PathOutput writer(build_dir, ESCAPE_SHELL, false);
+ PathOutput writer(build_dir, ESCAPE_NINJA_COMMAND);
writer.set_inhibit_quoting(true);
+
+ writer.set_escape_platform(ESCAPE_PLATFORM_WIN);
{
// We should get unescaped spaces in the output with no quotes.
std::ostringstream out;
writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
- EXPECT_EQ("../../foo/foo bar.cc", out.str());
+ EXPECT_EQ("../../foo/foo$ bar.cc", out.str());
+ }
+
+ writer.set_escape_platform(ESCAPE_PLATFORM_POSIX);
+ {
+ // Escapes the space.
+ std::ostringstream out;
+ writer.WriteFile(out, SourceFile("//foo/foo bar.cc"));
+ EXPECT_EQ("../../foo/foo\\$ bar.cc", out.str());
}
}
TEST(PathOutput, WriteDir) {
{
SourceDir build_dir("//out/Debug/");
- PathOutput writer(build_dir, ESCAPE_NINJA, false);
+ PathOutput writer(build_dir, ESCAPE_NINJA);
{
std::ostringstream out;
writer.WriteDir(out, SourceDir("//foo/bar/"),
@@ -191,6 +213,8 @@ TEST(PathOutput, WriteDir) {
// Output inside current dir.
{
std::ostringstream out;
+
+
writer.WriteDir(out, SourceDir("//out/Debug/"),
PathOutput::DIR_INCLUDE_LAST_SLASH);
EXPECT_EQ("./", out.str());
@@ -216,7 +240,7 @@ TEST(PathOutput, WriteDir) {
}
{
// Empty build dir writer.
- PathOutput root_writer(SourceDir("//"), ESCAPE_NINJA, false);
+ PathOutput root_writer(SourceDir("//"), ESCAPE_NINJA);
{
std::ostringstream out;
root_writer.WriteDir(out, SourceDir("//"),
generated by cgit v1.1 at 2025-02-13 08:55:27 +0000