diff options
author | Evan Martin <martine@danga.com> | 2013-09-12 02:14:12 (GMT) |
---|---|---|
committer | Evan Martin <martine@danga.com> | 2013-09-12 02:15:28 (GMT) |
commit | 63d5b1013cafb2db95687cf446eb5bb68cf6a27a (patch) | |
tree | f25d3219f9eb5e874fafed41a4042b2592ac5675 | |
parent | 045d00847b191da46faf1c1b91368a252412bb17 (diff) | |
parent | 6f7ea464bb9161ce2e15deb97977886de152c12d (diff) | |
download | Ninja-63d5b1013cafb2db95687cf446eb5bb68cf6a27a.zip Ninja-63d5b1013cafb2db95687cf446eb5bb68cf6a27a.tar.gz Ninja-63d5b1013cafb2db95687cf446eb5bb68cf6a27a.tar.bz2 |
v1.4.0v1.4.0
-rw-r--r-- | .gitignore | 8 | ||||
-rw-r--r-- | RELEASING | 18 | ||||
-rwxr-xr-x | bootstrap.py | 6 | ||||
-rwxr-xr-x | configure.py | 6 | ||||
-rw-r--r-- | doc/manual.asciidoc | 8 | ||||
-rw-r--r-- | platform_helper.py | 8 | ||||
-rw-r--r-- | src/build.cc | 58 | ||||
-rw-r--r-- | src/build.h | 7 | ||||
-rw-r--r-- | src/build_log.cc | 48 | ||||
-rw-r--r-- | src/build_log.h | 4 | ||||
-rw-r--r-- | src/build_test.cc | 249 | ||||
-rw-r--r-- | src/debug_flags.cc (renamed from src/explain.cc) | 2 | ||||
-rw-r--r-- | src/debug_flags.h (renamed from src/explain.h) | 2 | ||||
-rw-r--r-- | src/deps_log.cc | 115 | ||||
-rw-r--r-- | src/deps_log.h | 9 | ||||
-rw-r--r-- | src/deps_log_test.cc | 33 | ||||
-rw-r--r-- | src/disk_interface.cc | 2 | ||||
-rw-r--r-- | src/graph.cc | 96 | ||||
-rw-r--r-- | src/graph.h | 26 | ||||
-rw-r--r-- | src/graph_test.cc | 20 | ||||
-rw-r--r-- | src/hash_map.h | 6 | ||||
-rw-r--r-- | src/line_printer.cc | 6 | ||||
-rw-r--r-- | src/manifest_parser.cc | 33 | ||||
-rw-r--r-- | src/manifest_parser.h | 2 | ||||
-rw-r--r-- | src/manifest_parser_test.cc | 124 | ||||
-rw-r--r-- | src/msvc_helper_main-win32.cc | 7 | ||||
-rw-r--r-- | src/ninja.cc | 89 | ||||
-rw-r--r-- | src/subprocess-posix.cc | 6 | ||||
-rw-r--r-- | src/subprocess_test.cc | 4 | ||||
-rw-r--r-- | src/util.cc | 24 | ||||
-rw-r--r-- | src/version.cc | 2 |
31 files changed, 813 insertions, 215 deletions
@@ -15,8 +15,16 @@ TAGS /doc/manual.html /doc/doxygen /gtest-1.6.0 +*.patch # Eclipse project files .project .cproject +# SublimeText project files +*.sublime-project +*.sublime-workspace + +# Ninja output +.ninja_deps +.ninja_log @@ -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/bootstrap.py b/bootstrap.py index 5682bf1..66ec85b 100755 --- a/bootstrap.py +++ b/bootstrap.py @@ -37,7 +37,7 @@ parser.add_option('--platform', 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 and OpenBSD, but older versions might need to use pselect instead",) + help="ppoll() is used by default on Linux, OpenBSD and Bitrig, but older versions might need to use pselect instead",) (options, conf_args) = parser.parse_args() @@ -53,7 +53,7 @@ def run(*args, **kwargs): # g++ call as well as in the later configure.py. cflags = os.environ.get('CFLAGS', '').split() ldflags = os.environ.get('LDFLAGS', '').split() -if platform.is_freebsd() or platform.is_openbsd(): +if platform.is_freebsd() or platform.is_openbsd() or platform.is_bitrig(): cflags.append('-I/usr/local/include') ldflags.append('-L/usr/local/lib') @@ -109,7 +109,7 @@ else: cflags.append('-D_WIN32_WINNT=0x0501') if options.x64: cflags.append('-m64') -if (platform.is_linux() or platform.is_openbsd()) 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") diff --git a/configure.py b/configure.py index 22eb1e5..9fe3be8 100755 --- a/configure.py +++ b/configure.py @@ -48,7 +48,7 @@ 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 on Linux and OpenBSD, but older versions might 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) @@ -165,7 +165,7 @@ else: cflags.append('-fno-omit-frame-pointer') libs.extend(['-Wl,--no-as-needed', '-lprofiler']) -if (platform.is_linux() or platform.is_openbsd()) 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 +263,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', diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 4ceec26..6b2296f 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -1,7 +1,7 @@ Ninja ===== Evan Martin <martine@danga.com> -v1.3.0, May 2013 +v1.4.0, September 2013 Introduction @@ -55,7 +55,7 @@ Here are the design goals of Ninja: higher-level build systems have different opinions about how code should be built; for example, should built objects live alongside the sources or should all build output go into a separate directory? - Is there an "package" rule that builds a distributable package of + Is there a "package" rule that builds a distributable package of the project? Sidestep these decisions by trying to allow either to be implemented, rather than choosing, even if that results in more verbosity. @@ -589,6 +589,7 @@ rule cc command = cl /showIncludes -c $in /Fo$out ---- +[[ref_pool]] Pools ~~~~~ @@ -669,6 +670,9 @@ A file is a series of declarations. A declaration can be one of: +include _path_+. The difference between these is explained below <<ref_scope,in the discussion about scoping>>. +6. A pool declaration, which looks like +pool _poolname_+. Pools are explained + <<ref_pool, in the section on pools>>. + Lexical syntax ~~~~~~~~~~~~~~ diff --git a/platform_helper.py b/platform_helper.py index 5097f49..b7447a1 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'] + 'mingw', 'msvc', 'gnukfreebsd8', 'bitrig'] class Platform( object ): def __init__( self, platform): @@ -41,7 +41,8 @@ class Platform( object ): self._platform = 'mingw' elif self._platform.startswith('win'): self._platform = 'msvc' - + elif self._platform.startswith('bitrig'): + self._platform = 'bitrig' def platform(self): return self._platform @@ -69,3 +70,6 @@ class Platform( object ): def is_sunos5(self): return self._platform == 'sunos5' + + def is_bitrig(self): + return self._platform == 'bitrig' diff --git a/src/build.cc b/src/build.cc index 5cf9d27..9718f85 100644 --- a/src/build.cc +++ b/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" @@ -408,38 +409,32 @@ 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<Node*>::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<Node*>::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, recompute the dirty state of each output. - bool all_outputs_clean = true; - for (vector<Node*>::iterator ni = (*ei)->outputs_.begin(); - ni != (*ei)->outputs_.end(); ++ni) { - if (!(*ni)->dirty()) - continue; - - if (scan->RecomputeOutputDirty(*ei, most_recent_input, 0, - command, *ni)) { - (*ni)->MarkDirty(); - all_outputs_clean = false; - } else { + + // Now, this edge is dirty if any of the outputs are dirty. + // If the edge isn't dirty, clean the outputs and mark the edge as not + // wanted. + if (!scan->RecomputeOutputsDirty(*ei, most_recent_input)) { + for (vector<Node*>::iterator ni = (*ei)->outputs_.begin(); + ni != (*ei)->outputs_.end(); ++ni) { CleanNode(scan, *ni); } - } - // If we cleaned all outputs, mark the node as not wanted. - if (all_outputs_clean) { want_i->second = false; --wanted_edges_; if (!(*ei)->is_phony()) @@ -641,7 +636,10 @@ bool Builder::Build(string* err) { } --pending_commands; - FinishCommand(&result); + if (!FinishCommand(&result, err)) { + status_->BuildFinished(); + return false; + } if (!result.success()) { if (failures_allowed) @@ -704,7 +702,7 @@ bool Builder::StartEdge(Edge* edge, string* err) { return true; } -void Builder::FinishCommand(CommandRunner::Result* result) { +bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { METRIC_RECORD("FinishCommand"); Edge* edge = result->edge; @@ -733,7 +731,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) { // The rest of this function only applies to successful commands. if (!result->success()) - return; + return true; // Restat the edge outputs, if necessary. TimeStamp restat_mtime = 0; @@ -763,7 +761,7 @@ void Builder::FinishCommand(CommandRunner::Result* result) { } string depfile = edge->GetBinding("depfile"); - if (restat_mtime != 0 && !depfile.empty()) { + if (restat_mtime != 0 && deps_type.empty() && !depfile.empty()) { TimeStamp depfile_mtime = disk_interface_->Stat(depfile); if (depfile_mtime > restat_mtime) restat_mtime = depfile_mtime; @@ -779,21 +777,27 @@ void Builder::FinishCommand(CommandRunner::Result* result) { // 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()) { - scan_.build_log()->RecordCommand(edge, start_time, end_time, - restat_mtime); + if (!scan_.build_log()->RecordCommand(edge, start_time, end_time, + restat_mtime)) { + *err = string("Error writing to build log: ") + strerror(errno); + return false; + } } if (!deps_type.empty() && !config_.dry_run) { assert(edge->outputs_.size() == 1 && "should have been rejected by parser"); Node* out = edge->outputs_[0]; TimeStamp deps_mtime = disk_interface_->Stat(out->path()); - scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes); + if (!scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes)) { + *err = string("Error writing to deps log: ") + strerror(errno); + return false; + } } - + return true; } bool Builder::ExtractDeps(CommandRunner::Result* result, diff --git a/src/build.h b/src/build.h index 2715c0c..5b6c83c 100644 --- a/src/build.h +++ b/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 wanted_edges_; } + bool more_to_do() const { return wanted_edges_ > 0 && command_edges_ > 0; } /// Dumps the current state of the plan. void Dump(); @@ -163,7 +163,10 @@ struct Builder { bool Build(string* err); bool StartEdge(Edge* edge, string* err); - void FinishCommand(CommandRunner::Result* result); + + /// Update status ninja logs following a command termination. + /// @return false if the build can not proceed further due to a fatal error. + bool FinishCommand(CommandRunner::Result* result, string* err); /// Used for tests. void SetBuildLog(BuildLog* log) { diff --git a/src/build_log.cc b/src/build_log.cc index 6b73002..b92a06f 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -54,26 +54,27 @@ 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 uint64_t * data = (const uint64_t *)key; - const uint64_t * end = data + (len/8); - while (data != end) { - uint64_t k = *data++; + const unsigned char* data = (const unsigned char*)key; + while (len >= 8) { + uint64_t k; + memcpy(&k, data, sizeof k); k *= m; k ^= k >> r; k *= m; h ^= k; h *= m; + data += 8; + len -= 8; } - const unsigned char* data2 = (const unsigned char*)data; switch (len & 7) { - case 7: h ^= uint64_t(data2[6]) << 48; - case 6: h ^= uint64_t(data2[5]) << 40; - case 5: h ^= uint64_t(data2[4]) << 32; - case 4: h ^= uint64_t(data2[3]) << 24; - case 3: h ^= uint64_t(data2[2]) << 16; - case 2: h ^= uint64_t(data2[1]) << 8; - case 1: h ^= uint64_t(data2[0]); + case 7: h ^= uint64_t(data[6]) << 48; + case 6: h ^= uint64_t(data[5]) << 40; + case 5: h ^= uint64_t(data[4]) << 32; + case 4: h ^= uint64_t(data[3]) << 24; + case 3: h ^= uint64_t(data[2]) << 16; + case 2: h ^= uint64_t(data[1]) << 8; + case 1: h ^= uint64_t(data[0]); h *= m; }; h ^= h >> r; @@ -109,7 +110,6 @@ BuildLog::~BuildLog() { bool BuildLog::OpenForWrite(const string& path, string* err) { if (needs_recompaction_) { - Close(); if (!Recompact(path, err)) return false; } @@ -136,7 +136,7 @@ bool BuildLog::OpenForWrite(const string& path, string* err) { return true; } -void BuildLog::RecordCommand(Edge* edge, int start_time, int end_time, +bool BuildLog::RecordCommand(Edge* edge, int start_time, int end_time, TimeStamp restat_mtime) { string command = edge->EvaluateCommand(true); uint64_t command_hash = LogEntry::HashCommand(command); @@ -156,9 +156,12 @@ void BuildLog::RecordCommand(Edge* edge, int start_time, int end_time, log_entry->end_time = end_time; log_entry->restat_mtime = restat_mtime; - if (log_file_) - WriteEntry(log_file_, *log_entry); + if (log_file_) { + if (!WriteEntry(log_file_, *log_entry)) + return false; + } } + return true; } void BuildLog::Close() { @@ -341,16 +344,17 @@ BuildLog::LogEntry* BuildLog::LookupByOutput(const string& path) { return NULL; } -void BuildLog::WriteEntry(FILE* f, const LogEntry& entry) { - fprintf(f, "%d\t%d\t%d\t%s\t%" PRIx64 "\n", +bool BuildLog::WriteEntry(FILE* f, const LogEntry& entry) { + return fprintf(f, "%d\t%d\t%d\t%s\t%" PRIx64 "\n", entry.start_time, entry.end_time, entry.restat_mtime, - entry.output.c_str(), entry.command_hash); + entry.output.c_str(), entry.command_hash) > 0; } bool BuildLog::Recompact(const string& path, string* err) { METRIC_RECORD(".ninja_log recompact"); printf("Recompacting log...\n"); + Close(); string temp_path = path + ".recompact"; FILE* f = fopen(temp_path.c_str(), "wb"); if (!f) { @@ -365,7 +369,11 @@ bool BuildLog::Recompact(const string& path, string* err) { } for (Entries::iterator i = entries_.begin(); i != entries_.end(); ++i) { - WriteEntry(f, *i->second); + if (!WriteEntry(f, *i->second)) { + *err = strerror(errno); + fclose(f); + return false; + } } fclose(f); diff --git a/src/build_log.h b/src/build_log.h index 6eae89f..eeac5b3 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -37,7 +37,7 @@ struct BuildLog { ~BuildLog(); bool OpenForWrite(const string& path, string* err); - void RecordCommand(Edge* edge, int start_time, int end_time, + bool RecordCommand(Edge* edge, int start_time, int end_time, TimeStamp restat_mtime = 0); void Close(); @@ -69,7 +69,7 @@ struct BuildLog { LogEntry* LookupByOutput(const string& path); /// Serialize an entry into a log file. - void WriteEntry(FILE* f, const LogEntry& entry); + bool WriteEntry(FILE* f, const LogEntry& entry); /// Rewrite the known log entries, throwing away old data. bool Recompact(const string& path, string* err); diff --git a/src/build_test.cc b/src/build_test.cc index ed9ade3..e206cd8 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -435,6 +435,13 @@ struct BuildTest : public StateTestWithBuiltinRules { builder_.command_runner_.release(); } + /// 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 +459,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, &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 +1060,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(); @@ -1094,6 +1137,46 @@ TEST_F(BuildWithLogTest, RestatMissingFile) { ASSERT_EQ(1u, command_runner_.commands_ran_.size()); } +TEST_F(BuildWithLogTest, RestatSingleDependentOutputDirty) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, + "rule true\n" + " command = true\n" + " restat = 1\n" + "rule touch\n" + " command = touch\n" + "build out1: true in\n" + "build out2 out3: touch out1\n" + "build out4: touch out2\n" + )); + + // Create the necessary files + fs_.Create("in", ""); + + string err; + EXPECT_TRUE(builder_.AddTarget("out4", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder_.Build(&err)); + ASSERT_EQ("", err); + ASSERT_EQ(3u, command_runner_.commands_ran_.size()); + + fs_.Tick(); + fs_.Create("in", ""); + fs_.RemoveFile("out3"); + + // Since "in" is missing, out1 will be built. Since "out3" is missing, + // out2 and out3 will be built even though "in" is not touched when built. + // Then, since out2 is rebuilt, out4 should be rebuilt -- the restat on the + // "true" rule should not lead to the "touch" edge writing out2 and out3 being + // cleard. + command_runner_.commands_ran_.clear(); + state_.Reset(); + EXPECT_TRUE(builder_.AddTarget("out4", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder_.Build(&err)); + ASSERT_EQ("", err); + ASSERT_EQ(3u, command_runner_.commands_ran_.size()); +} + // Test scenario, in which an input file is removed, but output isn't changed // https://github.com/martine/ninja/issues/295 TEST_F(BuildWithLogTest, RestatMissingInput) { @@ -1126,7 +1209,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); @@ -1297,7 +1380,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", @@ -1604,7 +1687,30 @@ TEST_F(BuildWithDepsLogTest, DepsIgnoredInDryRun) { } /// Check that a restat rule generating a header cancels compilations correctly. -TEST_F(BuildWithDepsLogTest, RestatDepfileDependency) { +TEST_F(BuildTest, RestatDepfileDependency) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"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 in1\n" +" depfile = in1.d\n")); + + fs_.Create("header.h", ""); + fs_.Create("in1.d", "out: header.h"); + fs_.Tick(); + fs_.Create("header.in", ""); + + string err; + EXPECT_TRUE(builder_.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ("", err); +} + +/// Check that a restat rule generating a header cancels compilations correctly, +/// depslog case. +TEST_F(BuildWithDepsLogTest, RestatDepfileDependencyDepsLog) { string err; // Note: in1 was created by the superclass SetUp(). const char* manifest = @@ -1666,3 +1772,140 @@ TEST_F(BuildWithDepsLogTest, RestatDepfileDependency) { builder.command_runner_.release(); } } + +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"; + + fs_.Create("foo.c", ""); + + { + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + // Run the build once, everything should be ok. + DepsLog deps_log; + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + EXPECT_TRUE(builder.AddTarget("foo.o", &err)); + ASSERT_EQ("", err); + fs_.Create("foo.o.d", "foo.o: blah.h bar.h\n"); + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + + deps_log.Close(); + builder.command_runner_.release(); + } + + { + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + + Edge* edge = state.edges_.back(); + + state.GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. + EXPECT_TRUE(builder.AddTarget("foo.o", &err)); + ASSERT_EQ("", err); + + // Expect three new edges: one generating foo.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. + ASSERT_EQ(3u, edge->inputs_.size()); + + // Expect the command line we generate to only use the original input. + ASSERT_EQ("cc foo.c", edge->EvaluateCommand()); + + deps_log.Close(); + 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/src/explain.cc b/src/debug_flags.cc index 4e14c25..75f1ea5 100644 --- a/src/explain.cc +++ b/src/debug_flags.cc @@ -13,3 +13,5 @@ // limitations under the License. bool g_explaining = false; + +bool g_keep_rsp = false; diff --git a/src/explain.h b/src/debug_flags.h index d4f6a6c..ba3ebf3 100644 --- a/src/explain.h +++ b/src/debug_flags.h @@ -24,4 +24,6 @@ extern bool g_explaining; +extern bool g_keep_rsp; + #endif // NINJA_EXPLAIN_H_ diff --git a/src/deps_log.cc b/src/deps_log.cc index 931cc77..4f1214a 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -30,7 +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; + +// 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(); @@ -38,7 +42,6 @@ DepsLog::~DepsLog() { bool DepsLog::OpenForWrite(const string& path, string* err) { if (needs_recompaction_) { - Close(); if (!Recompact(path, err)) return false; } @@ -48,6 +51,9 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { *err = strerror(errno); return false; } + // 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 @@ -64,7 +70,10 @@ bool DepsLog::OpenForWrite(const string& path, string* err) { return false; } } - + if (fflush(file_) != 0) { + *err = strerror(errno); + return false; + } return true; } @@ -81,12 +90,14 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, // Assign ids to all nodes that are missing one. if (node->id() < 0) { - RecordId(node); + if (!RecordId(node)) + return false; made_change = true; } for (int i = 0; i < node_count; ++i) { if (nodes[i]->id() < 0) { - RecordId(nodes[i]); + if (!RecordId(nodes[i])) + return false; made_change = true; } } @@ -113,17 +124,27 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, return true; // Update on-disk representation. - uint16_t size = 4 * (1 + 1 + (uint16_t)node_count); - size |= 0x8000; // Deps record: set high bit. - fwrite(&size, 2, 1, file_); + unsigned size = 4 * (1 + 1 + node_count); + if (size > kMaxRecordSize) { + errno = ERANGE; + return false; + } + size |= 0x80000000; // Deps record: set high bit. + if (fwrite(&size, 4, 1, file_) < 1) + return false; int id = node->id(); - fwrite(&id, 4, 1, file_); + if (fwrite(&id, 4, 1, file_) < 1) + return false; int timestamp = mtime; - fwrite(×tamp, 4, 1, file_); + if (fwrite(×tamp, 4, 1, file_) < 1) + return false; for (int i = 0; i < node_count; ++i) { id = nodes[i]->id(); - fwrite(&id, 4, 1, file_); + if (fwrite(&id, 4, 1, file_) < 1) + return false; } + if (fflush(file_) != 0) + return false; // Update in-memory representation. Deps* deps = new Deps(mtime, node_count); @@ -142,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) @@ -155,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 @@ -172,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; } @@ -205,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<unsigned*>(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); } } @@ -257,6 +304,7 @@ bool DepsLog::Recompact(const string& path, string* err) { METRIC_RECORD(".ninja_deps recompact"); printf("Recompacting deps...\n"); + Close(); string temp_path = path + ".recompact"; // OpenForWrite() opens for append. Make sure it's not appending to a @@ -315,11 +363,30 @@ bool DepsLog::UpdateDeps(int out_id, Deps* deps) { } bool DepsLog::RecordId(Node* node) { - uint16_t size = (uint16_t)node->path().size(); - fwrite(&size, 2, 1, file_); - fwrite(node->path().data(), node->path().size(), 1, file_); + 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; + } + if (fwrite(&size, 4, 1, file_) < 1) + return false; + 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/src/deps_log.h b/src/deps_log.h index de0fe63..babf828 100644 --- a/src/deps_log.h +++ b/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 diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 3b32963..4e6cbac 100644 --- a/src/deps_log_test.cc +++ b/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<Node*> 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. diff --git a/src/disk_interface.cc b/src/disk_interface.cc index ee3e99a..3233144 100644 --- a/src/disk_interface.cc +++ b/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/src/graph.cc b/src/graph.cc index 7a57753..9801a7b 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -18,10 +18,10 @@ #include <stdio.h> #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<Node*>::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<Node*>::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. @@ -332,28 +325,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 +347,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,20 +387,29 @@ 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<Node*>::iterator implicit_dep = PreallocateSpace(edge, deps->node_count); for (int i = 0; i < deps->node_count; ++i, ++implicit_dep) { - *implicit_dep = deps->nodes[i]; - deps->nodes[i]->AddOutEdge(edge); - CreatePhonyInEdge(*implicit_dep); + Node* node = deps->nodes[i]; + *implicit_dep = node; + node->AddOutEdge(edge); + CreatePhonyInEdge(node); } return true; } diff --git a/src/graph.h b/src/graph.h index 428ba01..868413c 100644 --- a/src/graph.h +++ b/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<Node*> 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/src/graph_test.cc b/src/graph_test.cc index 63d5757..8521216 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -13,6 +13,7 @@ // limitations under the License. #include "graph.h" +#include "build.h" #include "test.h" @@ -226,3 +227,22 @@ TEST_F(GraphTest, DepfileOverrideParent) { Edge* edge = GetNode("out")->in_edge(); EXPECT_EQ("depfile is y", edge->GetBinding("command")); } + +// Verify that building a nested phony rule prints "no work to do" +TEST_F(GraphTest, NestedPhonyPrintsDone) { + AssertParse(&state_, +"build n1: phony \n" +"build n2: phony n1\n" + ); + string err; + Edge* edge = GetNode("n2")->in_edge(); + EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + ASSERT_EQ("", err); + + Plan plan_; + EXPECT_TRUE(plan_.AddTarget(GetNode("n2"), &err)); + ASSERT_EQ("", err); + + EXPECT_EQ(0, plan_.command_edge_count()); + ASSERT_FALSE(plan_.more_to_do()); +} diff --git a/src/hash_map.h b/src/hash_map.h index 076f6c0..c63aa88 100644 --- a/src/hash_map.h +++ b/src/hash_map.h @@ -15,6 +15,7 @@ #ifndef NINJA_MAP_H_ #define NINJA_MAP_H_ +#include <string.h> #include "string_piece.h" // MurmurHash2, by Austin Appleby @@ -24,9 +25,10 @@ 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 = *(unsigned int *)data; + unsigned int k; + memcpy(&k, data, sizeof k); k *= m; k ^= k >> r; k *= m; diff --git a/src/line_printer.cc b/src/line_printer.cc index a75eb05..3537e88 100644 --- a/src/line_printer.cc +++ b/src/line_printer.cc @@ -104,6 +104,10 @@ void LinePrinter::Print(string to_print, LineType type) { void LinePrinter::PrintOnNewLine(const string& to_print) { if (!have_blank_line_) printf("\n"); - printf("%s", to_print.c_str()); + if (!to_print.empty()) { + // 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); + } have_blank_line_ = to_print.empty() || *to_print.rbegin() == '\n'; } diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 3593567..20be7f3 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -15,6 +15,7 @@ #include "manifest_parser.h" #include <stdio.h> +#include <stdlib.h> #include <vector> #include "graph.h" @@ -27,20 +28,30 @@ ManifestParser::ManifestParser(State* state, FileReader* file_reader) : state_(state), file_reader_(file_reader) { env_ = &state->bindings_; } -bool ManifestParser::Load(const string& filename, string* err) { + +bool ManifestParser::Load(const string& filename, string* err, Lexer* parent) { + METRIC_RECORD(".ninja parse"); string contents; string read_err; if (!file_reader_->ReadFile(filename, &contents, &read_err)) { *err = "loading '" + filename + "': " + read_err; + if (parent) + parent->Error(string(*err), err); return false; } - contents.resize(contents.size() + 10); + + // The lexer needs a nul byte at the end of its input, to know when it's done. + // It takes a StringPiece, and StringPiece's string constructor uses + // string::data(). data()'s return value isn't guaranteed to be + // null-terminated (although in practice - libc++, libstdc++, msvc's stl -- + // it is, and C++11 demands that too), so add an explicit nul byte. + contents.resize(contents.size() + 1); + return Parse(filename, contents, err); } bool ManifestParser::Parse(const string& filename, const string& input, string* err) { - METRIC_RECORD(".ninja parse"); lexer_.Start(filename, input); for (;;) { @@ -145,10 +156,8 @@ bool ManifestParser::ParseRule(string* err) { if (!ExpectToken(Lexer::NEWLINE, err)) return false; - if (state_->LookupRule(name) != NULL) { - *err = "duplicate rule '" + name + "'"; - return false; - } + if (state_->LookupRule(name) != NULL) + return lexer_.Error("duplicate rule '" + name + "'", err); Rule* rule = new Rule(name); // XXX scoped_ptr @@ -306,7 +315,7 @@ bool ManifestParser::ParseEdge(string* err) { if (!pool_name.empty()) { Pool* pool = state_->LookupPool(pool_name); if (pool == NULL) - return lexer_.Error("unknown pool name", err); + return lexer_.Error("unknown pool name '" + pool_name + "'", err); edge->pool_ = pool; } @@ -339,17 +348,11 @@ bool ManifestParser::ParseEdge(string* err) { } bool ManifestParser::ParseFileInclude(bool new_scope, string* err) { - // XXX this should use ReadPath! EvalString eval; if (!lexer_.ReadPath(&eval, err)) return false; string path = eval.Evaluate(env_); - string contents; - string read_err; - if (!file_reader_->ReadFile(path, &contents, &read_err)) - return lexer_.Error("loading '" + path + "': " + read_err, err); - ManifestParser subparser(state_, file_reader_); if (new_scope) { subparser.env_ = new BindingEnv(env_); @@ -357,7 +360,7 @@ bool ManifestParser::ParseFileInclude(bool new_scope, string* err) { subparser.env_ = env_; } - if (!subparser.Parse(path, contents, err)) + if (!subparser.Load(path, err, &lexer_)) return false; if (!ExpectToken(Lexer::NEWLINE, err)) diff --git a/src/manifest_parser.h b/src/manifest_parser.h index 967dfdd..5212f72 100644 --- a/src/manifest_parser.h +++ b/src/manifest_parser.h @@ -35,7 +35,7 @@ struct ManifestParser { ManifestParser(State* state, FileReader* file_reader); /// Load and parse a file. - bool Load(const string& filename, string* err); + bool Load(const string& filename, string* err, Lexer* parent=NULL); /// Parse a text string of input. Used by tests. bool ParseTest(const string& input, string* err) { diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 2638edc..5ed1584 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -302,6 +302,17 @@ TEST_F(ParserTest, Errors) { State state; ManifestParser parser(&state, NULL); string err; + EXPECT_FALSE(parser.ParseTest(string("subn", 4), &err)); + EXPECT_EQ("input:1: expected '=', got eof\n" + "subn\n" + " ^ near here" + , err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; EXPECT_FALSE(parser.ParseTest("foobar", &err)); EXPECT_EQ("input:1: expected '=', got eof\n" "foobar\n" @@ -377,6 +388,17 @@ TEST_F(ParserTest, Errors) { State state; ManifestParser parser(&state, NULL); string err; + EXPECT_FALSE(parser.ParseTest("build\n", &err)); + EXPECT_EQ("input:1: expected path\n" + "build\n" + " ^ near here" + , err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; EXPECT_FALSE(parser.ParseTest("build x: y z\n", &err)); EXPECT_EQ("input:1: unknown build rule 'y'\n" "build x: y z\n" @@ -422,6 +444,32 @@ TEST_F(ParserTest, Errors) { ManifestParser parser(&state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" + " command = echo\n" + "rule cat\n" + " command = echo\n", &err)); + EXPECT_EQ("input:3: duplicate rule 'cat'\n" + "rule cat\n" + " ^ near here" + , err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("rule cat\n" + " command = echo\n" + " rspfile = cat.rsp\n", &err)); + EXPECT_EQ( + "input:4: rspfile and rspfile_content need to be both specified\n", + err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = ${fafsd\n" "foo = bar\n", &err)); @@ -583,6 +631,71 @@ TEST_F(ParserTest, Errors) { " generator = 1\n", &err)); EXPECT_EQ("input:4: unexpected indent\n", err); } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("pool\n", &err)); + EXPECT_EQ("input:1: expected pool name\n", err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("pool foo\n", &err)); + EXPECT_EQ("input:2: expected 'depth =' line\n", err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("pool foo\n" + " depth = 4\n" + "pool foo\n", &err)); + EXPECT_EQ("input:3: duplicate pool 'foo'\n" + "pool foo\n" + " ^ near here" + , err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("pool foo\n" + " depth = -1\n", &err)); + EXPECT_EQ("input:2: invalid pool depth\n" + " depth = -1\n" + " ^ near here" + , err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + EXPECT_FALSE(parser.ParseTest("pool foo\n" + " bar = 1\n", &err)); + EXPECT_EQ("input:2: unexpected variable 'bar'\n" + " bar = 1\n" + " ^ near here" + , err); + } + + { + State state; + ManifestParser parser(&state, NULL); + string err; + // Pool names are dereferenced at edge parsing time. + EXPECT_FALSE(parser.ParseTest("rule run\n" + " command = echo\n" + " pool = unnamed_pool\n" + "build out: run in\n", &err)); + EXPECT_EQ("input:5: unknown pool name 'unnamed_pool'\n", err); + } } TEST_F(ParserTest, MissingInput) { @@ -660,6 +773,17 @@ TEST_F(ParserTest, Include) { EXPECT_EQ("inner", state.bindings_.LookupVariable("var")); } +TEST_F(ParserTest, BrokenInclude) { + files_["include.ninja"] = "build\n"; + ManifestParser parser(&state, this); + string err; + EXPECT_FALSE(parser.ParseTest("include include.ninja\n", &err)); + EXPECT_EQ("include.ninja:1: expected path\n" + "build\n" + " ^ near here" + , err); +} + TEST_F(ParserTest, Implicit) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule cat\n" diff --git a/src/msvc_helper_main-win32.cc b/src/msvc_helper_main-win32.cc index 8a0479c..e3a7846 100644 --- a/src/msvc_helper_main-win32.cc +++ b/src/msvc_helper_main-win32.cc @@ -126,10 +126,15 @@ int MSVCHelperMain(int argc, char** argv) { 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/src/ninja.cc b/src/ninja.cc index 3b381b7..a313ecb 100644 --- a/src/ninja.cc +++ b/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" @@ -104,21 +104,23 @@ struct NinjaMain { // The various subcommands, run via "-t XXX". int ToolGraph(int argc, char* argv[]); int ToolQuery(int argc, char* argv[]); + int ToolDeps(int argc, char* argv[]); int ToolBrowse(int argc, char* argv[]); int ToolMSVC(int argc, char* argv[]); int ToolTargets(int argc, char* argv[]); int ToolCommands(int argc, char* argv[]); int ToolClean(int argc, char* argv[]); int ToolCompilationDatabase(int argc, char* argv[]); + int ToolRecompact(int argc, char* argv[]); int ToolUrtle(int argc, char** argv); /// Open the build log. /// @return false on error. - bool OpenBuildLog(); + bool OpenBuildLog(bool recompact_only = false); /// Open the deps log: load it, then open for writing. /// @return false on error. - bool OpenDepsLog(); + bool OpenDepsLog(bool recompact_only = false); /// Ensure the build directory exists, creating it if necessary. /// @return false on error. @@ -437,6 +439,45 @@ int ToolTargetsList(State* state) { return 0; } +int NinjaMain::ToolDeps(int argc, char** argv) { + vector<Node*> nodes; + if (argc == 0) { + for (vector<Node*>::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()) + nodes.push_back(*ni); + } + } else { + string err; + if (!CollectTargetsFromArgs(argc, argv, &nodes, &err)) { + Error("%s", err.c_str()); + return 1; + } + } + + RealDiskInterface disk_interface; + for (vector<Node*>::iterator it = nodes.begin(), end = nodes.end(); + it != end; ++it) { + DepsLog::Deps* deps = deps_log_.GetDeps(*it); + if (!deps) { + printf("%s: deps not found\n", (*it)->path().c_str()); + continue; + } + + TimeStamp mtime = disk_interface.Stat((*it)->path()); + printf("%s: #deps %d, deps mtime %d (%s)\n", + (*it)->path().c_str(), deps->node_count, deps->mtime, + (!mtime || mtime > deps->mtime ? "STALE":"VALID")); + for (int i = 0; i < deps->node_count; ++i) + printf(" %s\n", deps->nodes[i]->path().c_str()); + printf("\n"); + } + + return 0; +} + int NinjaMain::ToolTargets(int argc, char* argv[]) { int depth = 1; if (argc >= 1) { @@ -602,6 +643,17 @@ int NinjaMain::ToolCompilationDatabase(int argc, char* argv[]) { return 0; } +int NinjaMain::ToolRecompact(int argc, char* argv[]) { + if (!EnsureBuildDirExists()) + return 1; + + if (!OpenBuildLog(/*recompact_only=*/true) || + !OpenDepsLog(/*recompact_only=*/true)) + return 1; + + return 0; +} + int NinjaMain::ToolUrtle(int argc, char** argv) { // RLE encoded. const char* urtle = @@ -644,6 +696,8 @@ const Tool* ChooseTool(const string& tool_name) { Tool::RUN_AFTER_LOAD, &NinjaMain::ToolClean }, { "commands", "list all commands required to rebuild given targets", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolCommands }, + { "deps", "show dependencies stored in the deps log", + Tool::RUN_AFTER_LOGS, &NinjaMain::ToolDeps }, { "graph", "output graphviz dot file for targets", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolGraph }, { "query", "show inputs/outputs for a path", @@ -652,6 +706,8 @@ const Tool* ChooseTool(const string& tool_name) { Tool::RUN_AFTER_LOAD, &NinjaMain::ToolTargets }, { "compdb", "dump JSON compilation database to stdout", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolCompilationDatabase }, + { "recompact", "recompacts ninja-internal data structures", + Tool::RUN_AFTER_LOAD, &NinjaMain::ToolRecompact }, { "urtle", NULL, Tool::RUN_AFTER_FLAGS, &NinjaMain::ToolUrtle }, { NULL, NULL, Tool::RUN_AFTER_FLAGS, NULL } @@ -691,6 +747,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") { @@ -699,6 +756,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); @@ -712,7 +772,7 @@ bool DebugEnable(const string& name) { } } -bool NinjaMain::OpenBuildLog() { +bool NinjaMain::OpenBuildLog(bool recompact_only) { string log_path = ".ninja_log"; if (!build_dir_.empty()) log_path = build_dir_ + "/" + log_path; @@ -728,6 +788,13 @@ bool NinjaMain::OpenBuildLog() { err.clear(); } + if (recompact_only) { + bool success = build_log_.Recompact(log_path, &err); + if (!success) + Error("failed recompaction: %s", err.c_str()); + return success; + } + if (!config_.dry_run) { if (!build_log_.OpenForWrite(log_path, &err)) { Error("opening build log: %s", err.c_str()); @@ -740,7 +807,7 @@ bool NinjaMain::OpenBuildLog() { /// Open the deps log: load it, then open for writing. /// @return false on error. -bool NinjaMain::OpenDepsLog() { +bool NinjaMain::OpenDepsLog(bool recompact_only) { string path = ".ninja_deps"; if (!build_dir_.empty()) path = build_dir_ + "/" + path; @@ -756,6 +823,13 @@ bool NinjaMain::OpenDepsLog() { err.clear(); } + if (recompact_only) { + bool success = deps_log_.Recompact(path, &err); + if (!success) + Error("failed recompaction: %s", err.c_str()); + return success; + } + if (!config_.dry_run) { if (!deps_log_.OpenForWrite(path, &err)) { Error("opening deps log: %s", err.c_str()); @@ -936,6 +1010,7 @@ int real_main(int argc, char** argv) { options.input_file = "build.ninja"; setvbuf(stdout, NULL, _IOLBF, BUFSIZ); + const char* ninja_command = argv[0]; int exit_code = ReadFlags(&argc, &argv, &options, &config); if (exit_code >= 0) @@ -944,7 +1019,7 @@ int real_main(int argc, char** argv) { if (options.tool && options.tool->when == Tool::RUN_AFTER_FLAGS) { // None of the RUN_AFTER_FLAGS actually use a NinjaMain, but it's needed // by other tools. - NinjaMain ninja(argv[0], config); + NinjaMain ninja(ninja_command, config); return (ninja.*options.tool->func)(argc, argv); } @@ -964,7 +1039,7 @@ int real_main(int argc, char** argv) { // The build can take up to 2 passes: one to rebuild the manifest, then // another to build the desired target. for (int cycle = 0; cycle < 2; ++cycle) { - NinjaMain ninja(argv[0], config); + NinjaMain ninja(ninja_command, config); RealFileReader file_reader; ManifestParser parser(&ninja.state_, &file_reader); diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index b396f84..a9af756 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -41,7 +41,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { Fatal("pipe: %s", strerror(errno)); fd_ = output_pipe[0]; #if !defined(USE_PPOLL) - // On Linux and OpenBSD, we use ppoll in DoWork(); elsewhere we use pselect + // If available, we use ppoll in DoWork(); otherwise we use pselect // and so must avoid overly-large FDs. if (fd_ >= static_cast<int>(FD_SETSIZE)) Fatal("pipe: %s", strerror(EMFILE)); @@ -224,7 +224,7 @@ bool SubprocessSet::DoWork() { return interrupted_; } -#else // linux || __OpenBSD__ +#else // !defined(USE_PPOLL) bool SubprocessSet::DoWork() { fd_set set; int nfds = 0; @@ -266,7 +266,7 @@ bool SubprocessSet::DoWork() { return interrupted_; } -#endif // linux || __OpenBSD__ +#endif // !defined(USE_PPOLL) Subprocess* SubprocessSet::NextFinished() { if (finished_.empty()) diff --git a/src/subprocess_test.cc b/src/subprocess_test.cc index afd9008..9f8dcea 100644 --- a/src/subprocess_test.cc +++ b/src/subprocess_test.cc @@ -152,7 +152,7 @@ TEST_F(SubprocessTest, SetWithMulti) { // OS X's process limit is less than 1025 by default // (|sysctl kern.maxprocperuid| is 709 on 10.7 and 10.8 and less prior to that). -#if defined(linux) || defined(__OpenBSD__) +#if !defined(__APPLE__) && !defined(_WIN32) TEST_F(SubprocessTest, SetWithLots) { // Arbitrary big number; needs to be over 1024 to confirm we're no longer // hostage to pselect. @@ -179,7 +179,7 @@ TEST_F(SubprocessTest, SetWithLots) { } ASSERT_EQ(kNumProcs, subprocs_.finished_.size()); } -#endif // linux || __OpenBSD__ +#endif // !__APPLE__ && !_WIN32 // TODO: this test could work on Windows, just not sure how to simply // read stdin. diff --git a/src/util.cc b/src/util.cc index b9c2c0d..6ba3c6c 100644 --- a/src/util.cc +++ b/src/util.cc @@ -300,35 +300,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/src/version.cc b/src/version.cc index 28280aa..17c71aa 100644 --- a/src/version.cc +++ b/src/version.cc @@ -18,7 +18,7 @@ #include "util.h" -const char* kNinjaVersion = "1.3.4"; +const char* kNinjaVersion = "1.4.0"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.'); |