From a90b279e469ec4ad3c57dff5075acecabc41e70a Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Thu, 17 Oct 2013 21:12:11 -0700 Subject: Fix compilation on VS2013 --- configure.py | 2 ++ platform_helper.py | 12 ++++++++++-- src/edit_distance.cc | 1 + src/hash_map.h | 1 + src/metrics.cc | 2 ++ 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/configure.py b/configure.py index 9fe3be8..431b03e 100755 --- a/configure.py +++ b/configure.py @@ -125,6 +125,8 @@ if platform.is_msvc(): '/DNOMINMAX', '/D_CRT_SECURE_NO_WARNINGS', '/D_VARIADIC_MAX=10', '/DNINJA_PYTHON="%s"' % options.with_python] + if platform.msvc_needs_fs(): + cflags.append('/FS') ldflags = ['/DEBUG', '/libpath:$builddir'] if not options.debug: cflags += ['/Ox', '/DNDEBUG', '/GL'] diff --git a/platform_helper.py b/platform_helper.py index b7447a1..dac7c02 100644 --- a/platform_helper.py +++ b/platform_helper.py @@ -21,8 +21,8 @@ def platforms(): return ['linux', 'darwin', 'freebsd', 'openbsd', 'solaris', 'sunos5', 'mingw', 'msvc', 'gnukfreebsd8', 'bitrig'] -class Platform( object ): - def __init__( self, platform): +class Platform(object): + def __init__(self, platform): self._platform = platform if not self._platform is None: return @@ -56,6 +56,14 @@ class Platform( object ): def is_msvc(self): return self._platform == 'msvc' + def msvc_needs_fs(self): + import subprocess + popen = subprocess.Popen('cl /nologo /?', + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + out, err = popen.communicate() + return '/FS ' in out + def is_windows(self): return self.is_mingw() or self.is_msvc() diff --git a/src/edit_distance.cc b/src/edit_distance.cc index cc4483f..9553c6e 100644 --- a/src/edit_distance.cc +++ b/src/edit_distance.cc @@ -14,6 +14,7 @@ #include "edit_distance.h" +#include #include int EditDistance(const StringPiece& s1, diff --git a/src/hash_map.h b/src/hash_map.h index c63aa88..77e7586 100644 --- a/src/hash_map.h +++ b/src/hash_map.h @@ -15,6 +15,7 @@ #ifndef NINJA_MAP_H_ #define NINJA_MAP_H_ +#include #include #include "string_piece.h" diff --git a/src/metrics.cc b/src/metrics.cc index ca4f97a..a7d3c7a 100644 --- a/src/metrics.cc +++ b/src/metrics.cc @@ -24,6 +24,8 @@ #include #endif +#include + #include "util.h" Metrics* g_metrics = NULL; -- cgit v0.12 From 037b0934f929ba17434906fb781aeb1acb583385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20K=C3=BCmmel?= Date: Sun, 13 Oct 2013 12:20:17 +0200 Subject: add deps_prefix for localized /showIncludes' output parsing --- doc/manual.asciidoc | 10 +++++++++- src/build.cc | 6 ++++-- src/build.h | 2 +- src/includes_normalize_test.cc | 2 +- src/msvc_helper-win32.cc | 15 +++++++-------- src/msvc_helper.h | 4 ++-- src/msvc_helper_main-win32.cc | 9 +++++++-- src/msvc_helper_test.cc | 29 +++++++++++++++++++---------- 8 files changed, 50 insertions(+), 27 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index a735257..67fcbfd 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -580,9 +580,13 @@ Ninja supports this processing in two forms. http://msdn.microsoft.com/en-us/library/hdkef6tk(v=vs.90).aspx[`/showIncludes` flag]. Briefly, this means the tool outputs specially-formatted lines to its stdout. Ninja then filters these lines from the displayed - output. No `depfile` attribute is necessary. + output. No `depfile` attribute is necessary, but the localized string + in front of the the header file path. For instance + `msvc_deps_prefix = Note: including file: ` + for a English Visual Studio (the default). Should be globally defined. + ---- +msvc_deps_prefix = Note: including file: rule cc deps = msvc command = cl /showIncludes -c $in /Fo$out @@ -772,6 +776,10 @@ keys. stored as `.ninja_deps` in the `builddir`, see <>. +`msvc_deps_prefix`:: _(Available since Ninja 1.5.)_ defines the string + which should be stripped from msvc's /showIncludes output. Only + needed when `deps = msvc` and no English Visual Studio version is used. + `description`:: a short description of the command, used to pretty-print the command as it's running. The `-v` flag controls whether to print the full command or its description; if a command fails, the full command diff --git a/src/build.cc b/src/build.cc index 9718f85..33aa85b 100644 --- a/src/build.cc +++ b/src/build.cc @@ -714,9 +714,10 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { // build perspective. vector deps_nodes; string deps_type = edge->GetBinding("deps"); + const string deps_prefix = edge->GetBinding("msvc_deps_prefix"); if (!deps_type.empty()) { string extract_err; - if (!ExtractDeps(result, deps_type, &deps_nodes, &extract_err) && + if (!ExtractDeps(result, deps_type, deps_prefix, &deps_nodes, &extract_err) && result->success()) { if (!result->output.empty()) result->output.append("\n"); @@ -802,12 +803,13 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { bool Builder::ExtractDeps(CommandRunner::Result* result, const string& deps_type, + const string& deps_prefix, vector* deps_nodes, string* err) { #ifdef _WIN32 if (deps_type == "msvc") { CLParser parser; - result->output = parser.Parse(result->output); + result->output = parser.Parse(result->output, deps_prefix); for (set::iterator i = parser.includes_.begin(); i != parser.includes_.end(); ++i) { deps_nodes->push_back(state_->GetNode(*i)); diff --git a/src/build.h b/src/build.h index 5b6c83c..1122d84 100644 --- a/src/build.h +++ b/src/build.h @@ -180,7 +180,7 @@ struct Builder { BuildStatus* status_; private: - bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, + bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, const string& deps_prefix, vector* deps_nodes, string* err); DiskInterface* disk_interface_; diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index 1713d5d..419996f 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -38,7 +38,7 @@ string GetCurDir() { } // namespace TEST(IncludesNormalize, WithRelative) { - string currentdir = IncludesNormalize::ToLower(GetCurDir()); + string currentdir = GetCurDir(); EXPECT_EQ("c", IncludesNormalize::Normalize("a/b/c", "a/b")); EXPECT_EQ("a", IncludesNormalize::Normalize(IncludesNormalize::AbsPath("a"), NULL)); diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index 7c45029..3065ab0 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -48,14 +48,13 @@ string EscapeForDepfile(const string& path) { } // static -string CLParser::FilterShowIncludes(const string& line) { - static const char kMagicPrefix[] = "Note: including file: "; +string CLParser::FilterShowIncludes(const string& line, const string& deps_prefix) { + static const string deps_prefix_english = "Note: including file: "; const char* in = line.c_str(); const char* end = in + line.size(); - - if (end - in > (int)sizeof(kMagicPrefix) - 1 && - memcmp(in, kMagicPrefix, sizeof(kMagicPrefix) - 1) == 0) { - in += sizeof(kMagicPrefix) - 1; + const string& prefix = deps_prefix.empty() ? deps_prefix_english : deps_prefix; + if (end - in > (int)prefix.size() && memcmp(in, prefix.c_str(), (int)prefix.size()) == 0) { + in += prefix.size(); while (*in == ' ') ++in; return line.substr(in - line.c_str()); @@ -81,7 +80,7 @@ bool CLParser::FilterInputFilename(string line) { EndsWith(line, ".cpp"); } -string CLParser::Parse(const string& output) { +string CLParser::Parse(const string& output, const string& deps_prefix) { string filtered_output; // Loop over all lines in the output to process them. @@ -92,7 +91,7 @@ string CLParser::Parse(const string& output) { end = output.size(); string line = output.substr(start, end - start); - string include = FilterShowIncludes(line); + string include = FilterShowIncludes(line, deps_prefix); if (!include.empty()) { include = IncludesNormalize::Normalize(include, NULL); if (!IsSystemInclude(include)) diff --git a/src/msvc_helper.h b/src/msvc_helper.h index e207485..0433769 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -27,7 +27,7 @@ struct CLParser { /// Parse a line of cl.exe output and extract /showIncludes info. /// If a dependency is extracted, returns a nonempty string. /// Exposed for testing. - static string FilterShowIncludes(const string& line); + static string FilterShowIncludes(const string& line, const string& deps_prefix); /// Return true if a mentioned include file is a system path. /// Filtering these out reduces dependency information considerably. @@ -41,7 +41,7 @@ struct CLParser { /// Parse the full output of cl, returning the output (if any) that /// should printed. - string Parse(const string& output); + string Parse(const string& output, const string& deps_prefix); set includes_; }; diff --git a/src/msvc_helper_main-win32.cc b/src/msvc_helper_main-win32.cc index e3a7846..58bc797 100644 --- a/src/msvc_helper_main-win32.cc +++ b/src/msvc_helper_main-win32.cc @@ -31,6 +31,7 @@ void Usage() { "options:\n" " -e ENVFILE load environment block from ENVFILE as environment\n" " -o FILE write output dependency information to FILE.d\n" +" -p STRING localized prefix of msvc's /showIncludes output\n" ); } @@ -84,7 +85,8 @@ int MSVCHelperMain(int argc, char** argv) { { NULL, 0, NULL, 0 } }; int opt; - while ((opt = getopt_long(argc, argv, "e:o:h", kLongOptions, NULL)) != -1) { + string deps_prefix; + while ((opt = getopt_long(argc, argv, "e:o:p:h", kLongOptions, NULL)) != -1) { switch (opt) { case 'e': envfile = optarg; @@ -92,6 +94,9 @@ int MSVCHelperMain(int argc, char** argv) { case 'o': output_filename = optarg; break; + case 'p': + deps_prefix = optarg; + break; case 'h': default: Usage(); @@ -122,7 +127,7 @@ int MSVCHelperMain(int argc, char** argv) { if (output_filename) { CLParser parser; - output = parser.Parse(output); + output = parser.Parse(output, deps_prefix); WriteDepFileOrDie(output_filename, parser); } diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 02f2863..48fbe21 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -20,15 +20,19 @@ #include "util.h" TEST(CLParserTest, ShowIncludes) { - ASSERT_EQ("", CLParser::FilterShowIncludes("")); + ASSERT_EQ("", CLParser::FilterShowIncludes("", "")); - ASSERT_EQ("", CLParser::FilterShowIncludes("Sample compiler output")); + ASSERT_EQ("", CLParser::FilterShowIncludes("Sample compiler output", "")); ASSERT_EQ("c:\\Some Files\\foobar.h", CLParser::FilterShowIncludes("Note: including file: " - "c:\\Some Files\\foobar.h")); + "c:\\Some Files\\foobar.h", "")); ASSERT_EQ("c:\\initspaces.h", CLParser::FilterShowIncludes("Note: including file: " - "c:\\initspaces.h")); + "c:\\initspaces.h", "")); + ASSERT_EQ("c:\\initspaces.h", + CLParser::FilterShowIncludes("Non-default prefix: inc file: " + "c:\\initspaces.h", + "Non-default prefix: inc file:")); } TEST(CLParserTest, FilterInputFilename) { @@ -46,8 +50,9 @@ TEST(CLParserTest, ParseSimple) { CLParser parser; string output = parser.Parse( "foo\r\n" - "Note: including file: foo.h\r\n" - "bar\r\n"); + "Note: inc file prefix: foo.h\r\n" + "bar\r\n", + "Note: inc file prefix:"); ASSERT_EQ("foo\nbar\n", output); ASSERT_EQ(1u, parser.includes_.size()); @@ -58,7 +63,8 @@ TEST(CLParserTest, ParseFilenameFilter) { CLParser parser; string output = parser.Parse( "foo.cc\r\n" - "cl: warning\r\n"); + "cl: warning\r\n", + ""); ASSERT_EQ("cl: warning\n", output); } @@ -67,7 +73,8 @@ TEST(CLParserTest, ParseSystemInclude) { string output = parser.Parse( "Note: including file: c:\\Program Files\\foo.h\r\n" "Note: including file: d:\\Microsoft Visual Studio\\bar.h\r\n" - "Note: including file: path.h\r\n"); + "Note: including file: path.h\r\n", + ""); // We should have dropped the first two includes because they look like // system headers. ASSERT_EQ("", output); @@ -80,7 +87,8 @@ TEST(CLParserTest, DuplicatedHeader) { string output = parser.Parse( "Note: including file: foo.h\r\n" "Note: including file: bar.h\r\n" - "Note: including file: foo.h\r\n"); + "Note: including file: foo.h\r\n", + ""); // We should have dropped one copy of foo.h. ASSERT_EQ("", output); ASSERT_EQ(2u, parser.includes_.size()); @@ -91,7 +99,8 @@ TEST(CLParserTest, DuplicatedHeaderPathConverted) { string output = parser.Parse( "Note: including file: sub/foo.h\r\n" "Note: including file: bar.h\r\n" - "Note: including file: sub\\foo.h\r\n"); + "Note: including file: sub\\foo.h\r\n", + ""); // We should have dropped one copy of foo.h. ASSERT_EQ("", output); ASSERT_EQ(2u, parser.includes_.size()); -- cgit v0.12 From fffb8b1cddfd314acd2509f0507038d403222ab2 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 19 Oct 2013 11:03:15 -0700 Subject: Minor style fixes. No functionality change. --- src/build.cc | 3 ++- src/build.h | 5 +++-- src/msvc_helper-win32.cc | 10 ++++++---- src/msvc_helper.h | 3 ++- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/build.cc b/src/build.cc index 33aa85b..f91ff2f 100644 --- a/src/build.cc +++ b/src/build.cc @@ -717,7 +717,8 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { const string deps_prefix = edge->GetBinding("msvc_deps_prefix"); if (!deps_type.empty()) { string extract_err; - if (!ExtractDeps(result, deps_type, deps_prefix, &deps_nodes, &extract_err) && + if (!ExtractDeps(result, deps_type, deps_prefix, &deps_nodes, + &extract_err) && result->success()) { if (!result->output.empty()) result->output.append("\n"); diff --git a/src/build.h b/src/build.h index 1122d84..eb3636a 100644 --- a/src/build.h +++ b/src/build.h @@ -180,8 +180,9 @@ struct Builder { BuildStatus* status_; private: - bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, const string& deps_prefix, - vector* deps_nodes, string* err); + bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, + const string& deps_prefix, vector* deps_nodes, + string* err); DiskInterface* disk_interface_; DependencyScan scan_; diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index 3065ab0..d2e2eb5 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -48,12 +48,14 @@ string EscapeForDepfile(const string& path) { } // static -string CLParser::FilterShowIncludes(const string& line, const string& deps_prefix) { - static const string deps_prefix_english = "Note: including file: "; +string CLParser::FilterShowIncludes(const string& line, + const string& deps_prefix) { + const string kDepsPrefixEnglish = "Note: including file: "; const char* in = line.c_str(); const char* end = in + line.size(); - const string& prefix = deps_prefix.empty() ? deps_prefix_english : deps_prefix; - if (end - in > (int)prefix.size() && memcmp(in, prefix.c_str(), (int)prefix.size()) == 0) { + const string& prefix = deps_prefix.empty() ? kDepsPrefixEnglish : deps_prefix; + if (end - in > (int)prefix.size() && + memcmp(in, prefix.c_str(), (int)prefix.size()) == 0) { in += prefix.size(); while (*in == ' ') ++in; diff --git a/src/msvc_helper.h b/src/msvc_helper.h index 0433769..5d7dcb0 100644 --- a/src/msvc_helper.h +++ b/src/msvc_helper.h @@ -27,7 +27,8 @@ struct CLParser { /// Parse a line of cl.exe output and extract /showIncludes info. /// If a dependency is extracted, returns a nonempty string. /// Exposed for testing. - static string FilterShowIncludes(const string& line, const string& deps_prefix); + static string FilterShowIncludes(const string& line, + const string& deps_prefix); /// Return true if a mentioned include file is a system path. /// Filtering these out reduces dependency information considerably. -- cgit v0.12 From 3aa641ec82d39760b6aa413c2870d0d418c48898 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 26 Oct 2013 20:15:23 -0700 Subject: Wrap to 79 colums. No functionality change. --- bootstrap.py | 13 ++++++++----- configure.py | 12 ++++++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/bootstrap.py b/bootstrap.py index 66ec85b..026396b 100755 --- a/bootstrap.py +++ b/bootstrap.py @@ -34,10 +34,12 @@ parser.add_option('--verbose', action='store_true', parser.add_option('--x64', action='store_true', help='force 64-bit build (Windows)',) parser.add_option('--platform', - help='target platform (' + '/'.join(platform_helper.platforms()) + ')', + help='target platform (' + + '/'.join(platform_helper.platforms()) + ')', choices=platform_helper.platforms()) parser.add_option('--force-pselect', action='store_true', - help="ppoll() is used by default on Linux, OpenBSD and Bitrig, but older versions might need to use pselect instead",) + help='ppoll() is used by default where available, ' + 'but some platforms might need to use pselect instead',) (options, conf_args) = parser.parse_args() @@ -109,7 +111,8 @@ else: cflags.append('-D_WIN32_WINNT=0x0501') if options.x64: cflags.append('-m64') -if (platform.is_linux() or platform.is_openbsd() or platform.is_bitrig()) and not options.force_pselect: +if (platform.is_linux() or platform.is_openbsd() or platform.is_bitrig()) and \ + not options.force_pselect: cflags.append('-DUSE_PPOLL') if options.force_pselect: conf_args.append("--force-pselect") @@ -153,8 +156,8 @@ if platform.is_windows(): Done! Note: to work around Windows file locking, where you can't rebuild an -in-use binary, to run ninja after making any changes to build ninja itself -you should run ninja.bootstrap instead.""") +in-use binary, to run ninja after making any changes to build ninja +itself you should run ninja.bootstrap instead.""") else: print('Building ninja using itself...') run([sys.executable, 'configure.py'] + conf_args) diff --git a/configure.py b/configure.py index 431b03e..cceb0f7 100755 --- a/configure.py +++ b/configure.py @@ -32,10 +32,12 @@ import ninja_syntax parser = OptionParser() profilers = ['gmon', 'pprof'] parser.add_option('--platform', - help='target platform (' + '/'.join(platform_helper.platforms()) + ')', + help='target platform (' + + '/'.join(platform_helper.platforms()) + ')', choices=platform_helper.platforms()) parser.add_option('--host', - help='host platform (' + '/'.join(platform_helper.platforms()) + ')', + help='host platform (' + + '/'.join(platform_helper.platforms()) + ')', choices=platform_helper.platforms()) parser.add_option('--debug', action='store_true', help='enable debugging extras',) @@ -48,7 +50,8 @@ parser.add_option('--with-python', metavar='EXE', help='use EXE as the Python interpreter', default=os.path.basename(sys.executable)) parser.add_option('--force-pselect', action='store_true', - help="ppoll() is used by default where available, but some platforms may need to use pselect instead",) + help='ppoll() is used by default where available, ' + 'but some platforms may need to use pselect instead',) (options, args) = parser.parse_args() if args: print('ERROR: extra unparsed command-line arguments:', args) @@ -167,7 +170,8 @@ else: cflags.append('-fno-omit-frame-pointer') libs.extend(['-Wl,--no-as-needed', '-lprofiler']) -if (platform.is_linux() or platform.is_openbsd() or platform.is_bitrig()) and not options.force_pselect: +if (platform.is_linux() or platform.is_openbsd() or platform.is_bitrig()) and \ + not options.force_pselect: cflags.append('-DUSE_PPOLL') def shell_escape(str): -- cgit v0.12 From a36038f990db1b347ecb4cc3bfb984fc3045d04f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 27 Oct 2013 12:38:34 -0700 Subject: Add a test that shows that there is a single global namespace for rules. --- src/manifest_parser_test.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 5ed1584..0b4c042 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -762,6 +762,21 @@ TEST_F(ParserTest, MissingSubNinja) { , err); } +TEST_F(ParserTest, DuplicateRuleInDifferentSubninjas) { + // Test that rules live in a global namespace and aren't scoped to subninjas. + files_["test.ninja"] = "rule cat\n" + " command = cat\n"; + ManifestParser parser(&state, this); + string err; + EXPECT_FALSE(parser.ParseTest("rule cat\n" + " command = cat\n" + "subninja test.ninja\n", &err)); + EXPECT_EQ("test.ninja:1: duplicate rule 'cat'\n" + "rule cat\n" + " ^ near here" + , err); +} + TEST_F(ParserTest, Include) { files_["include.ninja"] = "var = inner\n"; ASSERT_NO_FATAL_FAILURE(AssertParse( -- cgit v0.12 From 6b9104734ae3b58824f4850b356737616b1753f7 Mon Sep 17 00:00:00 2001 From: Neil Mitchell Date: Mon, 18 Nov 2013 06:32:32 +0000 Subject: Fix up platform_helper for MSVC, with Python 2.6.8 the arguments have to be in a list, not space separated (unless you set Shell to true) --- platform_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform_helper.py b/platform_helper.py index dac7c02..1b45e19 100644 --- a/platform_helper.py +++ b/platform_helper.py @@ -58,7 +58,7 @@ class Platform(object): def msvc_needs_fs(self): import subprocess - popen = subprocess.Popen('cl /nologo /?', + popen = subprocess.Popen(['cl', '/nologo', '/?'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) out, err = popen.communicate() -- cgit v0.12 From abf3546ae6645bcb7f53d60bbfdcf3951a37fcbb Mon Sep 17 00:00:00 2001 From: Patrick von Reth Date: Thu, 5 Dec 2013 10:37:52 +0100 Subject: fixed platform_helper.py msvc_needs_fs test --- platform_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform_helper.py b/platform_helper.py index 1b45e19..de102b5 100644 --- a/platform_helper.py +++ b/platform_helper.py @@ -62,7 +62,7 @@ class Platform(object): stdout=subprocess.PIPE, stderr=subprocess.PIPE) out, err = popen.communicate() - return '/FS ' in out + return '/FS ' in str(out) def is_windows(self): return self.is_mingw() or self.is_msvc() -- cgit v0.12 From 81f935bf2da05ec97da6d6268c7a283859779275 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 13 Dec 2013 13:01:02 -0800 Subject: Fix additional range in character class. The range was added in 7ab6dcbdb6447861eefafc47fc3e10f3273cede2, but that change only tried to add ! to the character class. Fix by moving '-' to the end of the class. Fixes #694. --- src/depfile_parser.cc | 71 ++++++++++++++++++++++++++++-------------------- src/depfile_parser.in.cc | 2 +- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index 5a30c6b..49c7d7b 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -53,10 +53,10 @@ bool DepfileParser::Parse(string* content, string* err) { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 128, 128, 128, 128, 128, 128, 128, - 128, 128, 128, 128, 128, 128, 128, 128, + 0, 128, 0, 0, 0, 0, 0, 0, + 128, 128, 0, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, - 128, 128, 128, 128, 128, 128, 0, 0, + 128, 128, 128, 0, 0, 128, 0, 0, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, @@ -84,30 +84,45 @@ bool DepfileParser::Parse(string* content, string* err) { }; yych = *in; - if (yych <= '[') { + if (yych <= '=') { if (yych <= '$') { - if (yych <= 0x00) goto yy7; - if (yych <= ' ') goto yy9; - if (yych <= '#') goto yy6; - goto yy4; + if (yych <= ' ') { + if (yych <= 0x00) goto yy7; + goto yy9; + } else { + if (yych <= '!') goto yy5; + if (yych <= '#') goto yy9; + goto yy4; + } } else { - if (yych <= '=') goto yy6; - if (yych <= '?') goto yy9; - if (yych <= 'Z') goto yy6; - goto yy9; + if (yych <= '*') { + if (yych <= '\'') goto yy9; + if (yych <= ')') goto yy5; + goto yy9; + } else { + if (yych <= ':') goto yy5; + if (yych <= '<') goto yy9; + goto yy5; + } } } else { - if (yych <= '`') { - if (yych <= '\\') goto yy2; - if (yych == '_') goto yy6; - goto yy9; + if (yych <= '^') { + if (yych <= 'Z') { + if (yych <= '?') goto yy9; + goto yy5; + } else { + if (yych != '\\') goto yy9; + } } else { - if (yych <= 'z') goto yy6; - if (yych == '~') goto yy6; - goto yy9; + if (yych <= 'z') { + if (yych == '`') goto yy9; + goto yy5; + } else { + if (yych == '~') goto yy5; + goto yy9; + } } } -yy2: ++in; if ((yych = *in) <= '#') { if (yych <= '\n') { @@ -135,10 +150,14 @@ yy3: break; } yy4: + yych = *++in; + if (yych == '$') goto yy12; + goto yy3; +yy5: ++in; - if ((yych = *in) == '$') goto yy12; + yych = *in; goto yy11; -yy5: +yy6: { // Got a span of plain text. int len = (int)(in - start); @@ -148,9 +167,6 @@ yy5: out += len; continue; } -yy6: - yych = *++in; - goto yy11; yy7: ++in; { @@ -166,12 +182,9 @@ yy11: if (yybm[0+yych] & 128) { goto yy10; } - goto yy5; + goto yy6; yy12: ++in; - if (yybm[0+(yych = *in)] & 128) { - goto yy10; - } { // De-escape dollar character. *out++ = '$'; diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc index cf24a09..8bb6d84 100644 --- a/src/depfile_parser.in.cc +++ b/src/depfile_parser.in.cc @@ -73,7 +73,7 @@ bool DepfileParser::Parse(string* content, string* err) { *out++ = yych; continue; } - [a-zA-Z0-9+,/_:.~()@=-!]+ { + [a-zA-Z0-9+,/_:.~()@=!-]+ { // Got a span of plain text. int len = (int)(in - start); // Need to shift it over if we're overwriting backslashes. -- cgit v0.12 From b2e6fcf7031cfaf995c65820d14d4aa390daf9fb Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 1 Jan 2014 20:31:34 -0800 Subject: Remove dead entries in .ninja_log and .ninja_deps while recompacting. For .ninja_deps, remove objects that have no in-edges or whose in-edges do not have a "deps" attribute. (This matches the behaviour of `-t deps`). BuildLog doesn't know about state, so let its recompact method take delegate that decides is a path is life or not, and implement it in NinjaMain. --- src/build_log.cc | 15 ++++++++++++--- src/build_log.h | 8 ++++++-- src/deps_log.cc | 7 +++++++ src/ninja.cc | 11 ++++++++--- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/build_log.cc b/src/build_log.cc index b92a06f..825a8f5 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -108,9 +108,9 @@ BuildLog::~BuildLog() { Close(); } -bool BuildLog::OpenForWrite(const string& path, string* err) { +bool BuildLog::OpenForWrite(const string& path, IsDead* is_dead, string* err) { if (needs_recompaction_) { - if (!Recompact(path, err)) + if (!Recompact(path, is_dead, err)) return false; } @@ -350,7 +350,7 @@ bool BuildLog::WriteEntry(FILE* f, const LogEntry& entry) { entry.output.c_str(), entry.command_hash) > 0; } -bool BuildLog::Recompact(const string& path, string* err) { +bool BuildLog::Recompact(const string& path, IsDead* is_dead, string* err) { METRIC_RECORD(".ninja_log recompact"); printf("Recompacting log...\n"); @@ -368,7 +368,13 @@ bool BuildLog::Recompact(const string& path, string* err) { return false; } + vector dead_outputs; for (Entries::iterator i = entries_.begin(); i != entries_.end(); ++i) { + if (is_dead->IsPathDead(i->first)) { + dead_outputs.push_back(i->first); + continue; + } + if (!WriteEntry(f, *i->second)) { *err = strerror(errno); fclose(f); @@ -376,6 +382,9 @@ bool BuildLog::Recompact(const string& path, string* err) { } } + for (size_t i = 0; i < dead_outputs.size(); ++i) + entries_.erase(dead_outputs[i]); + fclose(f); if (unlink(path.c_str()) < 0) { *err = strerror(errno); diff --git a/src/build_log.h b/src/build_log.h index eeac5b3..bb474fc 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -25,6 +25,10 @@ using namespace std; struct Edge; +struct IsDead { + virtual bool IsPathDead(StringPiece s) = 0; +}; + /// Store a log of every command ran for every build. /// It has a few uses: /// @@ -36,7 +40,7 @@ struct BuildLog { BuildLog(); ~BuildLog(); - bool OpenForWrite(const string& path, string* err); + bool OpenForWrite(const string& path, IsDead* is_dead, string* err); // XXX bool RecordCommand(Edge* edge, int start_time, int end_time, TimeStamp restat_mtime = 0); void Close(); @@ -72,7 +76,7 @@ struct BuildLog { bool WriteEntry(FILE* f, const LogEntry& entry); /// Rewrite the known log entries, throwing away old data. - bool Recompact(const string& path, string* err); + bool Recompact(const string& path, IsDead* is_dead, string* err); // XXX typedef ExternalStringHashMap::Type Entries; const Entries& entries() const { return entries_; } diff --git a/src/deps_log.cc b/src/deps_log.cc index 4f1214a..0a3f7e5 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -325,6 +325,13 @@ bool DepsLog::Recompact(const string& path, string* err) { Deps* deps = deps_[old_id]; if (!deps) continue; // If nodes_[old_id] is a leaf, it has no deps. + Node* n = nodes_[old_id]; + Edge* e = n->in_edge(); + // FIXME: move this condition to a helper: (also used in src/ninja.cc) + if (!e || e->GetBinding("deps").empty()) { + continue; + } + if (!new_log.RecordDeps(nodes_[old_id], deps->mtime, deps->node_count, deps->nodes)) { new_log.Close(); diff --git a/src/ninja.cc b/src/ninja.cc index a313ecb..298d993 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -68,7 +68,7 @@ struct Options { /// The Ninja main() loads up a series of data structures; various tools need /// to poke into these, so store them as fields on an object. -struct NinjaMain { +struct NinjaMain : public IsDead { NinjaMain(const char* ninja_command, const BuildConfig& config) : ninja_command_(ninja_command), config_(config) {} @@ -137,6 +137,11 @@ struct NinjaMain { /// Dump the output requested by '-d stats'. void DumpMetrics(); + + virtual bool IsPathDead(StringPiece s) { + Node* n = state_.LookupNode(s); + return !n || !n->in_edge(); + } }; /// Subtools, accessible via "-t foo". @@ -789,14 +794,14 @@ bool NinjaMain::OpenBuildLog(bool recompact_only) { } if (recompact_only) { - bool success = build_log_.Recompact(log_path, &err); + bool success = build_log_.Recompact(log_path, this, &err); if (!success) Error("failed recompaction: %s", err.c_str()); return success; } if (!config_.dry_run) { - if (!build_log_.OpenForWrite(log_path, &err)) { + if (!build_log_.OpenForWrite(log_path, this, &err)) { Error("opening build log: %s", err.c_str()); return false; } -- cgit v0.12 From 4ecc34493fdebeea25f7ddf4392282b8adf39f05 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 1 Jan 2014 20:46:18 -0800 Subject: Make tests compile (one fails atm). --- src/build_log_test.cc | 13 +++++++------ src/build_test.cc | 6 ++++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/build_log_test.cc b/src/build_log_test.cc index 4639bc9..60f7423 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -30,7 +30,7 @@ namespace { const char kTestFilename[] = "BuildLogTest-tempfile"; -struct BuildLogTest : public StateTestWithBuiltinRules { +struct BuildLogTest : public StateTestWithBuiltinRules, public IsDead { virtual void SetUp() { // In case a crashing test left a stale file behind. unlink(kTestFilename); @@ -38,6 +38,7 @@ struct BuildLogTest : public StateTestWithBuiltinRules { virtual void TearDown() { unlink(kTestFilename); } + virtual bool IsPathDead(StringPiece s) { return false; } }; TEST_F(BuildLogTest, WriteRead) { @@ -47,7 +48,7 @@ TEST_F(BuildLogTest, WriteRead) { BuildLog log1; string err; - EXPECT_TRUE(log1.OpenForWrite(kTestFilename, &err)); + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, this, &err)); ASSERT_EQ("", err); log1.RecordCommand(state_.edges_[0], 15, 18); log1.RecordCommand(state_.edges_[1], 20, 25); @@ -75,7 +76,7 @@ TEST_F(BuildLogTest, FirstWriteAddsSignature) { BuildLog log; string contents, err; - EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); + EXPECT_TRUE(log.OpenForWrite(kTestFilename, this, &err)); ASSERT_EQ("", err); log.Close(); @@ -86,7 +87,7 @@ TEST_F(BuildLogTest, FirstWriteAddsSignature) { EXPECT_EQ(kExpectedVersion, contents); // Opening the file anew shouldn't add a second version string. - EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); + EXPECT_TRUE(log.OpenForWrite(kTestFilename, this, &err)); ASSERT_EQ("", err); log.Close(); @@ -122,7 +123,7 @@ TEST_F(BuildLogTest, Truncate) { BuildLog log1; string err; - EXPECT_TRUE(log1.OpenForWrite(kTestFilename, &err)); + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, this, &err)); ASSERT_EQ("", err); log1.RecordCommand(state_.edges_[0], 15, 18); log1.RecordCommand(state_.edges_[1], 20, 25); @@ -137,7 +138,7 @@ TEST_F(BuildLogTest, Truncate) { for (off_t size = statbuf.st_size; size > 0; --size) { BuildLog log2; string err; - EXPECT_TRUE(log2.OpenForWrite(kTestFilename, &err)); + EXPECT_TRUE(log2.OpenForWrite(kTestFilename, this, &err)); ASSERT_EQ("", err); log2.RecordCommand(state_.edges_[0], 15, 18); log2.RecordCommand(state_.edges_[1], 20, 25); diff --git a/src/build_test.cc b/src/build_test.cc index e206cd8..19625c6 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -412,7 +412,7 @@ struct FakeCommandRunner : public CommandRunner { VirtualFileSystem* fs_; }; -struct BuildTest : public StateTestWithBuiltinRules { +struct BuildTest : public StateTestWithBuiltinRules, public IsDead { BuildTest() : config_(MakeConfig()), command_runner_(&fs_), builder_(&state_, config_, NULL, NULL, &fs_), status_(config_) { @@ -435,6 +435,8 @@ struct BuildTest : public StateTestWithBuiltinRules { builder_.command_runner_.release(); } + virtual bool IsPathDead(StringPiece s) { return false; } + /// Rebuild target in the 'working tree' (fs_). /// State of command_runner_ and logs contents (if specified) ARE MODIFIED. /// Handy to check for NOOP builds, and higher-level rebuild tests. @@ -469,7 +471,7 @@ void BuildTest::RebuildTarget(const string& target, const char* manifest, BuildLog build_log, *pbuild_log = NULL; if (log_path) { ASSERT_TRUE(build_log.Load(log_path, &err)); - ASSERT_TRUE(build_log.OpenForWrite(log_path, &err)); + ASSERT_TRUE(build_log.OpenForWrite(log_path, this, &err)); ASSERT_EQ("", err); pbuild_log = &build_log; } -- cgit v0.12 From 7a5fc52601d6e1db5786e0a83d7cc93e2d32a097 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 1 Jan 2014 21:37:47 -0800 Subject: Fix DepsLogTest.Recompact by making sure outputs aren't garbag-collected. --- src/deps_log_test.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 4e6cbac..3304edb 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -163,10 +163,18 @@ TEST_F(DepsLogTest, DoubleEntry) { // Verify that adding the new deps works and can be compacted away. TEST_F(DepsLogTest, Recompact) { + const char kManifest[] = +"rule cc\n" +" command = cc\n" +" deps = gcc\n" +"build out.o: cc\n" +"build other_out.o: cc\n"; + // Write some deps to the file and grab its size. int file_size; { State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest)); DepsLog log; string err; ASSERT_TRUE(log.OpenForWrite(kTestFilename, &err)); @@ -194,6 +202,7 @@ TEST_F(DepsLogTest, Recompact) { int file_size_2; { State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest)); DepsLog log; string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); @@ -217,6 +226,7 @@ TEST_F(DepsLogTest, Recompact) { // recompact. { State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest)); DepsLog log; string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); -- cgit v0.12 From 691474973aeeb4d1002cd3c8fae3e106767e36f6 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 2 Jan 2014 21:16:38 -0800 Subject: Add a test for collection of dead deps entries. --- src/deps_log.cc | 6 ++++++ src/deps_log_test.cc | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 0a3f7e5..157b65c 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -329,6 +329,12 @@ bool DepsLog::Recompact(const string& path, string* err) { Edge* e = n->in_edge(); // FIXME: move this condition to a helper: (also used in src/ninja.cc) if (!e || e->GetBinding("deps").empty()) { + // Skip entries that don't have in-edges or whose edges don't have a + // "deps" attribute. They were in the deps log from previous builds, but + // the the files they were for were removed from the build and their deps + // entries are no longer needed. + // (Without the check for "deps", a chain of two or more nodes that each + // had deps wouldn't be collected in a single recompaction.) continue; } diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 3304edb..e8e5138 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -224,6 +224,7 @@ TEST_F(DepsLogTest, Recompact) { // Now reload the file, verify the new deps have replaced the old, then // recompact. + int file_size_3; { State state; ASSERT_NO_FATAL_FAILURE(AssertParse(&state, kManifest)); @@ -267,9 +268,53 @@ TEST_F(DepsLogTest, Recompact) { // The file should have shrunk a bit for the smaller deps. struct stat st; ASSERT_EQ(0, stat(kTestFilename, &st)); - int file_size_3 = (int)st.st_size; + file_size_3 = (int)st.st_size; ASSERT_LT(file_size_3, file_size_2); } + + // Now reload the file and recompact with an empty manifest. The previous + // entries should be removed. + { + State state; + // Intentionally not parsing kManifest here. + DepsLog log; + string err; + ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); + + Node* out = state.GetNode("out.o"); + DepsLog::Deps* deps = log.GetDeps(out); + ASSERT_TRUE(deps); + ASSERT_EQ(1, deps->mtime); + ASSERT_EQ(1, deps->node_count); + ASSERT_EQ("foo.h", deps->nodes[0]->path()); + + Node* other_out = state.GetNode("other_out.o"); + deps = log.GetDeps(other_out); + ASSERT_TRUE(deps); + ASSERT_EQ(1, deps->mtime); + ASSERT_EQ(2, deps->node_count); + ASSERT_EQ("foo.h", deps->nodes[0]->path()); + ASSERT_EQ("baz.h", deps->nodes[1]->path()); + + ASSERT_TRUE(log.Recompact(kTestFilename, &err)); + + // The previous entries should have been removed. + deps = log.GetDeps(out); + ASSERT_FALSE(deps); + + deps = log.GetDeps(other_out); + ASSERT_FALSE(deps); + + // The .h files pulled in via deps should no longer have ids either. + ASSERT_EQ(-1, state.LookupNode("foo.h")->id()); + ASSERT_EQ(-1, state.LookupNode("baz.h")->id()); + + // The file should have shrunk more. + struct stat st; + ASSERT_EQ(0, stat(kTestFilename, &st)); + int file_size_4 = (int)st.st_size; + ASSERT_LT(file_size_4, file_size_3); + } } // Verify that invalid file headers cause a new build. -- cgit v0.12 From 637b4572ba9c98b812858a37521af1d442b3ccc4 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 3 Jan 2014 20:43:42 -0800 Subject: Add a test for collection of dead log entries. --- src/build_log_test.cc | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/build_log_test.cc b/src/build_log_test.cc index 60f7423..db1e0be 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -262,4 +262,44 @@ TEST_F(BuildLogTest, MultiTargetEdge) { ASSERT_EQ(22, e2->end_time); } +struct BuildLogRecompactTest : public BuildLogTest { + virtual bool IsPathDead(StringPiece s) { return s == "out2"; } +}; + +TEST_F(BuildLogRecompactTest, Recompact) { + AssertParse(&state_, +"build out: cat in\n" +"build out2: cat in\n"); + + BuildLog log1; + string err; + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, this, &err)); + ASSERT_EQ("", err); + // Record the same edge several times, to trigger recompaction + // the next time the log is opened. + for (int i = 0; i < 200; ++i) + log1.RecordCommand(state_.edges_[0], 15, 18 + i); + log1.RecordCommand(state_.edges_[1], 21, 22); + log1.Close(); + + // Load... + BuildLog log2; + EXPECT_TRUE(log2.Load(kTestFilename, &err)); + ASSERT_EQ("", err); + ASSERT_EQ(2u, log2.entries().size()); + ASSERT_TRUE(log2.LookupByOutput("out")); + ASSERT_TRUE(log2.LookupByOutput("out2")); + // ...and force a recompaction. + EXPECT_TRUE(log2.OpenForWrite(kTestFilename, this, &err)); + log2.Close(); + + // "out2" is dead, it should've been removed. + BuildLog log3; + EXPECT_TRUE(log2.Load(kTestFilename, &err)); + ASSERT_EQ("", err); + ASSERT_EQ(1u, log2.entries().size()); + ASSERT_TRUE(log2.LookupByOutput("out")); + ASSERT_FALSE(log2.LookupByOutput("out2")); +} + } // anonymous namespace -- cgit v0.12 From c8b5ad32899503a1f992f7db75fd2c56e8d49238 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 3 Jan 2014 21:12:31 -0800 Subject: Move duplicated code into a helper function. --- src/deps_log.cc | 22 +++++++++++----------- src/deps_log.h | 8 ++++++++ src/ninja.cc | 4 +--- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 157b65c..61df387 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -325,18 +325,8 @@ bool DepsLog::Recompact(const string& path, string* err) { Deps* deps = deps_[old_id]; if (!deps) continue; // If nodes_[old_id] is a leaf, it has no deps. - Node* n = nodes_[old_id]; - Edge* e = n->in_edge(); - // FIXME: move this condition to a helper: (also used in src/ninja.cc) - if (!e || e->GetBinding("deps").empty()) { - // Skip entries that don't have in-edges or whose edges don't have a - // "deps" attribute. They were in the deps log from previous builds, but - // the the files they were for were removed from the build and their deps - // entries are no longer needed. - // (Without the check for "deps", a chain of two or more nodes that each - // had deps wouldn't be collected in a single recompaction.) + if (!IsDepsEntryLiveFor(nodes_[old_id])) continue; - } if (!new_log.RecordDeps(nodes_[old_id], deps->mtime, deps->node_count, deps->nodes)) { @@ -364,6 +354,16 @@ bool DepsLog::Recompact(const string& path, string* err) { return true; } +bool DepsLog::IsDepsEntryLiveFor(Node* node) { + // Skip entries that don't have in-edges or whose edges don't have a + // "deps" attribute. They were in the deps log from previous builds, but + // the the files they were for were removed from the build and their deps + // entries are no longer needed. + // (Without the check for "deps", a chain of two or more nodes that each + // had deps wouldn't be collected in a single recompaction.) + return node->in_edge() && !node->in_edge()->GetBinding("deps").empty(); +} + bool DepsLog::UpdateDeps(int out_id, Deps* deps) { if (out_id >= (int)deps_.size()) deps_.resize(out_id + 1); diff --git a/src/deps_log.h b/src/deps_log.h index babf828..cec0257 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -88,6 +88,14 @@ struct DepsLog { /// Rewrite the known log entries, throwing away old data. bool Recompact(const string& path, string* err); + /// Returns if the deps entry for a node is still reachable from the manifest. + /// + /// The deps log can contain deps entries for files that were built in the + /// past but are no longer part of the manifest. This function returns if + /// this is the case for a given node. This function is slow, don't call + /// it from code that runs on every build. + bool IsDepsEntryLiveFor(Node* node); + /// Used for tests. const vector& nodes() const { return nodes_; } const vector& deps() const { return deps_; } diff --git a/src/ninja.cc b/src/ninja.cc index 298d993..095132e 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -449,9 +449,7 @@ int NinjaMain::ToolDeps(int argc, char** argv) { if (argc == 0) { for (vector::const_iterator ni = deps_log_.nodes().begin(); ni != deps_log_.nodes().end(); ++ni) { - // Only query for targets with an incoming edge and deps - Edge* e = (*ni)->in_edge(); - if (e && !e->GetBinding("deps").empty()) + if (deps_log_.IsDepsEntryLiveFor(*ni)) nodes.push_back(*ni); } } else { -- cgit v0.12 From 1513693dce3aa6aa3af74965b6eb55a3cd37111e Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 3 Jan 2014 21:26:50 -0800 Subject: Rename "IsDead" to "BuildLogUser". --- src/build_log.cc | 9 +++++---- src/build_log.h | 9 ++++++--- src/build_log_perftest.cc | 7 ++++++- src/build_log_test.cc | 16 ++++++++-------- src/build_test.cc | 4 ++-- src/ninja.cc | 6 +++--- 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/build_log.cc b/src/build_log.cc index 825a8f5..c041514 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -108,9 +108,10 @@ BuildLog::~BuildLog() { Close(); } -bool BuildLog::OpenForWrite(const string& path, IsDead* is_dead, string* err) { +bool BuildLog::OpenForWrite(const string& path, BuildLogUser& user, + string* err) { if (needs_recompaction_) { - if (!Recompact(path, is_dead, err)) + if (!Recompact(path, user, err)) return false; } @@ -350,7 +351,7 @@ bool BuildLog::WriteEntry(FILE* f, const LogEntry& entry) { entry.output.c_str(), entry.command_hash) > 0; } -bool BuildLog::Recompact(const string& path, IsDead* is_dead, string* err) { +bool BuildLog::Recompact(const string& path, BuildLogUser& user, string* err) { METRIC_RECORD(".ninja_log recompact"); printf("Recompacting log...\n"); @@ -370,7 +371,7 @@ bool BuildLog::Recompact(const string& path, IsDead* is_dead, string* err) { vector dead_outputs; for (Entries::iterator i = entries_.begin(); i != entries_.end(); ++i) { - if (is_dead->IsPathDead(i->first)) { + if (user.IsPathDead(i->first)) { dead_outputs.push_back(i->first); continue; } diff --git a/src/build_log.h b/src/build_log.h index bb474fc..fc44854 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -25,7 +25,10 @@ using namespace std; struct Edge; -struct IsDead { +/// Can answer questions about the manifest for the BuildLog. +struct BuildLogUser { + /// Return if a given output no longer part of the build manifest. + /// This is only called during recompaction and doesn't have to be fast. virtual bool IsPathDead(StringPiece s) = 0; }; @@ -40,7 +43,7 @@ struct BuildLog { BuildLog(); ~BuildLog(); - bool OpenForWrite(const string& path, IsDead* is_dead, string* err); // XXX + bool OpenForWrite(const string& path, BuildLogUser& user, string* err); bool RecordCommand(Edge* edge, int start_time, int end_time, TimeStamp restat_mtime = 0); void Close(); @@ -76,7 +79,7 @@ struct BuildLog { bool WriteEntry(FILE* f, const LogEntry& entry); /// Rewrite the known log entries, throwing away old data. - bool Recompact(const string& path, IsDead* is_dead, string* err); // XXX + bool Recompact(const string& path, BuildLogUser& user, string* err); typedef ExternalStringHashMap::Type Entries; const Entries& entries() const { return entries_; } diff --git a/src/build_log_perftest.cc b/src/build_log_perftest.cc index a09beb8..12cae99 100644 --- a/src/build_log_perftest.cc +++ b/src/build_log_perftest.cc @@ -28,10 +28,15 @@ const char kTestFilename[] = "BuildLogPerfTest-tempfile"; +struct NoDeadPaths : public BuildLogUser { + virtual bool IsPathDead(StringPiece) { return false; } +}; + bool WriteTestData(string* err) { BuildLog log; - if (!log.OpenForWrite(kTestFilename, err)) + NoDeadPaths no_dead_paths; + if (!log.OpenForWrite(kTestFilename, no_dead_paths, err)) return false; /* diff --git a/src/build_log_test.cc b/src/build_log_test.cc index db1e0be..55f8560 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -30,7 +30,7 @@ namespace { const char kTestFilename[] = "BuildLogTest-tempfile"; -struct BuildLogTest : public StateTestWithBuiltinRules, public IsDead { +struct BuildLogTest : public StateTestWithBuiltinRules, public BuildLogUser { virtual void SetUp() { // In case a crashing test left a stale file behind. unlink(kTestFilename); @@ -48,7 +48,7 @@ TEST_F(BuildLogTest, WriteRead) { BuildLog log1; string err; - EXPECT_TRUE(log1.OpenForWrite(kTestFilename, this, &err)); + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); log1.RecordCommand(state_.edges_[0], 15, 18); log1.RecordCommand(state_.edges_[1], 20, 25); @@ -76,7 +76,7 @@ TEST_F(BuildLogTest, FirstWriteAddsSignature) { BuildLog log; string contents, err; - EXPECT_TRUE(log.OpenForWrite(kTestFilename, this, &err)); + EXPECT_TRUE(log.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); log.Close(); @@ -87,7 +87,7 @@ TEST_F(BuildLogTest, FirstWriteAddsSignature) { EXPECT_EQ(kExpectedVersion, contents); // Opening the file anew shouldn't add a second version string. - EXPECT_TRUE(log.OpenForWrite(kTestFilename, this, &err)); + EXPECT_TRUE(log.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); log.Close(); @@ -123,7 +123,7 @@ TEST_F(BuildLogTest, Truncate) { BuildLog log1; string err; - EXPECT_TRUE(log1.OpenForWrite(kTestFilename, this, &err)); + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); log1.RecordCommand(state_.edges_[0], 15, 18); log1.RecordCommand(state_.edges_[1], 20, 25); @@ -138,7 +138,7 @@ TEST_F(BuildLogTest, Truncate) { for (off_t size = statbuf.st_size; size > 0; --size) { BuildLog log2; string err; - EXPECT_TRUE(log2.OpenForWrite(kTestFilename, this, &err)); + EXPECT_TRUE(log2.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); log2.RecordCommand(state_.edges_[0], 15, 18); log2.RecordCommand(state_.edges_[1], 20, 25); @@ -273,7 +273,7 @@ TEST_F(BuildLogRecompactTest, Recompact) { BuildLog log1; string err; - EXPECT_TRUE(log1.OpenForWrite(kTestFilename, this, &err)); + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, *this, &err)); ASSERT_EQ("", err); // Record the same edge several times, to trigger recompaction // the next time the log is opened. @@ -290,7 +290,7 @@ TEST_F(BuildLogRecompactTest, Recompact) { ASSERT_TRUE(log2.LookupByOutput("out")); ASSERT_TRUE(log2.LookupByOutput("out2")); // ...and force a recompaction. - EXPECT_TRUE(log2.OpenForWrite(kTestFilename, this, &err)); + EXPECT_TRUE(log2.OpenForWrite(kTestFilename, *this, &err)); log2.Close(); // "out2" is dead, it should've been removed. diff --git a/src/build_test.cc b/src/build_test.cc index 19625c6..8a7fc20 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -412,7 +412,7 @@ struct FakeCommandRunner : public CommandRunner { VirtualFileSystem* fs_; }; -struct BuildTest : public StateTestWithBuiltinRules, public IsDead { +struct BuildTest : public StateTestWithBuiltinRules, public BuildLogUser { BuildTest() : config_(MakeConfig()), command_runner_(&fs_), builder_(&state_, config_, NULL, NULL, &fs_), status_(config_) { @@ -471,7 +471,7 @@ void BuildTest::RebuildTarget(const string& target, const char* manifest, BuildLog build_log, *pbuild_log = NULL; if (log_path) { ASSERT_TRUE(build_log.Load(log_path, &err)); - ASSERT_TRUE(build_log.OpenForWrite(log_path, this, &err)); + ASSERT_TRUE(build_log.OpenForWrite(log_path, *this, &err)); ASSERT_EQ("", err); pbuild_log = &build_log; } diff --git a/src/ninja.cc b/src/ninja.cc index 095132e..5bf8221 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -68,7 +68,7 @@ struct Options { /// The Ninja main() loads up a series of data structures; various tools need /// to poke into these, so store them as fields on an object. -struct NinjaMain : public IsDead { +struct NinjaMain : public BuildLogUser { NinjaMain(const char* ninja_command, const BuildConfig& config) : ninja_command_(ninja_command), config_(config) {} @@ -792,14 +792,14 @@ bool NinjaMain::OpenBuildLog(bool recompact_only) { } if (recompact_only) { - bool success = build_log_.Recompact(log_path, this, &err); + bool success = build_log_.Recompact(log_path, *this, &err); if (!success) Error("failed recompaction: %s", err.c_str()); return success; } if (!config_.dry_run) { - if (!build_log_.OpenForWrite(log_path, this, &err)) { + if (!build_log_.OpenForWrite(log_path, *this, &err)) { Error("opening build log: %s", err.c_str()); return false; } -- cgit v0.12 From d571e79baaf58032e291325043d7870eb13cb70b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 3 Jan 2014 21:32:41 -0800 Subject: Add a comment. --- src/ninja.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ninja.cc b/src/ninja.cc index 5bf8221..008c052 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -140,6 +140,13 @@ struct NinjaMain : public BuildLogUser { virtual bool IsPathDead(StringPiece s) { Node* n = state_.LookupNode(s); + // Just checking n isn't enough: If an old output is both in the build log + // and in the deps log, it will have a Node object in state_. (It will also + // have an in edge if one of its inputs is another output that's in the deps + // log, but having a deps edge product an output thats input to another deps + // edge is rare, and the first recompaction will delete all old outputs from + // the deps log, and then a second recompaction will clear the build log, + // which seems good enough for this corner case.) return !n || !n->in_edge(); } }; -- cgit v0.12 From 664edba804ddd5363299f2af5183109bdc9715b1 Mon Sep 17 00:00:00 2001 From: Nicholas Hutchinson Date: Sat, 30 Nov 2013 22:33:02 +0000 Subject: More robust escaping of $in, $out paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In summary: don’t escape if the path doesn’t contain problematic characters, otherwise: - Shell: enclose string in single quotes, escape embedded single quotes with the magic quote-backslash-quote sequence - Win32: Escape double quotes by doubling the number of consecutive backslashes that precede them (if any) and adding one more. Finally, double the number of trailing backslashes, and enclose the whole thing in double quotes. --- src/graph.cc | 15 +++---- src/graph_test.cc | 11 +++-- src/manifest_parser_test.cc | 4 ++ src/util.cc | 104 ++++++++++++++++++++++++++++++++++++++++++++ src/util.h | 7 +++ src/util_test.cc | 23 ++++++++++ 6 files changed, 152 insertions(+), 12 deletions(-) diff --git a/src/graph.cc b/src/graph.cc index 9801a7b..65f9244 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -250,16 +250,13 @@ string EdgeEnv::MakePathList(vector::iterator begin, char sep) { string result; for (vector::iterator i = begin; i != end; ++i) { - if (!result.empty()) - result.push_back(sep); + if (!result.empty()) result.push_back(sep); const string& path = (*i)->path(); - if (path.find(" ") != string::npos) { - result.append("\""); - result.append(path); - result.append("\""); - } else { - result.append(path); - } +#if _WIN32 + GetWin32EscapedString(path, &result); +#else + GetShellEscapedString(path, &result); +#endif } return result; } diff --git a/src/graph_test.cc b/src/graph_test.cc index 8521216..14dc678 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -139,13 +139,18 @@ TEST_F(GraphTest, RootNodes) { } } -TEST_F(GraphTest, VarInOutQuoteSpaces) { +TEST_F(GraphTest, VarInOutPathEscaping) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"build a$ b: cat nospace with$ space nospace2\n")); +"build a$ b: cat no'space with$ space$$ no\"space2\n")); Edge* edge = GetNode("a b")->in_edge(); - EXPECT_EQ("cat nospace \"with space\" nospace2 > \"a b\"", +#if _WIN32 + EXPECT_EQ("cat no'space \"with space$\" \"no\\\"space2\" > \"a b\"", edge->EvaluateCommand()); +#else + EXPECT_EQ("cat 'no'\\''space' 'with space$' 'no\"space2' > 'a b'", + edge->EvaluateCommand()); +#endif } // Regression test for https://github.com/martine/ninja/issues/380 diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 0b4c042..152b965 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -236,7 +236,11 @@ TEST_F(ParserTest, Dollars) { "build $x: foo y\n" )); EXPECT_EQ("$dollar", state.bindings_.LookupVariable("x")); +#ifdef _WIN32 EXPECT_EQ("$dollarbar$baz$blah", state.edges_[0]->EvaluateCommand()); +#else + EXPECT_EQ("'$dollar'bar$baz$blah", state.edges_[0]->EvaluateCommand()); +#endif } TEST_F(ParserTest, EscapeSpaces) { diff --git a/src/util.cc b/src/util.cc index 6ba3c6c..0e4dc59 100644 --- a/src/util.cc +++ b/src/util.cc @@ -20,6 +20,7 @@ #include #endif +#include #include #include #include @@ -175,6 +176,109 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { return true; } +static inline bool IsKnownShellSafeCharacter(char ch) { + if ('A' <= ch && ch <= 'Z') return true; + if ('a' <= ch && ch <= 'z') return true; + if ('0' <= ch && ch <= '9') return true; + + switch (ch) { + case '_': + case '-': + case '.': + case '/': + return true; + default: + return false; + } +} + +static inline bool IsKnownWin32SafeCharacter(char ch) { + switch (ch) { + case '\\': + case ' ': + case '"': + return false; + default: + return true; + } +} + +static inline bool StringNeedsShellEscaping(const string& input) { + for (size_t i = 0; i < input.size(); ++i) { + if (!IsKnownShellSafeCharacter(input[i])) return true; + } + return false; +} + +static inline bool StringNeedsWin32Escaping(const string& input) { + for (size_t i = 0; i < input.size(); ++i) { + if (!IsKnownWin32SafeCharacter(input[i])) return true; + } + return false; +} + +void GetShellEscapedString(const string& input, string* result) { + assert(result); + + if (!StringNeedsShellEscaping(input)) { + result->append(input); + return; + } + + const char kQuote = '\''; + const char kEscapeSequence[] = "'\\'"; + + result->push_back(kQuote); + + string::const_iterator span_begin = input.begin(); + for (string::const_iterator it = input.begin(), end = input.end(); it != end; + ++it) { + if (*it == kQuote) { + result->append(span_begin, it); + result->append(kEscapeSequence); + span_begin = it; + } + } + result->append(span_begin, input.end()); + result->push_back(kQuote); +} + + +void GetWin32EscapedString(const string& input, string* result) { + assert(result); + if (!StringNeedsWin32Escaping(input)) { + result->append(input); + return; + } + + const char kQuote = '"'; + const char kBackslash = '\\'; + + result->push_back(kQuote); + size_t consecutive_backslash_count = 0; + string::const_iterator span_begin = input.begin(); + for (string::const_iterator it = input.begin(), end = input.end(); it != end; + ++it) { + switch (*it) { + case kBackslash: + ++consecutive_backslash_count; + break; + case kQuote: + result->append(span_begin, it); + result->append(consecutive_backslash_count + 1, kBackslash); + span_begin = it; + consecutive_backslash_count = 0; + break; + default: + consecutive_backslash_count = 0; + break; + } + } + result->append(span_begin, input.end()); + result->append(consecutive_backslash_count, kBackslash); + result->push_back(kQuote); +} + int ReadFile(const string& path, string* contents, string* err) { FILE* f = fopen(path.c_str(), "r"); if (!f) { diff --git a/src/util.h b/src/util.h index 6788410..7101770 100644 --- a/src/util.h +++ b/src/util.h @@ -45,6 +45,13 @@ bool CanonicalizePath(string* path, string* err); bool CanonicalizePath(char* path, size_t* len, string* err); +/// Appends |input| to |*result|, escaping according to the whims of either +/// Bash, or Win32's CommandLineToArgvW(). +/// Appends the string directly to |result| without modification if we can +/// determine that it contains no problematic characters. +void GetShellEscapedString(const string& input, string* result); +void GetWin32EscapedString(const string& input, string* result); + /// Read a file to a string (in text mode: with CRLF conversion /// on Windows). /// Returns -errno and fills in \a err on error. diff --git a/src/util_test.cc b/src/util_test.cc index 1e29053..f6728fb 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -136,6 +136,29 @@ TEST(CanonicalizePath, NotNullTerminated) { EXPECT_EQ("file ./file bar/.", string(path)); } +TEST(PathEscaping, TortureTest) { + string result; + + GetWin32EscapedString("foo bar\\\"'$@d!st!c'\\path'\\", &result); + EXPECT_EQ("\"foo bar\\\\\\\"'$@d!st!c'\\path'\\\\\"", result); + result.clear(); + + GetShellEscapedString("foo bar\"/'$@d!st!c'/path'", &result); + EXPECT_EQ("'foo bar\"/'\\''$@d!st!c'\\''/path'\\'''", result); +} + +TEST(PathEscaping, SensiblePathsAreNotNeedlesslyEscaped) { + const char* path = "some/sensible/path/without/crazy/characters.cc"; + string result; + + GetWin32EscapedString(path, &result); + EXPECT_EQ(path, result); + result.clear(); + + GetShellEscapedString(path, &result); + EXPECT_EQ(path, result); +} + TEST(StripAnsiEscapeCodes, EscapeAtEnd) { string stripped = StripAnsiEscapeCodes("foo\33"); EXPECT_EQ("foo", stripped); -- cgit v0.12 From 8e20867e1de55489d47ec65552a2f561126c070b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 6 Jan 2014 16:15:23 -0800 Subject: Make BuildLogUser reference constant. --- src/build_log.cc | 5 +++-- src/build_log.h | 6 +++--- src/build_log_perftest.cc | 2 +- src/build_log_test.cc | 4 ++-- src/build_test.cc | 2 +- src/ninja.cc | 2 +- src/state.cc | 4 ++-- src/state.h | 2 +- 8 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/build_log.cc b/src/build_log.cc index c041514..3f24c16 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -108,7 +108,7 @@ BuildLog::~BuildLog() { Close(); } -bool BuildLog::OpenForWrite(const string& path, BuildLogUser& user, +bool BuildLog::OpenForWrite(const string& path, const BuildLogUser& user, string* err) { if (needs_recompaction_) { if (!Recompact(path, user, err)) @@ -351,7 +351,8 @@ bool BuildLog::WriteEntry(FILE* f, const LogEntry& entry) { entry.output.c_str(), entry.command_hash) > 0; } -bool BuildLog::Recompact(const string& path, BuildLogUser& user, string* err) { +bool BuildLog::Recompact(const string& path, const BuildLogUser& user, + string* err) { METRIC_RECORD(".ninja_log recompact"); printf("Recompacting log...\n"); diff --git a/src/build_log.h b/src/build_log.h index fc44854..fe81a85 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -29,7 +29,7 @@ struct Edge; struct BuildLogUser { /// Return if a given output no longer part of the build manifest. /// This is only called during recompaction and doesn't have to be fast. - virtual bool IsPathDead(StringPiece s) = 0; + virtual bool IsPathDead(StringPiece s) const = 0; }; /// Store a log of every command ran for every build. @@ -43,7 +43,7 @@ struct BuildLog { BuildLog(); ~BuildLog(); - bool OpenForWrite(const string& path, BuildLogUser& user, string* err); + bool OpenForWrite(const string& path, const BuildLogUser& user, string* err); bool RecordCommand(Edge* edge, int start_time, int end_time, TimeStamp restat_mtime = 0); void Close(); @@ -79,7 +79,7 @@ struct BuildLog { bool WriteEntry(FILE* f, const LogEntry& entry); /// Rewrite the known log entries, throwing away old data. - bool Recompact(const string& path, BuildLogUser& user, string* err); + bool Recompact(const string& path, const BuildLogUser& user, string* err); typedef ExternalStringHashMap::Type Entries; const Entries& entries() const { return entries_; } diff --git a/src/build_log_perftest.cc b/src/build_log_perftest.cc index 12cae99..810c065 100644 --- a/src/build_log_perftest.cc +++ b/src/build_log_perftest.cc @@ -29,7 +29,7 @@ const char kTestFilename[] = "BuildLogPerfTest-tempfile"; struct NoDeadPaths : public BuildLogUser { - virtual bool IsPathDead(StringPiece) { return false; } + virtual bool IsPathDead(StringPiece) const { return false; } }; bool WriteTestData(string* err) { diff --git a/src/build_log_test.cc b/src/build_log_test.cc index 55f8560..6738c7b 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -38,7 +38,7 @@ struct BuildLogTest : public StateTestWithBuiltinRules, public BuildLogUser { virtual void TearDown() { unlink(kTestFilename); } - virtual bool IsPathDead(StringPiece s) { return false; } + virtual bool IsPathDead(StringPiece s) const { return false; } }; TEST_F(BuildLogTest, WriteRead) { @@ -263,7 +263,7 @@ TEST_F(BuildLogTest, MultiTargetEdge) { } struct BuildLogRecompactTest : public BuildLogTest { - virtual bool IsPathDead(StringPiece s) { return s == "out2"; } + virtual bool IsPathDead(StringPiece s) const { return s == "out2"; } }; TEST_F(BuildLogRecompactTest, Recompact) { diff --git a/src/build_test.cc b/src/build_test.cc index 8a7fc20..86a911b 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -435,7 +435,7 @@ struct BuildTest : public StateTestWithBuiltinRules, public BuildLogUser { builder_.command_runner_.release(); } - virtual bool IsPathDead(StringPiece s) { return false; } + virtual bool IsPathDead(StringPiece s) const { return false; } /// Rebuild target in the 'working tree' (fs_). /// State of command_runner_ and logs contents (if specified) ARE MODIFIED. diff --git a/src/ninja.cc b/src/ninja.cc index 008c052..03ca83b 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -138,7 +138,7 @@ struct NinjaMain : public BuildLogUser { /// Dump the output requested by '-d stats'. void DumpMetrics(); - virtual bool IsPathDead(StringPiece s) { + virtual bool IsPathDead(StringPiece s) const { Node* n = state_.LookupNode(s); // Just checking n isn't enough: If an old output is both in the build log // and in the deps log, it will have a Node object in state_. (It will also diff --git a/src/state.cc b/src/state.cc index 9b6160b..33f8423 100644 --- a/src/state.cc +++ b/src/state.cc @@ -118,9 +118,9 @@ Node* State::GetNode(StringPiece path) { return node; } -Node* State::LookupNode(StringPiece path) { +Node* State::LookupNode(StringPiece path) const { METRIC_RECORD("lookup node"); - Paths::iterator i = paths_.find(path); + Paths::const_iterator i = paths_.find(path); if (i != paths_.end()) return i->second; return NULL; diff --git a/src/state.h b/src/state.h index bde75ff..bcb0eff 100644 --- a/src/state.h +++ b/src/state.h @@ -95,7 +95,7 @@ struct State { Edge* AddEdge(const Rule* rule); Node* GetNode(StringPiece path); - Node* LookupNode(StringPiece path); + Node* LookupNode(StringPiece path) const; Node* SpellcheckNode(const string& path); void AddIn(Edge* edge, StringPiece path); -- cgit v0.12 From 5d03a85eaa10804243686acdd1e5fd6a71831176 Mon Sep 17 00:00:00 2001 From: Mostyn Bramley-Moore Date: Tue, 7 Jan 2014 10:33:09 +0100 Subject: don't Fail if trying to mkdir when the dir already exists --- src/disk_interface.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 3233144..090e94e 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -146,6 +146,9 @@ bool RealDiskInterface::WriteFile(const string& path, const string& contents) { bool RealDiskInterface::MakeDir(const string& path) { if (::MakeDir(path) < 0) { + if (errno == EEXIST) { + return true; + } Error("mkdir(%s): %s", path.c_str(), strerror(errno)); return false; } -- cgit v0.12 From 98a33759ddfe166ac684db4f4e1d0e174c89d2b5 Mon Sep 17 00:00:00 2001 From: Nicholas Hutchinson Date: Thu, 9 Jan 2014 09:38:20 +1300 Subject: =?UTF-8?q?Don=E2=80=99t=20unnecessarily=20escape=20backslashes=20?= =?UTF-8?q?in=20Win32=20paths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under ::CommandLineToArgvW() rules, the backslash character only gets special treatment if it’s immediately followed by a double quote. So, when checking to see if a string needs Win32 escaping, it’s sufficient to check for the presence of a double quote character. This allows paths like "foo\bar" to be recognised as “sensible” paths, which don’t require the full escaping. --- src/util.cc | 1 - src/util_test.cc | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/util.cc b/src/util.cc index 0e4dc59..24d231f 100644 --- a/src/util.cc +++ b/src/util.cc @@ -194,7 +194,6 @@ static inline bool IsKnownShellSafeCharacter(char ch) { static inline bool IsKnownWin32SafeCharacter(char ch) { switch (ch) { - case '\\': case ' ': case '"': return false; diff --git a/src/util_test.cc b/src/util_test.cc index f6728fb..f827e5a 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -159,6 +159,14 @@ TEST(PathEscaping, SensiblePathsAreNotNeedlesslyEscaped) { EXPECT_EQ(path, result); } +TEST(PathEscaping, SensibleWin32PathsAreNotNeedlesslyEscaped) { + const char* path = "some\\sensible\\path\\without\\crazy\\characters.cc"; + string result; + + GetWin32EscapedString(path, &result); + EXPECT_EQ(path, result); +} + TEST(StripAnsiEscapeCodes, EscapeAtEnd) { string stripped = StripAnsiEscapeCodes("foo\33"); EXPECT_EQ("foo", stripped); -- cgit v0.12 From c0b1e4d75e2839add8c856ce6ceabb3e91a8666a Mon Sep 17 00:00:00 2001 From: Nicholas Hutchinson Date: Thu, 9 Jan 2014 15:35:36 +1300 Subject: configure.py: use /FS flag under vs2013 when compiling gtest --- configure.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/configure.py b/configure.py index cceb0f7..da2f6ef 100755 --- a/configure.py +++ b/configure.py @@ -328,7 +328,10 @@ if options.with_gtest: gtest_all_incs = '-I%s -I%s' % (path, os.path.join(path, 'include')) if platform.is_msvc(): - gtest_cflags = '/nologo /EHsc /Zi /D_VARIADIC_MAX=10 ' + gtest_all_incs + gtest_cflags = '/nologo /EHsc /Zi /D_VARIADIC_MAX=10 ' + if platform.msvc_needs_fs(): + gtest_cflags += '/FS ' + gtest_cflags += gtest_all_incs else: gtest_cflags = '-fvisibility=hidden ' + gtest_all_incs objs += n.build(built('gtest-all' + objext), 'cxx', -- cgit v0.12 From e4dc39bdbf3ce36cb80183daa51c81587055fabf Mon Sep 17 00:00:00 2001 From: Yasuyuki Oka Date: Tue, 21 Jan 2014 22:19:19 +0900 Subject: emacs: ELPA compatibility --- misc/ninja-mode.el | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index d939206..8c6a197 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -1,3 +1,5 @@ +;;; ninja-mode.el --- Major mode for editing .ninja files + ;; Copyright 2011 Google Inc. All Rights Reserved. ;; ;; Licensed under the Apache License, Version 2.0 (the "License"); @@ -12,9 +14,13 @@ ;; See the License for the specific language governing permissions and ;; limitations under the License. +;;; Commentary: + ;; Simple emacs mode for editing .ninja files. ;; Just some syntax highlighting for now. +;;; Code: + (setq ninja-keywords (list '("^#.*" . font-lock-comment-face) @@ -40,3 +46,5 @@ ;; Run ninja-mode for files ending in .ninja. ;;;###autoload (add-to-list 'auto-mode-alist '("\\.ninja$" . ninja-mode)) + +;;; ninja-mode.el ends here -- cgit v0.12 From 8596ba60814f2ca57bf4969253b09807b4f3cac4 Mon Sep 17 00:00:00 2001 From: Yasuyuki Oka Date: Tue, 21 Jan 2014 22:29:41 +0900 Subject: emacs: Fix a warning Fix a following byte-compile warning: ninja-mode.el:18:7:Warning: assignment to free variable `ninja-keywords' --- misc/ninja-mode.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index d939206..cd31be6 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -15,7 +15,7 @@ ;; Simple emacs mode for editing .ninja files. ;; Just some syntax highlighting for now. -(setq ninja-keywords +(defvar ninja-keywords (list '("^#.*" . font-lock-comment-face) (cons (concat "^" (regexp-opt '("rule" "build" "subninja" "include" -- cgit v0.12 From 9da5b0c862326c837e7d131fadcd8866d9053922 Mon Sep 17 00:00:00 2001 From: Yasuyuki Oka Date: Tue, 21 Jan 2014 22:33:35 +0900 Subject: emacs: Place provide form at end of file Any errors in the code after "provide" will leave the feature registered, so it should always be at the end. --- misc/ninja-mode.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index d939206..e4d9006 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -35,8 +35,8 @@ (setq font-lock-defaults '(ninja-keywords t)) ) -(provide 'ninja-mode) - ;; Run ninja-mode for files ending in .ninja. ;;;###autoload (add-to-list 'auto-mode-alist '("\\.ninja$" . ninja-mode)) + +(provide 'ninja-mode) -- cgit v0.12 From 2b9fefbdd471bd1b276fd5eadd5d8043dcc7f8ac Mon Sep 17 00:00:00 2001 From: Steve Purcell Date: Wed, 22 Jan 2014 12:01:16 +0000 Subject: ninja-mode: add autoload cookie for the mode itself --- misc/ninja-mode.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 54b9b62..9fe6fc8 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -34,6 +34,8 @@ ;; Rule names '("rule \\([[:alnum:]_]+\\)" . (1 font-lock-function-name-face)) )) + +;;;###autoload (define-derived-mode ninja-mode fundamental-mode "ninja" (setq comment-start "#") ; Pass extra "t" to turn off syntax-based fontification -- we don't want -- cgit v0.12 From 2832613dc7c1a4a8ff3b9df729954715762a8381 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Wed, 29 Jan 2014 21:57:58 -0800 Subject: Introduce the "console" pool This is a pre-defined pool with a depth of 1. It has the special property that any task in the pool has direct access to the console. This can be useful for interactive tasks or long-running tasks which produce status updates on the console (such as test suites). --- doc/manual.asciidoc | 17 +++++++++++++ src/build.cc | 14 +++++++++-- src/build_test.cc | 64 +++++++++++++++++++++++++++++++++++++++++-------- src/graph.cc | 4 ++++ src/graph.h | 1 + src/line_printer.cc | 51 ++++++++++++++++++++++++++++++++++----- src/line_printer.h | 20 ++++++++++++++++ src/manifest_parser.cc | 4 ++++ src/state.cc | 2 ++ src/state.h | 1 + src/subprocess-posix.cc | 50 +++++++++++++++++++++----------------- src/subprocess-win32.cc | 5 +++- src/subprocess.h | 8 +++++-- src/subprocess_test.cc | 15 ++++++++++++ 14 files changed, 213 insertions(+), 43 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 67fcbfd..5b0c1fe 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -648,6 +648,23 @@ build heavy_object2.obj: cc heavy_obj2.cc ---------------- +The `console` pool +^^^^^^^^^^^^^^^^^^ + +_Available since Ninja 1.5._ + +There exists a pre-defined pool named `console` with a depth of 1. It has +the special property that any task in the pool has direct access to the +standard input, output and error streams provided to Ninja, which are +normally connected to the user's console (hence the name) but could be +redirected. This can be useful for interactive tasks or long-running tasks +which produce status updates on the console (such as test suites). + +While a task in the `console` pool is running, Ninja's regular output (such +as progress status and output from concurrent tasks) is buffered until +it completes. + +This feature is not yet available on Windows. Ninja file reference -------------------- diff --git a/src/build.cc b/src/build.cc index f91ff2f..91f1754 100644 --- a/src/build.cc +++ b/src/build.cc @@ -97,6 +97,9 @@ void BuildStatus::BuildEdgeStarted(Edge* edge) { ++started_edges_; PrintStatus(edge); + + if (edge->use_console()) + printer_.SetConsoleLocked(true); } void BuildStatus::BuildEdgeFinished(Edge* edge, @@ -112,10 +115,13 @@ void BuildStatus::BuildEdgeFinished(Edge* edge, *end_time = (int)(now - start_time_millis_); running_edges_.erase(i); + if (edge->use_console()) + printer_.SetConsoleLocked(false); + if (config_.verbosity == BuildConfig::QUIET) return; - if (printer_.is_smart_terminal()) + if (!edge->use_console() && printer_.is_smart_terminal()) PrintStatus(edge); // Print the command that is spewing before printing its output. @@ -145,6 +151,7 @@ void BuildStatus::BuildEdgeFinished(Edge* edge, } void BuildStatus::BuildFinished() { + printer_.SetConsoleLocked(false); printer_.PrintOnNewLine(""); } @@ -488,7 +495,7 @@ bool RealCommandRunner::CanRunMore() { bool RealCommandRunner::StartCommand(Edge* edge) { string command = edge->EvaluateCommand(); - Subprocess* subproc = subprocs_.Add(command); + Subprocess* subproc = subprocs_.Add(command, edge->use_console()); if (!subproc) return false; subproc_to_edge_.insert(make_pair(subproc, edge)); @@ -610,6 +617,7 @@ bool Builder::Build(string* err) { if (failures_allowed && command_runner_->CanRunMore()) { if (Edge* edge = plan_.FindWork()) { if (!StartEdge(edge, err)) { + Cleanup(); status_->BuildFinished(); return false; } @@ -630,6 +638,7 @@ bool Builder::Build(string* err) { CommandRunner::Result result; if (!command_runner_->WaitForCommand(&result) || result.status == ExitInterrupted) { + Cleanup(); status_->BuildFinished(); *err = "interrupted by user"; return false; @@ -637,6 +646,7 @@ bool Builder::Build(string* err) { --pending_commands; if (!FinishCommand(&result, err)) { + Cleanup(); status_->BuildFinished(); return false; } diff --git a/src/build_test.cc b/src/build_test.cc index 86a911b..119521e 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -44,6 +44,8 @@ struct PlanTest : public StateTestWithBuiltinRules { ASSERT_FALSE(plan_.FindWork()); sort(ret->begin(), ret->end(), CompareEdgesByOutput::cmp); } + + void TestPoolWithDepthOne(const char *test_case); }; TEST_F(PlanTest, Basic) { @@ -197,15 +199,8 @@ TEST_F(PlanTest, DependencyCycle) { ASSERT_EQ("dependency cycle: out -> mid -> in -> pre -> out", err); } -TEST_F(PlanTest, PoolWithDepthOne) { - ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"pool foobar\n" -" depth = 1\n" -"rule poolcat\n" -" command = cat $in > $out\n" -" pool = foobar\n" -"build out1: poolcat in\n" -"build out2: poolcat in\n")); +void PlanTest::TestPoolWithDepthOne(const char* test_case) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, test_case)); GetNode("out1")->MarkDirty(); GetNode("out2")->MarkDirty(); string err; @@ -239,6 +234,28 @@ TEST_F(PlanTest, PoolWithDepthOne) { ASSERT_EQ(0, edge); } +TEST_F(PlanTest, PoolWithDepthOne) { + TestPoolWithDepthOne( +"pool foobar\n" +" depth = 1\n" +"rule poolcat\n" +" command = cat $in > $out\n" +" pool = foobar\n" +"build out1: poolcat in\n" +"build out2: poolcat in\n"); +} + +#ifndef _WIN32 +TEST_F(PlanTest, ConsolePool) { + TestPoolWithDepthOne( +"rule poolcat\n" +" command = cat $in > $out\n" +" pool = console\n" +"build out1: poolcat in\n" +"build out2: poolcat in\n"); +} +#endif + TEST_F(PlanTest, PoolsWithDepthTwo) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "pool foobar\n" @@ -515,7 +532,8 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { } } else if (edge->rule().name() == "true" || edge->rule().name() == "fail" || - edge->rule().name() == "interrupt") { + edge->rule().name() == "interrupt" || + edge->rule().name() == "console") { // Don't do anything. } else { printf("unknown command\n"); @@ -539,6 +557,15 @@ bool FakeCommandRunner::WaitForCommand(Result* result) { return true; } + if (edge->rule().name() == "console") { + if (edge->use_console()) + result->status = ExitSuccess; + else + result->status = ExitFailure; + last_command_ = NULL; + return true; + } + if (edge->rule().name() == "fail") result->status = ExitFailure; else @@ -1911,3 +1938,20 @@ TEST_F(BuildWithDepsLogTest, RestatMissingDepfileDepslog) { RebuildTarget("out", manifest, "build_log", "ninja_deps2"); ASSERT_EQ(0u, command_runner_.commands_ran_.size()); } + +TEST_F(BuildTest, Console) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule console\n" +" command = console\n" +" pool = console\n" +"build con: console in.txt\n")); + + fs_.Create("in.txt", ""); + + string err; + EXPECT_TRUE(builder_.AddTarget("con", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ("", err); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); +} diff --git a/src/graph.cc b/src/graph.cc index 65f9244..7121342 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -305,6 +305,10 @@ bool Edge::is_phony() const { return rule_ == &State::kPhonyRule; } +bool Edge::use_console() const { + return pool() == &State::kConsolePool; +} + void Node::Dump(const char* prefix) const { printf("%s <%s 0x%p> mtime: %d%s, (:%s), ", prefix, path().c_str(), this, diff --git a/src/graph.h b/src/graph.h index 868413c..6cd7f25 100644 --- a/src/graph.h +++ b/src/graph.h @@ -183,6 +183,7 @@ struct Edge { } bool is_phony() const; + bool use_console() const; }; diff --git a/src/line_printer.cc b/src/line_printer.cc index 3537e88..ef1609c 100644 --- a/src/line_printer.cc +++ b/src/line_printer.cc @@ -26,7 +26,7 @@ #include "util.h" -LinePrinter::LinePrinter() : have_blank_line_(true) { +LinePrinter::LinePrinter() : have_blank_line_(true), console_locked_(false) { #ifndef _WIN32 const char* term = getenv("TERM"); smart_terminal_ = isatty(1) && term && string(term) != "dumb"; @@ -43,6 +43,12 @@ LinePrinter::LinePrinter() : have_blank_line_(true) { } void LinePrinter::Print(string to_print, LineType type) { + if (console_locked_) { + line_buffer_ = to_print; + line_type_ = type; + return; + } + #ifdef _WIN32 CONSOLE_SCREEN_BUFFER_INFO csbi; GetConsoleScreenBufferInfo(console_, &csbi); @@ -101,13 +107,46 @@ void LinePrinter::Print(string to_print, LineType type) { } } -void LinePrinter::PrintOnNewLine(const string& to_print) { - if (!have_blank_line_) - printf("\n"); - if (!to_print.empty()) { +void LinePrinter::PrintOrBuffer(const char* data, size_t size) { + if (console_locked_) { + output_buffer_.append(data, size); + } else { // Avoid printf and C strings, since the actual output might contain null // bytes like UTF-16 does (yuck). - fwrite(&to_print[0], sizeof(char), to_print.size(), stdout); + fwrite(data, 1, size, stdout); + } +} + +void LinePrinter::PrintOnNewLine(const string& to_print) { + if (console_locked_ && !line_buffer_.empty()) { + output_buffer_.append(line_buffer_); + output_buffer_.append(1, '\n'); + line_buffer_.clear(); + } + if (!have_blank_line_) { + PrintOrBuffer("\n", 1); + } + if (!to_print.empty()) { + PrintOrBuffer(&to_print[0], to_print.size()); } have_blank_line_ = to_print.empty() || *to_print.rbegin() == '\n'; } + +void LinePrinter::SetConsoleLocked(bool locked) { + if (locked == console_locked_) + return; + + if (locked) + PrintOnNewLine(""); + + console_locked_ = locked; + + if (!locked) { + PrintOnNewLine(output_buffer_); + if (!line_buffer_.empty()) { + Print(line_buffer_, line_type_); + } + output_buffer_.clear(); + line_buffer_.clear(); + } +} diff --git a/src/line_printer.h b/src/line_printer.h index aea2817..55225e5 100644 --- a/src/line_printer.h +++ b/src/line_printer.h @@ -15,6 +15,7 @@ #ifndef NINJA_LINE_PRINTER_H_ #define NINJA_LINE_PRINTER_H_ +#include #include using namespace std; @@ -37,6 +38,10 @@ struct LinePrinter { /// Prints a string on a new line, not overprinting previous output. void PrintOnNewLine(const string& to_print); + /// Lock or unlock the console. Any output sent to the LinePrinter while the + /// console is locked will not be printed until it is unlocked. + void SetConsoleLocked(bool locked); + private: /// Whether we can do fancy terminal control codes. bool smart_terminal_; @@ -44,9 +49,24 @@ struct LinePrinter { /// Whether the caret is at the beginning of a blank line. bool have_blank_line_; + /// Whether console is locked. + bool console_locked_; + + /// Buffered current line while console is locked. + string line_buffer_; + + /// Buffered line type while console is locked. + LineType line_type_; + + /// Buffered console output while console is locked. + string output_buffer_; + #ifdef _WIN32 void* console_; #endif + + /// Print the given data to the console, or buffer it if it is locked. + void PrintOrBuffer(const char *data, size_t size); }; #endif // NINJA_LINE_PRINTER_H_ diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 20be7f3..17584dd 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -316,6 +316,10 @@ bool ManifestParser::ParseEdge(string* err) { Pool* pool = state_->LookupPool(pool_name); if (pool == NULL) return lexer_.Error("unknown pool name '" + pool_name + "'", err); +#ifdef _WIN32 + if (pool == &State::kConsolePool) + return lexer_.Error("console pool unsupported on Windows", err); +#endif edge->pool_ = pool; } diff --git a/src/state.cc b/src/state.cc index 33f8423..7258272 100644 --- a/src/state.cc +++ b/src/state.cc @@ -69,11 +69,13 @@ bool Pool::WeightedEdgeCmp(const Edge* a, const Edge* b) { } Pool State::kDefaultPool("", 0); +Pool State::kConsolePool("console", 1); const Rule State::kPhonyRule("phony"); State::State() { AddRule(&kPhonyRule); AddPool(&kDefaultPool); + AddPool(&kConsolePool); } void State::AddRule(const Rule* rule) { diff --git a/src/state.h b/src/state.h index bcb0eff..c382dc0 100644 --- a/src/state.h +++ b/src/state.h @@ -82,6 +82,7 @@ struct Pool { /// Global state (file status, loaded rules) for a single run. struct State { static Pool kDefaultPool; + static Pool kConsolePool; static const Rule kPhonyRule; State(); diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index a9af756..793d48f 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -25,7 +25,8 @@ #include "util.h" -Subprocess::Subprocess() : fd_(-1), pid_(-1) { +Subprocess::Subprocess(bool use_console) : fd_(-1), pid_(-1), + use_console_(use_console) { } Subprocess::~Subprocess() { if (fd_ >= 0) @@ -58,29 +59,31 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { // Track which fd we use to report errors on. int error_pipe = output_pipe[1]; do { - if (setpgid(0, 0) < 0) - break; - if (sigaction(SIGINT, &set->old_act_, 0) < 0) break; if (sigprocmask(SIG_SETMASK, &set->old_mask_, 0) < 0) break; - // Open /dev/null over stdin. - int devnull = open("/dev/null", O_RDONLY); - if (devnull < 0) - break; - if (dup2(devnull, 0) < 0) - break; - close(devnull); - - if (dup2(output_pipe[1], 1) < 0 || - dup2(output_pipe[1], 2) < 0) - break; - - // Now can use stderr for errors. - error_pipe = 2; - close(output_pipe[1]); + if (!use_console_) { + if (setpgid(0, 0) < 0) + break; + + // Open /dev/null over stdin. + int devnull = open("/dev/null", O_RDONLY); + if (devnull < 0) + break; + if (dup2(devnull, 0) < 0) + break; + close(devnull); + + if (dup2(output_pipe[1], 1) < 0 || + dup2(output_pipe[1], 2) < 0) + break; + + // Now can use stderr for errors. + error_pipe = 2; + close(output_pipe[1]); + } execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL); } while (false); @@ -168,8 +171,8 @@ SubprocessSet::~SubprocessSet() { Fatal("sigprocmask: %s", strerror(errno)); } -Subprocess *SubprocessSet::Add(const string& command) { - Subprocess *subprocess = new Subprocess; +Subprocess *SubprocessSet::Add(const string& command, bool use_console) { + Subprocess *subprocess = new Subprocess(use_console); if (!subprocess->Start(this, command)) { delete subprocess; return 0; @@ -279,7 +282,10 @@ Subprocess* SubprocessSet::NextFinished() { void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) - kill(-(*i)->pid_, SIGINT); + // Since the foreground process is in our process group, it will receive a + // SIGINT at the same time as us. + if (!(*i)->use_console_) + kill(-(*i)->pid_, SIGINT); for (vector::iterator i = running_.begin(); i != running_.end(); ++i) delete *i; diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index 1b230b6..c9607e1 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -14,6 +14,7 @@ #include "subprocess.h" +#include #include #include @@ -213,7 +214,9 @@ BOOL WINAPI SubprocessSet::NotifyInterrupted(DWORD dwCtrlType) { return FALSE; } -Subprocess *SubprocessSet::Add(const string& command) { +Subprocess *SubprocessSet::Add(const string& command, bool use_console) { + assert(!use_console); // We don't support this yet on Windows. + Subprocess *subprocess = new Subprocess; if (!subprocess->Start(this, command)) { delete subprocess; diff --git a/src/subprocess.h b/src/subprocess.h index 4c1629c..6ea6f62 100644 --- a/src/subprocess.h +++ b/src/subprocess.h @@ -44,13 +44,14 @@ struct Subprocess { const string& GetOutput() const; private: - Subprocess(); bool Start(struct SubprocessSet* set, const string& command); void OnPipeReady(); string buf_; #ifdef _WIN32 + Subprocess(); + /// Set up pipe_ as the parent-side pipe of the subprocess; return the /// other end of the pipe, usable in the child process. HANDLE SetupPipe(HANDLE ioport); @@ -61,9 +62,12 @@ struct Subprocess { char overlapped_buf_[4 << 10]; bool is_reading_; #else + Subprocess(bool use_console); + int fd_; pid_t pid_; #endif + bool use_console_; friend struct SubprocessSet; }; @@ -75,7 +79,7 @@ struct SubprocessSet { SubprocessSet(); ~SubprocessSet(); - Subprocess* Add(const string& command); + Subprocess* Add(const string& command, bool use_console = false); bool DoWork(); Subprocess* NextFinished(); void Clear(); diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index 9f8dcea..775a13a 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -95,6 +95,21 @@ TEST_F(SubprocessTest, InterruptParent) { ADD_FAILURE() << "We should have been interrupted"; } +TEST_F(SubprocessTest, Console) { + // Skip test if we don't have the console ourselves. + if (isatty(0) && isatty(1) && isatty(2)) { + Subprocess* subproc = subprocs_.Add("test -t 0 -a -t 1 -a -t 2", + /*use_console=*/true); + ASSERT_NE((Subprocess *) 0, subproc); + + while (!subproc->Done()) { + subprocs_.DoWork(); + } + + EXPECT_EQ(ExitSuccess, subproc->Finish()); + } +} + #endif TEST_F(SubprocessTest, SetWithSingle) { -- cgit v0.12 From dd8c1d84699bb6b45ffa1a529d53fe2273c8b427 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 11 Feb 2014 12:19:01 -0800 Subject: Allocate per-edge BindingEnvs lazily. In chrome, only 2000 of 22000 build edges have bindings. A BindingEnv is 64 bytes, so allocating these only when needed saves a bit over 1 MB of memory. Since env chains are shorter for lookups, builds also become a tiny bit faster. --- src/manifest_parser.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 20be7f3..6fa4f7c 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -296,16 +296,17 @@ bool ManifestParser::ParseEdge(string* err) { if (!ExpectToken(Lexer::NEWLINE, err)) return false; - // XXX scoped_ptr to handle error case. - BindingEnv* env = new BindingEnv(env_); - - while (lexer_.PeekToken(Lexer::INDENT)) { + // Bindings on edges are rare, so allocate per-edge envs only when needed. + bool hasIdent = lexer_.PeekToken(Lexer::INDENT); + BindingEnv* env = hasIdent ? new BindingEnv(env_) : env_; + while (hasIdent) { string key; EvalString val; if (!ParseLet(&key, &val, err)) return false; env->AddBinding(key, val.Evaluate(env_)); + hasIdent = lexer_.PeekToken(Lexer::INDENT); } Edge* edge = state_->AddEdge(rule); -- cgit v0.12 From 9ea8bdd8d41508c8a2da125dea63abcd4bffc273 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 25 Mar 2014 08:33:21 -0700 Subject: add a script to generate large synthetic manifests To be used by a manifest parser perf test in a follow-up. --- misc/write_fake_manifests.py | 219 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 misc/write_fake_manifests.py diff --git a/misc/write_fake_manifests.py b/misc/write_fake_manifests.py new file mode 100644 index 0000000..837007e --- /dev/null +++ b/misc/write_fake_manifests.py @@ -0,0 +1,219 @@ +#!/usr/bin/env python + +"""Writes large manifest files, for manifest parser performance testing. + +The generated manifest files are (eerily) similar in appearance and size to the +ones used in the Chromium project. + +Usage: + python misc/write_fake_manifests.py outdir # Will run for about 5s. + +The program contains a hardcoded random seed, so it will generate the same +output every time it runs. By changing the seed, it's easy to generate many +different sets of manifest files. +""" + +import argparse +import contextlib +import os +import random +import sys + +import ninja_syntax + + +def paretoint(avg, alpha): + """Returns a random integer that's avg on average, following a power law. + alpha determines the shape of the power curve. alpha has to be larger + than 1. The closer alpha is to 1, the higher the variation of the returned + numbers.""" + return int(random.paretovariate(alpha) * avg / (alpha / (alpha - 1))) + + +# Based on http://neugierig.org/software/chromium/class-name-generator.html +def moar(avg_options, p_suffix): + kStart = ['render', 'web', 'browser', 'tab', 'content', 'extension', 'url', + 'file', 'sync', 'content', 'http', 'profile'] + kOption = ['view', 'host', 'holder', 'container', 'impl', 'ref', + 'delegate', 'widget', 'proxy', 'stub', 'context', + 'manager', 'master', 'watcher', 'service', 'file', 'data', + 'resource', 'device', 'info', 'provider', 'internals', 'tracker', + 'api', 'layer'] + kOS = ['win', 'mac', 'aura', 'linux', 'android', 'unittest', 'browsertest'] + num_options = min(paretoint(avg_options, alpha=4), 5) + # The original allows kOption to repeat as long as no consecutive options + # repeat. This version doesn't allow any option repetition. + name = [random.choice(kStart)] + random.sample(kOption, num_options) + if random.random() < p_suffix: + name.append(random.choice(kOS)) + return '_'.join(name) + + +class GenRandom(object): + def __init__(self): + self.seen_names = set([None]) + self.seen_defines = set([None]) + + def _unique_string(self, seen, avg_options=1.3, p_suffix=0.1): + s = None + while s in seen: + s = moar(avg_options, p_suffix) + seen.add(s) + return s + + def _n_unique_strings(self, n): + seen = set([None]) + return [self._unique_string(seen, avg_options=3, p_suffix=0.4) + for _ in xrange(n)] + + def target_name(self): + return self._unique_string(p_suffix=0, seen=self.seen_names) + + def path(self): + return os.path.sep.join([ + self._unique_string(self.seen_names, avg_options=1, p_suffix=0) + for _ in xrange(1 + paretoint(0.6, alpha=4))]) + + def src_obj_pairs(self, path, name): + num_sources = paretoint(55, alpha=2) + 1 + return [(os.path.join('..', '..', path, s + '.cc'), + os.path.join('obj', path, '%s.%s.o' % (name, s))) + for s in self._n_unique_strings(num_sources)] + + def defines(self): + return [ + '-DENABLE_' + self._unique_string(self.seen_defines).upper() + for _ in xrange(paretoint(20, alpha=3))] + + +LIB, EXE = 0, 1 +class Target(object): + def __init__(self, gen, kind): + self.name = gen.target_name() + self.dir_path = gen.path() + self.ninja_file_path = os.path.join( + 'obj', self.dir_path, self.name + '.ninja') + self.src_obj_pairs = gen.src_obj_pairs(self.dir_path, self.name) + if kind == LIB: + self.output = os.path.join('lib' + self.name + '.a') + elif kind == EXE: + self.output = os.path.join(self.name) + self.defines = gen.defines() + self.deps = [] + self.kind = kind + self.has_compile_depends = random.random() < 0.4 + + @property + def includes(self): + return ['-I' + dep.dir_path for dep in self.deps] + + +def write_target_ninja(ninja, target): + compile_depends = None + if target.has_compile_depends: + compile_depends = os.path.join( + 'obj', target.dir_path, target.name + '.stamp') + ninja.build(compile_depends, 'stamp', target.src_obj_pairs[0][0]) + ninja.newline() + + ninja.variable('defines', target.defines) + if target.deps: + ninja.variable('includes', target.includes) + ninja.variable('cflags', ['-Wall', '-fno-rtti', '-fno-exceptions']) + ninja.newline() + + for src, obj in target.src_obj_pairs: + ninja.build(obj, 'cxx', src, implicit=compile_depends) + ninja.newline() + + deps = [dep.output for dep in target.deps] + libs = [dep.output for dep in target.deps if dep.kind == LIB] + if target.kind == EXE: + ninja.variable('ldflags', '-Wl,pie') + ninja.variable('libs', libs) + link = { LIB: 'alink', EXE: 'link'}[target.kind] + ninja.build(target.output, link, [obj for _, obj in target.src_obj_pairs], + implicit=deps) + + +def write_master_ninja(master_ninja, targets): + """Writes master build.ninja file, referencing all given subninjas.""" + master_ninja.variable('cxx', 'c++') + master_ninja.variable('ld', '$cxx') + master_ninja.newline() + + master_ninja.pool('link_pool', depth=4) + master_ninja.newline() + + master_ninja.rule('cxx', description='CXX $out', + command='$cxx -MMD -MF $out.d $defines $includes $cflags -c $in -o $out', + depfile='$out.d', deps='gcc') + master_ninja.rule('alink', description='LIBTOOL-STATIC $out', + command='rm -f $out && libtool -static -o $out $in') + master_ninja.rule('link', description='LINK $out', pool='link_pool', + command='$ld $ldflags -o $out $in $libs') + master_ninja.rule('stamp', description='STAMP $out', command='touch $out') + master_ninja.newline() + + for target in targets: + master_ninja.subninja(target.ninja_file_path) + master_ninja.newline() + + master_ninja.comment('Short names for targets.') + for target in targets: + if target.name != target.output: + master_ninja.build(target.name, 'phony', target.output) + master_ninja.newline() + + master_ninja.build('all', 'phony', [target.output for target in targets]) + master_ninja.default('all') + + +@contextlib.contextmanager +def FileWriter(path): + """Context manager for a ninja_syntax object writing to a file.""" + try: + os.makedirs(os.path.dirname(path)) + except OSError: + pass + f = open(path, 'w') + yield ninja_syntax.Writer(f) + f.close() + + +def random_targets(): + num_targets = 800 + gen = GenRandom() + + # N-1 static libraries, and 1 executable depending on all of them. + targets = [Target(gen, LIB) for i in xrange(num_targets - 1)] + for i in range(len(targets)): + targets[i].deps = [t for t in targets[0:i] if random.random() < 0.05] + + last_target = Target(gen, EXE) + last_target.deps = targets[:] + last_target.src_obj_pairs = last_target.src_obj_pairs[0:10] # Trim. + targets.append(last_target) + return targets + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('outdir', help='output directory') + args = parser.parse_args() + root_dir = args.outdir + + random.seed(12345) + + targets = random_targets() + for target in targets: + with FileWriter(os.path.join(root_dir, target.ninja_file_path)) as n: + write_target_ninja(n, target) + + with FileWriter(os.path.join(root_dir, 'build.ninja')) as master_ninja: + master_ninja.width = 120 + write_master_ninja(master_ninja, targets) + + +if __name__ == '__main__': + sys.exit(main()) -- cgit v0.12 From e6f9d04661f8bffa01bc306b6d191582ec62e23c Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 2 Apr 2014 14:11:26 -0700 Subject: Support both slashes on Windows when making output dirs --- src/disk_interface.cc | 13 ++++++++----- src/disk_interface_test.cc | 13 ++++++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 3233144..bfacb81 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -14,6 +14,8 @@ #include "disk_interface.h" +#include + #include #include #include @@ -31,15 +33,16 @@ namespace { string DirName(const string& path) { #ifdef _WIN32 - const char kPathSeparator = '\\'; + const char kPathSeparators[] = "\\/"; #else - const char kPathSeparator = '/'; + const char kPathSeparators[] = "/"; #endif - - string::size_type slash_pos = path.rfind(kPathSeparator); + string::size_type slash_pos = path.find_last_of(kPathSeparators); if (slash_pos == string::npos) return string(); // Nothing to do. - while (slash_pos > 0 && path[slash_pos - 1] == kPathSeparator) + const char* const kEnd = kPathSeparators + strlen(kPathSeparators); + while (slash_pos > 0 && + std::find(kPathSeparators, kEnd, path[slash_pos - 1]) != kEnd) --slash_pos; return path.substr(0, slash_pos); } diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 55822a6..51a1d14 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -93,7 +93,18 @@ TEST_F(DiskInterfaceTest, ReadFile) { } TEST_F(DiskInterfaceTest, MakeDirs) { - EXPECT_TRUE(disk_.MakeDirs("path/with/double//slash/")); + string path = "path/with/double//slash/"; + EXPECT_TRUE(disk_.MakeDirs(path.c_str())); + FILE* f = fopen((path + "a_file").c_str(), "w"); + EXPECT_TRUE(f); + EXPECT_EQ(0, fclose(f)); +#ifdef _WIN32 + string path2 = "another\\with\\back\\\\slashes\\"; + EXPECT_TRUE(disk_.MakeDirs(path2.c_str())); + FILE* f2 = fopen((path2 + "a_file").c_str(), "w"); + EXPECT_TRUE(f2); + EXPECT_EQ(0, fclose(f2)); +#endif } TEST_F(DiskInterfaceTest, RemoveFile) { -- cgit v0.12 From cc533ecb298191eceb95a4edf8f1f6ac110e1f07 Mon Sep 17 00:00:00 2001 From: Daniel Bratell Date: Mon, 14 Apr 2014 13:52:02 +0200 Subject: performance: Writer.build should copy less. The build method copies the input lists several times. That cost about 0.1s in the gyp generation for the Chromium project for no gain. --- misc/ninja_syntax.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index d69e3e4..4b9b547 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -61,16 +61,15 @@ class Writer(object): def build(self, outputs, rule, inputs=None, implicit=None, order_only=None, variables=None): outputs = self._as_list(outputs) - all_inputs = self._as_list(inputs)[:] - out_outputs = list(map(escape_path, outputs)) - all_inputs = list(map(escape_path, all_inputs)) + out_outputs = [escape_path(x) for x in outputs] + all_inputs = [escape_path(x) for x in self._as_list(inputs)] if implicit: - implicit = map(escape_path, self._as_list(implicit)) + implicit = [escape_path(x) for x in self._as_list(implicit)] all_inputs.append('|') all_inputs.extend(implicit) if order_only: - order_only = map(escape_path, self._as_list(order_only)) + order_only = [escape_path(x) for x in self._as_list(order_only)] all_inputs.append('||') all_inputs.extend(order_only) -- cgit v0.12 From 0a4936bf28b1965bdabb5df05efa75bcd82da14d Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 14 Apr 2014 13:37:06 -0700 Subject: CLParser shouldn't read stderr --- src/msvc_helper-win32.cc | 2 +- src/msvc_helper_test.cc | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/msvc_helper-win32.cc b/src/msvc_helper-win32.cc index d2e2eb5..e465279 100644 --- a/src/msvc_helper-win32.cc +++ b/src/msvc_helper-win32.cc @@ -141,7 +141,7 @@ int CLWrapper::Run(const string& command, string* output) { STARTUPINFO startup_info = {}; startup_info.cb = sizeof(STARTUPINFO); startup_info.hStdInput = nul; - startup_info.hStdError = stdout_write; + startup_info.hStdError = ::GetStdHandle(STD_ERROR_HANDLE); startup_info.hStdOutput = stdout_write; startup_info.dwFlags |= STARTF_USESTDHANDLES; diff --git a/src/msvc_helper_test.cc b/src/msvc_helper_test.cc index 48fbe21..391c045 100644 --- a/src/msvc_helper_test.cc +++ b/src/msvc_helper_test.cc @@ -119,3 +119,10 @@ TEST(MSVCHelperTest, EnvBlock) { cl.Run("cmd /c \"echo foo is %foo%", &output); ASSERT_EQ("foo is bar\r\n", output); } + +TEST(MSVCHelperTest, NoReadOfStderr) { + CLWrapper cl; + string output; + cl.Run("cmd /c \"echo to stdout&& echo to stderr 1>&2", &output); + ASSERT_EQ("to stdout\r\n", output); +} -- cgit v0.12 From 1f38478eb976d3c11db7885972664c0422317ea8 Mon Sep 17 00:00:00 2001 From: Allan Odgaard Date: Tue, 15 Apr 2014 14:23:47 +0700 Subject: Fix bash completion when using command options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By quoting the ‘line’ variable we are making it a single word, but ‘getopts’ wants each option as its own word. Previously bash completion would output an error for a line like: ‘ninja -vn targ‸’. In addition to removing the quotes (to enable word expansion) I also used it as a regular variable, as that is what it is (not an array). --- misc/bash-completion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/bash-completion b/misc/bash-completion index 2d6975b..93bc9a1 100644 --- a/misc/bash-completion +++ b/misc/bash-completion @@ -26,7 +26,7 @@ _ninja_target() { dir="." line=$(echo ${COMP_LINE} | cut -d" " -f 2-) # filter out all non relevant arguments but keep C for dirs - while getopts C:f:j:l:k:nvd:t: opt "${line[@]}"; do + while getopts C:f:j:l:k:nvd:t: opt $line; do case $opt in C) dir="$OPTARG" ;; esac -- cgit v0.12 From 4835a2b1952d6f7c21b6a02cf27b31f79fe89a25 Mon Sep 17 00:00:00 2001 From: Allan Odgaard Date: Tue, 15 Apr 2014 14:27:37 +0700 Subject: =?UTF-8?q?Expand=20the=20-C=20argument=20via=20=E2=80=98eval?= =?UTF-8?q?=E2=80=99=20in=20bash=20completion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously completion would not work for ‘ninja -C $HOME/Source/foo targ‸’. We still do not support using tilde in the directory argument. --- misc/bash-completion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/bash-completion b/misc/bash-completion index 93bc9a1..719e7a8 100644 --- a/misc/bash-completion +++ b/misc/bash-completion @@ -31,7 +31,7 @@ _ninja_target() { C) dir="$OPTARG" ;; esac done; - targets_command="ninja -C ${dir} -t targets all" + targets_command="eval ninja -C \"${dir}\" -t targets all" targets=$((${targets_command} 2>/dev/null) | awk -F: '{print $1}') COMPREPLY=($(compgen -W "$targets" -- "$cur")) fi -- cgit v0.12 From 68a4bb27d1a747b240ea68cd320a51dbd261ed14 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 15 Apr 2014 17:43:03 -0700 Subject: Rename parser_perftest to depfile_parser_perftest. --- configure.py | 4 +-- src/depfile_parser_perftest.cc | 77 ++++++++++++++++++++++++++++++++++++++++++ src/parser_perftest.cc | 77 ------------------------------------------ 3 files changed, 79 insertions(+), 79 deletions(-) create mode 100644 src/depfile_parser_perftest.cc delete mode 100644 src/parser_perftest.cc diff --git a/configure.py b/configure.py index da2f6ef..50a4176 100755 --- a/configure.py +++ b/configure.py @@ -377,8 +377,8 @@ all_targets += ninja_test n.comment('Ancillary executables.') -objs = cxx('parser_perftest') -all_targets += n.build(binary('parser_perftest'), 'link', objs, +objs = cxx('depfile_parser_perftest') +all_targets += n.build(binary('depfile_parser_perftest'), 'link', objs, implicit=ninja_lib, variables=[('libs', libs)]) objs = cxx('build_log_perftest') all_targets += n.build(binary('build_log_perftest'), 'link', objs, diff --git a/src/depfile_parser_perftest.cc b/src/depfile_parser_perftest.cc new file mode 100644 index 0000000..b215221 --- /dev/null +++ b/src/depfile_parser_perftest.cc @@ -0,0 +1,77 @@ +// Copyright 2011 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include + +#include "depfile_parser.h" +#include "util.h" +#include "metrics.h" + +int main(int argc, char* argv[]) { + if (argc < 2) { + printf("usage: %s \n", argv[0]); + return 1; + } + + vector times; + for (int i = 1; i < argc; ++i) { + const char* filename = argv[i]; + + for (int limit = 1 << 10; limit < (1<<20); limit *= 2) { + int64_t start = GetTimeMillis(); + for (int rep = 0; rep < limit; ++rep) { + string buf; + string err; + if (ReadFile(filename, &buf, &err) < 0) { + printf("%s: %s\n", filename, err.c_str()); + return 1; + } + + DepfileParser parser; + if (!parser.Parse(&buf, &err)) { + printf("%s: %s\n", filename, err.c_str()); + return 1; + } + } + int64_t end = GetTimeMillis(); + + if (end - start > 100) { + int delta = (int)(end - start); + float time = delta*1000 / (float)limit; + printf("%s: %.1fus\n", filename, time); + times.push_back(time); + break; + } + } + } + + if (!times.empty()) { + float min = times[0]; + float max = times[0]; + float total = 0; + for (size_t i = 0; i < times.size(); ++i) { + total += times[i]; + if (times[i] < min) + min = times[i]; + else if (times[i] > max) + max = times[i]; + } + + printf("min %.1fus max %.1fus avg %.1fus\n", + min, max, total / times.size()); + } + + return 0; +} diff --git a/src/parser_perftest.cc b/src/parser_perftest.cc deleted file mode 100644 index b215221..0000000 --- a/src/parser_perftest.cc +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2011 Google Inc. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include -#include - -#include "depfile_parser.h" -#include "util.h" -#include "metrics.h" - -int main(int argc, char* argv[]) { - if (argc < 2) { - printf("usage: %s \n", argv[0]); - return 1; - } - - vector times; - for (int i = 1; i < argc; ++i) { - const char* filename = argv[i]; - - for (int limit = 1 << 10; limit < (1<<20); limit *= 2) { - int64_t start = GetTimeMillis(); - for (int rep = 0; rep < limit; ++rep) { - string buf; - string err; - if (ReadFile(filename, &buf, &err) < 0) { - printf("%s: %s\n", filename, err.c_str()); - return 1; - } - - DepfileParser parser; - if (!parser.Parse(&buf, &err)) { - printf("%s: %s\n", filename, err.c_str()); - return 1; - } - } - int64_t end = GetTimeMillis(); - - if (end - start > 100) { - int delta = (int)(end - start); - float time = delta*1000 / (float)limit; - printf("%s: %.1fus\n", filename, time); - times.push_back(time); - break; - } - } - } - - if (!times.empty()) { - float min = times[0]; - float max = times[0]; - float total = 0; - for (size_t i = 0; i < times.size(); ++i) { - total += times[i]; - if (times[i] < min) - min = times[i]; - else if (times[i] > max) - max = times[i]; - } - - printf("min %.1fus max %.1fus avg %.1fus\n", - min, max, total / times.size()); - } - - return 0; -} -- cgit v0.12 From a4dc70737a208f45095cd08a18b9c57120a6b8a8 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 15 Apr 2014 22:06:01 -0700 Subject: Add a .clang-format file. This isn't meant to be authoritative. It's good enough to let the "indent current line using clang-format" hotkey do the right thing often enough to be useful. --- .clang-format | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .clang-format diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..1841c03 --- /dev/null +++ b/.clang-format @@ -0,0 +1,25 @@ +# Copyright 2014 Google Inc. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# This isn't meant to be authoritative, but it's good enough to be useful. +# Still use your best judgement for formatting decisions: clang-format +# sometimes makes strange choices. + +BasedOnStyle: Google +AllowShortFunctionsOnASingleLine: Inline +AllowShortIfStatementsOnASingleLine: false +AllowShortLoopsOnASingleLine: false +ConstructorInitializerAllOnOneLineOrOnePerLine: false +Cpp11BracedListStyle: false +IndentCaseLabels: false -- cgit v0.12 From 263f5b618a8289cab6078beaa7d3dab7abdeefc0 Mon Sep 17 00:00:00 2001 From: Nicolas Despres Date: Wed, 16 Apr 2014 08:24:38 +0200 Subject: Propagate file rename to gitignore. This was introduced by 68a4bb27d1a747b240ea68cd320a51dbd261ed14. --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 501a02d..503c109 100644 --- a/.gitignore +++ b/.gitignore @@ -10,7 +10,7 @@ TAGS /canon_perftest /hash_collision_bench /ninja_test -/parser_perftest +/depfile_parser_perftest /graph.png /doc/manual.html /doc/doxygen -- cgit v0.12 From 993803ed2a22c24b45d09f750e4527a05c7900c0 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 16 Apr 2014 21:56:49 -0700 Subject: Add a simple manifest parsing perftest. --- configure.py | 9 +++-- src/manifest_parser_perftest.cc | 82 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 src/manifest_parser_perftest.cc diff --git a/configure.py b/configure.py index 50a4176..c5a6abd 100755 --- a/configure.py +++ b/configure.py @@ -377,18 +377,21 @@ all_targets += ninja_test n.comment('Ancillary executables.') -objs = cxx('depfile_parser_perftest') -all_targets += n.build(binary('depfile_parser_perftest'), 'link', objs, - implicit=ninja_lib, variables=[('libs', libs)]) objs = cxx('build_log_perftest') all_targets += n.build(binary('build_log_perftest'), 'link', objs, implicit=ninja_lib, variables=[('libs', libs)]) objs = cxx('canon_perftest') all_targets += n.build(binary('canon_perftest'), 'link', objs, implicit=ninja_lib, variables=[('libs', libs)]) +objs = cxx('depfile_parser_perftest') +all_targets += n.build(binary('depfile_parser_perftest'), 'link', objs, + implicit=ninja_lib, variables=[('libs', libs)]) objs = cxx('hash_collision_bench') all_targets += n.build(binary('hash_collision_bench'), 'link', objs, implicit=ninja_lib, variables=[('libs', libs)]) +objs = cxx('manifest_parser_perftest') +all_targets += n.build(binary('manifest_parser_perftest'), 'link', objs, + implicit=ninja_lib, variables=[('libs', libs)]) n.newline() n.comment('Generate a graph using the "graph" tool.') diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc new file mode 100644 index 0000000..208d3a3 --- /dev/null +++ b/src/manifest_parser_perftest.cc @@ -0,0 +1,82 @@ +// Copyright 2014 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Tests manifest parser performance. Expects to be run in ninja's root +// directory. + +#include + +#ifdef _WIN32 +#include +#else +#include +#endif + +#include "disk_interface.h" +#include "manifest_parser.h" +#include "metrics.h" +#include "state.h" +#include "util.h" + +struct RealFileReader : public ManifestParser::FileReader { + virtual bool ReadFile(const string& path, string* content, string* err) { + return ::ReadFile(path, content, err) == 0; + } +}; + +bool WriteFakeManifests(const string& dir) { + RealDiskInterface disk_interface; + if (disk_interface.Stat(dir + "/build.ninja") > 0) + return true; + + printf("Creating manifest data..."); fflush(stdout); + int err = system(("python misc/write_fake_manifests.py " + dir).c_str()); + printf("done.\n"); + return err == 0; +} + +int main() { + const char kManifestDir[] = "build/manifest_perftest"; + RealFileReader file_reader; + vector times; + string err; + + if (!WriteFakeManifests(kManifestDir)) { + fprintf(stderr, "Failed to write test data\n"); + return 1; + } + + chdir(kManifestDir); + + const int kNumRepetitions = 5; + for (int i = 0; i < kNumRepetitions; ++i) { + int64_t start = GetTimeMillis(); + + State state; + ManifestParser parser(&state, &file_reader); + if (!parser.Load("build.ninja", &err)) { + fprintf(stderr, "Failed to read test data: %s\n", err.c_str()); + return 1; + } + + int delta = (int)(GetTimeMillis() - start); + printf("%dms\n", delta); + times.push_back(delta); + } + + int min = *min_element(times.begin(), times.end()); + int max = *max_element(times.begin(), times.end()); + float total = accumulate(times.begin(), times.end(), 0); + printf("min %dms max %dms avg %.1fms\n", min, max, total / times.size()); +} -- cgit v0.12 From 8d43f6ba5ad7d974b70b9bff3da4b515978d7082 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 16 Apr 2014 22:05:32 -0700 Subject: Manifest perftest: Also measure command evaluation time. --- src/manifest_parser_perftest.cc | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc index 208d3a3..4fb6a76 100644 --- a/src/manifest_parser_perftest.cc +++ b/src/manifest_parser_perftest.cc @@ -24,6 +24,7 @@ #endif #include "disk_interface.h" +#include "graph.h" #include "manifest_parser.h" #include "metrics.h" #include "state.h" @@ -46,11 +47,26 @@ bool WriteFakeManifests(const string& dir) { return err == 0; } -int main() { +int main(int argc, char* argv[]) { + bool measure_command_evaluation = true; + int opt; + while ((opt = getopt(argc, argv, const_cast("fh"))) != -1) { + switch (opt) { + case 'f': + measure_command_evaluation = false; + break; + case 'h': + default: + printf("usage: manifest_parser_perftest\n" +"\n" +"options:\n" +" -f only measure manifest load time, not command evaluation time\n" + ); + return 1; + } + } + const char kManifestDir[] = "build/manifest_perftest"; - RealFileReader file_reader; - vector times; - string err; if (!WriteFakeManifests(kManifestDir)) { fprintf(stderr, "Failed to write test data\n"); @@ -60,6 +76,9 @@ int main() { chdir(kManifestDir); const int kNumRepetitions = 5; + RealFileReader file_reader; + vector times; + string err; for (int i = 0; i < kNumRepetitions; ++i) { int64_t start = GetTimeMillis(); @@ -69,9 +88,16 @@ int main() { fprintf(stderr, "Failed to read test data: %s\n", err.c_str()); return 1; } + // Doing an empty build involves reading the manifest and evaluating all + // commands required for the requested targets. So include command + // evaluation in the perftest by default. + int optimization_guard = 0; + if (measure_command_evaluation) + for (size_t i = 0; i < state.edges_.size(); ++i) + optimization_guard += state.edges_[i]->EvaluateCommand().size(); int delta = (int)(GetTimeMillis() - start); - printf("%dms\n", delta); + printf("%dms (hash: %x)\n", delta, optimization_guard); times.push_back(delta); } -- cgit v0.12 From e5604290ba366ce114a070d5008c2ab69de5d59e Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 16 Apr 2014 22:06:34 -0700 Subject: Add manifest_parser_perftest to .gitignore. --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 503c109..40a610d 100644 --- a/.gitignore +++ b/.gitignore @@ -8,9 +8,10 @@ TAGS /ninja /build_log_perftest /canon_perftest +/depfile_parser_perftest /hash_collision_bench /ninja_test -/depfile_parser_perftest +/manifest_parser_perftest /graph.png /doc/manual.html /doc/doxygen -- cgit v0.12 From 6e014f6e21205f38259eb651f81953867779bc7d Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 16 Apr 2014 22:12:30 -0700 Subject: Manifest perftest: Pull manifest parsing into own function. --- src/manifest_parser_perftest.cc | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc index 4fb6a76..765ea1c 100644 --- a/src/manifest_parser_perftest.cc +++ b/src/manifest_parser_perftest.cc @@ -47,6 +47,25 @@ bool WriteFakeManifests(const string& dir) { return err == 0; } +int LoadManifests(bool measure_command_evaluation) { + string err; + RealFileReader file_reader; + State state; + ManifestParser parser(&state, &file_reader); + if (!parser.Load("build.ninja", &err)) { + fprintf(stderr, "Failed to read test data: %s\n", err.c_str()); + exit(1); + } + // Doing an empty build involves reading the manifest and evaluating all + // commands required for the requested targets. So include command + // evaluation in the perftest by default. + int optimization_guard = 0; + if (measure_command_evaluation) + for (size_t i = 0; i < state.edges_.size(); ++i) + optimization_guard += state.edges_[i]->EvaluateCommand().size(); + return optimization_guard; +} + int main(int argc, char* argv[]) { bool measure_command_evaluation = true; int opt; @@ -76,26 +95,10 @@ int main(int argc, char* argv[]) { chdir(kManifestDir); const int kNumRepetitions = 5; - RealFileReader file_reader; vector times; - string err; for (int i = 0; i < kNumRepetitions; ++i) { int64_t start = GetTimeMillis(); - - State state; - ManifestParser parser(&state, &file_reader); - if (!parser.Load("build.ninja", &err)) { - fprintf(stderr, "Failed to read test data: %s\n", err.c_str()); - return 1; - } - // Doing an empty build involves reading the manifest and evaluating all - // commands required for the requested targets. So include command - // evaluation in the perftest by default. - int optimization_guard = 0; - if (measure_command_evaluation) - for (size_t i = 0; i < state.edges_.size(); ++i) - optimization_guard += state.edges_[i]->EvaluateCommand().size(); - + int optimization_guard = LoadManifests(measure_command_evaluation); int delta = (int)(GetTimeMillis() - start); printf("%dms (hash: %x)\n", delta, optimization_guard); times.push_back(delta); -- cgit v0.12 From 95f08c1b4ec8e81feb11d77e7aff6bf629eb125b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 16 Apr 2014 22:15:09 -0700 Subject: Manifest perftest: Try to make it build on Linux. --- src/manifest_parser_perftest.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc index 765ea1c..ed3a89a 100644 --- a/src/manifest_parser_perftest.cc +++ b/src/manifest_parser_perftest.cc @@ -16,6 +16,7 @@ // directory. #include +#include #ifdef _WIN32 #include -- cgit v0.12 From 28d64afc726d38a411efeda6b61a4f9ff77547d8 Mon Sep 17 00:00:00 2001 From: Taylor Braun-Jones Date: Fri, 25 Apr 2014 13:50:09 -0400 Subject: Fix bash-completion support for -C option to expand tilde --- misc/bash-completion | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/misc/bash-completion b/misc/bash-completion index 719e7a8..ecb6c54 100644 --- a/misc/bash-completion +++ b/misc/bash-completion @@ -28,7 +28,8 @@ _ninja_target() { # filter out all non relevant arguments but keep C for dirs while getopts C:f:j:l:k:nvd:t: opt $line; do case $opt in - C) dir="$OPTARG" ;; + # eval for tilde expansion + C) eval dir="$OPTARG" ;; esac done; targets_command="eval ninja -C \"${dir}\" -t targets all" -- cgit v0.12 From 43645ff15bdd376c6484305ef35191a5bf43e446 Mon Sep 17 00:00:00 2001 From: Taylor Braun-Jones Date: Fri, 25 Apr 2014 13:59:53 -0400 Subject: Style: Fix inconsistent indentation --- misc/bash-completion | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/misc/bash-completion b/misc/bash-completion index ecb6c54..0ca5235 100644 --- a/misc/bash-completion +++ b/misc/bash-completion @@ -19,23 +19,23 @@ _ninja_target() { local cur targets dir line targets_command OPTIND cur="${COMP_WORDS[COMP_CWORD]}" - if [[ "$cur" == "--"* ]]; then - # there is currently only one argument that takes -- - COMPREPLY=($(compgen -P '--' -W 'version' -- "${cur:2}")) - else - dir="." - line=$(echo ${COMP_LINE} | cut -d" " -f 2-) - # filter out all non relevant arguments but keep C for dirs - while getopts C:f:j:l:k:nvd:t: opt $line; do - case $opt in - # eval for tilde expansion - C) eval dir="$OPTARG" ;; - esac - done; - targets_command="eval ninja -C \"${dir}\" -t targets all" - targets=$((${targets_command} 2>/dev/null) | awk -F: '{print $1}') - COMPREPLY=($(compgen -W "$targets" -- "$cur")) - fi + if [[ "$cur" == "--"* ]]; then + # there is currently only one argument that takes -- + COMPREPLY=($(compgen -P '--' -W 'version' -- "${cur:2}")) + else + dir="." + line=$(echo ${COMP_LINE} | cut -d" " -f 2-) + # filter out all non relevant arguments but keep C for dirs + while getopts C:f:j:l:k:nvd:t: opt $line; do + case $opt in + # eval for tilde expansion + C) eval dir="$OPTARG" ;; + esac + done; + targets_command="eval ninja -C \"${dir}\" -t targets all" + targets=$((${targets_command} 2>/dev/null) | awk -F: '{print $1}') + COMPREPLY=($(compgen -W "$targets" -- "$cur")) + fi return } complete -F _ninja_target ninja -- cgit v0.12 From 7cae968bff6a760f4c46f2505c4ff2aec4b915cd Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 27 Apr 2014 12:57:25 -0700 Subject: Make manifest_parser_perftest build on Windows. --- src/manifest_parser_perftest.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc index ed3a89a..e40468f 100644 --- a/src/manifest_parser_perftest.cc +++ b/src/manifest_parser_perftest.cc @@ -19,8 +19,10 @@ #include #ifdef _WIN32 +#include "getopt.h" #include #else +#include #include #endif @@ -107,6 +109,6 @@ int main(int argc, char* argv[]) { int min = *min_element(times.begin(), times.end()); int max = *max_element(times.begin(), times.end()); - float total = accumulate(times.begin(), times.end(), 0); + float total = accumulate(times.begin(), times.end(), 0.0f); printf("min %dms max %dms avg %.1fms\n", min, max, total / times.size()); } -- cgit v0.12 From 4f9ac2aab919f3ce522afe7cc1b65c24897b2306 Mon Sep 17 00:00:00 2001 From: Taylor Braun-Jones Date: Fri, 25 Apr 2014 14:20:42 -0400 Subject: Support completion of arguments to -f and -C options Note: This is only available for bash_completion users. --- misc/bash-completion | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/misc/bash-completion b/misc/bash-completion index 0ca5235..b30b773 100644 --- a/misc/bash-completion +++ b/misc/bash-completion @@ -16,8 +16,25 @@ # . path/to/ninja/misc/bash-completion _ninja_target() { - local cur targets dir line targets_command OPTIND - cur="${COMP_WORDS[COMP_CWORD]}" + local cur prev targets dir line targets_command OPTIND + + # When available, use bash_completion to: + # 1) Complete words when the cursor is in the middle of the word + # 2) Complete paths with files or directories, as appropriate + if _get_comp_words_by_ref cur prev &>/dev/null ; then + case $prev in + -f) + _filedir + return 0 + ;; + -C) + _filedir -d + return 0 + ;; + esac + else + cur="${COMP_WORDS[COMP_CWORD]}" + fi if [[ "$cur" == "--"* ]]; then # there is currently only one argument that takes -- -- cgit v0.12 From b1d4668260ef142837c76c6ca95c32e1a43d4aef Mon Sep 17 00:00:00 2001 From: Taylor Braun-Jones Date: Sun, 27 Apr 2014 18:31:40 -0400 Subject: Fix getopts barfing over the terminal when trying to complete -f and -C Note that this is only applicable for when bash_completion is unavailable. --- misc/bash-completion | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/bash-completion b/misc/bash-completion index b30b773..6edf4df 100644 --- a/misc/bash-completion +++ b/misc/bash-completion @@ -43,7 +43,7 @@ _ninja_target() { dir="." line=$(echo ${COMP_LINE} | cut -d" " -f 2-) # filter out all non relevant arguments but keep C for dirs - while getopts C:f:j:l:k:nvd:t: opt $line; do + while getopts :C:f:j:l:k:nvd:t: opt $line; do case $opt in # eval for tilde expansion C) eval dir="$OPTARG" ;; -- cgit v0.12 From c2b7e472ee5db11a65d113a2d7e11668b9e4608f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 30 Apr 2014 20:46:12 -0700 Subject: Accept \r\n line endings in depfiles. Fixes #752. --- src/depfile_parser.cc | 13 +++++++------ src/depfile_parser.in.cc | 4 ++-- src/depfile_parser_test.cc | 12 ++++++++++++ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index 49c7d7b..d052a4b 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -124,17 +124,18 @@ bool DepfileParser::Parse(string* content, string* err) { } } ++in; - if ((yych = *in) <= '#') { - if (yych <= '\n') { + if ((yych = *in) <= '"') { + if (yych <= '\f') { if (yych <= 0x00) goto yy3; - if (yych <= '\t') goto yy14; + if (yych != '\n') goto yy14; } else { + if (yych <= '\r') goto yy3; if (yych == ' ') goto yy16; - if (yych <= '"') goto yy14; - goto yy16; + goto yy14; } } else { if (yych <= 'Z') { + if (yych <= '#') goto yy16; if (yych == '*') goto yy16; goto yy14; } else { @@ -224,7 +225,7 @@ yy16: } else if (!out_.str_) { out_ = StringPiece(filename, len); } else if (out_ != StringPiece(filename, len)) { - *err = "depfile has multiple output paths."; + *err = "depfile has multiple output paths"; return false; } } diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc index 8bb6d84..ba77079 100644 --- a/src/depfile_parser.in.cc +++ b/src/depfile_parser.in.cc @@ -67,7 +67,7 @@ bool DepfileParser::Parse(string* content, string* err) { *out++ = '$'; continue; } - '\\' [^\000\n] { + '\\' [^\000\r\n] { // Let backslash before other characters through verbatim. *out++ = '\\'; *out++ = yych; @@ -108,7 +108,7 @@ bool DepfileParser::Parse(string* content, string* err) { } else if (!out_.str_) { out_ = StringPiece(filename, len); } else if (out_ != StringPiece(filename, len)) { - *err = "depfile has multiple output paths."; + *err = "depfile has multiple output paths"; return false; } } diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc index 0f6771a..581f3a2 100644 --- a/src/depfile_parser_test.cc +++ b/src/depfile_parser_test.cc @@ -58,6 +58,17 @@ TEST_F(DepfileParserTest, Continuation) { EXPECT_EQ(2u, parser_.ins_.size()); } +TEST_F(DepfileParserTest, CarriageReturnContinuation) { + string err; + EXPECT_TRUE(Parse( +"foo.o: \\\r\n" +" bar.h baz.h\r\n", + &err)); + ASSERT_EQ("", err); + EXPECT_EQ("foo.o", parser_.out_.AsString()); + EXPECT_EQ(2u, parser_.ins_.size()); +} + TEST_F(DepfileParserTest, BackSlashes) { string err; EXPECT_TRUE(Parse( @@ -136,4 +147,5 @@ TEST_F(DepfileParserTest, RejectMultipleDifferentOutputs) { // check that multiple different outputs are rejected by the parser string err; EXPECT_FALSE(Parse("foo bar: x y z", &err)); + ASSERT_EQ("depfile has multiple output paths", err); } -- cgit v0.12 From 1bea14c77c8ffd6da39bc9a95af421b5e40b7fd7 Mon Sep 17 00:00:00 2001 From: Ronny Chevalier Date: Sun, 4 May 2014 04:55:40 +0200 Subject: improve zsh completion --- misc/zsh-completion | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/misc/zsh-completion b/misc/zsh-completion index cd0edfb..2fe16fb 100644 --- a/misc/zsh-completion +++ b/misc/zsh-completion @@ -1,3 +1,4 @@ +#compdef ninja # Copyright 2011 Google Inc. All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -15,7 +16,47 @@ # Add the following to your .zshrc to tab-complete ninja targets # . path/to/ninja/misc/zsh-completion -_ninja() { - reply=(`(ninja -t targets all 2&>/dev/null) | awk -F: '{print $1}'`) +__get_targets() { + ninja -t targets 2>/dev/null | while read -r a b; do echo $a | cut -d ':' -f1; done; } -compctl -K _ninja ninja + +__get_tools() { + ninja -t list 2>/dev/null | while read -r a b; do echo $a; done | tail -n +2 +} + +__get_modes() { + ninja -d list 2>/dev/null | while read -r a b; do echo $a; done | tail -n +2 | head -n -1 +} + +__modes() { + local -a modes + modes=(${(fo)"$(__get_modes)"}) + _describe 'modes' modes +} + +__tools() { + local -a tools + tools=(${(fo)"$(__get_tools)"}) + _describe 'tools' tools +} + +__targets() { + local -a targets + targets=(${(fo)"$(__get_targets)"}) + _describe 'targets' targets +} + +_arguments \ + {-h,--help}'[Show help]' \ + '--version[Print ninja version]' \ + '-C+[Change to directory before doing anything else]:directories:_directories' \ + '-f+[Specify input build file (default=build.ninja)]:files:_files' \ + '-j+[Run N jobs in parallel (default=number of CPUs available)]:number of jobs' \ + '-l+[Do not start new jobs if the load average is greater than N]:number of jobs' \ + '-k+[Keep going until N jobs fail (default=1)]:number of jobs' \ + '-n[Dry run (do not run commands but act like they succeeded)]' \ + '-v[Show all command lines while building]' \ + '-d+[Enable debugging (use -d list to list modes)]:modes:__modes' \ + '-t+[Run a subtool (use -t list to list subtools)]:tools:__tools' \ + '*::targets:__targets' + -- cgit v0.12 From c5ee738460d6b4ef4657c1ca5f1da5c5be4d05bb Mon Sep 17 00:00:00 2001 From: Chris Drake Date: Sun, 4 May 2014 18:34:55 -0700 Subject: Use consistent indentation conventions --- misc/ninja_syntax.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index 4b9b547..32dbefb 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -11,7 +11,7 @@ import textwrap import re def escape_path(word): - return word.replace('$ ','$$ ').replace(' ','$ ').replace(':', '$:') + return word.replace('$ ', '$$ ').replace(' ', '$ ').replace(':', '$:') class Writer(object): def __init__(self, output, width=78): @@ -74,7 +74,7 @@ class Writer(object): all_inputs.extend(order_only) self._line('build %s: %s' % (' '.join(out_outputs), - ' '.join([rule] + all_inputs))) + ' '.join([rule] + all_inputs))) if variables: if isinstance(variables, dict): @@ -97,13 +97,13 @@ class Writer(object): self._line('default %s' % ' '.join(self._as_list(paths))) def _count_dollars_before_index(self, s, i): - """Returns the number of '$' characters right in front of s[i].""" - dollar_count = 0 - dollar_index = i - 1 - while dollar_index > 0 and s[dollar_index] == '$': - dollar_count += 1 - dollar_index -= 1 - return dollar_count + """Returns the number of '$' characters right in front of s[i].""" + dollar_count = 0 + dollar_index = i - 1 + while dollar_index > 0 and s[dollar_index] == '$': + dollar_count += 1 + dollar_index -= 1 + return dollar_count def _line(self, text, indent=0): """Write 'text' word-wrapped at self.width characters.""" @@ -116,19 +116,19 @@ class Writer(object): available_space = self.width - len(leading_space) - len(' $') space = available_space while True: - space = text.rfind(' ', 0, space) - if space < 0 or \ - self._count_dollars_before_index(text, space) % 2 == 0: - break + space = text.rfind(' ', 0, space) + if (space < 0 or + self._count_dollars_before_index(text, space) % 2 == 0): + break if space < 0: # No such space; just use the first unescaped space we can find. space = available_space - 1 while True: - space = text.find(' ', space + 1) - if space < 0 or \ - self._count_dollars_before_index(text, space) % 2 == 0: - break + space = text.find(' ', space + 1) + if (space < 0 or + self._count_dollars_before_index(text, space) % 2 == 0): + break if space < 0: # Give up on breaking. break -- cgit v0.12 From fca5ea6ece9bcbf84cc71bcf883948e73c96a92c Mon Sep 17 00:00:00 2001 From: Chris Drake Date: Sun, 4 May 2014 18:35:28 -0700 Subject: Get rid of unused import --- misc/ninja_syntax.py | 1 - 1 file changed, 1 deletion(-) diff --git a/misc/ninja_syntax.py b/misc/ninja_syntax.py index 32dbefb..14b932f 100644 --- a/misc/ninja_syntax.py +++ b/misc/ninja_syntax.py @@ -8,7 +8,6 @@ use Python. """ import textwrap -import re def escape_path(word): return word.replace('$ ', '$$ ').replace(' ', '$ ').replace(':', '$:') -- cgit v0.12 From 03d5b7b0bd06cab5f43e8b85accc7b6450626f39 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 5 May 2014 15:00:26 -0400 Subject: compdb: check that inputs is not empty --- src/ninja.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ninja.cc b/src/ninja.cc index 03ca83b..50de43e 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -631,6 +631,8 @@ int NinjaMain::ToolCompilationDatabase(int argc, char* argv[]) { putchar('['); for (vector::iterator e = state_.edges_.begin(); e != state_.edges_.end(); ++e) { + if ((*e)->inputs_.empty()) + continue; for (int i = 0; i != argc; ++i) { if ((*e)->rule_->name() == argv[i]) { if (!first) -- cgit v0.12 From 2136ca304ad492189b59c0a9e747541c6ea29002 Mon Sep 17 00:00:00 2001 From: Maxim Kalaev Date: Wed, 7 May 2014 20:56:32 +0300 Subject: Allow paths with '{' '}' in depfiles --- src/depfile_parser.cc | 7 ++++--- src/depfile_parser.in.cc | 2 +- src/depfile_parser_test.cc | 7 +++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/depfile_parser.cc b/src/depfile_parser.cc index d052a4b..4ca3943 100644 --- a/src/depfile_parser.cc +++ b/src/depfile_parser.cc @@ -64,7 +64,7 @@ bool DepfileParser::Parse(string* content, string* err) { 0, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, 128, - 128, 128, 128, 0, 0, 0, 128, 0, + 128, 128, 128, 128, 0, 128, 128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -114,11 +114,12 @@ bool DepfileParser::Parse(string* content, string* err) { if (yych != '\\') goto yy9; } } else { - if (yych <= 'z') { + if (yych <= '{') { if (yych == '`') goto yy9; goto yy5; } else { - if (yych == '~') goto yy5; + if (yych <= '|') goto yy9; + if (yych <= '~') goto yy5; goto yy9; } } diff --git a/src/depfile_parser.in.cc b/src/depfile_parser.in.cc index ba77079..b59baf0 100644 --- a/src/depfile_parser.in.cc +++ b/src/depfile_parser.in.cc @@ -73,7 +73,7 @@ bool DepfileParser::Parse(string* content, string* err) { *out++ = yych; continue; } - [a-zA-Z0-9+,/_:.~()@=!-]+ { + [a-zA-Z0-9+,/_:.~()}{@=!-]+ { // Got a span of plain text. int len = (int)(in - start); // Need to shift it over if we're overwriting backslashes. diff --git a/src/depfile_parser_test.cc b/src/depfile_parser_test.cc index 581f3a2..a5f3321 100644 --- a/src/depfile_parser_test.cc +++ b/src/depfile_parser_test.cc @@ -120,16 +120,19 @@ TEST_F(DepfileParserTest, SpecialChars) { string err; EXPECT_TRUE(Parse( "C:/Program\\ Files\\ (x86)/Microsoft\\ crtdefs.h: \n" -" en@quot.header~ t+t-x!=1", +" en@quot.header~ t+t-x!=1 \n" +" openldap/slapd.d/cn=config/cn=schema/cn={0}core.ldif", &err)); ASSERT_EQ("", err); EXPECT_EQ("C:/Program Files (x86)/Microsoft crtdefs.h", parser_.out_.AsString()); - ASSERT_EQ(2u, parser_.ins_.size()); + ASSERT_EQ(3u, parser_.ins_.size()); EXPECT_EQ("en@quot.header~", parser_.ins_[0].AsString()); EXPECT_EQ("t+t-x!=1", parser_.ins_[1].AsString()); + EXPECT_EQ("openldap/slapd.d/cn=config/cn=schema/cn={0}core.ldif", + parser_.ins_[2].AsString()); } TEST_F(DepfileParserTest, UnifyMultipleOutputs) { -- cgit v0.12 From 47993664be821df46b009d3e48d251e6266cefc9 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 12:35:52 -0700 Subject: wip for console pool on windows --- doc/manual.asciidoc | 2 -- src/manifest_parser.cc | 4 ---- src/subprocess-posix.cc | 2 ++ src/subprocess-win32.cc | 24 +++++++++++++++--------- src/subprocess.h | 5 +---- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 5b0c1fe..18760dd 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -664,8 +664,6 @@ While a task in the `console` pool is running, Ninja's regular output (such as progress status and output from concurrent tasks) is buffered until it completes. -This feature is not yet available on Windows. - Ninja file reference -------------------- diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index a566eda..6fa4f7c 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -317,10 +317,6 @@ bool ManifestParser::ParseEdge(string* err) { Pool* pool = state_->LookupPool(pool_name); if (pool == NULL) return lexer_.Error("unknown pool name '" + pool_name + "'", err); -#ifdef _WIN32 - if (pool == &State::kConsolePool) - return lexer_.Error("console pool unsupported on Windows", err); -#endif edge->pool_ = pool; } diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 793d48f..7311f64 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -84,6 +84,8 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { error_pipe = 2; close(output_pipe[1]); } + // In the console case, child_pipe is still inherited by the child and + // closed when the subprocess finishes, which then notifies ninja. execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL); } while (false); diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index c9607e1..c71f95b 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -21,7 +21,9 @@ #include "util.h" -Subprocess::Subprocess() : child_(NULL) , overlapped_(), is_reading_(false) { +Subprocess::Subprocess(bool use_console) : child_(NULL) , overlapped_(), + is_reading_(false), + use_console_(use_console) { } Subprocess::~Subprocess() { @@ -87,10 +89,14 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { STARTUPINFOA startup_info; memset(&startup_info, 0, sizeof(startup_info)); startup_info.cb = sizeof(STARTUPINFO); - startup_info.dwFlags = STARTF_USESTDHANDLES; - startup_info.hStdInput = nul; - startup_info.hStdOutput = child_pipe; - startup_info.hStdError = child_pipe; + if (!use_console_) { + startup_info.dwFlags = STARTF_USESTDHANDLES; + startup_info.hStdInput = nul; + startup_info.hStdOutput = child_pipe; + startup_info.hStdError = child_pipe; + } + // In the console case, child_pipe is still inherited by the child and closed + // when the subprocess finishes, which then notifies ninja. PROCESS_INFORMATION process_info; memset(&process_info, 0, sizeof(process_info)); @@ -215,9 +221,7 @@ BOOL WINAPI SubprocessSet::NotifyInterrupted(DWORD dwCtrlType) { } Subprocess *SubprocessSet::Add(const string& command, bool use_console) { - assert(!use_console); // We don't support this yet on Windows. - - Subprocess *subprocess = new Subprocess; + Subprocess *subprocess = new Subprocess(use_console); if (!subprocess->Start(this, command)) { delete subprocess; return 0; @@ -269,7 +273,9 @@ Subprocess* SubprocessSet::NextFinished() { void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) { - if ((*i)->child_) { + // Since the foreground process is in our process group, it will receive a + // SIGINT at the same time as us. XXX is this true on windows? + if ((*i)->child_ && !(*i)->use_console_) { if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_))) { Win32Fatal("GenerateConsoleCtrlEvent"); diff --git a/src/subprocess.h b/src/subprocess.h index 6ea6f62..b7a1a4c 100644 --- a/src/subprocess.h +++ b/src/subprocess.h @@ -44,14 +44,13 @@ struct Subprocess { const string& GetOutput() const; private: + Subprocess(bool use_console); bool Start(struct SubprocessSet* set, const string& command); void OnPipeReady(); string buf_; #ifdef _WIN32 - Subprocess(); - /// Set up pipe_ as the parent-side pipe of the subprocess; return the /// other end of the pipe, usable in the child process. HANDLE SetupPipe(HANDLE ioport); @@ -62,8 +61,6 @@ struct Subprocess { char overlapped_buf_[4 << 10]; bool is_reading_; #else - Subprocess(bool use_console); - int fd_; pid_t pid_; #endif -- cgit v0.12 From c57b771cc3e86f80bf16d36eb1b1f3ab2ddde1de Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 12:47:08 -0700 Subject: win console wip: enable test --- src/build_test.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/build_test.cc b/src/build_test.cc index 119521e..c414c88 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -245,7 +245,6 @@ TEST_F(PlanTest, PoolWithDepthOne) { "build out2: poolcat in\n"); } -#ifndef _WIN32 TEST_F(PlanTest, ConsolePool) { TestPoolWithDepthOne( "rule poolcat\n" @@ -254,7 +253,6 @@ TEST_F(PlanTest, ConsolePool) { "build out1: poolcat in\n" "build out2: poolcat in\n"); } -#endif TEST_F(PlanTest, PoolsWithDepthTwo) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, @@ -1944,12 +1942,12 @@ TEST_F(BuildTest, Console) { "rule console\n" " command = console\n" " pool = console\n" -"build con: console in.txt\n")); +"build cons: console in.txt\n")); fs_.Create("in.txt", ""); string err; - EXPECT_TRUE(builder_.AddTarget("con", &err)); + EXPECT_TRUE(builder_.AddTarget("cons", &err)); ASSERT_EQ("", err); EXPECT_TRUE(builder_.Build(&err)); EXPECT_EQ("", err); -- cgit v0.12 From 6d5cdede695cd08064f9b7e76b7e4b77a5a78c0c Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 12:58:01 -0700 Subject: win console wip: ctrl-c should reach commands running in console pools --- src/subprocess-win32.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index c71f95b..59b2d37 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -101,10 +101,13 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { PROCESS_INFORMATION process_info; memset(&process_info, 0, sizeof(process_info)); + // Ninja handles ctrl-c, except for subprocesses in console pools. + DWORD process_flags = use_console_ ? 0 : CREATE_NEW_PROCESS_GROUP; + // Do not prepend 'cmd /c' on Windows, this breaks command // lines greater than 8,191 chars. if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL, - /* inherit handles */ TRUE, CREATE_NEW_PROCESS_GROUP, + /* inherit handles */ TRUE, process_flags, NULL, NULL, &startup_info, &process_info)) { DWORD error = GetLastError(); -- cgit v0.12 From 4213308cb98c809190424cb43052f8d1cd264ac5 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 13 May 2014 23:51:51 +0200 Subject: vim syntax: Correctly highlight $$a as ($$)a instead of $($a). --- misc/ninja.vim | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/misc/ninja.vim b/misc/ninja.vim index d813267..9e6ee5c 100644 --- a/misc/ninja.vim +++ b/misc/ninja.vim @@ -1,8 +1,8 @@ " ninja build file syntax. " Language: ninja build file as described at " http://martine.github.com/ninja/manual.html -" Version: 1.3 -" Last Change: 2013/04/16 +" Version: 1.4 +" Last Change: 2014/05/13 " Maintainer: Nicolas Weber " Version 1.3 of this script is in the upstream vim repository and will be " included in the next vim release. If you change this, please send your change @@ -55,6 +55,7 @@ syn keyword ninjaPoolCommand contained depth " $simple_varname -> variable " ${varname} -> variable +syn match ninjaDollar "\$\$" syn match ninjaWrapLineOperator "\$$" syn match ninjaSimpleVar "\$[a-zA-Z0-9_-]\+" syn match ninjaVar "\${[a-zA-Z0-9_.-]\+}" @@ -70,6 +71,7 @@ hi def link ninjaComment Comment hi def link ninjaKeyword Keyword hi def link ninjaRuleCommand Statement hi def link ninjaPoolCommand Statement +hi def link ninjaDollar ninjaOperator hi def link ninjaWrapLineOperator ninjaOperator hi def link ninjaOperator Operator hi def link ninjaSimpleVar ninjaVar -- cgit v0.12 From c246b586a4191d8eca21761b15466b047008489f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 14 May 2014 03:00:34 -0700 Subject: win console wip: resolve FIXME --- src/subprocess-posix.cc | 1 + src/subprocess-win32.cc | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 7311f64..6e4b47c 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -65,6 +65,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { break; if (!use_console_) { + // Put the child in its own process group, so ctrl-c won't reach it. if (setpgid(0, 0) < 0) break; diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index 59b2d37..4fa24c6 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -277,7 +277,7 @@ void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) { // Since the foreground process is in our process group, it will receive a - // SIGINT at the same time as us. XXX is this true on windows? + // CTRL_BREAK_EVENT at the same time as us. if ((*i)->child_ && !(*i)->use_console_) { if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_))) { -- cgit v0.12 From b0ada0efd9ee1d80d38c781fa15d73bece4a3d69 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 17 May 2014 17:20:20 -0700 Subject: win console wip: Fix comments based on review feedback. --- src/subprocess-posix.cc | 2 +- src/subprocess-win32.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 6e4b47c..743e406 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -85,7 +85,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { error_pipe = 2; close(output_pipe[1]); } - // In the console case, child_pipe is still inherited by the child and + // In the console case, output_pipe is still inherited by the child and // closed when the subprocess finishes, which then notifies ninja. execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL); diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc index 4fa24c6..fad66e8 100644 --- a/src/subprocess-win32.cc +++ b/src/subprocess-win32.cc @@ -277,7 +277,7 @@ void SubprocessSet::Clear() { for (vector::iterator i = running_.begin(); i != running_.end(); ++i) { // Since the foreground process is in our process group, it will receive a - // CTRL_BREAK_EVENT at the same time as us. + // CTRL_C_EVENT or CTRL_BREAK_EVENT at the same time as us. if ((*i)->child_ && !(*i)->use_console_) { if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_))) { -- cgit v0.12 From 9c3cada3bf6e56f5ab202689dd61bfff845795e8 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 21 May 2014 14:16:18 -0700 Subject: CleanTest cleanups: * $in only makes sense on rules, not edges (see issue #687) * Remove unneccesary clear() line at end of test --- src/clean_test.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/clean_test.cc b/src/clean_test.cc index 04cff73..4a00fd8 100644 --- a/src/clean_test.cc +++ b/src/clean_test.cc @@ -286,8 +286,7 @@ TEST_F(CleanTest, CleanRspFile) { " rspfile = $rspfile\n" " rspfile_content=$in\n" "build out1: cc in1\n" -" rspfile = cc1.rsp\n" -" rspfile_content=$in\n")); +" rspfile = cc1.rsp\n")); fs_.Create("out1", ""); fs_.Create("cc1.rsp", ""); @@ -307,10 +306,9 @@ TEST_F(CleanTest, CleanRsp) { "build out1: cat in1\n" "build in2: cat_rsp src2\n" " rspfile=in2.rsp\n" -" rspfile_content=$in\n" "build out2: cat_rsp in2\n" " rspfile=out2.rsp\n" -" rspfile_content=$in\n")); +)); fs_.Create("in1", ""); fs_.Create("out1", ""); fs_.Create("in2.rsp", ""); @@ -336,8 +334,6 @@ TEST_F(CleanTest, CleanRsp) { EXPECT_EQ(0, fs_.Stat("out2")); EXPECT_EQ(0, fs_.Stat("in2.rsp")); EXPECT_EQ(0, fs_.Stat("out2.rsp")); - - fs_.files_removed_.clear(); } TEST_F(CleanTest, CleanFailure) { -- cgit v0.12 From bc239cca4f3f0757ba34d0306fbc2a2b75d067a2 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 21 May 2014 15:07:47 -0700 Subject: Make "depfile=$out.d" work if $out contains escaped characters, rspfile too. Fixes #730. This has always been broken, but due to #690 more paths are now escaped (e.g. paths containing + characters, like file.c++). Also see discussion in #689. The approach is to give EdgeEnv an enum deciding on whether or not to escape file names, and provide functions that evaluate depfile and rspfile with that set that to kNoEscape. (depfile=$out.d doesn't make sense on edges with multiple outputs.) This should be relatively safe, as $in and $out can't be used on edges, only on rules (#687). --- doc/manual.asciidoc | 6 ++++-- src/build.cc | 10 +++++----- src/build_test.cc | 45 ++++++++++++++++++++++++++++----------------- src/clean.cc | 4 ++-- src/clean_test.cc | 28 ++++++++++++++++++++++++++++ src/graph.cc | 31 +++++++++++++++++++++++++------ src/graph.h | 6 ++++++ 7 files changed, 98 insertions(+), 32 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 18760dd..aea1784 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -783,7 +783,8 @@ keys. `depfile`:: path to an optional `Makefile` that contains extra _implicit dependencies_ (see <>). This is explicitly to support C/C++ header - dependencies; see <>. + dependencies; see <>. `out`, `in`, and + `in_newline` are not shell-quoted when used to set `depfile`. `deps`:: _(Available since Ninja 1.3.)_ if present, must be one of `gcc` or `msvc` to specify special dependency processing. See @@ -830,7 +831,8 @@ keys. response file for the given command, i.e. write the selected string (`rspfile_content`) to the given file (`rspfile`) before calling the command and delete the file after successful execution of the - command. + command. `out`, `in`, and `in_newline` are not shell-quoted when used to set + `rspfile`. + This is particularly useful on Windows OS, where the maximal length of a command line is limited and response files must be used instead. diff --git a/src/build.cc b/src/build.cc index 91f1754..64bcea3 100644 --- a/src/build.cc +++ b/src/build.cc @@ -541,7 +541,7 @@ void Builder::Cleanup() { for (vector::iterator i = active_edges.begin(); i != active_edges.end(); ++i) { - string depfile = (*i)->GetBinding("depfile"); + string depfile = (*i)->GetUnescapedDepfile(); for (vector::iterator ni = (*i)->outputs_.begin(); ni != (*i)->outputs_.end(); ++ni) { // Only delete this output if it was actually modified. This is @@ -696,7 +696,7 @@ bool Builder::StartEdge(Edge* edge, string* err) { // Create response file, if needed // XXX: this may also block; do we care? - string rspfile = edge->GetBinding("rspfile"); + string rspfile = edge->GetUnescapedRspfile(); if (!rspfile.empty()) { string content = edge->GetBinding("rspfile_content"); if (!disk_interface_->WriteFile(rspfile, content)) @@ -772,7 +772,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { restat_mtime = input_mtime; } - string depfile = edge->GetBinding("depfile"); + string depfile = edge->GetUnescapedDepfile(); if (restat_mtime != 0 && deps_type.empty() && !depfile.empty()) { TimeStamp depfile_mtime = disk_interface_->Stat(depfile); if (depfile_mtime > restat_mtime) @@ -788,7 +788,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { plan_.EdgeFinished(edge); // Delete any left over response file. - string rspfile = edge->GetBinding("rspfile"); + string rspfile = edge->GetUnescapedRspfile(); if (!rspfile.empty() && !g_keep_rsp) disk_interface_->RemoveFile(rspfile); @@ -828,7 +828,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, } else #endif if (deps_type == "gcc") { - string depfile = result->edge->GetBinding("depfile"); + string depfile = result->edge->GetUnescapedDepfile(); if (depfile.empty()) { *err = string("edge with deps=gcc but no depfile makes no sense"); return false; diff --git a/src/build_test.cc b/src/build_test.cc index c414c88..dad69dc 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -521,6 +521,7 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { commands_ran_.push_back(edge->EvaluateCommand()); if (edge->rule().name() == "cat" || edge->rule().name() == "cat_rsp" || + edge->rule().name() == "cat_rsp_out" || edge->rule().name() == "cc" || edge->rule().name() == "touch" || edge->rule().name() == "touch-interrupt") { @@ -775,13 +776,13 @@ TEST_F(BuildTest, DepFileMissing) { string err; ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule cc\n command = cc $in\n depfile = $out.d\n" -"build foo.o: cc foo.c\n")); +"build fo$ o.o: cc foo.c\n")); fs_.Create("foo.c", ""); - EXPECT_TRUE(builder_.AddTarget("foo.o", &err)); + EXPECT_TRUE(builder_.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); ASSERT_EQ(1u, fs_.files_read_.size()); - EXPECT_EQ("foo.o.d", fs_.files_read_[0]); + EXPECT_EQ("fo o.o.d", fs_.files_read_[0]); } TEST_F(BuildTest, DepFileOK) { @@ -1302,14 +1303,20 @@ TEST_F(BuildTest, RspFileSuccess) " command = cat $rspfile > $out\n" " rspfile = $rspfile\n" " rspfile_content = $long_command\n" + "rule cat_rsp_out\n" + " command = cat $rspfile > $out\n" + " rspfile = $out.rsp\n" + " rspfile_content = $long_command\n" "build out1: cat in\n" "build out2: cat_rsp in\n" - " rspfile = out2.rsp\n" + " rspfile = out 2.rsp\n" + " long_command = Some very long command\n" + "build out$ 3: cat_rsp_out in\n" " long_command = Some very long command\n")); fs_.Create("out1", ""); fs_.Create("out2", ""); - fs_.Create("out3", ""); + fs_.Create("out 3", ""); fs_.Tick(); @@ -1320,20 +1327,24 @@ TEST_F(BuildTest, RspFileSuccess) ASSERT_EQ("", err); EXPECT_TRUE(builder_.AddTarget("out2", &err)); ASSERT_EQ("", err); + EXPECT_TRUE(builder_.AddTarget("out 3", &err)); + ASSERT_EQ("", err); size_t files_created = fs_.files_created_.size(); size_t files_removed = fs_.files_removed_.size(); EXPECT_TRUE(builder_.Build(&err)); - ASSERT_EQ(2u, command_runner_.commands_ran_.size()); // cat + cat_rsp + ASSERT_EQ(3u, command_runner_.commands_ran_.size()); - // The RSP file was created - ASSERT_EQ(files_created + 1, fs_.files_created_.size()); - ASSERT_EQ(1u, fs_.files_created_.count("out2.rsp")); + // The RSP files were created + ASSERT_EQ(files_created + 2, fs_.files_created_.size()); + ASSERT_EQ(1u, fs_.files_created_.count("out 2.rsp")); + ASSERT_EQ(1u, fs_.files_created_.count("out 3.rsp")); - // The RSP file was removed - ASSERT_EQ(files_removed + 1, fs_.files_removed_.size()); - ASSERT_EQ(1u, fs_.files_removed_.count("out2.rsp")); + // The RSP files were removed + ASSERT_EQ(files_removed + 2, fs_.files_removed_.size()); + ASSERT_EQ(1u, fs_.files_removed_.count("out 2.rsp")); + ASSERT_EQ(1u, fs_.files_removed_.count("out 3.rsp")); } // Test that RSP file is created but not removed for commands, which fail @@ -1804,7 +1815,7 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { string err; const char* manifest = "rule cc\n command = cc $in\n depfile = $out.d\n deps = gcc\n" - "build foo.o: cc foo.c\n"; + "build fo$ o.o: cc foo.c\n"; fs_.Create("foo.c", ""); @@ -1819,9 +1830,9 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { Builder builder(&state, config_, NULL, &deps_log, &fs_); builder.command_runner_.reset(&command_runner_); - EXPECT_TRUE(builder.AddTarget("foo.o", &err)); + EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); - fs_.Create("foo.o.d", "foo.o: blah.h bar.h\n"); + fs_.Create("fo o.o.d", "fo\\ o.o: blah.h bar.h\n"); EXPECT_TRUE(builder.Build(&err)); EXPECT_EQ("", err); @@ -1844,10 +1855,10 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { Edge* edge = state.edges_.back(); state.GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. - EXPECT_TRUE(builder.AddTarget("foo.o", &err)); + EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); - // Expect three new edges: one generating foo.o, and two more from + // Expect three new edges: one generating fo o.o, and two more from // loading the depfile. ASSERT_EQ(3u, state.edges_.size()); // Expect our edge to now have three inputs: foo.c and two headers. diff --git a/src/clean.cc b/src/clean.cc index 5d1974e..98c638c 100644 --- a/src/clean.cc +++ b/src/clean.cc @@ -80,11 +80,11 @@ bool Cleaner::IsAlreadyRemoved(const string& path) { } void Cleaner::RemoveEdgeFiles(Edge* edge) { - string depfile = edge->GetBinding("depfile"); + string depfile = edge->GetUnescapedDepfile(); if (!depfile.empty()) Remove(depfile); - string rspfile = edge->GetBinding("rspfile"); + string rspfile = edge->GetUnescapedRspfile(); if (!rspfile.empty()) Remove(rspfile); } diff --git a/src/clean_test.cc b/src/clean_test.cc index 4a00fd8..5869bbb 100644 --- a/src/clean_test.cc +++ b/src/clean_test.cc @@ -368,3 +368,31 @@ TEST_F(CleanTest, CleanPhony) { EXPECT_EQ(2, cleaner.cleaned_files_count()); EXPECT_NE(0, fs_.Stat("phony")); } + +TEST_F(CleanTest, CleanDepFileAndRspFileWithSpaces) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule cc_dep\n" +" command = cc $in > $out\n" +" depfile = $out.d\n" +"rule cc_rsp\n" +" command = cc $in > $out\n" +" rspfile = $out.rsp\n" +" rspfile_content = $in\n" +"build out$ 1: cc_dep in$ 1\n" +"build out$ 2: cc_rsp in$ 1\n" +)); + fs_.Create("out 1", ""); + fs_.Create("out 2", ""); + fs_.Create("out 1.d", ""); + fs_.Create("out 2.rsp", ""); + + Cleaner cleaner(&state_, config_, &fs_); + EXPECT_EQ(0, cleaner.CleanAll()); + EXPECT_EQ(4, cleaner.cleaned_files_count()); + EXPECT_EQ(4u, fs_.files_removed_.size()); + + EXPECT_EQ(0, fs_.Stat("out 1")); + EXPECT_EQ(0, fs_.Stat("out 2")); + EXPECT_EQ(0, fs_.Stat("out 1.d")); + EXPECT_EQ(0, fs_.Stat("out 2.rsp")); +} diff --git a/src/graph.cc b/src/graph.cc index 7121342..aa9c0e8 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -215,7 +215,10 @@ bool Edge::AllInputsReady() const { /// An Env for an Edge, providing $in and $out. struct EdgeEnv : public Env { - explicit EdgeEnv(Edge* edge) : edge_(edge) {} + enum EscapeKind { kShellEscape, kDoNotEscape }; + + explicit EdgeEnv(Edge* edge, EscapeKind escape) + : edge_(edge), escape_in_out_(escape) {} virtual string LookupVariable(const string& var); /// Given a span of Nodes, construct a list of paths suitable for a command @@ -225,6 +228,7 @@ struct EdgeEnv : public Env { char sep); Edge* edge_; + EscapeKind escape_in_out_; }; string EdgeEnv::LookupVariable(const string& var) { @@ -250,13 +254,18 @@ string EdgeEnv::MakePathList(vector::iterator begin, char sep) { string result; for (vector::iterator i = begin; i != end; ++i) { - if (!result.empty()) result.push_back(sep); + if (!result.empty()) + result.push_back(sep); const string& path = (*i)->path(); + if (escape_in_out_ == kShellEscape) { #if _WIN32 - GetWin32EscapedString(path, &result); + GetWin32EscapedString(path, &result); #else - GetShellEscapedString(path, &result); + GetShellEscapedString(path, &result); #endif + } else { + result.append(path); + } } return result; } @@ -272,7 +281,7 @@ string Edge::EvaluateCommand(bool incl_rsp_file) { } string Edge::GetBinding(const string& key) { - EdgeEnv env(this); + EdgeEnv env(this, EdgeEnv::kShellEscape); return env.LookupVariable(key); } @@ -280,6 +289,16 @@ bool Edge::GetBindingBool(const string& key) { return !GetBinding(key).empty(); } +string Edge::GetUnescapedDepfile() { + EdgeEnv env(this, EdgeEnv::kDoNotEscape); + return env.LookupVariable("depfile"); +} + +string Edge::GetUnescapedRspfile() { + EdgeEnv env(this, EdgeEnv::kDoNotEscape); + return env.LookupVariable("rspfile"); +} + void Edge::Dump(const char* prefix) const { printf("%s[ ", prefix); for (vector::const_iterator i = inputs_.begin(); @@ -331,7 +350,7 @@ bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { if (!deps_type.empty()) return LoadDepsFromLog(edge, err); - string depfile = edge->GetBinding("depfile"); + string depfile = edge->GetUnescapedDepfile(); if (!depfile.empty()) return LoadDepFile(edge, depfile, err); diff --git a/src/graph.h b/src/graph.h index 6cd7f25..66e31b5 100644 --- a/src/graph.h +++ b/src/graph.h @@ -146,9 +146,15 @@ struct Edge { /// full contents of a response file (if applicable) string EvaluateCommand(bool incl_rsp_file = false); + /// Returns the shell-escaped value of |key|. string GetBinding(const string& key); bool GetBindingBool(const string& key); + /// Like GetBinding("depfile"), but without shell escaping. + string GetUnescapedDepfile(); + /// Like GetBinding("rspfile"), but without shell escaping. + string GetUnescapedRspfile(); + void Dump(const char* prefix="") const; const Rule* rule_; -- cgit v0.12 From 385b7f39f5eb602dd90d9aedee0f6147b695c762 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 24 May 2014 15:52:52 -0700 Subject: reword manual for depfile/rspfile escaping change --- doc/manual.asciidoc | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index aea1784..fcf3db3 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -783,8 +783,7 @@ keys. `depfile`:: path to an optional `Makefile` that contains extra _implicit dependencies_ (see <>). This is explicitly to support C/C++ header - dependencies; see <>. `out`, `in`, and - `in_newline` are not shell-quoted when used to set `depfile`. + dependencies; see <>. `deps`:: _(Available since Ninja 1.3.)_ if present, must be one of `gcc` or `msvc` to specify special dependency processing. See @@ -807,9 +806,9 @@ keys. rebuilt if the command line changes; and secondly, they are not cleaned by default. -`in`:: the shell-quoted space-separated list of files provided as - inputs to the build line referencing this `rule`. (`$in` is provided - solely for convenience; if you need some subset or variant of this +`in`:: the space-separated list of files provided as inputs to the build line + referencing this `rule`, shell-quoted if it appears in commands. (`$in` is + provided solely for convenience; if you need some subset or variant of this list of files, just construct a new variable with that list and use that instead.) @@ -818,8 +817,8 @@ keys. `$rspfile_content`; this works around a bug in the MSVC linker where it uses a fixed-size buffer for processing input.) -`out`:: the shell-quoted space-separated list of files provided as - outputs to the build line referencing this `rule`. +`out`:: the space-separated list of files provided as outputs to the build line + referencing this `rule`, shell-quoted if it appears in commands. `restat`:: if present, causes Ninja to re-stat the command's outputs after execution of the command. Each output whose modification time @@ -831,8 +830,7 @@ keys. response file for the given command, i.e. write the selected string (`rspfile_content`) to the given file (`rspfile`) before calling the command and delete the file after successful execution of the - command. `out`, `in`, and `in_newline` are not shell-quoted when used to set - `rspfile`. + command. + This is particularly useful on Windows OS, where the maximal length of a command line is limited and response files must be used instead. -- cgit v0.12 From 8d84ba41dee869935243da46e4bf61fc84117676 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 30 May 2014 03:14:48 +0200 Subject: Allow + in filenames without escaping Due to #690, file.c++ used to be escaped. + seems as safe as -, so allow it to not be escaped, to keep compile command lines with a fairly common extension slightly cleaner. --- src/util.cc | 1 + src/util_test.cc | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util.cc b/src/util.cc index 24d231f..484b0c1 100644 --- a/src/util.cc +++ b/src/util.cc @@ -183,6 +183,7 @@ static inline bool IsKnownShellSafeCharacter(char ch) { switch (ch) { case '_': + case '+': case '-': case '.': case '/': diff --git a/src/util_test.cc b/src/util_test.cc index f827e5a..b58d15e 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -148,7 +148,7 @@ TEST(PathEscaping, TortureTest) { } TEST(PathEscaping, SensiblePathsAreNotNeedlesslyEscaped) { - const char* path = "some/sensible/path/without/crazy/characters.cc"; + const char* path = "some/sensible/path/without/crazy/characters.c++"; string result; GetWin32EscapedString(path, &result); @@ -160,7 +160,7 @@ TEST(PathEscaping, SensiblePathsAreNotNeedlesslyEscaped) { } TEST(PathEscaping, SensibleWin32PathsAreNotNeedlesslyEscaped) { - const char* path = "some\\sensible\\path\\without\\crazy\\characters.cc"; + const char* path = "some\\sensible\\path\\without\\crazy\\characters.c++"; string result; GetWin32EscapedString(path, &result); -- cgit v0.12 From 8324fcc7425cb2bc285b042c357a954e7a1a2013 Mon Sep 17 00:00:00 2001 From: Felix Geyer Date: Fri, 30 May 2014 22:59:02 +0200 Subject: Use unversioned gnukfreebsd platform. --- platform_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform_helper.py b/platform_helper.py index de102b5..bc3a125 100644 --- a/platform_helper.py +++ b/platform_helper.py @@ -19,7 +19,7 @@ import sys def platforms(): return ['linux', 'darwin', 'freebsd', 'openbsd', 'solaris', 'sunos5', - 'mingw', 'msvc', 'gnukfreebsd8', 'bitrig'] + 'mingw', 'msvc', 'gnukfreebsd', 'bitrig'] class Platform(object): def __init__(self, platform): @@ -31,7 +31,7 @@ class Platform(object): self._platform = 'linux' elif self._platform.startswith('freebsd'): self._platform = 'freebsd' - elif self._platform.startswith('gnukfreebsd8'): + elif self._platform.startswith('gnukfreebsd'): self._platform = 'freebsd' elif self._platform.startswith('openbsd'): self._platform = 'openbsd' -- cgit v0.12 From 23a88eaf4c6023cab27cbb5efd4ef1d12cbd3878 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 12 Jun 2014 13:22:35 -0700 Subject: Version 1.4 of the vim syntax file was merged. See https://code.google.com/p/vim/source/detail?r=92751673cc37c9ef4d1ad1ac4d42d36faa67f88f --- misc/ninja.vim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ninja.vim b/misc/ninja.vim index 9e6ee5c..f34588f 100644 --- a/misc/ninja.vim +++ b/misc/ninja.vim @@ -4,7 +4,7 @@ " Version: 1.4 " Last Change: 2014/05/13 " Maintainer: Nicolas Weber -" Version 1.3 of this script is in the upstream vim repository and will be +" Version 1.4 of this script is in the upstream vim repository and will be " included in the next vim release. If you change this, please send your change " upstream. -- cgit v0.12 From ef647f1e940162611c23d4004491bdaafa28fa81 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 22:47:05 -0700 Subject: create a slightly nicer build.ninja on windows --- configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.py b/configure.py index c5a6abd..64123a0 100755 --- a/configure.py +++ b/configure.py @@ -365,7 +365,7 @@ for name in ['build_log_test', objs += cxx(name, variables=[('cflags', '$test_cflags')]) if platform.is_windows(): for name in ['includes_normalize_test', 'msvc_helper_test']: - objs += cxx(name, variables=[('cflags', test_cflags)]) + objs += cxx(name, variables=[('cflags', '$test_cflags')]) if not platform.is_windows(): test_libs.append('-lpthread') -- cgit v0.12 From f7c8dd18acb4806ca3a84aa230d6ebec459ad507 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 22:49:01 -0700 Subject: spellcheck "keeprsp" in -d options --- src/ninja.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ninja.cc b/src/ninja.cc index 50de43e..4c8dab7 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -773,7 +773,7 @@ bool DebugEnable(const string& name) { return true; } else { const char* suggestion = - SpellcheckString(name.c_str(), "stats", "explain", NULL); + SpellcheckString(name.c_str(), "stats", "explain", "keeprsp", NULL); if (suggestion) { Error("unknown debug setting '%s', did you mean '%s'?", name.c_str(), suggestion); -- cgit v0.12 From 945a1080afc7f79d50e166428af754a3e5d5d91c Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 24 May 2014 21:17:35 -0700 Subject: Add a stat cache. Demo-quality, and disabled atm. --- src/disk_interface.cc | 119 ++++++++++++++++++++++++++++++++++++++++++-------- src/disk_interface.h | 9 +++- src/ninja.cc | 6 +++ 3 files changed, 114 insertions(+), 20 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 4dfae1a..2175051 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -55,6 +55,29 @@ int MakeDir(const string& path) { #endif } +TimeStamp StatSingleFile(const string& path, bool quiet) { + WIN32_FILE_ATTRIBUTE_DATA attrs; + if (!GetFileAttributesEx(path.c_str(), GetFileExInfoStandard, &attrs)) { + DWORD err = GetLastError(); + if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) + return 0; + if (!quiet) { + Error("GetFileAttributesEx(%s): %s", path.c_str(), + GetLastErrorString().c_str()); + } + return -1; + } + const FILETIME& filetime = attrs.ftLastWriteTime; + // FILETIME is in 100-nanosecond increments since the Windows epoch. + // We don't much care about epoch correctness but we do want the + // resulting value to fit in an integer. + uint64_t mtime = ((uint64_t)filetime.dwHighDateTime << 32) | + ((uint64_t)filetime.dwLowDateTime); + mtime /= 1000000000LL / 100; // 100ns -> s. + mtime -= 12622770400LL; // 1600 epoch -> 2000 epoch (subtract 400 years). + return (TimeStamp)mtime; +} + } // namespace // DiskInterface --------------------------------------------------------------- @@ -89,26 +112,84 @@ TimeStamp RealDiskInterface::Stat(const string& path) { } return -1; } - WIN32_FILE_ATTRIBUTE_DATA attrs; - if (!GetFileAttributesEx(path.c_str(), GetFileExInfoStandard, &attrs)) { - DWORD err = GetLastError(); - if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) - return 0; - if (!quiet_) { - Error("GetFileAttributesEx(%s): %s", path.c_str(), - GetLastErrorString().c_str()); - } - return -1; + if (!use_cache_) + return StatSingleFile(path, quiet_); + + string dir = DirName(path); + int offs = dir.size(); + if (offs) ++offs; // skip \ too + string base(path.substr(offs)); + + std::transform(dir.begin(), dir.end(), dir.begin(), ::tolower); + std::transform(base.begin(), base.end(), base.begin(), ::tolower); + + Cache::iterator ci = cache_.find(dir); + if (ci != cache_.end()) { + DirCache::iterator di = ci->second->find(base); + if (di != ci->second->end()) + return di->second; + return 0; } - const FILETIME& filetime = attrs.ftLastWriteTime; - // FILETIME is in 100-nanosecond increments since the Windows epoch. - // We don't much care about epoch correctness but we do want the - // resulting value to fit in an integer. - uint64_t mtime = ((uint64_t)filetime.dwHighDateTime << 32) | - ((uint64_t)filetime.dwLowDateTime); - mtime /= 1000000000LL / 100; // 100ns -> s. - mtime -= 12622770400LL; // 1600 epoch -> 2000 epoch (subtract 400 years). - return (TimeStamp)mtime; + + if (dir.empty()) + dir = "."; + // XXX fill in dir using FFF / FNF + HANDLE hFind = INVALID_HANDLE_VALUE; + WIN32_FIND_DATAA ffd; + +#if 0 + hFind = FindFirstFileA((dir + "\\*").c_str(), &ffd); +#else + hFind = FindFirstFileExA((dir + "\\*").c_str(), + //FindExInfoStandard, + FindExInfoBasic, // 30% faster than FindExInfoStandard! + &ffd, + FindExSearchNameMatch, + NULL, + 0 ); // XXX: check FIND_FIRST_EX_LARGE_FETCH +#endif + + if (hFind == INVALID_HANDLE_VALUE) { + fprintf(stderr, "fail %s", dir.c_str()); + exit(-1); + } + DirCache* dc = new DirCache; + do { + if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) + continue; + + string lowername = ffd.cFileName; + std::transform(lowername.begin(), lowername.end(), + lowername.begin(), ::tolower); + +//fprintf(stderr, "%s %s\n", dir.c_str(), lowername.c_str()); + + const FILETIME& filetime = ffd.ftLastWriteTime; + // FILETIME is in 100-nanosecond increments since the Windows epoch. + // We don't much care about epoch correctness but we do want the + // resulting value to fit in an integer. + uint64_t mtime = ((uint64_t)filetime.dwHighDateTime << 32) | + ((uint64_t)filetime.dwLowDateTime); + mtime /= 1000000000LL / 100; // 100ns -> s. +//if(mtime == 0) +//printf(" asdasdfadsfsadf\n"); + + mtime -= 12622770400LL; // 1600 epoch -> 2000 epoch (subtract 400 years). + + (*dc).insert(make_pair(lowername, (TimeStamp)mtime)); + } while (FindNextFileA(hFind, &ffd)); + FindClose(hFind); + + if (dir == ".") + cache_.insert(make_pair("", dc)); + else + cache_.insert(make_pair(dir, dc)); + + DirCache::iterator di = dc->find(base); + if (di != dc->end()) + return di->second; + return 0; + #else struct stat st; if (stat(path.c_str(), &st) < 0) { diff --git a/src/disk_interface.h b/src/disk_interface.h index ff1e21c..d368c38 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -15,6 +15,7 @@ #ifndef NINJA_DISK_INTERFACE_H_ #define NINJA_DISK_INTERFACE_H_ +#include #include using namespace std; @@ -55,7 +56,7 @@ struct DiskInterface { /// Implementation of DiskInterface that actually hits the disk. struct RealDiskInterface : public DiskInterface { - RealDiskInterface() : quiet_(false) {} + RealDiskInterface() : quiet_(false), use_cache_(false) {} virtual ~RealDiskInterface() {} virtual TimeStamp Stat(const string& path); virtual bool MakeDir(const string& path); @@ -65,6 +66,12 @@ struct RealDiskInterface : public DiskInterface { /// Whether to print on errors. Used to make a test quieter. bool quiet_; + /// Whether stat information can be cached. + bool use_cache_; + + typedef map DirCache; + typedef map Cache; + Cache cache_; }; #endif // NINJA_DISK_INTERFACE_H_ diff --git a/src/ninja.cc b/src/ninja.cc index 4c8dab7..962480e 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -882,6 +882,9 @@ int NinjaMain::RunBuild(int argc, char** argv) { return 1; } +// XXX allow stat caching + //disk_interface_.use_cache_ = true; + Builder builder(&state_, config_, &build_log_, &deps_log_, &disk_interface_); for (size_t i = 0; i < targets.size(); ++i) { if (!builder.AddTarget(targets[i], &err)) { @@ -895,6 +898,9 @@ int NinjaMain::RunBuild(int argc, char** argv) { } } +// XXX disallow stat caching + disk_interface_.use_cache_ = false; + if (builder.AlreadyUpToDate()) { printf("ninja: no work to do.\n"); return 0; -- cgit v0.12 From ef73a646ea32eb243c9571d5f7a5f386bb1461b7 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 24 May 2014 21:36:57 -0700 Subject: Turn on stat cache. Empty builds of chrome on my laptop 4s -> 1.3s (!) --- src/ninja.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ninja.cc b/src/ninja.cc index 962480e..a2f3111 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -883,7 +883,7 @@ int NinjaMain::RunBuild(int argc, char** argv) { } // XXX allow stat caching - //disk_interface_.use_cache_ = true; + disk_interface_.use_cache_ = true; Builder builder(&state_, config_, &build_log_, &deps_log_, &disk_interface_); for (size_t i = 0; i < targets.size(); ++i) { -- cgit v0.12 From 238b259fd7f0361aa4cd2037b4695cbb668d9f7c Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 21:29:18 -0700 Subject: minor cleanups --- src/disk_interface.cc | 96 +++++++++++++++++++++------------------------------ 1 file changed, 40 insertions(+), 56 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 2175051..b4a00b4 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -55,6 +55,17 @@ int MakeDir(const string& path) { #endif } +TimeStamp TimeStampFromFileTime(const FILETIME& filetime) { + // FILETIME is in 100-nanosecond increments since the Windows epoch. + // We don't much care about epoch correctness but we do want the + // resulting value to fit in an integer. + uint64_t mtime = ((uint64_t)filetime.dwHighDateTime << 32) | + ((uint64_t)filetime.dwLowDateTime); + mtime /= 1000000000LL / 100; // 100ns -> s. + mtime -= 12622770400LL; // 1600 epoch -> 2000 epoch (subtract 400 years). + return (TimeStamp)mtime; +} + TimeStamp StatSingleFile(const string& path, bool quiet) { WIN32_FILE_ATTRIBUTE_DATA attrs; if (!GetFileAttributesEx(path.c_str(), GetFileExInfoStandard, &attrs)) { @@ -67,15 +78,32 @@ TimeStamp StatSingleFile(const string& path, bool quiet) { } return -1; } - const FILETIME& filetime = attrs.ftLastWriteTime; - // FILETIME is in 100-nanosecond increments since the Windows epoch. - // We don't much care about epoch correctness but we do want the - // resulting value to fit in an integer. - uint64_t mtime = ((uint64_t)filetime.dwHighDateTime << 32) | - ((uint64_t)filetime.dwLowDateTime); - mtime /= 1000000000LL / 100; // 100ns -> s. - mtime -= 12622770400LL; // 1600 epoch -> 2000 epoch (subtract 400 years). - return (TimeStamp)mtime; + return TimeStampFromFileTime(attrs.ftLastWriteTime); +} + +void StatAllFilesInDir(const string& dir, map* stamps) { + HANDLE hFind = INVALID_HANDLE_VALUE; + WIN32_FIND_DATAA ffd; + + // FindExInfoBasic is 30% faster than FindExInfoStandard. + hFind = FindFirstFileExA((dir + "\\*").c_str(), FindExInfoBasic, &ffd, + FindExSearchNameMatch, NULL, 0); + + if (hFind == INVALID_HANDLE_VALUE) { + fprintf(stderr, "fail %s", dir.c_str()); + exit(-1); + } + do { + if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) + continue; + + string lowername = ffd.cFileName; + transform(lowername.begin(), lowername.end(), lowername.begin(), ::tolower); + + const FILETIME& filetime = ffd.ftLastWriteTime; + (*stamps).insert(make_pair(lowername, TimeStampFromFileTime(filetime))); + } while (FindNextFileA(hFind, &ffd)); + FindClose(hFind); } } // namespace @@ -120,8 +148,8 @@ TimeStamp RealDiskInterface::Stat(const string& path) { if (offs) ++offs; // skip \ too string base(path.substr(offs)); - std::transform(dir.begin(), dir.end(), dir.begin(), ::tolower); - std::transform(base.begin(), base.end(), base.begin(), ::tolower); + transform(dir.begin(), dir.end(), dir.begin(), ::tolower); + transform(base.begin(), base.end(), base.begin(), ::tolower); Cache::iterator ci = cache_.find(dir); if (ci != cache_.end()) { @@ -133,52 +161,8 @@ TimeStamp RealDiskInterface::Stat(const string& path) { if (dir.empty()) dir = "."; - // XXX fill in dir using FFF / FNF - HANDLE hFind = INVALID_HANDLE_VALUE; - WIN32_FIND_DATAA ffd; - -#if 0 - hFind = FindFirstFileA((dir + "\\*").c_str(), &ffd); -#else - hFind = FindFirstFileExA((dir + "\\*").c_str(), - //FindExInfoStandard, - FindExInfoBasic, // 30% faster than FindExInfoStandard! - &ffd, - FindExSearchNameMatch, - NULL, - 0 ); // XXX: check FIND_FIRST_EX_LARGE_FETCH -#endif - - if (hFind == INVALID_HANDLE_VALUE) { - fprintf(stderr, "fail %s", dir.c_str()); - exit(-1); - } DirCache* dc = new DirCache; - do { - if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) - continue; - - string lowername = ffd.cFileName; - std::transform(lowername.begin(), lowername.end(), - lowername.begin(), ::tolower); - -//fprintf(stderr, "%s %s\n", dir.c_str(), lowername.c_str()); - - const FILETIME& filetime = ffd.ftLastWriteTime; - // FILETIME is in 100-nanosecond increments since the Windows epoch. - // We don't much care about epoch correctness but we do want the - // resulting value to fit in an integer. - uint64_t mtime = ((uint64_t)filetime.dwHighDateTime << 32) | - ((uint64_t)filetime.dwLowDateTime); - mtime /= 1000000000LL / 100; // 100ns -> s. -//if(mtime == 0) -//printf(" asdasdfadsfsadf\n"); - - mtime -= 12622770400LL; // 1600 epoch -> 2000 epoch (subtract 400 years). - - (*dc).insert(make_pair(lowername, (TimeStamp)mtime)); - } while (FindNextFileA(hFind, &ffd)); - FindClose(hFind); + StatAllFilesInDir(dir, dc); if (dir == ".") cache_.insert(make_pair("", dc)); -- cgit v0.12 From 20e03b33a340bbc709b0ecd8c50a4b55e709322b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 21:36:30 -0700 Subject: more minor cleanups --- src/disk_interface.cc | 27 +++++++-------------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index b4a00b4..f211698 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -82,12 +82,10 @@ TimeStamp StatSingleFile(const string& path, bool quiet) { } void StatAllFilesInDir(const string& dir, map* stamps) { - HANDLE hFind = INVALID_HANDLE_VALUE; - WIN32_FIND_DATAA ffd; - // FindExInfoBasic is 30% faster than FindExInfoStandard. - hFind = FindFirstFileExA((dir + "\\*").c_str(), FindExInfoBasic, &ffd, - FindExSearchNameMatch, NULL, 0); + WIN32_FIND_DATAA ffd; + HANDLE hFind = FindFirstFileExA((dir + "\\*").c_str(), FindExInfoBasic, &ffd, + FindExSearchNameMatch, NULL, 0); if (hFind == INVALID_HANDLE_VALUE) { fprintf(stderr, "fail %s", dir.c_str()); @@ -154,26 +152,15 @@ TimeStamp RealDiskInterface::Stat(const string& path) { Cache::iterator ci = cache_.find(dir); if (ci != cache_.end()) { DirCache::iterator di = ci->second->find(base); - if (di != ci->second->end()) - return di->second; - return 0; + return di != ci->second->end() ? di->second : 0; } - if (dir.empty()) - dir = "."; DirCache* dc = new DirCache; - StatAllFilesInDir(dir, dc); - - if (dir == ".") - cache_.insert(make_pair("", dc)); - else - cache_.insert(make_pair(dir, dc)); + StatAllFilesInDir(dir.empty() ? "." : dir, dc); + cache_.insert(make_pair(dir, dc)); DirCache::iterator di = dc->find(base); - if (di != dc->end()) - return di->second; - return 0; - + return di != dc->end() ? di->second : 0; #else struct stat st; if (stat(path.c_str(), &st) < 0) { -- cgit v0.12 From f5f7c80897cbe2d7dca454283e4e9c5e484dfadb Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 21:38:44 -0700 Subject: more minor cleanups --- src/disk_interface.cc | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index f211698..784f5d5 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -150,17 +150,13 @@ TimeStamp RealDiskInterface::Stat(const string& path) { transform(base.begin(), base.end(), base.begin(), ::tolower); Cache::iterator ci = cache_.find(dir); - if (ci != cache_.end()) { - DirCache::iterator di = ci->second->find(base); - return di != ci->second->end() ? di->second : 0; + if (ci == cache_.end()) { + DirCache* dc = new DirCache; + StatAllFilesInDir(dir.empty() ? "." : dir, dc); + ci = cache_.insert(make_pair(dir, dc)).first; } - - DirCache* dc = new DirCache; - StatAllFilesInDir(dir.empty() ? "." : dir, dc); - cache_.insert(make_pair(dir, dc)); - - DirCache::iterator di = dc->find(base); - return di != dc->end() ? di->second : 0; + DirCache::iterator di = ci->second->find(base); + return di != ci->second->end() ? di->second : 0; #else struct stat st; if (stat(path.c_str(), &st) < 0) { -- cgit v0.12 From d64f672e5083e01c579191b02c1e1667d9e63ef2 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 21:50:05 -0700 Subject: simplify more, move behind flag --- src/debug_flags.cc | 2 ++ src/debug_flags.h | 4 ++++ src/disk_interface.cc | 4 +--- src/ninja.cc | 15 +++++++++++---- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/debug_flags.cc b/src/debug_flags.cc index 75f1ea5..786a100 100644 --- a/src/debug_flags.cc +++ b/src/debug_flags.cc @@ -15,3 +15,5 @@ bool g_explaining = false; bool g_keep_rsp = false; + +bool g_experimental_win_statcache = false; diff --git a/src/debug_flags.h b/src/debug_flags.h index ba3ebf3..ce3c292 100644 --- a/src/debug_flags.h +++ b/src/debug_flags.h @@ -26,4 +26,8 @@ extern bool g_explaining; extern bool g_keep_rsp; +#ifdef _WIN32 +extern bool g_experimental_win_statcache; +#endif // _WIN32 + #endif // NINJA_EXPLAIN_H_ diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 784f5d5..b7943ef 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -142,9 +142,7 @@ TimeStamp RealDiskInterface::Stat(const string& path) { return StatSingleFile(path, quiet_); string dir = DirName(path); - int offs = dir.size(); - if (offs) ++offs; // skip \ too - string base(path.substr(offs)); + string base(path.substr(dir.size() ? dir.size() + 1 : 0)); transform(dir.begin(), dir.end(), dir.begin(), ::tolower); transform(base.begin(), base.end(), base.begin(), ::tolower); diff --git a/src/ninja.cc b/src/ninja.cc index a2f3111..acb793e 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -760,6 +760,9 @@ bool DebugEnable(const string& name) { " stats print operation counts/timing info\n" " explain explain what caused a command to execute\n" " keeprsp don't delete @response files on success\n" +#ifdef _WIN32 +" experimentalwinstatcache use an alternative method for stat()ing files\n" +#endif "multiple modes can be enabled via -d FOO -d BAR\n"); return false; } else if (name == "stats") { @@ -771,9 +774,13 @@ bool DebugEnable(const string& name) { } else if (name == "keeprsp") { g_keep_rsp = true; return true; + } else if (name == "experimentalwinstatcache") { + g_experimental_win_statcache = true; + return true; } else { const char* suggestion = - SpellcheckString(name.c_str(), "stats", "explain", "keeprsp", NULL); + SpellcheckString(name.c_str(), "stats", "explain", "keeprsp", + "experimentalwinstatcache", NULL); if (suggestion) { Error("unknown debug setting '%s', did you mean '%s'?", name.c_str(), suggestion); @@ -882,8 +889,8 @@ int NinjaMain::RunBuild(int argc, char** argv) { return 1; } -// XXX allow stat caching - disk_interface_.use_cache_ = true; + if (g_experimental_win_statcache) + disk_interface_.use_cache_ = true; Builder builder(&state_, config_, &build_log_, &deps_log_, &disk_interface_); for (size_t i = 0; i < targets.size(); ++i) { @@ -898,7 +905,7 @@ int NinjaMain::RunBuild(int argc, char** argv) { } } -// XXX disallow stat caching + // Make sure restat rules do not see stale timestamps. disk_interface_.use_cache_ = false; if (builder.AlreadyUpToDate()) { -- cgit v0.12 From f6f63eb50855ac2ccf37e2f5629f6a9e962d6c5e Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 22:05:09 -0700 Subject: make win-only --- src/disk_interface.h | 2 ++ src/ninja.cc | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/disk_interface.h b/src/disk_interface.h index d368c38..bffb679 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -66,12 +66,14 @@ struct RealDiskInterface : public DiskInterface { /// Whether to print on errors. Used to make a test quieter. bool quiet_; +#ifdef _WIN32 /// Whether stat information can be cached. bool use_cache_; typedef map DirCache; typedef map Cache; Cache cache_; +#endif }; #endif // NINJA_DISK_INTERFACE_H_ diff --git a/src/ninja.cc b/src/ninja.cc index acb793e..ccfc14d 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -889,8 +889,10 @@ int NinjaMain::RunBuild(int argc, char** argv) { return 1; } +#ifdef _WIN32 if (g_experimental_win_statcache) disk_interface_.use_cache_ = true; +#endif Builder builder(&state_, config_, &build_log_, &deps_log_, &disk_interface_); for (size_t i = 0; i < targets.size(); ++i) { @@ -905,8 +907,10 @@ int NinjaMain::RunBuild(int argc, char** argv) { } } +#ifdef _WIN32 // Make sure restat rules do not see stale timestamps. disk_interface_.use_cache_ = false; +#endif if (builder.AlreadyUpToDate()) { printf("ninja: no work to do.\n"); -- cgit v0.12 From d902b5f24d4c56aa2fc82b34246b0f75766f10f8 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 22:07:04 -0700 Subject: comment --- src/disk_interface.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/disk_interface.h b/src/disk_interface.h index bffb679..bb40dc9 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -56,7 +56,11 @@ struct DiskInterface { /// Implementation of DiskInterface that actually hits the disk. struct RealDiskInterface : public DiskInterface { - RealDiskInterface() : quiet_(false), use_cache_(false) {} + RealDiskInterface() : quiet_(false) +#ifdef _WIN32 + , use_cache_(false) +#endif + {} virtual ~RealDiskInterface() {} virtual TimeStamp Stat(const string& path); virtual bool MakeDir(const string& path); @@ -71,6 +75,8 @@ struct RealDiskInterface : public DiskInterface { bool use_cache_; typedef map DirCache; + // TODO: Neither a map nor a hashmap seems ideal here. If the statcache + // works out, come up with a better data structure. typedef map Cache; Cache cache_; #endif -- cgit v0.12 From f9c23f8fad43997a08b7e2f7e4c56cc9d9bcb5e1 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 22:11:31 -0700 Subject: on by default --- src/debug_flags.cc | 2 +- src/ninja.cc | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/debug_flags.cc b/src/debug_flags.cc index 786a100..ccd4396 100644 --- a/src/debug_flags.cc +++ b/src/debug_flags.cc @@ -16,4 +16,4 @@ bool g_explaining = false; bool g_keep_rsp = false; -bool g_experimental_win_statcache = false; +bool g_experimental_win_statcache = true; diff --git a/src/ninja.cc b/src/ninja.cc index ccfc14d..cace2a0 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -761,7 +761,7 @@ bool DebugEnable(const string& name) { " explain explain what caused a command to execute\n" " keeprsp don't delete @response files on success\n" #ifdef _WIN32 -" experimentalwinstatcache use an alternative method for stat()ing files\n" +" nowinstatcache don't batch stat() calls per directory and cache them\n" #endif "multiple modes can be enabled via -d FOO -d BAR\n"); return false; @@ -774,13 +774,13 @@ bool DebugEnable(const string& name) { } else if (name == "keeprsp") { g_keep_rsp = true; return true; - } else if (name == "experimentalwinstatcache") { - g_experimental_win_statcache = true; + } else if (name == "nowinstatcache") { + g_experimental_win_statcache = false; return true; } else { const char* suggestion = SpellcheckString(name.c_str(), "stats", "explain", "keeprsp", - "experimentalwinstatcache", NULL); + "nowinstatcache", NULL); if (suggestion) { Error("unknown debug setting '%s', did you mean '%s'?", name.c_str(), suggestion); -- cgit v0.12 From 9970174e2ab8e4d5c9f333b795c8d284cf1cb8f5 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 14 Jun 2014 22:18:28 -0700 Subject: error checking --- src/disk_interface.cc | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index b7943ef..cd99915 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -81,25 +81,28 @@ TimeStamp StatSingleFile(const string& path, bool quiet) { return TimeStampFromFileTime(attrs.ftLastWriteTime); } -void StatAllFilesInDir(const string& dir, map* stamps) { +void StatAllFilesInDir(const string& dir, map* stamps, + bool quiet) { // FindExInfoBasic is 30% faster than FindExInfoStandard. WIN32_FIND_DATAA ffd; HANDLE hFind = FindFirstFileExA((dir + "\\*").c_str(), FindExInfoBasic, &ffd, FindExSearchNameMatch, NULL, 0); if (hFind == INVALID_HANDLE_VALUE) { - fprintf(stderr, "fail %s", dir.c_str()); - exit(-1); + DWORD err = GetLastError(); + if (err != ERROR_FILE_NOT_FOUND && err != ERROR_PATH_NOT_FOUND && !quiet) { + Error("FindFirstFileExA(%s): %s", dir.c_str(), + GetLastErrorString().c_str()); + } + return; } do { if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) continue; - string lowername = ffd.cFileName; transform(lowername.begin(), lowername.end(), lowername.begin(), ::tolower); - - const FILETIME& filetime = ffd.ftLastWriteTime; - (*stamps).insert(make_pair(lowername, TimeStampFromFileTime(filetime))); + stamps->insert(make_pair(lowername, + TimeStampFromFileTime(ffd.ftLastWriteTime))); } while (FindNextFileA(hFind, &ffd)); FindClose(hFind); } @@ -150,7 +153,7 @@ TimeStamp RealDiskInterface::Stat(const string& path) { Cache::iterator ci = cache_.find(dir); if (ci == cache_.end()) { DirCache* dc = new DirCache; - StatAllFilesInDir(dir.empty() ? "." : dir, dc); + StatAllFilesInDir(dir.empty() ? "." : dir, dc, quiet_); ci = cache_.insert(make_pair(dir, dc)).first; } DirCache::iterator di = ci->second->find(base); -- cgit v0.12 From 726afc8226a10cd6c5ce724a845ff5cd17169091 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 15 Jun 2014 14:11:27 -0700 Subject: Free cache memory once it's no longer used. Doesn't slow down empty build times measurably, and saves some memory on non-empty builds. --- src/disk_interface.cc | 16 ++++++++++++++++ src/disk_interface.h | 6 +++++- src/ninja.cc | 9 ++------- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index cd99915..ff86ed1 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -230,3 +230,19 @@ int RealDiskInterface::RemoveFile(const string& path) { return 0; } } + +void RealDiskInterface::AllowCache(bool allow) { +#ifdef _WIN32 + use_cache_ = allow; + if (!use_cache_) + ClearCache(); +#endif +} + +void RealDiskInterface::ClearCache() { +#ifdef _WIN32 + for (Cache::iterator it = cache_.begin(), end = cache_.end(); it != end; ++it) + delete it->second; + cache_.clear(); +#endif +} diff --git a/src/disk_interface.h b/src/disk_interface.h index bb40dc9..40c24b6 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -61,7 +61,7 @@ struct RealDiskInterface : public DiskInterface { , use_cache_(false) #endif {} - virtual ~RealDiskInterface() {} + virtual ~RealDiskInterface() { ClearCache(); } virtual TimeStamp Stat(const string& path); virtual bool MakeDir(const string& path); virtual bool WriteFile(const string& path, const string& contents); @@ -70,6 +70,9 @@ struct RealDiskInterface : public DiskInterface { /// Whether to print on errors. Used to make a test quieter. bool quiet_; + + /// Whether stat information can be cached. + void AllowCache(bool allow); #ifdef _WIN32 /// Whether stat information can be cached. bool use_cache_; @@ -80,6 +83,7 @@ struct RealDiskInterface : public DiskInterface { typedef map Cache; Cache cache_; #endif + void ClearCache(); }; #endif // NINJA_DISK_INTERFACE_H_ diff --git a/src/ninja.cc b/src/ninja.cc index cace2a0..eedfec0 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -889,10 +889,7 @@ int NinjaMain::RunBuild(int argc, char** argv) { return 1; } -#ifdef _WIN32 - if (g_experimental_win_statcache) - disk_interface_.use_cache_ = true; -#endif + disk_interface_.AllowCache(g_experimental_win_statcache); Builder builder(&state_, config_, &build_log_, &deps_log_, &disk_interface_); for (size_t i = 0; i < targets.size(); ++i) { @@ -907,10 +904,8 @@ int NinjaMain::RunBuild(int argc, char** argv) { } } -#ifdef _WIN32 // Make sure restat rules do not see stale timestamps. - disk_interface_.use_cache_ = false; -#endif + disk_interface_.AllowCache(false); if (builder.AlreadyUpToDate()) { printf("ninja: no work to do.\n"); -- cgit v0.12 From 4eb8309251c4839de25502a5390270b53d9706eb Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 15 Jun 2014 14:42:42 -0700 Subject: add some statcache tests --- src/disk_interface.cc | 16 +++++++++++----- src/disk_interface.h | 4 +++- src/disk_interface_test.cc | 27 +++++++++++++++++++++++++++ src/ninja.cc | 4 ++-- 4 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index ff86ed1..c4531e6 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -81,7 +81,7 @@ TimeStamp StatSingleFile(const string& path, bool quiet) { return TimeStampFromFileTime(attrs.ftLastWriteTime); } -void StatAllFilesInDir(const string& dir, map* stamps, +bool StatAllFilesInDir(const string& dir, map* stamps, bool quiet) { // FindExInfoBasic is 30% faster than FindExInfoStandard. WIN32_FIND_DATAA ffd; @@ -90,11 +90,13 @@ void StatAllFilesInDir(const string& dir, map* stamps, if (hFind == INVALID_HANDLE_VALUE) { DWORD err = GetLastError(); - if (err != ERROR_FILE_NOT_FOUND && err != ERROR_PATH_NOT_FOUND && !quiet) { + if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) + return true; + if (!quiet) { Error("FindFirstFileExA(%s): %s", dir.c_str(), GetLastErrorString().c_str()); } - return; + return false; } do { if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) @@ -105,6 +107,7 @@ void StatAllFilesInDir(const string& dir, map* stamps, TimeStampFromFileTime(ffd.ftLastWriteTime))); } while (FindNextFileA(hFind, &ffd)); FindClose(hFind); + return true; } } // namespace @@ -153,7 +156,10 @@ TimeStamp RealDiskInterface::Stat(const string& path) { Cache::iterator ci = cache_.find(dir); if (ci == cache_.end()) { DirCache* dc = new DirCache; - StatAllFilesInDir(dir.empty() ? "." : dir, dc, quiet_); + if (!StatAllFilesInDir(dir.empty() ? "." : dir, dc, quiet_)) { + delete dc; + return -1; + } ci = cache_.insert(make_pair(dir, dc)).first; } DirCache::iterator di = ci->second->find(base); @@ -231,7 +237,7 @@ int RealDiskInterface::RemoveFile(const string& path) { } } -void RealDiskInterface::AllowCache(bool allow) { +void RealDiskInterface::AllowStatCache(bool allow) { #ifdef _WIN32 use_cache_ = allow; if (!use_cache_) diff --git a/src/disk_interface.h b/src/disk_interface.h index 40c24b6..a8a3023 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -72,7 +72,9 @@ struct RealDiskInterface : public DiskInterface { bool quiet_; /// Whether stat information can be cached. - void AllowCache(bool allow); + void AllowStatCache(bool allow); + + private: #ifdef _WIN32 /// Whether stat information can be cached. bool use_cache_; diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 51a1d14..69fd1ab 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -76,6 +76,33 @@ TEST_F(DiskInterfaceTest, StatExistingFile) { EXPECT_GT(disk_.Stat("file"), 1); } +#ifdef _WIN32 +TEST_F(DiskInterfaceTest, StatCache) { + disk_.AllowStatCache(true); + + ASSERT_TRUE(Touch("file1")); + ASSERT_TRUE(Touch("fiLE2")); + ASSERT_TRUE(disk_.MakeDir("subdir")); + ASSERT_TRUE(Touch("subdir\\subfile1")); + ASSERT_TRUE(Touch("subdir\\SUBFILE2")); + ASSERT_TRUE(Touch("subdir\\SUBFILE3")); + + EXPECT_GT(disk_.Stat("FIle1"), 1); + EXPECT_GT(disk_.Stat("file1"), 1); + + EXPECT_GT(disk_.Stat("subdir/subfile2"), 1); + EXPECT_GT(disk_.Stat("sUbdir\\suBFile1"), 1); + + // Test error cases. + disk_.quiet_ = true; + string bad_path("cc:\\foo"); + EXPECT_EQ(-1, disk_.Stat(bad_path)); + EXPECT_EQ(-1, disk_.Stat(bad_path)); + EXPECT_EQ(0, disk_.Stat("nosuchfile")); + EXPECT_EQ(0, disk_.Stat("nosuchdir/nosuchfile")); +} +#endif + TEST_F(DiskInterfaceTest, ReadFile) { string err; EXPECT_EQ("", disk_.ReadFile("foobar", &err)); diff --git a/src/ninja.cc b/src/ninja.cc index eedfec0..e555df4 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -889,7 +889,7 @@ int NinjaMain::RunBuild(int argc, char** argv) { return 1; } - disk_interface_.AllowCache(g_experimental_win_statcache); + disk_interface_.AllowStatCache(g_experimental_win_statcache); Builder builder(&state_, config_, &build_log_, &deps_log_, &disk_interface_); for (size_t i = 0; i < targets.size(); ++i) { @@ -905,7 +905,7 @@ int NinjaMain::RunBuild(int argc, char** argv) { } // Make sure restat rules do not see stale timestamps. - disk_interface_.AllowCache(false); + disk_interface_.AllowStatCache(false); if (builder.AlreadyUpToDate()) { printf("ninja: no work to do.\n"); -- cgit v0.12 From 13cdf9b1812397a969288c755c99926b4011a955 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 15 Jun 2014 15:16:50 -0700 Subject: make bool exist everywhere, for simpler calling code --- src/debug_flags.h | 2 -- src/disk_interface.h | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/debug_flags.h b/src/debug_flags.h index ce3c292..4ffef75 100644 --- a/src/debug_flags.h +++ b/src/debug_flags.h @@ -26,8 +26,6 @@ extern bool g_explaining; extern bool g_keep_rsp; -#ifdef _WIN32 extern bool g_experimental_win_statcache; -#endif // _WIN32 #endif // NINJA_EXPLAIN_H_ diff --git a/src/disk_interface.h b/src/disk_interface.h index a8a3023..0f6dfd3 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -71,7 +71,7 @@ struct RealDiskInterface : public DiskInterface { /// Whether to print on errors. Used to make a test quieter. bool quiet_; - /// Whether stat information can be cached. + /// Whether stat information can be cached. Only has an effect on Windows. void AllowStatCache(bool allow); private: -- cgit v0.12 From b503fdc5367de37c8db42442de511c82a2d35399 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 15 Jun 2014 15:26:37 -0700 Subject: add missing _WIN32 checks --- src/disk_interface.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index c4531e6..6437761 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -55,6 +55,7 @@ int MakeDir(const string& path) { #endif } +#ifdef _WIN32 TimeStamp TimeStampFromFileTime(const FILETIME& filetime) { // FILETIME is in 100-nanosecond increments since the Windows epoch. // We don't much care about epoch correctness but we do want the @@ -109,6 +110,7 @@ bool StatAllFilesInDir(const string& dir, map* stamps, FindClose(hFind); return true; } +#endif // _WIN32 } // namespace -- cgit v0.12 From f6f86d38c09e7caa2e386ce132a63960480ea48b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 15 Jun 2014 16:00:13 -0700 Subject: s/hFind/find_handle/ --- src/disk_interface.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 6437761..50a2d97 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -86,10 +86,10 @@ bool StatAllFilesInDir(const string& dir, map* stamps, bool quiet) { // FindExInfoBasic is 30% faster than FindExInfoStandard. WIN32_FIND_DATAA ffd; - HANDLE hFind = FindFirstFileExA((dir + "\\*").c_str(), FindExInfoBasic, &ffd, - FindExSearchNameMatch, NULL, 0); + HANDLE find_handle = FindFirstFileExA((dir + "\\*").c_str(), FindExInfoBasic, + &ffd, FindExSearchNameMatch, NULL, 0); - if (hFind == INVALID_HANDLE_VALUE) { + if (find_handle == INVALID_HANDLE_VALUE) { DWORD err = GetLastError(); if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) return true; @@ -106,8 +106,8 @@ bool StatAllFilesInDir(const string& dir, map* stamps, transform(lowername.begin(), lowername.end(), lowername.begin(), ::tolower); stamps->insert(make_pair(lowername, TimeStampFromFileTime(ffd.ftLastWriteTime))); - } while (FindNextFileA(hFind, &ffd)); - FindClose(hFind); + } while (FindNextFileA(find_handle, &ffd)); + FindClose(find_handle); return true; } #endif // _WIN32 -- cgit v0.12 From 4a61415871343250789124837a316a55d4499d1f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 16 Jun 2014 09:59:16 -0700 Subject: only use FindExInfoBasic on win7+ --- src/disk_interface.cc | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 50a2d97..fa0aa3d 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -82,12 +82,23 @@ TimeStamp StatSingleFile(const string& path, bool quiet) { return TimeStampFromFileTime(attrs.ftLastWriteTime); } +bool IsWindows7OrLater() { + OSVERSIONINFO version_info = { sizeof(version_info) }; + if (!GetVersionEx(&version_info)) + Fatal("GetVersionEx: %s", GetLastErrorString().c_str()); + return version_info.dwMajorVersion > 6 || + version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 1; +} + bool StatAllFilesInDir(const string& dir, map* stamps, bool quiet) { // FindExInfoBasic is 30% faster than FindExInfoStandard. + static bool can_use_basic_info = IsWindows7OrLater(); + FINDEX_INFO_LEVELS level = + can_use_basic_info ? FindExInfoBasic : FindExInfoStandard; WIN32_FIND_DATAA ffd; - HANDLE find_handle = FindFirstFileExA((dir + "\\*").c_str(), FindExInfoBasic, - &ffd, FindExSearchNameMatch, NULL, 0); + HANDLE find_handle = FindFirstFileExA((dir + "\\*").c_str(), level, &ffd, + FindExSearchNameMatch, NULL, 0); if (find_handle == INVALID_HANDLE_VALUE) { DWORD err = GetLastError(); -- cgit v0.12 From 9bdb0644d141a1e376e4fd0aee1b373a3a7c3468 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 16 Jun 2014 10:27:10 -0700 Subject: simplify statcache code more --- src/disk_interface.cc | 21 ++++++--------------- src/disk_interface.h | 5 ++--- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index fa0aa3d..290975b 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -168,15 +168,14 @@ TimeStamp RealDiskInterface::Stat(const string& path) { Cache::iterator ci = cache_.find(dir); if (ci == cache_.end()) { - DirCache* dc = new DirCache; - if (!StatAllFilesInDir(dir.empty() ? "." : dir, dc, quiet_)) { - delete dc; + ci = cache_.insert(make_pair(dir, DirCache())).first; + if (!StatAllFilesInDir(dir.empty() ? "." : dir, &ci->second, quiet_)) { + cache_.erase(ci); return -1; } - ci = cache_.insert(make_pair(dir, dc)).first; } - DirCache::iterator di = ci->second->find(base); - return di != ci->second->end() ? di->second : 0; + DirCache::iterator di = ci->second.find(base); + return di != ci->second.end() ? di->second : 0; #else struct stat st; if (stat(path.c_str(), &st) < 0) { @@ -254,14 +253,6 @@ void RealDiskInterface::AllowStatCache(bool allow) { #ifdef _WIN32 use_cache_ = allow; if (!use_cache_) - ClearCache(); -#endif -} - -void RealDiskInterface::ClearCache() { -#ifdef _WIN32 - for (Cache::iterator it = cache_.begin(), end = cache_.end(); it != end; ++it) - delete it->second; - cache_.clear(); + cache_.clear(); #endif } diff --git a/src/disk_interface.h b/src/disk_interface.h index 0f6dfd3..b152a62 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -61,7 +61,7 @@ struct RealDiskInterface : public DiskInterface { , use_cache_(false) #endif {} - virtual ~RealDiskInterface() { ClearCache(); } + virtual ~RealDiskInterface() {} virtual TimeStamp Stat(const string& path); virtual bool MakeDir(const string& path); virtual bool WriteFile(const string& path, const string& contents); @@ -82,10 +82,9 @@ struct RealDiskInterface : public DiskInterface { typedef map DirCache; // TODO: Neither a map nor a hashmap seems ideal here. If the statcache // works out, come up with a better data structure. - typedef map Cache; + typedef map Cache; Cache cache_; #endif - void ClearCache(); }; #endif // NINJA_DISK_INTERFACE_H_ -- cgit v0.12 From d61278c352ace3dfe7bd2d12ca0b360d850929e0 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 16 Jun 2014 10:56:24 -0700 Subject: suppress warning on win8.1 sdk --- src/disk_interface.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 290975b..dfea469 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -82,6 +82,8 @@ TimeStamp StatSingleFile(const string& path, bool quiet) { return TimeStampFromFileTime(attrs.ftLastWriteTime); } +#pragma warning(push) +#pragma warning(disable: 4996) // GetVersionExA is deprecated post SDK 8.1. bool IsWindows7OrLater() { OSVERSIONINFO version_info = { sizeof(version_info) }; if (!GetVersionEx(&version_info)) @@ -89,6 +91,7 @@ bool IsWindows7OrLater() { return version_info.dwMajorVersion > 6 || version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 1; } +#pragma warning(pop) bool StatAllFilesInDir(const string& dir, map* stamps, bool quiet) { -- cgit v0.12 From 78d6bb9b6794c01308dba313f75e205d43f95e23 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Thu, 19 Jun 2014 11:22:44 -0700 Subject: use local definition of FindExInfoBasic for earlier sdks --- src/disk_interface.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index dfea469..c329aa3 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -97,8 +97,11 @@ bool StatAllFilesInDir(const string& dir, map* stamps, bool quiet) { // FindExInfoBasic is 30% faster than FindExInfoStandard. static bool can_use_basic_info = IsWindows7OrLater(); + // This is not in earlier SDKs. + const FINDEX_INFO_LEVELS kFindExInfoBasic = + static_cast(1); FINDEX_INFO_LEVELS level = - can_use_basic_info ? FindExInfoBasic : FindExInfoStandard; + can_use_basic_info ? kFindExInfoBasic : FindExInfoStandard; WIN32_FIND_DATAA ffd; HANDLE find_handle = FindFirstFileExA((dir + "\\*").c_str(), level, &ffd, FindExSearchNameMatch, NULL, 0); -- cgit v0.12 From e88a8daa10bb27a0617c74f3bc6eec66823f3a99 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 19 Jun 2014 16:00:35 -0700 Subject: make Stat() a const method --- src/disk_interface.cc | 2 +- src/disk_interface.h | 6 +++--- src/disk_interface_test.cc | 8 ++++---- src/test.cc | 4 ++-- src/test.h | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index c329aa3..ae2146e 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -152,7 +152,7 @@ bool DiskInterface::MakeDirs(const string& path) { // RealDiskInterface ----------------------------------------------------------- -TimeStamp RealDiskInterface::Stat(const string& path) { +TimeStamp RealDiskInterface::Stat(const string& path) const { #ifdef _WIN32 // MSDN: "Naming Files, Paths, and Namespaces" // http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx diff --git a/src/disk_interface.h b/src/disk_interface.h index b152a62..a13bced 100644 --- a/src/disk_interface.h +++ b/src/disk_interface.h @@ -30,7 +30,7 @@ struct DiskInterface { /// stat() a file, returning the mtime, or 0 if missing and -1 on /// other errors. - virtual TimeStamp Stat(const string& path) = 0; + virtual TimeStamp Stat(const string& path) const = 0; /// Create a directory, returning false on failure. virtual bool MakeDir(const string& path) = 0; @@ -62,7 +62,7 @@ struct RealDiskInterface : public DiskInterface { #endif {} virtual ~RealDiskInterface() {} - virtual TimeStamp Stat(const string& path); + virtual TimeStamp Stat(const string& path) const; virtual bool MakeDir(const string& path); virtual bool WriteFile(const string& path, const string& contents); virtual string ReadFile(const string& path, string* err); @@ -83,7 +83,7 @@ struct RealDiskInterface : public DiskInterface { // TODO: Neither a map nor a hashmap seems ideal here. If the statcache // works out, come up with a better data structure. typedef map Cache; - Cache cache_; + mutable Cache cache_; #endif }; diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 69fd1ab..f4e0bb0 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -147,7 +147,7 @@ struct StatTest : public StateTestWithBuiltinRules, StatTest() : scan_(&state_, NULL, NULL, this) {} // DiskInterface implementation. - virtual TimeStamp Stat(const string& path); + virtual TimeStamp Stat(const string& path) const; virtual bool WriteFile(const string& path, const string& contents) { assert(false); return true; @@ -167,12 +167,12 @@ struct StatTest : public StateTestWithBuiltinRules, DependencyScan scan_; map mtimes_; - vector stats_; + mutable vector stats_; }; -TimeStamp StatTest::Stat(const string& path) { +TimeStamp StatTest::Stat(const string& path) const { stats_.push_back(path); - map::iterator i = mtimes_.find(path); + map::const_iterator i = mtimes_.find(path); if (i == mtimes_.end()) return 0; // File not found. return i->second; diff --git a/src/test.cc b/src/test.cc index 45a9226..21015ed 100644 --- a/src/test.cc +++ b/src/test.cc @@ -105,8 +105,8 @@ void VirtualFileSystem::Create(const string& path, files_created_.insert(path); } -TimeStamp VirtualFileSystem::Stat(const string& path) { - FileMap::iterator i = files_.find(path); +TimeStamp VirtualFileSystem::Stat(const string& path) const { + FileMap::const_iterator i = files_.find(path); if (i != files_.end()) return i->second.mtime; return 0; diff --git a/src/test.h b/src/test.h index 9f29e07..f34b877 100644 --- a/src/test.h +++ b/src/test.h @@ -59,7 +59,7 @@ struct VirtualFileSystem : public DiskInterface { } // DiskInterface - virtual TimeStamp Stat(const string& path); + virtual TimeStamp Stat(const string& path) const; virtual bool WriteFile(const string& path, const string& contents); virtual bool MakeDir(const string& path); virtual string ReadFile(const string& path, string* err); -- cgit v0.12 From e4be3232d947abb419f2d0ce542b9b70f3343434 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 19 Jun 2014 16:09:50 -0700 Subject: do not delete files from the logs that still exist on disk This is to keep the possibility of maybe having a tool that deletes old files in the future, or for having a tool which exposes this information to generators so they can do that. See https://github.com/martine/ninja/pull/697#issuecomment-37140762 and the discussion on #762. Idea from @maximuska! --- src/ninja.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ninja.cc b/src/ninja.cc index e555df4..15e265b 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -147,7 +147,9 @@ struct NinjaMain : public BuildLogUser { // edge is rare, and the first recompaction will delete all old outputs from // the deps log, and then a second recompaction will clear the build log, // which seems good enough for this corner case.) - return !n || !n->in_edge(); + // Do keep entries around for files which still exist on disk, for + // generators that want to use this information. + return (!n || !n->in_edge()) && disk_interface_.Stat(s.AsString()) == 0; } }; -- cgit v0.12 From 2120c57e611db9d6af03cbe21a0ed814c119d5df Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 24 Jun 2014 17:49:22 -0700 Subject: Fix -Wunused-result warning for chdir on linux. --- src/manifest_parser_perftest.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc index e40468f..ca62fb2 100644 --- a/src/manifest_parser_perftest.cc +++ b/src/manifest_parser_perftest.cc @@ -16,7 +16,10 @@ // directory. #include + +#include #include +#include #ifdef _WIN32 #include "getopt.h" @@ -95,7 +98,8 @@ int main(int argc, char* argv[]) { return 1; } - chdir(kManifestDir); + if (chdir(kManifestDir) < 0) + Fatal("chdir: %s", strerror(errno)); const int kNumRepetitions = 5; vector times; -- cgit v0.12 From 583632513c19bd647b6b420783113939d5cb5e9f Mon Sep 17 00:00:00 2001 From: donkopotamus Date: Thu, 26 Jun 2014 16:34:35 +1200 Subject: Update ninja-mode for emacs to handle hyphens in rule names --- misc/ninja-mode.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/ninja-mode.el b/misc/ninja-mode.el index 9fe6fc8..36ada6f 100644 --- a/misc/ninja-mode.el +++ b/misc/ninja-mode.el @@ -32,7 +32,7 @@ ;; Variable expansion. '("\\($[[:alnum:]_]+\\)" . (1 font-lock-variable-name-face)) ;; Rule names - '("rule \\([[:alnum:]_]+\\)" . (1 font-lock-function-name-face)) + '("rule \\([[:alnum:]_-]+\\)" . (1 font-lock-function-name-face)) )) ;;;###autoload -- cgit v0.12 From f6baa1b8e83efe007af3485f0595bec00f830a8f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 27 Jun 2014 13:03:41 -0700 Subject: Rename -d nowinstatcache to -d nostatcache; might become useful elsewhere (#787) --- src/debug_flags.cc | 2 +- src/debug_flags.h | 2 +- src/ninja.cc | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/debug_flags.cc b/src/debug_flags.cc index ccd4396..8065001 100644 --- a/src/debug_flags.cc +++ b/src/debug_flags.cc @@ -16,4 +16,4 @@ bool g_explaining = false; bool g_keep_rsp = false; -bool g_experimental_win_statcache = true; +bool g_experimental_statcache = true; diff --git a/src/debug_flags.h b/src/debug_flags.h index 4ffef75..7965585 100644 --- a/src/debug_flags.h +++ b/src/debug_flags.h @@ -26,6 +26,6 @@ extern bool g_explaining; extern bool g_keep_rsp; -extern bool g_experimental_win_statcache; +extern bool g_experimental_statcache; #endif // NINJA_EXPLAIN_H_ diff --git a/src/ninja.cc b/src/ninja.cc index 15e265b..a381e83 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -763,7 +763,7 @@ bool DebugEnable(const string& name) { " explain explain what caused a command to execute\n" " keeprsp don't delete @response files on success\n" #ifdef _WIN32 -" nowinstatcache don't batch stat() calls per directory and cache them\n" +" nostatcache don't batch stat() calls per directory and cache them\n" #endif "multiple modes can be enabled via -d FOO -d BAR\n"); return false; @@ -776,13 +776,13 @@ bool DebugEnable(const string& name) { } else if (name == "keeprsp") { g_keep_rsp = true; return true; - } else if (name == "nowinstatcache") { - g_experimental_win_statcache = false; + } else if (name == "nostatcache") { + g_experimental_statcache = false; return true; } else { const char* suggestion = SpellcheckString(name.c_str(), "stats", "explain", "keeprsp", - "nowinstatcache", NULL); + "nostatcache", NULL); if (suggestion) { Error("unknown debug setting '%s', did you mean '%s'?", name.c_str(), suggestion); @@ -891,7 +891,7 @@ int NinjaMain::RunBuild(int argc, char** argv) { return 1; } - disk_interface_.AllowStatCache(g_experimental_win_statcache); + disk_interface_.AllowStatCache(g_experimental_statcache); Builder builder(&state_, config_, &build_log_, &deps_log_, &disk_interface_); for (size_t i = 0; i < targets.size(); ++i) { -- cgit v0.12 From 7c231a3d0d800bfc2602da097b34d1edca2f600f Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Fri, 27 Jun 2014 13:15:57 -0700 Subject: mark this 1.5.0.git, update RELEASING --- RELEASING | 17 +++++++++-------- src/version.cc | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/RELEASING b/RELEASING index 25926db..c4973b2 100644 --- a/RELEASING +++ b/RELEASING @@ -1,17 +1,18 @@ Notes to myself on all the steps to make for a Ninja release. Push new release branch: -1. update src/version.cc with new version (with ".git") -2. git checkout release; git merge master -3. fix version number in src/version.cc (it will likely conflict in the above) -4. fix version in doc/manual.asciidoc -5. commit, tag, push (don't forget to push --tags) -6. construct release notes from prior notes +1. Consider sending a heads-up to the ninja-build mailing list first +2. update src/version.cc with new version (with ".git"), commit to master +3. git checkout release; git merge master +4. fix version number in src/version.cc (it will likely conflict in the above) +5. fix version in doc/manual.asciidoc +6. commit, tag, push (don't forget to push --tags) +7. construct release notes from prior notes credits: git shortlog -s --no-merges REV.. Release on github: -1. (haven't tried this yet) - https://github.com/blog/1547-release-your-software +1. https://github.com/blog/1547-release-your-software + Add binaries to https://github.com/martine/ninja/releases Make announcement on mailing list: 1. copy old mail diff --git a/src/version.cc b/src/version.cc index 1406d91..562fce4 100644 --- a/src/version.cc +++ b/src/version.cc @@ -18,7 +18,7 @@ #include "util.h" -const char* kNinjaVersion = "1.4.0.git"; +const char* kNinjaVersion = "1.5.0.git"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.'); -- cgit v0.12