From 3f0f86b0caed75241fa71c95a5d73bc0164348c5 Mon Sep 17 00:00:00 2001 From: Andras Becsi Date: Tue, 18 Mar 2014 13:16:26 +0100 Subject: Update to new stable branch 1750 This also includes an updated ninja and chromium dependencies needed on Windows. Change-Id: Icd597d80ed3fa4425933c9f1334c3c2e31291c42 Reviewed-by: Zoltan Arvai Reviewed-by: Zeno Albisser --- ninja/RELEASING | 18 +++-- ninja/bootstrap.py | 13 ++-- ninja/configure.py | 21 ++++-- ninja/doc/manual.asciidoc | 10 ++- ninja/misc/ninja-mode.el | 16 ++++- ninja/platform_helper.py | 12 +++- ninja/src/build.cc | 31 ++++----- ninja/src/build.h | 7 +- ninja/src/build_log.cc | 19 ++++-- ninja/src/build_log.h | 11 ++- ninja/src/build_log_perftest.cc | 7 +- ninja/src/build_log_test.cc | 53 ++++++++++++-- ninja/src/build_test.cc | 129 +++++++++++++++++++++++++++++++++-- ninja/src/debug_flags.cc | 17 +++++ ninja/src/debug_flags.h | 29 ++++++++ ninja/src/depfile_parser.cc | 71 +++++++++++-------- ninja/src/depfile_parser.in.cc | 2 +- ninja/src/deps_log.cc | 100 +++++++++++++++++++-------- ninja/src/deps_log.h | 17 ++++- ninja/src/deps_log_test.cc | 90 +++++++++++++++++++++++- ninja/src/disk_interface.cc | 2 +- ninja/src/edit_distance.cc | 1 + ninja/src/explain.cc | 15 ---- ninja/src/explain.h | 27 -------- ninja/src/graph.cc | 104 ++++++++++++---------------- ninja/src/graph.h | 26 ++++--- ninja/src/graph_test.cc | 11 ++- ninja/src/hash_map.h | 3 +- ninja/src/includes_normalize_test.cc | 2 +- ninja/src/manifest_parser_test.cc | 19 ++++++ ninja/src/metrics.cc | 2 + ninja/src/msvc_helper-win32.cc | 17 ++--- ninja/src/msvc_helper.h | 5 +- ninja/src/msvc_helper_main-win32.cc | 16 ++++- ninja/src/msvc_helper_test.cc | 29 +++++--- ninja/src/ninja.cc | 28 ++++++-- ninja/src/state.cc | 4 +- ninja/src/state.h | 2 +- ninja/src/util.cc | 127 ++++++++++++++++++++++++++++------ ninja/src/util.h | 7 ++ ninja/src/util_test.cc | 31 +++++++++ ninja/src/version.cc | 2 +- 42 files changed, 859 insertions(+), 294 deletions(-) create mode 100644 ninja/src/debug_flags.cc create mode 100644 ninja/src/debug_flags.h delete mode 100644 ninja/src/explain.cc delete mode 100644 ninja/src/explain.h (limited to 'ninja') diff --git a/ninja/RELEASING b/ninja/RELEASING index 1110f0b57b0..25926dbc6aa 100644 --- a/ninja/RELEASING +++ b/ninja/RELEASING @@ -1,12 +1,22 @@ 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. rebuild manual, put in place on website -6. commit, tag, push (don't forget to push --tags) -7. construct release notes from prior notes +5. commit, tag, push (don't forget to push --tags) +6. construct release notes from prior notes credits: git shortlog -s --no-merges REV.. -8. update home page mention of latest version. +Release on github: +1. (haven't tried this yet) + https://github.com/blog/1547-release-your-software + +Make announcement on mailing list: +1. copy old mail + +Update website: +(note to self: website is now in github.com/martine/martine.github.io) +1. rebuild manual, put in place on website +2. update home page mention of latest version. diff --git a/ninja/bootstrap.py b/ninja/bootstrap.py index 66ec85bccba..026396b5c36 100755 --- a/ninja/bootstrap.py +++ b/ninja/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/ninja/configure.py b/ninja/configure.py index c838392895d..da2f6ef478d 100755 --- a/ninja/configure.py +++ b/ninja/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) @@ -125,6 +128,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'] @@ -165,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): @@ -263,12 +269,12 @@ n.comment('Core source files all build into ninja library.') for name in ['build', 'build_log', 'clean', + 'debug_flags', 'depfile_parser', 'deps_log', 'disk_interface', 'edit_distance', 'eval_env', - 'explain', 'graph', 'graphviz', 'lexer', @@ -322,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', diff --git a/ninja/doc/manual.asciidoc b/ninja/doc/manual.asciidoc index a7352571c84..67fcbfd2788 100644 --- a/ninja/doc/manual.asciidoc +++ b/ninja/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/ninja/misc/ninja-mode.el b/ninja/misc/ninja-mode.el index d939206de1f..9fe6fc8cb5f 100644 --- a/ninja/misc/ninja-mode.el +++ b/ninja/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,10 +14,14 @@ ;; 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. -(setq ninja-keywords +;;; Code: + +(defvar ninja-keywords (list '("^#.*" . font-lock-comment-face) (cons (concat "^" (regexp-opt '("rule" "build" "subninja" "include" @@ -28,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 @@ -35,8 +43,10 @@ (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) + +;;; ninja-mode.el ends here diff --git a/ninja/platform_helper.py b/ninja/platform_helper.py index b7447a1a122..de102b51e8c 100644 --- a/ninja/platform_helper.py +++ b/ninja/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 str(out) + def is_windows(self): return self.is_mingw() or self.is_msvc() diff --git a/ninja/src/build.cc b/ninja/src/build.cc index 6d23f3bb40f..f91ff2fb2b3 100644 --- a/ninja/src/build.cc +++ b/ninja/src/build.cc @@ -25,6 +25,7 @@ #endif #include "build_log.h" +#include "debug_flags.h" #include "depfile_parser.h" #include "deps_log.h" #include "disk_interface.h" @@ -362,11 +363,8 @@ void Plan::ResumeDelayedJobs(Edge* edge) { void Plan::EdgeFinished(Edge* edge) { map::iterator i = want_.find(edge); assert(i != want_.end()); - if (i->second) { + if (i->second) --wanted_edges_; - if (!edge->is_phony()) - --command_edges_; - } want_.erase(i); edge->outputs_ready_ = true; @@ -411,31 +409,27 @@ void Plan::CleanNode(DependencyScan* scan, Node* node) { if (want_i == want_.end() || !want_i->second) continue; + // Don't attempt to clean an edge if it failed to load deps. + if ((*ei)->deps_missing_) + continue; + // If all non-order-only inputs for this edge are now clean, // we might have changed the dirty state of the outputs. vector::iterator begin = (*ei)->inputs_.begin(), end = (*ei)->inputs_.end() - (*ei)->order_only_deps_; if (find_if(begin, end, mem_fun(&Node::dirty)) == end) { - // Recompute most_recent_input and command. + // Recompute most_recent_input. Node* most_recent_input = NULL; for (vector::iterator ni = begin; ni != end; ++ni) { if (!most_recent_input || (*ni)->mtime() > most_recent_input->mtime()) most_recent_input = *ni; } - string command = (*ei)->EvaluateCommand(true); // Now, this edge is dirty if any of the outputs are dirty. - bool dirty = false; - for (vector::iterator ni = (*ei)->outputs_.begin(); - !dirty && ni != (*ei)->outputs_.end(); ++ni) { - dirty = scan->RecomputeOutputDirty(*ei, most_recent_input, 0, - command, *ni); - } - // If the edge isn't dirty, clean the outputs and mark the edge as not // wanted. - if (!dirty) { + if (!scan->RecomputeOutputsDirty(*ei, most_recent_input)) { for (vector::iterator ni = (*ei)->outputs_.begin(); ni != (*ei)->outputs_.end(); ++ni) { CleanNode(scan, *ni); @@ -720,9 +714,11 @@ 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"); @@ -783,7 +779,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { // Delete any left over response file. string rspfile = edge->GetBinding("rspfile"); - if (!rspfile.empty()) + if (!rspfile.empty() && !g_keep_rsp) disk_interface_->RemoveFile(rspfile); if (scan_.build_log()) { @@ -808,12 +804,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/ninja/src/build.h b/ninja/src/build.h index a872f6cf282..eb3636a9f3f 100644 --- a/ninja/src/build.h +++ b/ninja/src/build.h @@ -51,7 +51,7 @@ struct Plan { Edge* FindWork(); /// Returns true if there's more work to be done. - bool more_to_do() const { return (command_edges_ > 0); } + bool more_to_do() const { return wanted_edges_ > 0 && command_edges_ > 0; } /// Dumps the current state of the plan. void Dump(); @@ -180,8 +180,9 @@ struct Builder { BuildStatus* status_; private: - bool ExtractDeps(CommandRunner::Result* result, const string& deps_type, - 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/ninja/src/build_log.cc b/ninja/src/build_log.cc index 1374bd068f8..3f24c16d387 100644 --- a/ninja/src/build_log.cc +++ b/ninja/src/build_log.cc @@ -54,7 +54,7 @@ uint64_t MurmurHash64A(const void* key, size_t len) { const uint64_t m = BIG_CONSTANT(0xc6a4a7935bd1e995); const int r = 47; uint64_t h = seed ^ (len * m); - const unsigned char * data = (const unsigned char *)key; + const unsigned char* data = (const unsigned char*)key; while (len >= 8) { uint64_t k; memcpy(&k, data, sizeof k); @@ -108,9 +108,10 @@ BuildLog::~BuildLog() { Close(); } -bool BuildLog::OpenForWrite(const string& path, string* err) { +bool BuildLog::OpenForWrite(const string& path, const BuildLogUser& user, + string* err) { if (needs_recompaction_) { - if (!Recompact(path, err)) + if (!Recompact(path, user, err)) return false; } @@ -350,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, string* err) { +bool BuildLog::Recompact(const string& path, const BuildLogUser& user, + string* err) { METRIC_RECORD(".ninja_log recompact"); printf("Recompacting log...\n"); @@ -368,7 +370,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 (user.IsPathDead(i->first)) { + dead_outputs.push_back(i->first); + continue; + } + if (!WriteEntry(f, *i->second)) { *err = strerror(errno); fclose(f); @@ -376,6 +384,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/ninja/src/build_log.h b/ninja/src/build_log.h index eeac5b3b0b4..fe81a851f41 100644 --- a/ninja/src/build_log.h +++ b/ninja/src/build_log.h @@ -25,6 +25,13 @@ using namespace std; struct Edge; +/// 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) const = 0; +}; + /// Store a log of every command ran for every build. /// It has a few uses: /// @@ -36,7 +43,7 @@ struct BuildLog { BuildLog(); ~BuildLog(); - bool OpenForWrite(const string& path, 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(); @@ -72,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, 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/ninja/src/build_log_perftest.cc b/ninja/src/build_log_perftest.cc index a09beb827e8..810c06554aa 100644 --- a/ninja/src/build_log_perftest.cc +++ b/ninja/src/build_log_perftest.cc @@ -28,10 +28,15 @@ const char kTestFilename[] = "BuildLogPerfTest-tempfile"; +struct NoDeadPaths : public BuildLogUser { + virtual bool IsPathDead(StringPiece) const { 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/ninja/src/build_log_test.cc b/ninja/src/build_log_test.cc index 4639bc93309..6738c7b5dcc 100644 --- a/ninja/src/build_log_test.cc +++ b/ninja/src/build_log_test.cc @@ -30,7 +30,7 @@ namespace { const char kTestFilename[] = "BuildLogTest-tempfile"; -struct BuildLogTest : public StateTestWithBuiltinRules { +struct BuildLogTest : public StateTestWithBuiltinRules, public BuildLogUser { 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) const { 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); @@ -261,4 +262,44 @@ TEST_F(BuildLogTest, MultiTargetEdge) { ASSERT_EQ(22, e2->end_time); } +struct BuildLogRecompactTest : public BuildLogTest { + virtual bool IsPathDead(StringPiece s) const { 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 diff --git a/ninja/src/build_test.cc b/ninja/src/build_test.cc index d1ac0efb8bb..86a911bd281 100644 --- a/ninja/src/build_test.cc +++ b/ninja/src/build_test.cc @@ -412,7 +412,7 @@ struct FakeCommandRunner : public CommandRunner { VirtualFileSystem* fs_; }; -struct BuildTest : public StateTestWithBuiltinRules { +struct BuildTest : public StateTestWithBuiltinRules, public BuildLogUser { BuildTest() : config_(MakeConfig()), command_runner_(&fs_), builder_(&state_, config_, NULL, NULL, &fs_), status_(config_) { @@ -435,6 +435,15 @@ struct BuildTest : public StateTestWithBuiltinRules { builder_.command_runner_.release(); } + 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. + /// Handy to check for NOOP builds, and higher-level rebuild tests. + void RebuildTarget(const string& target, const char* manifest, + const char* log_path = NULL, + const char* deps_path = NULL); + // Mark a path dirty. void Dirty(const string& path); @@ -452,6 +461,41 @@ struct BuildTest : public StateTestWithBuiltinRules { BuildStatus status_; }; +void BuildTest::RebuildTarget(const string& target, const char* manifest, + const char* log_path, const char* deps_path) { + State state; + ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); + AssertParse(&state, manifest); + + string err; + 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_EQ("", err); + pbuild_log = &build_log; + } + + DepsLog deps_log, *pdeps_log = NULL; + if (deps_path) { + ASSERT_TRUE(deps_log.Load(deps_path, &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite(deps_path, &err)); + ASSERT_EQ("", err); + pdeps_log = &deps_log; + } + + Builder builder(&state, config_, pbuild_log, pdeps_log, &fs_); + EXPECT_TRUE(builder.AddTarget(target, &err)); + + command_runner_.commands_ran_.clear(); + builder.command_runner_.reset(&command_runner_); + if (!builder.AlreadyUpToDate()) { + bool build_res = builder.Build(&err); + EXPECT_TRUE(build_res) << "builder.Build(&err)"; + } + builder.command_runner_.release(); +} + bool FakeCommandRunner::CanRunMore() { // Only run one at a time. return last_command_ == NULL; @@ -1018,6 +1062,7 @@ TEST_F(BuildWithLogTest, RestatTest) { ASSERT_EQ("", err); EXPECT_TRUE(builder_.Build(&err)); ASSERT_EQ("", err); + EXPECT_EQ("[3/3]", builder_.status_->FormatProgressStatus("[%s/%t]")); command_runner_.commands_ran_.clear(); state_.Reset(); @@ -1166,7 +1211,7 @@ TEST_F(BuildWithLogTest, RestatMissingInput) { // See that an entry in the logfile is created, capturing // the right mtime - BuildLog::LogEntry * log_entry = build_log_.LookupByOutput("out1"); + BuildLog::LogEntry* log_entry = build_log_.LookupByOutput("out1"); ASSERT_TRUE(NULL != log_entry); ASSERT_EQ(restat_mtime, log_entry->restat_mtime); @@ -1337,7 +1382,7 @@ TEST_F(BuildWithLogTest, RspFileCmdLineChange) { // 3. Alter the entry in the logfile // (to simulate a change in the command line between 2 builds) - BuildLog::LogEntry * log_entry = build_log_.LookupByOutput("out"); + BuildLog::LogEntry* log_entry = build_log_.LookupByOutput("out"); ASSERT_TRUE(NULL != log_entry); ASSERT_NO_FATAL_FAILURE(AssertHash( "cat out.rsp > out;rspfile=Original very long command", @@ -1779,7 +1824,7 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { // Expect three new edges: one generating foo.o, and two more from // loading the depfile. - ASSERT_EQ(3, (int)state.edges_.size()); + ASSERT_EQ(3u, state.edges_.size()); // Expect our edge to now have three inputs: foo.c and two headers. ASSERT_EQ(3u, edge->inputs_.size()); @@ -1790,3 +1835,79 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { builder.command_runner_.release(); } } + +/// Check that a restat rule doesn't clear an edge if the depfile is missing. +/// Follows from: https://github.com/martine/ninja/issues/603 +TEST_F(BuildTest, RestatMissingDepfile) { +const char* manifest = +"rule true\n" +" command = true\n" // Would be "write if out-of-date" in reality. +" restat = 1\n" +"build header.h: true header.in\n" +"build out: cat header.h\n" +" depfile = out.d\n"; + + fs_.Create("header.h", ""); + fs_.Tick(); + fs_.Create("out", ""); + fs_.Create("header.in", ""); + + // Normally, only 'header.h' would be rebuilt, as + // its rule doesn't touch the output and has 'restat=1' set. + // But we are also missing the depfile for 'out', + // which should force its command to run anyway! + RebuildTarget("out", manifest); + ASSERT_EQ(2u, command_runner_.commands_ran_.size()); +} + +/// Check that a restat rule doesn't clear an edge if the deps are missing. +/// https://github.com/martine/ninja/issues/603 +TEST_F(BuildWithDepsLogTest, RestatMissingDepfileDepslog) { + string err; + const char* manifest = +"rule true\n" +" command = true\n" // Would be "write if out-of-date" in reality. +" restat = 1\n" +"build header.h: true header.in\n" +"build out: cat header.h\n" +" deps = gcc\n" +" depfile = out.d\n"; + + // Build once to populate ninja deps logs from out.d + fs_.Create("header.in", ""); + fs_.Create("out.d", "out: header.h"); + fs_.Create("header.h", ""); + + RebuildTarget("out", manifest, "build_log", "ninja_deps"); + ASSERT_EQ(2u, command_runner_.commands_ran_.size()); + + // Sanity: this rebuild should be NOOP + RebuildTarget("out", manifest, "build_log", "ninja_deps"); + ASSERT_EQ(0u, command_runner_.commands_ran_.size()); + + // Touch 'header.in', blank dependencies log (create a different one). + // Building header.h triggers 'restat' outputs cleanup. + // Validate that out is rebuilt netherless, as deps are missing. + fs_.Tick(); + fs_.Create("header.in", ""); + + // (switch to a new blank deps_log "ninja_deps2") + RebuildTarget("out", manifest, "build_log", "ninja_deps2"); + ASSERT_EQ(2u, command_runner_.commands_ran_.size()); + + // Sanity: this build should be NOOP + RebuildTarget("out", manifest, "build_log", "ninja_deps2"); + ASSERT_EQ(0u, command_runner_.commands_ran_.size()); + + // Check that invalidating deps by target timestamp also works here + // Repeat the test but touch target instead of blanking the log. + fs_.Tick(); + fs_.Create("header.in", ""); + fs_.Create("out", ""); + RebuildTarget("out", manifest, "build_log", "ninja_deps2"); + ASSERT_EQ(2u, command_runner_.commands_ran_.size()); + + // And this build should be NOOP again + RebuildTarget("out", manifest, "build_log", "ninja_deps2"); + ASSERT_EQ(0u, command_runner_.commands_ran_.size()); +} diff --git a/ninja/src/debug_flags.cc b/ninja/src/debug_flags.cc new file mode 100644 index 00000000000..75f1ea57502 --- /dev/null +++ b/ninja/src/debug_flags.cc @@ -0,0 +1,17 @@ +// Copyright 2012 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. + +bool g_explaining = false; + +bool g_keep_rsp = false; diff --git a/ninja/src/debug_flags.h b/ninja/src/debug_flags.h new file mode 100644 index 00000000000..ba3ebf36a6c --- /dev/null +++ b/ninja/src/debug_flags.h @@ -0,0 +1,29 @@ +// Copyright 2012 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. + +#ifndef NINJA_EXPLAIN_H_ +#define NINJA_EXPLAIN_H_ + +#include + +#define EXPLAIN(fmt, ...) { \ + if (g_explaining) \ + fprintf(stderr, "ninja explain: " fmt "\n", __VA_ARGS__); \ +} + +extern bool g_explaining; + +extern bool g_keep_rsp; + +#endif // NINJA_EXPLAIN_H_ diff --git a/ninja/src/depfile_parser.cc b/ninja/src/depfile_parser.cc index 5a30c6b1bb8..49c7d7b316c 100644 --- a/ninja/src/depfile_parser.cc +++ b/ninja/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/ninja/src/depfile_parser.in.cc b/ninja/src/depfile_parser.in.cc index cf24a09cc20..8bb6d8444d7 100644 --- a/ninja/src/depfile_parser.in.cc +++ b/ninja/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/ninja/src/deps_log.cc b/ninja/src/deps_log.cc index 2c4e3c29a1f..61df387e0db 100644 --- a/ninja/src/deps_log.cc +++ b/ninja/src/deps_log.cc @@ -30,15 +30,11 @@ // The version is stored as 4 bytes after the signature and also serves as a // byte order mark. Signature and version combined are 16 bytes long. const char kFileSignature[] = "# ninjadeps\n"; -const int kCurrentVersion = 1; +const int kCurrentVersion = 3; -// Since the size field is 2 bytes and the top bit marks deps entries, a single -// record can be at most 32 kB. Set the buffer size to this and flush the file -// buffer after every record to make sure records aren't written partially. -const int kMaxBufferSize = 1 << 15; - -// Record size is currently limited to 15 bit -const size_t kMaxRecordSize = (1 << 15) - 1; +// Record size is currently limited to less than the full 32 bit, due to +// internal buffers having to have this size. +const unsigned kMaxRecordSize = (1 << 19) - 1; DepsLog::~DepsLog() { Close(); @@ -55,7 +51,9 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { *err = strerror(errno); return false; } - setvbuf(file_, NULL, _IOFBF, kMaxBufferSize); + // Set the buffer size to this and flush the file buffer after every record + // to make sure records aren't written partially. + setvbuf(file_, NULL, _IOFBF, kMaxRecordSize + 1); SetCloseOnExec(fileno(file_)); // Opening a file in append mode doesn't set the file pointer to the file's @@ -126,14 +124,13 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, return true; // Update on-disk representation. - size_t size = 4 * (1 + 1 + (uint16_t)node_count); + unsigned size = 4 * (1 + 1 + node_count); if (size > kMaxRecordSize) { errno = ERANGE; return false; } - size |= 0x8000; // Deps record: set high bit. - uint16_t size16 = (uint16_t)size; - if (fwrite(&size16, 2, 1, file_) < 1) + size |= 0x80000000; // Deps record: set high bit. + if (fwrite(&size, 4, 1, file_) < 1) return false; int id = node->id(); if (fwrite(&id, 4, 1, file_) < 1) @@ -147,7 +144,7 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, return false; } if (fflush(file_) != 0) - return false; + return false; // Update in-memory representation. Deps* deps = new Deps(mtime, node_count); @@ -166,7 +163,7 @@ void DepsLog::Close() { bool DepsLog::Load(const string& path, State* state, string* err) { METRIC_RECORD(".ninja_deps load"); - char buf[32 << 10]; + char buf[kMaxRecordSize + 1]; FILE* f = fopen(path.c_str(), "rb"); if (!f) { if (errno == ENOENT) @@ -179,9 +176,16 @@ bool DepsLog::Load(const string& path, State* state, string* err) { int version = 0; if (!fgets(buf, sizeof(buf), f) || fread(&version, 4, 1, f) < 1) valid_header = false; + // Note: For version differences, this should migrate to the new format. + // But the v1 format could sometimes (rarely) end up with invalid data, so + // don't migrate v1 to v3 to force a rebuild. (v2 only existed for a few days, + // and there was no release with it, so pretend that it never happened.) if (!valid_header || strcmp(buf, kFileSignature) != 0 || version != kCurrentVersion) { - *err = "bad deps log signature or version; starting over"; + if (version == 1) + *err = "deps log version change; rebuilding"; + else + *err = "bad deps log signature or version; starting over"; fclose(f); unlink(path.c_str()); // Don't report this as a failure. An empty deps log will cause @@ -196,16 +200,16 @@ bool DepsLog::Load(const string& path, State* state, string* err) { for (;;) { offset = ftell(f); - uint16_t size; - if (fread(&size, 2, 1, f) < 1) { + unsigned size; + if (fread(&size, 4, 1, f) < 1) { if (!feof(f)) read_failed = true; break; } - bool is_deps = (size >> 15) != 0; - size = size & 0x7FFF; + bool is_deps = (size >> 31) != 0; + size = size & 0x7FFFFFFF; - if (fread(buf, size, 1, f) < 1) { + if (fread(buf, size, 1, f) < 1 || size > kMaxRecordSize) { read_failed = true; break; } @@ -229,10 +233,29 @@ bool DepsLog::Load(const string& path, State* state, string* err) { if (!UpdateDeps(out_id, deps)) ++unique_dep_record_count; } else { - StringPiece path(buf, size); + int path_size = size - 4; + assert(path_size > 0); // CanonicalizePath() rejects empty paths. + // There can be up to 3 bytes of padding. + if (buf[path_size - 1] == '\0') --path_size; + if (buf[path_size - 1] == '\0') --path_size; + if (buf[path_size - 1] == '\0') --path_size; + StringPiece path(buf, path_size); Node* node = state->GetNode(path); + + // Check that the expected index matches the actual index. This can only + // happen if two ninja processes write to the same deps log concurrently. + // (This uses unary complement to make the checksum look less like a + // dependency record entry.) + unsigned checksum = *reinterpret_cast(buf + size - 4); + int expected_id = ~checksum; + int id = nodes_.size(); + if (id != expected_id) { + read_failed = true; + break; + } + assert(node->id() < 0); - node->set_id(nodes_.size()); + node->set_id(id); nodes_.push_back(node); } } @@ -302,6 +325,9 @@ 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. + if (!IsDepsEntryLiveFor(nodes_[old_id])) + continue; + if (!new_log.RecordDeps(nodes_[old_id], deps->mtime, deps->node_count, deps->nodes)) { new_log.Close(); @@ -328,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); @@ -340,22 +376,30 @@ bool DepsLog::UpdateDeps(int out_id, Deps* deps) { } bool DepsLog::RecordId(Node* node) { - size_t size = node->path().size(); + int path_size = node->path().size(); + int padding = (4 - path_size % 4) % 4; // Pad path to 4 byte boundary. + + unsigned size = path_size + padding + 4; if (size > kMaxRecordSize) { errno = ERANGE; return false; } - uint16_t size16 = (uint16_t)size; - if (fwrite(&size16, 2, 1, file_) < 1) + if (fwrite(&size, 4, 1, file_) < 1) return false; - if (fwrite(node->path().data(), node->path().size(), 1, file_) < 1) { + if (fwrite(node->path().data(), path_size, 1, file_) < 1) { assert(node->path().size() > 0); return false; } + if (padding && fwrite("\0\0", padding, 1, file_) < 1) + return false; + int id = nodes_.size(); + unsigned checksum = ~(unsigned)id; + if (fwrite(&checksum, 4, 1, file_) < 1) + return false; if (fflush(file_) != 0) return false; - node->set_id(nodes_.size()); + node->set_id(id); nodes_.push_back(node); return true; diff --git a/ninja/src/deps_log.h b/ninja/src/deps_log.h index de0fe639d14..cec0257ceff 100644 --- a/ninja/src/deps_log.h +++ b/ninja/src/deps_log.h @@ -50,9 +50,12 @@ struct State; /// A dependency list maps an output id to a list of input ids. /// /// Concretely, a record is: -/// two bytes record length, high bit indicates record type -/// (implies max record length 32k) -/// path records contain just the string name of the path +/// four bytes record length, high bit indicates record type +/// (but max record sizes are capped at 512kB) +/// path records contain the string name of the path, followed by up to 3 +/// padding bytes to align on 4 byte boundaries, followed by the +/// one's complement of the expected index of the record (to detect +/// concurrent writes of multiple ninja processes to the log). /// dependency records are an array of 4-byte integers /// [output path id, output path mtime, input path id, input path id...] /// (The mtime is compared against the on-disk output path mtime @@ -85,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/ninja/src/deps_log_test.cc b/ninja/src/deps_log_test.cc index 3b329635519..e8e5138fc2b 100644 --- a/ninja/src/deps_log_test.cc +++ b/ninja/src/deps_log_test.cc @@ -82,6 +82,39 @@ TEST_F(DepsLogTest, WriteRead) { ASSERT_EQ("bar2.h", log_deps->nodes[1]->path()); } +TEST_F(DepsLogTest, LotsOfDeps) { + const int kNumDeps = 100000; // More than 64k. + + State state1; + DepsLog log1; + string err; + EXPECT_TRUE(log1.OpenForWrite(kTestFilename, &err)); + ASSERT_EQ("", err); + + { + vector deps; + for (int i = 0; i < kNumDeps; ++i) { + char buf[32]; + sprintf(buf, "file%d.h", i); + deps.push_back(state1.GetNode(buf)); + } + log1.RecordDeps(state1.GetNode("out.o"), 1, deps); + + DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o")); + ASSERT_EQ(kNumDeps, log_deps->node_count); + } + + log1.Close(); + + State state2; + DepsLog log2; + EXPECT_TRUE(log2.Load(kTestFilename, &state2, &err)); + ASSERT_EQ("", err); + + DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out.o")); + ASSERT_EQ(kNumDeps, log_deps->node_count); +} + // Verify that adding the same deps twice doesn't grow the file. TEST_F(DepsLogTest, DoubleEntry) { // Write some deps to the file and grab its size. @@ -130,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)); @@ -161,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)); @@ -182,8 +224,10 @@ 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)); DepsLog log; string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); @@ -224,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. diff --git a/ninja/src/disk_interface.cc b/ninja/src/disk_interface.cc index ee3e99a3b98..3233144c893 100644 --- a/ninja/src/disk_interface.cc +++ b/ninja/src/disk_interface.cc @@ -121,7 +121,7 @@ TimeStamp RealDiskInterface::Stat(const string& path) { } bool RealDiskInterface::WriteFile(const string& path, const string& contents) { - FILE * fp = fopen(path.c_str(), "w"); + FILE* fp = fopen(path.c_str(), "w"); if (fp == NULL) { Error("WriteFile(%s): Unable to create file. %s", path.c_str(), strerror(errno)); diff --git a/ninja/src/edit_distance.cc b/ninja/src/edit_distance.cc index cc4483ffa5b..9553c6ea89d 100644 --- a/ninja/src/edit_distance.cc +++ b/ninja/src/edit_distance.cc @@ -14,6 +14,7 @@ #include "edit_distance.h" +#include #include int EditDistance(const StringPiece& s1, diff --git a/ninja/src/explain.cc b/ninja/src/explain.cc deleted file mode 100644 index 4e14c25a07a..00000000000 --- a/ninja/src/explain.cc +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2012 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. - -bool g_explaining = false; diff --git a/ninja/src/explain.h b/ninja/src/explain.h deleted file mode 100644 index d4f6a6c2505..00000000000 --- a/ninja/src/explain.h +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2012 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. - -#ifndef NINJA_EXPLAIN_H_ -#define NINJA_EXPLAIN_H_ - -#include - -#define EXPLAIN(fmt, ...) { \ - if (g_explaining) \ - fprintf(stderr, "ninja explain: " fmt "\n", __VA_ARGS__); \ -} - -extern bool g_explaining; - -#endif // NINJA_EXPLAIN_H_ diff --git a/ninja/src/graph.cc b/ninja/src/graph.cc index fdd93de9f4c..65f92442170 100644 --- a/ninja/src/graph.cc +++ b/ninja/src/graph.cc @@ -18,10 +18,10 @@ #include #include "build_log.h" +#include "debug_flags.h" #include "depfile_parser.h" #include "deps_log.h" #include "disk_interface.h" -#include "explain.h" #include "manifest_parser.h" #include "metrics.h" #include "state.h" @@ -60,13 +60,13 @@ bool Rule::IsReservedBinding(const string& var) { bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { bool dirty = false; edge->outputs_ready_ = true; + edge->deps_missing_ = false; - TimeStamp deps_mtime = 0; - if (!dep_loader_.LoadDeps(edge, &deps_mtime, err)) { + if (!dep_loader_.LoadDeps(edge, err)) { if (!err->empty()) return false; // Failed to load dependency info: rebuild to regenerate it. - dirty = true; + dirty = edge->deps_missing_ = true; } // Visit all inputs; we're dirty if any of the inputs are dirty. @@ -107,19 +107,8 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { // We may also be dirty due to output state: missing outputs, out of // date outputs, etc. Visit all outputs and determine whether they're dirty. - if (!dirty) { - string command = edge->EvaluateCommand(true); - - for (vector::iterator i = edge->outputs_.begin(); - i != edge->outputs_.end(); ++i) { - (*i)->StatIfNecessary(disk_interface_); - if (RecomputeOutputDirty(edge, most_recent_input, deps_mtime, - command, *i)) { - dirty = true; - break; - } - } - } + if (!dirty) + dirty = RecomputeOutputsDirty(edge, most_recent_input); // Finally, visit each output to mark off that we've visited it, and update // their dirty state if necessary. @@ -141,9 +130,20 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { return true; } +bool DependencyScan::RecomputeOutputsDirty(Edge* edge, + Node* most_recent_input) { + string command = edge->EvaluateCommand(true); + for (vector::iterator i = edge->outputs_.begin(); + i != edge->outputs_.end(); ++i) { + (*i)->StatIfNecessary(disk_interface_); + if (RecomputeOutputDirty(edge, most_recent_input, command, *i)) + return true; + } + return false; +} + bool DependencyScan::RecomputeOutputDirty(Edge* edge, Node* most_recent_input, - TimeStamp deps_mtime, const string& command, Node* output) { if (edge->is_phony()) { @@ -185,13 +185,6 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge, } } - // Dirty if the output is newer than the deps. - if (deps_mtime && output->mtime() > deps_mtime) { - EXPLAIN("stored deps info out of date for for %s (%d vs %d)", - output->path().c_str(), deps_mtime, output->mtime()); - return true; - } - // May also be dirty due to the command changing since the last build. // But if this is a generator rule, the command changing does not make us // dirty. @@ -257,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; } @@ -332,28 +322,14 @@ void Node::Dump(const char* prefix) const { } } -bool ImplicitDepLoader::LoadDeps(Edge* edge, TimeStamp* mtime, string* err) { +bool ImplicitDepLoader::LoadDeps(Edge* edge, string* err) { string deps_type = edge->GetBinding("deps"); - if (!deps_type.empty()) { - if (!LoadDepsFromLog(edge, mtime, err)) { - if (!err->empty()) - return false; - EXPLAIN("deps for %s are missing", edge->outputs_[0]->path().c_str()); - return false; - } - return true; - } + if (!deps_type.empty()) + return LoadDepsFromLog(edge, err); string depfile = edge->GetBinding("depfile"); - if (!depfile.empty()) { - if (!LoadDepFile(edge, depfile, err)) { - if (!err->empty()) - return false; - EXPLAIN("depfile '%s' is missing", depfile.c_str()); - return false; - } - return true; - } + if (!depfile.empty()) + return LoadDepFile(edge, depfile, err); // No deps to load. return true; @@ -368,8 +344,10 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, return false; } // On a missing depfile: return false and empty *err. - if (content.empty()) + if (content.empty()) { + EXPLAIN("depfile '%s' is missing", path.c_str()); return false; + } DepfileParser depfile; string depfile_err; @@ -406,13 +384,21 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, return true; } -bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, TimeStamp* deps_mtime, - string* err) { - DepsLog::Deps* deps = deps_log_->GetDeps(edge->outputs_[0]); - if (!deps) +bool ImplicitDepLoader::LoadDepsFromLog(Edge* edge, string* err) { + // NOTE: deps are only supported for single-target edges. + Node* output = edge->outputs_[0]; + DepsLog::Deps* deps = deps_log_->GetDeps(output); + if (!deps) { + EXPLAIN("deps for '%s' are missing", output->path().c_str()); return false; + } - *deps_mtime = deps->mtime; + // Deps are invalid if the output is newer than the deps. + if (output->mtime() > deps->mtime) { + EXPLAIN("stored deps info out of date for for '%s' (%d vs %d)", + output->path().c_str(), deps->mtime, output->mtime()); + return false; + } vector::iterator implicit_dep = PreallocateSpace(edge, deps->node_count); diff --git a/ninja/src/graph.h b/ninja/src/graph.h index 428ba01e65d..868413c3bce 100644 --- a/ninja/src/graph.h +++ b/ninja/src/graph.h @@ -135,8 +135,8 @@ struct Rule { /// An edge in the dependency graph; links between Nodes using Rules. struct Edge { - Edge() : rule_(NULL), env_(NULL), outputs_ready_(false), implicit_deps_(0), - order_only_deps_(0) {} + Edge() : rule_(NULL), env_(NULL), outputs_ready_(false), deps_missing_(false), + implicit_deps_(0), order_only_deps_(0) {} /// Return true if all inputs' in-edges are ready. bool AllInputsReady() const; @@ -157,6 +157,7 @@ struct Edge { vector outputs_; BindingEnv* env_; bool outputs_ready_; + bool deps_missing_; const Rule& rule() const { return *rule_; } Pool* pool() const { return pool_; } @@ -192,10 +193,10 @@ struct ImplicitDepLoader { DiskInterface* disk_interface) : state_(state), disk_interface_(disk_interface), deps_log_(deps_log) {} - /// Load implicit dependencies for \a edge. May fill in \a mtime with - /// the timestamp of the loaded information. - /// @return false on error (without filling \a err if info is just missing). - bool LoadDeps(Edge* edge, TimeStamp* mtime, string* err); + /// Load implicit dependencies for \a edge. + /// @return false on error (without filling \a err if info is just missing + // or out of date). + bool LoadDeps(Edge* edge, string* err); DepsLog* deps_log() const { return deps_log_; @@ -208,7 +209,7 @@ struct ImplicitDepLoader { /// Load implicit dependencies for \a edge from the DepsLog. /// @return false on error (without filling \a err if info is just missing). - bool LoadDepsFromLog(Edge* edge, TimeStamp* mtime, string* err); + bool LoadDepsFromLog(Edge* edge, string* err); /// Preallocate \a count spaces in the input array on \a edge, returning /// an iterator pointing at the first new space. @@ -240,11 +241,9 @@ struct DependencyScan { /// Returns false on failure. bool RecomputeDirty(Edge* edge, string* err); - /// Recompute whether a given single output should be marked dirty. + /// Recompute whether any output of the edge is dirty. /// Returns true if so. - bool RecomputeOutputDirty(Edge* edge, Node* most_recent_input, - TimeStamp deps_mtime, - const string& command, Node* output); + bool RecomputeOutputsDirty(Edge* edge, Node* most_recent_input); BuildLog* build_log() const { return build_log_; @@ -258,6 +257,11 @@ struct DependencyScan { } private: + /// Recompute whether a given single output should be marked dirty. + /// Returns true if so. + bool RecomputeOutputDirty(Edge* edge, Node* most_recent_input, + const string& command, Node* output); + BuildLog* build_log_; DiskInterface* disk_interface_; ImplicitDepLoader dep_loader_; diff --git a/ninja/src/graph_test.cc b/ninja/src/graph_test.cc index 85212167e95..14dc6780b53 100644 --- a/ninja/src/graph_test.cc +++ b/ninja/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/ninja/src/hash_map.h b/ninja/src/hash_map.h index 919b6fc5d14..77e75865901 100644 --- a/ninja/src/hash_map.h +++ b/ninja/src/hash_map.h @@ -15,6 +15,7 @@ #ifndef NINJA_MAP_H_ #define NINJA_MAP_H_ +#include #include #include "string_piece.h" @@ -25,7 +26,7 @@ unsigned int MurmurHash2(const void* key, size_t len) { const unsigned int m = 0x5bd1e995; const int r = 24; unsigned int h = seed ^ len; - const unsigned char * data = (const unsigned char *)key; + const unsigned char* data = (const unsigned char*)key; while (len >= 4) { unsigned int k; memcpy(&k, data, sizeof k); diff --git a/ninja/src/includes_normalize_test.cc b/ninja/src/includes_normalize_test.cc index 1713d5d516d..419996f4e49 100644 --- a/ninja/src/includes_normalize_test.cc +++ b/ninja/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/ninja/src/manifest_parser_test.cc b/ninja/src/manifest_parser_test.cc index 5ed158408fc..152b965d786 100644 --- a/ninja/src/manifest_parser_test.cc +++ b/ninja/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) { @@ -762,6 +766,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( diff --git a/ninja/src/metrics.cc b/ninja/src/metrics.cc index ca4f97a350b..a7d3c7ad5b9 100644 --- a/ninja/src/metrics.cc +++ b/ninja/src/metrics.cc @@ -24,6 +24,8 @@ #include #endif +#include + #include "util.h" Metrics* g_metrics = NULL; diff --git a/ninja/src/msvc_helper-win32.cc b/ninja/src/msvc_helper-win32.cc index 7c45029ccea..d2e2eb536c4 100644 --- a/ninja/src/msvc_helper-win32.cc +++ b/ninja/src/msvc_helper-win32.cc @@ -48,14 +48,15 @@ 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) { + const string kDepsPrefixEnglish = "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() ? kDepsPrefixEnglish : 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 +82,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 +93,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/ninja/src/msvc_helper.h b/ninja/src/msvc_helper.h index e207485bfcd..5d7dcb0c0b2 100644 --- a/ninja/src/msvc_helper.h +++ b/ninja/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); + 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 +42,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/ninja/src/msvc_helper_main-win32.cc b/ninja/src/msvc_helper_main-win32.cc index 8a0479c6497..58bc797144d 100644 --- a/ninja/src/msvc_helper_main-win32.cc +++ b/ninja/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,14 +127,19 @@ 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); } + if (output.empty()) + return exit_code; + // CLWrapper's output already as \r\n line endings, make sure the C runtime // doesn't expand this to \r\r\n. _setmode(_fileno(stdout), _O_BINARY); - printf("%s", output.c_str()); + // Avoid printf and C strings, since the actual output might contain null + // bytes like UTF-16 does (yuck). + fwrite(&output[0], 1, output.size(), stdout); return exit_code; } diff --git a/ninja/src/msvc_helper_test.cc b/ninja/src/msvc_helper_test.cc index 02f2863426f..48fbe21dfee 100644 --- a/ninja/src/msvc_helper_test.cc +++ b/ninja/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()); diff --git a/ninja/src/ninja.cc b/ninja/src/ninja.cc index 0586bdca907..03ca83b544c 100644 --- a/ninja/src/ninja.cc +++ b/ninja/src/ninja.cc @@ -32,8 +32,8 @@ #include "build_log.h" #include "deps_log.h" #include "clean.h" +#include "debug_flags.h" #include "disk_interface.h" -#include "explain.h" #include "graph.h" #include "graphviz.h" #include "manifest_parser.h" @@ -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 BuildLogUser { NinjaMain(const char* ninja_command, const BuildConfig& config) : ninja_command_(ninja_command), config_(config) {} @@ -137,6 +137,18 @@ struct NinjaMain { /// Dump the output requested by '-d stats'. void DumpMetrics(); + + 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 + // 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(); + } }; /// Subtools, accessible via "-t foo". @@ -444,9 +456,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 { @@ -747,6 +757,7 @@ bool DebugEnable(const string& name) { printf("debugging modes:\n" " 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" "multiple modes can be enabled via -d FOO -d BAR\n"); return false; } else if (name == "stats") { @@ -755,6 +766,9 @@ bool DebugEnable(const string& name) { } else if (name == "explain") { g_explaining = true; return true; + } else if (name == "keeprsp") { + g_keep_rsp = true; + return true; } else { const char* suggestion = SpellcheckString(name.c_str(), "stats", "explain", NULL); @@ -785,14 +799,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; } diff --git a/ninja/src/state.cc b/ninja/src/state.cc index 9b6160bc2ca..33f8423312a 100644 --- a/ninja/src/state.cc +++ b/ninja/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/ninja/src/state.h b/ninja/src/state.h index bde75ff774f..bcb0eff75ff 100644 --- a/ninja/src/state.h +++ b/ninja/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); diff --git a/ninja/src/util.cc b/ninja/src/util.cc index b9c2c0d59b9..24d231f9fd4 100644 --- a/ninja/src/util.cc +++ b/ninja/src/util.cc @@ -20,6 +20,7 @@ #include #endif +#include #include #include #include @@ -175,6 +176,108 @@ 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 '"': + 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) { @@ -300,35 +403,15 @@ string StripAnsiEscapeCodes(const string& in) { return stripped; } -#if defined(linux) || defined(__GLIBC__) -int GetProcessorCount() { - return get_nprocs(); -} -#elif defined(__APPLE__) || defined(__FreeBSD__) -int GetProcessorCount() { - int processors; - size_t processors_size = sizeof(processors); - int name[] = {CTL_HW, HW_NCPU}; - if (sysctl(name, sizeof(name) / sizeof(int), - &processors, &processors_size, - NULL, 0) < 0) { - return 0; - } - return processors; -} -#elif defined(_WIN32) int GetProcessorCount() { +#ifdef _WIN32 SYSTEM_INFO info; GetSystemInfo(&info); return info.dwNumberOfProcessors; -} #else -// This is what get_nprocs() should be doing in the Linux implementation -// above, but in a more standard way. -int GetProcessorCount() { return sysconf(_SC_NPROCESSORS_ONLN); -} #endif +} #if defined(_WIN32) || defined(__CYGWIN__) double GetLoadAverage() { diff --git a/ninja/src/util.h b/ninja/src/util.h index 6788410d2ee..71017703ff0 100644 --- a/ninja/src/util.h +++ b/ninja/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/ninja/src/util_test.cc b/ninja/src/util_test.cc index 1e290537c29..f827e5aaa07 100644 --- a/ninja/src/util_test.cc +++ b/ninja/src/util_test.cc @@ -136,6 +136,37 @@ 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(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); diff --git a/ninja/src/version.cc b/ninja/src/version.cc index 18fa96a07eb..1406d914e25 100644 --- a/ninja/src/version.cc +++ b/ninja/src/version.cc @@ -18,7 +18,7 @@ #include "util.h" -const char* kNinjaVersion = "1.3.0.git"; +const char* kNinjaVersion = "1.4.0.git"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.'); -- cgit v1.2.1