From 7222513f47e41eff30d3f267e4c1ce10ea859fb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?okhowang=28=E7=8E=8B=E6=B2=9B=E6=96=87=29?= Date: Tue, 18 Aug 2020 12:04:16 +0800 Subject: [PATCH 01/18] feat: support cpu limit by job api on windows --- src/util.cc | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/util.cc b/src/util.cc index 080883e066..2e85513c8d 100644 --- a/src/util.cc +++ b/src/util.cc @@ -500,6 +500,7 @@ string StripAnsiEscapeCodes(const string& in) { int GetProcessorCount() { #ifdef _WIN32 + DWORD cpuCount = 0; #ifndef _WIN64 // Need to use GetLogicalProcessorInformationEx to get real core count on // machines with >64 cores. See https://stackoverflow.com/a/31209344/21475 @@ -524,12 +525,25 @@ int GetProcessorCount() { i += info->Size; } if (cores != 0) { - return cores; + cpuCount = cores; } } } #endif - return GetActiveProcessorCount(ALL_PROCESSOR_GROUPS); + if (cpuCount == 0) { + cpuCount = GetActiveProcessorCount(ALL_PROCESSOR_GROUPS); + } + JOBOBJECT_CPU_RATE_CONTROL_INFORMATION info; + // reference: + // https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-jobobject_cpu_rate_control_information + if (QueryInformationJobObject(NULL, JobObjectCpuRateControlInformation, &info, + sizeof(info), NULL)) { + if (info.ControlFlags & (JOB_OBJECT_CPU_RATE_CONTROL_ENABLE | + JOB_OBJECT_CPU_RATE_CONTROL_HARD_CAP)) { + return cpuCount * info.CpuRate / 10000; + } + } + return cpuCount; #else // The number of exposed processors might not represent the actual number of // processors threads can run on. This happens when a CPU set limitation is From 540be336f5639ee6a89e959e6f9f434c01900ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?okhowang=28=E7=8E=8B=E6=B2=9B=E6=96=87=29?= Date: Tue, 18 Aug 2020 14:38:04 +0800 Subject: [PATCH 02/18] feat: support cpu limit by cgroups on linux --- src/util.cc | 163 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 2 deletions(-) diff --git a/src/util.cc b/src/util.cc index 2e85513c8d..287debc12e 100644 --- a/src/util.cc +++ b/src/util.cc @@ -49,6 +49,9 @@ #include #elif defined(linux) || defined(__GLIBC__) #include +#include +#include +#include "string_piece_util.h" #endif #if defined(__FreeBSD__) @@ -498,6 +501,155 @@ string StripAnsiEscapeCodes(const string& in) { return stripped; } +#if defined(linux) || defined(__GLIBC__) +std::pair readCount(const std::string& path) { + std::ifstream file(path.c_str()); + if (!file.is_open()) + return std::make_pair(0, false); + int64_t n = 0; + file >> n; + if (file.good()) + return std::make_pair(n, true); + return std::make_pair(0, false); +} + +struct MountPoint { + int mountId; + int parentId; + StringPiece deviceId; + StringPiece root; + StringPiece mountPoint; + vector options; + vector optionalFields; + StringPiece fsType; + StringPiece mountSource; + vector superOptions; + bool parse(const string& line) { + vector pieces = SplitStringPiece(line, ' '); + if (pieces.size() < 10) + return false; + size_t optionalStart = 0; + for (size_t i = 6; i < pieces.size(); i++) { + if (pieces[i] == "-") { + optionalStart = i + 1; + break; + } + } + if (optionalStart == 0) + return false; + if (optionalStart + 3 != pieces.size()) + return false; + mountId = atoi(pieces[0].AsString().c_str()); + parentId = atoi(pieces[1].AsString().c_str()); + deviceId = pieces[2]; + root = pieces[3]; + mountPoint = pieces[4]; + options = SplitStringPiece(pieces[5], ','); + optionalFields = + vector(&pieces[6], &pieces[optionalStart - 1]); + fsType = pieces[optionalStart]; + mountSource = pieces[optionalStart + 1]; + superOptions = SplitStringPiece(pieces[optionalStart + 2], ','); + return true; + } + string translate(string& path) const { + // path must be sub dir of root + if (path.compare(0, root.len_, root.str_, root.len_) != 0) { + return string(); + } + path.erase(0, root.len_); + if (path == ".." || (path.length() > 2 && path.compare(0, 3, "../") == 0)) { + return string(); + } + return mountPoint.AsString() + "/" + path; + } +}; + +struct CGroupSubSys { + int id; + string name; + vector subsystems; + bool parse(string& line) { + size_t first = line.find(':'); + if (first == string::npos) + return false; + line[first] = '\0'; + size_t second = line.find(':', first + 1); + if (second == string::npos) + return false; + line[second] = '\0'; + id = atoi(line.c_str()); + name = line.substr(second + 1); + vector pieces = + SplitStringPiece(StringPiece(line.c_str() + first + 1), ','); + for (size_t i = 0; i < pieces.size(); i++) { + subsystems.push_back(pieces[i].AsString()); + } + return true; + } +}; + +map ParseMountInfo(map& subsystems) { + map cgroups; + ifstream mountinfo("/proc/self/mountinfo"); + if (!mountinfo.is_open()) + return cgroups; + while (!mountinfo.eof()) { + string line; + getline(mountinfo, line); + MountPoint mp; + if (!mp.parse(line)) + continue; + if (mp.fsType != "cgroup") + continue; + for (size_t i = 0; i < mp.superOptions.size(); i++) { + string opt = mp.superOptions[i].AsString(); + map::iterator subsys = subsystems.find(opt); + if (subsys == subsystems.end()) + continue; + string newPath = mp.translate(subsys->second.name); + if (!newPath.empty()) + cgroups.insert(make_pair(opt, newPath)); + } + } + return cgroups; +} + +map ParseSelfCGroup() { + map cgroups; + ifstream cgroup("/proc/self/cgroup"); + if (!cgroup.is_open()) + return cgroups; + string line; + while (!cgroup.eof()) { + getline(cgroup, line); + CGroupSubSys subsys; + if (!subsys.parse(line)) + continue; + for (size_t i = 0; i < subsys.subsystems.size(); i++) { + cgroups.insert(make_pair(subsys.subsystems[i], subsys)); + } + } + return cgroups; +} + +int ParseCPUFromCGroup() { + map subsystems = ParseSelfCGroup(); + map cgroups = ParseMountInfo(subsystems); + map::iterator cpu = cgroups.find("cpu"); + if (cpu == cgroups.end()) + return -1; + std::pair quota = readCount(cpu->second + "/cpu.cfs_quota_us"); + if (!quota.second || quota.first == -1) + return -1; + std::pair period = + readCount(cpu->second + "/cpu.cfs_period_us"); + if (!period.second) + return -1; + return quota.first / period.first; +} +#endif + int GetProcessorCount() { #ifdef _WIN32 DWORD cpuCount = 0; @@ -545,6 +697,11 @@ int GetProcessorCount() { } return cpuCount; #else + int cgroupCount = -1; + int schedCount = -1; +#if defined(linux) || defined(__GLIBC__) + cgroupCount = ParseCPUFromCGroup(); +#endif // The number of exposed processors might not represent the actual number of // processors threads can run on. This happens when a CPU set limitation is // active, see https://github.com/ninja-build/ninja/issues/1278 @@ -558,10 +715,12 @@ int GetProcessorCount() { #elif defined(CPU_COUNT) cpu_set_t set; if (sched_getaffinity(getpid(), sizeof(set), &set) == 0) { - return CPU_COUNT(&set); + schedCount = CPU_COUNT(&set); } #endif - return sysconf(_SC_NPROCESSORS_ONLN); + if (cgroupCount >= 0 && schedCount >= 0) return std::min(cgroupCount, schedCount); + if (cgroupCount < 0 && schedCount < 0) return sysconf(_SC_NPROCESSORS_ONLN); + return std::max(cgroupCount, schedCount); #endif } From 2187403594b951c50bfb9ffe7fedaf4178e9455a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?okhowang=28=E7=8E=8B=E6=B2=9B=E6=96=87=29?= Date: Mon, 24 Aug 2020 14:14:07 +0800 Subject: [PATCH 03/18] feat: don't detect cpu if -j set --- src/ninja.cc | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/ninja.cc b/src/ninja.cc index 3e5c971fae..2d43b1d953 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -1305,11 +1305,28 @@ int ExceptionFilter(unsigned int code, struct _EXCEPTION_POINTERS *ep) { #endif // _MSC_VER +class DeferGuessParallelism { + public: + bool needGuess; + BuildConfig* config; + + DeferGuessParallelism(BuildConfig* config) + : config(config), needGuess(true) {} + + void Refresh() { + if (needGuess) { + needGuess = false; + config->parallelism = GuessParallelism(); + } + } + ~DeferGuessParallelism() { Refresh(); } +}; + /// Parse argv for command-line options. /// Returns an exit code, or -1 if Ninja should continue. int ReadFlags(int* argc, char*** argv, Options* options, BuildConfig* config) { - config->parallelism = GuessParallelism(); + DeferGuessParallelism deferGuessParallelism(config); enum { OPT_VERSION = 1, OPT_QUIET = 2 }; const option kLongOptions[] = { @@ -1341,6 +1358,7 @@ int ReadFlags(int* argc, char*** argv, // We want to run N jobs in parallel. For N = 0, INT_MAX // is close enough to infinite for most sane builds. config->parallelism = value > 0 ? value : INT_MAX; + deferGuessParallelism.needGuess = false; break; } case 'k': { @@ -1389,6 +1407,7 @@ int ReadFlags(int* argc, char*** argv, return 0; case 'h': default: + deferGuessParallelism.Refresh(); Usage(*config); return 1; } From 9116613e39ba4e18f50494e97eb968a874effdcf Mon Sep 17 00:00:00 2001 From: David 'Digit' Turner Date: Fri, 18 Feb 2022 15:00:07 +0100 Subject: [PATCH 04/18] Fix ReadFile() handle leak on read error on Windows. Small change to fix a file handle leak in case of error. Also return -EIO instead of -1 to signal read errors, since it is more consistent with what is happening here. --- src/util.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util.cc b/src/util.cc index 080883e066..a2a0f278d8 100644 --- a/src/util.cc +++ b/src/util.cc @@ -350,7 +350,8 @@ int ReadFile(const string& path, string* contents, string* err) { if (!::ReadFile(f, buf, sizeof(buf), &len, NULL)) { err->assign(GetLastErrorString()); contents->clear(); - return -1; + ::CloseHandle(f); + return -EIO; } if (len == 0) break; From 2dd343b8649c8834d42421d5664d8241cabd8f8a Mon Sep 17 00:00:00 2001 From: Michael Hirsch Date: Sun, 20 Feb 2022 20:29:38 -0500 Subject: [PATCH 05/18] cmake: remove unnecessary install parameter The install(TARGETS ... DESTINATION bin) is not necessary as this subdirectory is implicit for executable targets. ref: https://cmake.org/cmake/help/v3.15/command/install.html --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 57ae548f5b..70fc5e99f0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -232,4 +232,4 @@ if(BUILD_TESTING) add_test(NAME NinjaTest COMMAND ninja_test) endif() -install(TARGETS ninja DESTINATION bin) +install(TARGETS ninja) From f0fd305a5772fe06be9627ddcdcf2950449900b9 Mon Sep 17 00:00:00 2001 From: Jan Niklas Hasse Date: Sat, 5 Mar 2022 15:45:11 +0100 Subject: [PATCH 06/18] Ignore Visual Studios folders --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index fdca01513f..ca36ec8810 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,7 @@ /.clangd/ /compile_commands.json /.cache/ + +# Visual Studio files +/.vs/ +/out/ From 7923d736c108cf19e15aa53f5a3fa30582530abb Mon Sep 17 00:00:00 2001 From: David 'Digit' Turner Date: Tue, 15 Feb 2022 12:38:58 +0100 Subject: [PATCH 07/18] Document the `msvc` tool --- doc/manual.asciidoc | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 9e2bec5d49..00b293cd66 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -312,6 +312,37 @@ file. _Available since Ninja 1.10._ to pass to +ninja -t targets rule _name_+ or +ninja -t compdb+. Adding the `-d` flag also prints the description of the rules. +`msvc`:: Available on Windows hosts only. +Helper tool to invoke the `cl.exe` compiler with a pre-defined set of +environment variables, as in: ++ +---- +ninja -t msvc -e ENVFILE -- cl.exe +---- ++ +Where `ENVFILE` is a binary file that contains an environment block suitable +for CreateProcessA() on Windows (i.e. a series of zero-terminated strings that +look like NAME=VALUE, followed by an extra zero terminator). Note that this uses +the local codepage encoding. + +This tool also supports a deprecated way of parsing the compiler's output when +the `/showIncludes` flag is used, and generating a GCC-compatible depfile from it. ++ +--- +ninja -t msvc -o DEPFILE [-p STRING] -- cl.exe /showIncludes +--- ++ + +When using this option, `-p STRING` can be used to pass the localized line prefix +that `cl.exe` uses to output dependency information. For English-speaking regions +this is `"Note: including file: "` without the double quotes, but will be different +for other regions. + +Note that Ninja supports this natively now, with the use of `deps = msvc` and +`msvc_deps_prefix` in Ninja files. Native support also avoids launching an extra +tool process each time the compiler must be called, which can speed up builds +noticeably on Windows. + `wincodepage`:: Available on Windows hosts (_since Ninja 1.11_). Prints the Windows code page whose encoding is expected in the build file. The output has the form: From 66b05496ff0ef6114e3534418c970dd92d34accb Mon Sep 17 00:00:00 2001 From: David 'Digit' Turner Date: Fri, 18 Feb 2022 15:05:10 +0100 Subject: [PATCH 08/18] Ensure the `msvc` tool is built in all Win32 Ninja binaries. It was only previously available when Ninja was built when `_MSVC` is defined (i.e. when compiling with the Microsoft compiler of with `clang-cl`). + Tag the tool as DEPRECATED --- src/ninja.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ninja.cc b/src/ninja.cc index 71dea210df..ad0912ecf5 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -52,7 +52,7 @@ using namespace std; -#ifdef _MSC_VER +#ifdef _WIN32 // Defined in msvc_helper_main-win32.cc. int MSVCHelperMain(int argc, char** argv); @@ -449,7 +449,7 @@ int NinjaMain::ToolBrowse(const Options*, int, char**) { } #endif -#if defined(_MSC_VER) +#if defined(_WIN32) int NinjaMain::ToolMSVC(const Options* options, int argc, char* argv[]) { // Reset getopt: push one argument onto the front of argv, reset optind. argc++; @@ -1043,8 +1043,8 @@ const Tool* ChooseTool(const string& tool_name) { static const Tool kTools[] = { { "browse", "browse dependency graph in a web browser", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolBrowse }, -#if defined(_MSC_VER) - { "msvc", "build helper for MSVC cl.exe (EXPERIMENTAL)", +#ifdef _WIN32 + { "msvc", "build helper for MSVC cl.exe (DEPRECATED)", Tool::RUN_AFTER_FLAGS, &NinjaMain::ToolMSVC }, #endif { "clean", "clean built files", From 988c847ee728de5520d07051843c02a17dd44777 Mon Sep 17 00:00:00 2001 From: David 'Digit' Turner Date: Mon, 21 Feb 2022 19:32:35 +0100 Subject: [PATCH 09/18] Make the output of `ninja -t inputs` deterministic This sorts the output of `ninja -t inputs` to make it deterministic and remove duplicates, and adds a regression test in output_test.py + Ensure all inputs are listed, not only explicit ones. + Document the `inputs` tool in doc/manual.asciidoc. --- doc/manual.asciidoc | 4 ++-- misc/output_test.py | 18 ++++++++++++++++ src/graph.cc | 22 +++++++++++++++++++ src/graph.h | 3 +++ src/graph_test.cc | 33 +++++++++++++++++++++++++++++ src/ninja.cc | 51 +++++++++++++++++++++++++++++++++++++++------ 6 files changed, 123 insertions(+), 8 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 00b293cd66..2062a2a210 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -257,8 +257,8 @@ than the _depth_ mode. executed in order, may be used to rebuild those targets, assuming that all output files are out of date. -`inputs`:: given a list of targets, print a list of all inputs which are used -to rebuild those targets. +`inputs`:: given a list of targets, print a list of all inputs used to +rebuild those targets. _Available since Ninja 1.11._ `clean`:: remove built files. By default it removes all built files diff --git a/misc/output_test.py b/misc/output_test.py index 45698f1e57..141716c136 100755 --- a/misc/output_test.py +++ b/misc/output_test.py @@ -134,5 +134,23 @@ def test_entering_directory_on_stdout(self): output = run(Output.BUILD_SIMPLE_ECHO, flags='-C$PWD', pipe=True) self.assertEqual(output.splitlines()[0][:25], "ninja: Entering directory") + def test_tool_inputs(self): + plan = ''' +rule cat + command = cat $in $out +build out1 : cat in1 +build out2 : cat in2 out1 +build out3 : cat out2 out1 | implicit || order_only +''' + self.assertEqual(run(plan, flags='-t inputs out3'), +'''implicit +in1 +in2 +order_only +out1 +out2 +''') + + if __name__ == '__main__': unittest.main() diff --git a/src/graph.cc b/src/graph.cc index 8d58587784..43ba45ae3d 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -456,6 +456,28 @@ std::string EdgeEnv::MakePathList(const Node* const* const span, return result; } +void Edge::CollectInputs(bool shell_escape, + std::vector* out) const { + for (std::vector::const_iterator it = inputs_.begin(); + it != inputs_.end(); ++it) { + std::string path = (*it)->PathDecanonicalized(); + if (shell_escape) { + std::string unescaped; + unescaped.swap(path); +#ifdef _WIN32 + GetWin32EscapedString(unescaped, &path); +#else + GetShellEscapedString(unescaped, &path); +#endif + } +#if __cplusplus >= 201103L + out->push_back(std::move(path)); +#else + out->push_back(path); +#endif + } +} + std::string Edge::EvaluateCommand(const bool incl_rsp_file) const { string command = GetBinding("command"); if (incl_rsp_file) { diff --git a/src/graph.h b/src/graph.h index 141b43993e..9de67d2718 100644 --- a/src/graph.h +++ b/src/graph.h @@ -195,6 +195,9 @@ struct Edge { void Dump(const char* prefix="") const; + // Append all edge explicit inputs to |*out|. Possibly with shell escaping. + void CollectInputs(bool shell_escape, std::vector* out) const; + const Rule* rule_; Pool* pool_; std::vector inputs_; diff --git a/src/graph_test.cc b/src/graph_test.cc index 5314bc5f5f..9dba8afc3c 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -215,6 +215,39 @@ TEST_F(GraphTest, RootNodes) { } } +TEST_F(GraphTest, CollectInputs) { + ASSERT_NO_FATAL_FAILURE(AssertParse( + &state_, + "build out$ 1: cat in1 in2 in$ with$ space | implicit || order_only\n")); + + std::vector inputs; + Edge* edge = GetNode("out 1")->in_edge(); + + // Test without shell escaping. + inputs.clear(); + edge->CollectInputs(false, &inputs); + EXPECT_EQ(5u, inputs.size()); + EXPECT_EQ("in1", inputs[0]); + EXPECT_EQ("in2", inputs[1]); + EXPECT_EQ("in with space", inputs[2]); + EXPECT_EQ("implicit", inputs[3]); + EXPECT_EQ("order_only", inputs[4]); + + // Test with shell escaping. + inputs.clear(); + edge->CollectInputs(true, &inputs); + EXPECT_EQ(5u, inputs.size()); + EXPECT_EQ("in1", inputs[0]); + EXPECT_EQ("in2", inputs[1]); +#ifdef _WIN32 + EXPECT_EQ("\"in with space\"", inputs[2]); +#else + EXPECT_EQ("'in with space'", inputs[2]); +#endif + EXPECT_EQ("implicit", inputs[3]); + EXPECT_EQ("order_only", inputs[4]); +} + TEST_F(GraphTest, VarInOutPathEscaping) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build a$ b: cat no'space with$ space$$ no\"space2\n")); diff --git a/src/ninja.cc b/src/ninja.cc index ad0912ecf5..aea430d112 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -17,6 +17,8 @@ #include #include #include + +#include #include #ifdef _WIN32 @@ -744,7 +746,8 @@ int NinjaMain::ToolCommands(const Options* options, int argc, char* argv[]) { return 0; } -void PrintInputs(Edge* edge, set* seen) { +void CollectInputs(Edge* edge, std::set* seen, + std::vector* result) { if (!edge) return; if (!seen->insert(edge).second) @@ -752,13 +755,41 @@ void PrintInputs(Edge* edge, set* seen) { for (vector::iterator in = edge->inputs_.begin(); in != edge->inputs_.end(); ++in) - PrintInputs((*in)->in_edge(), seen); + CollectInputs((*in)->in_edge(), seen, result); - if (!edge->is_phony()) - puts(edge->GetBinding("in_newline").c_str()); + if (!edge->is_phony()) { + edge->CollectInputs(true, result); + } } int NinjaMain::ToolInputs(const Options* options, int argc, char* argv[]) { + // The inputs tool uses getopt, and expects argv[0] to contain the name of + // the tool, i.e. "inputs". + argc++; + argv--; + optind = 1; + int opt; + const option kLongOptions[] = { { "help", no_argument, NULL, 'h' }, + { NULL, 0, NULL, 0 } }; + while ((opt = getopt_long(argc, argv, "h", kLongOptions, NULL)) != -1) { + switch (opt) { + case 'h': + default: + // clang-format off + printf( +"Usage '-t inputs [options] [targets]\n" +"\n" +"List all inputs used for a set of targets. Note that this includes\n" +"explicit, implicit and order-only inputs, but not validation ones.\n\n" +"Options:\n" +" -h, --help Print this message.\n"); + // clang-format on + return 1; + } + } + argv += optind; + argc -= optind; + vector nodes; string err; if (!CollectTargetsFromArgs(argc, argv, &nodes, &err)) { @@ -766,9 +797,17 @@ int NinjaMain::ToolInputs(const Options* options, int argc, char* argv[]) { return 1; } - set seen; + std::set seen; + std::vector result; for (vector::iterator in = nodes.begin(); in != nodes.end(); ++in) - PrintInputs((*in)->in_edge(), &seen); + CollectInputs((*in)->in_edge(), &seen, &result); + + // Make output deterministic by sorting then removing duplicates. + std::sort(result.begin(), result.end()); + result.erase(std::unique(result.begin(), result.end()), result.end()); + + for (size_t n = 0; n < result.size(); ++n) + puts(result[n].c_str()); return 0; } From bb471e235a83fd2b146299cd7d4d3a95163de10a Mon Sep 17 00:00:00 2001 From: Jan Niklas Hasse Date: Sun, 15 May 2022 16:53:55 +0200 Subject: [PATCH 10/18] mark this 1.11.0.git --- src/version.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/version.cc b/src/version.cc index 97afa7e56e..bdcbc53e50 100644 --- a/src/version.cc +++ b/src/version.cc @@ -20,7 +20,7 @@ using namespace std; -const char* kNinjaVersion = "1.10.2.git"; +const char* kNinjaVersion = "1.11.0.git"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.'); From 99c1bc7442ff3109c8b91fb98b4a252045623296 Mon Sep 17 00:00:00 2001 From: Jan Niklas Hasse Date: Sun, 15 May 2022 17:52:42 +0200 Subject: [PATCH 11/18] doc: Add available since 1.11 to Validations --- doc/manual.asciidoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 2062a2a210..214dca4a57 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -1046,6 +1046,9 @@ relative path, pointing to the same file, are considered different by Ninja. [[validations]] Validations ~~~~~~~~~~~ + +_Available since Ninja 1.11._ + Validations listed on the build line cause the specified files to be added to the top level of the build graph (as if they were specified on the Ninja command line) whenever the build line is a transitive From 29e66e24199113535195d06a228a7e0e0b229c94 Mon Sep 17 00:00:00 2001 From: Gregor Jasny Date: Tue, 17 May 2022 16:51:46 +0200 Subject: [PATCH 12/18] chore: fix build warning --- src/ninja.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ninja.cc b/src/ninja.cc index 2b71eb170f..834e2846ee 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -1400,7 +1400,7 @@ class DeferGuessParallelism { BuildConfig* config; DeferGuessParallelism(BuildConfig* config) - : config(config), needGuess(true) {} + : needGuess(true), config(config) {} void Refresh() { if (needGuess) { From 8e0af0809cd8e5403ad0410e6fd75c94934ca75d Mon Sep 17 00:00:00 2001 From: Ken Matsui <26405363+ken-matsui@users.noreply.github.com> Date: Tue, 31 May 2022 22:25:47 +0900 Subject: [PATCH 13/18] Support building `libninja-re2c.a` on `configure.py` --- configure.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/configure.py b/configure.py index 43904349a8..f24d1f8a61 100755 --- a/configure.py +++ b/configure.py @@ -489,16 +489,27 @@ def has_re2c(): "changes to src/*.in.cc will not affect your build.") n.newline() -n.comment('Core source files all build into ninja library.') cxxvariables = [] if platform.is_msvc(): cxxvariables = [('pdb', 'ninja.pdb')] + +n.comment('Generate a library for `ninja-re2c`.') +re2c_objs = [] +for name in ['depfile_parser', 'lexer']: + re2c_objs += cxx(name, variables=cxxvariables) +if platform.is_msvc(): + n.build(built('ninja-re2c.lib'), 'ar', re2c_objs) +else: + n.build(built('libninja-re2c.a'), 'ar', re2c_objs) +n.newline() + +n.comment('Core source files all build into ninja library.') +objs.extend(re2c_objs) for name in ['build', 'build_log', 'clean', 'clparser', 'debug_flags', - 'depfile_parser', 'deps_log', 'disk_interface', 'dyndep', @@ -508,7 +519,6 @@ def has_re2c(): 'graph', 'graphviz', 'json', - 'lexer', 'line_printer', 'manifest_parser', 'metrics', From 57b8fee639a4290176086f3839c78bfc0d02c42b Mon Sep 17 00:00:00 2001 From: Ken Matsui <26405363+ken-matsui@users.noreply.github.com> Date: Sat, 28 May 2022 22:26:55 +0900 Subject: [PATCH 14/18] Add an option to avoid building binary when used as a library --- CMakeLists.txt | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 70fc5e99f0..22b815886c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,6 +3,8 @@ cmake_minimum_required(VERSION 3.15) include(CheckSymbolExists) include(CheckIPOSupported) +option(NINJA_BUILD_BINARY "Build ninja binary" ON) + project(ninja) # --- optional link-time optimization @@ -148,11 +150,13 @@ if(CMAKE_SYSTEM_NAME STREQUAL "OS400" OR CMAKE_SYSTEM_NAME STREQUAL "AIX") endif() # Main executable is library plus main() function. -add_executable(ninja src/ninja.cc) -target_link_libraries(ninja PRIVATE libninja libninja-re2c) +if(NINJA_BUILD_BINARY) + add_executable(ninja src/ninja.cc) + target_link_libraries(ninja PRIVATE libninja libninja-re2c) -if(WIN32) - target_sources(ninja PRIVATE windows/ninja.manifest) + if(WIN32) + target_sources(ninja PRIVATE windows/ninja.manifest) + endif() endif() # Adds browse mode into the ninja binary if it's supported by the host platform. @@ -171,8 +175,10 @@ if(platform_supports_ninja_browse) VERBATIM ) - target_compile_definitions(ninja PRIVATE NINJA_HAVE_BROWSE) - target_sources(ninja PRIVATE src/browse.cc) + if(NINJA_BUILD_BINARY) + target_compile_definitions(ninja PRIVATE NINJA_HAVE_BROWSE) + target_sources(ninja PRIVATE src/browse.cc) + endif() set_source_files_properties(src/browse.cc PROPERTIES OBJECT_DEPENDS "${PROJECT_BINARY_DIR}/build/browse_py.h" @@ -232,4 +238,6 @@ if(BUILD_TESTING) add_test(NAME NinjaTest COMMAND ninja_test) endif() -install(TARGETS ninja) +if(NINJA_BUILD_BINARY) + install(TARGETS ninja) +endif() From a2b5e6deff1545f5ca1947930fa59fa3ff236db7 Mon Sep 17 00:00:00 2001 From: John Drouhard Date: Fri, 26 Mar 2021 12:07:21 -0500 Subject: [PATCH 15/18] Introduce mechanism to provide resiliency for inputs changing while the build runs When an edge starts to run, create a temporary lock file in the build directory, stat it, and cache its mtime. When the command finishes, use the temporary lock file's mtime from when the edge started running as the mtime that is recorded in the build log for each of the edge's output(s). Subsequent runs will use that as the mtime for the output(s). This provides robustness against inputs changing while the command itself is running. If an input is changed, the subsequent run will detect the output as dirty since its recorded mtime reflects when the build command began, not when the output was actually written to disk. Generator and restat rules are exempt from this and will continue to record their actual mtime on disk at the time the command finished in the build log (unless the restat rule cleans the output). This avoids potential infinite loops when the generator rule touches input dependencies of the output(s) or a restat rule intentionally changes implicit dependencies of its output. --- src/build.cc | 88 +++++----- src/build.h | 1 + src/build_log.cc | 10 +- src/build_log.h | 2 +- src/build_test.cc | 320 ++++++++++++++++++++++++++++++++++++- src/disk_interface_test.cc | 2 +- src/graph.cc | 49 +++--- src/graph.h | 4 +- 8 files changed, 396 insertions(+), 80 deletions(-) diff --git a/src/build.cc b/src/build.cc index 6f11ed7a3c..76ff93af03 100644 --- a/src/build.cc +++ b/src/build.cc @@ -518,6 +518,10 @@ Builder::Builder(State* state, const BuildConfig& config, start_time_millis_(start_time_millis), disk_interface_(disk_interface), scan_(state, build_log, deps_log, disk_interface, &config_.depfile_parser_options) { + lock_file_path_ = ".ninja_lock"; + string build_dir = state_->bindings_.LookupVariable("builddir"); + if (!build_dir.empty()) + lock_file_path_ = build_dir + "/" + lock_file_path_; } Builder::~Builder() { @@ -552,6 +556,10 @@ void Builder::Cleanup() { disk_interface_->RemoveFile(depfile); } } + + string err; + if (disk_interface_->Stat(lock_file_path_, &err) > 0) + disk_interface_->RemoveFile(lock_file_path_); } Node* Builder::AddTarget(const string& name, string* err) { @@ -704,14 +712,25 @@ bool Builder::StartEdge(Edge* edge, string* err) { status_->BuildEdgeStarted(edge, start_time_millis); - // Create directories necessary for outputs. + TimeStamp build_start = -1; + + // Create directories necessary for outputs and remember the current + // filesystem mtime to record later // XXX: this will block; do we care? for (vector::iterator o = edge->outputs_.begin(); o != edge->outputs_.end(); ++o) { if (!disk_interface_->MakeDirs((*o)->path())) return false; + if (build_start == -1) { + disk_interface_->WriteFile(lock_file_path_, ""); + build_start = disk_interface_->Stat(lock_file_path_, err); + if (build_start == -1) + build_start = 0; + } } + edge->command_start_time_ = build_start; + // Create response file, if needed // XXX: this may also block; do we care? string rspfile = edge->GetUnescapedRspfile(); @@ -770,55 +789,42 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { } // Restat the edge outputs - TimeStamp output_mtime = 0; - bool restat = edge->GetBindingBool("restat"); + TimeStamp record_mtime = 0; if (!config_.dry_run) { + const bool restat = edge->GetBindingBool("restat"); + const bool generator = edge->GetBindingBool("generator"); bool node_cleaned = false; - - for (vector::iterator o = edge->outputs_.begin(); - o != edge->outputs_.end(); ++o) { - TimeStamp new_mtime = disk_interface_->Stat((*o)->path(), err); - if (new_mtime == -1) - return false; - if (new_mtime > output_mtime) - output_mtime = new_mtime; - if ((*o)->mtime() == new_mtime && restat) { - // The rule command did not change the output. Propagate the clean - // state through the build graph. - // Note that this also applies to nonexistent outputs (mtime == 0). - if (!plan_.CleanNode(&scan_, *o, err)) + record_mtime = edge->command_start_time_; + + // restat and generator rules must restat the outputs after the build + // has finished. if record_mtime == 0, then there was an error while + // attempting to touch/stat the temp file when the edge started and + // we should fall back to recording the outputs' current mtime in the + // log. + if (record_mtime == 0 || restat || generator) { + for (vector::iterator o = edge->outputs_.begin(); + o != edge->outputs_.end(); ++o) { + TimeStamp new_mtime = disk_interface_->Stat((*o)->path(), err); + if (new_mtime == -1) return false; - node_cleaned = true; + if (new_mtime > record_mtime) + record_mtime = new_mtime; + if ((*o)->mtime() == new_mtime && restat) { + // The rule command did not change the output. Propagate the clean + // state through the build graph. + // Note that this also applies to nonexistent outputs (mtime == 0). + if (!plan_.CleanNode(&scan_, *o, err)) + return false; + node_cleaned = true; + } } } - if (node_cleaned) { - TimeStamp restat_mtime = 0; - // If any output was cleaned, find the most recent mtime of any - // (existing) non-order-only input or the depfile. - for (vector::iterator i = edge->inputs_.begin(); - i != edge->inputs_.end() - edge->order_only_deps_; ++i) { - TimeStamp input_mtime = disk_interface_->Stat((*i)->path(), err); - if (input_mtime == -1) - return false; - if (input_mtime > restat_mtime) - restat_mtime = input_mtime; - } - - string depfile = edge->GetUnescapedDepfile(); - if (restat_mtime != 0 && deps_type.empty() && !depfile.empty()) { - TimeStamp depfile_mtime = disk_interface_->Stat(depfile, err); - if (depfile_mtime == -1) - return false; - if (depfile_mtime > restat_mtime) - restat_mtime = depfile_mtime; - } + record_mtime = edge->command_start_time_; // The total number of edges in the plan may have changed as a result // of a restat. status_->PlanHasTotalEdges(plan_.command_edge_count()); - - output_mtime = restat_mtime; } } @@ -832,7 +838,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { if (scan_.build_log()) { if (!scan_.build_log()->RecordCommand(edge, start_time_millis, - end_time_millis, output_mtime)) { + end_time_millis, record_mtime)) { *err = string("Error writing to build log: ") + strerror(errno); return false; } diff --git a/src/build.h b/src/build.h index d697dfb89e..d727a8a480 100644 --- a/src/build.h +++ b/src/build.h @@ -234,6 +234,7 @@ struct Builder { /// Time the build started. int64_t start_time_millis_; + std::string lock_file_path_; DiskInterface* disk_interface_; DependencyScan scan_; diff --git a/src/build_log.cc b/src/build_log.cc index 4dcd6cee53..b35279d410 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -116,9 +116,9 @@ BuildLog::LogEntry::LogEntry(const string& output) : output(output) {} BuildLog::LogEntry::LogEntry(const string& output, uint64_t command_hash, - int start_time, int end_time, TimeStamp restat_mtime) + int start_time, int end_time, TimeStamp mtime) : output(output), command_hash(command_hash), - start_time(start_time), end_time(end_time), mtime(restat_mtime) + start_time(start_time), end_time(end_time), mtime(mtime) {} BuildLog::BuildLog() @@ -303,7 +303,7 @@ LoadStatus BuildLog::Load(const string& path, string* err) { *end = 0; int start_time = 0, end_time = 0; - TimeStamp restat_mtime = 0; + TimeStamp mtime = 0; start_time = atoi(start); start = end + 1; @@ -319,7 +319,7 @@ LoadStatus BuildLog::Load(const string& path, string* err) { if (!end) continue; *end = 0; - restat_mtime = strtoll(start, NULL, 10); + mtime = strtoll(start, NULL, 10); start = end + 1; end = (char*)memchr(start, kFieldSeparator, line_end - start); @@ -343,7 +343,7 @@ LoadStatus BuildLog::Load(const string& path, string* err) { entry->start_time = start_time; entry->end_time = end_time; - entry->mtime = restat_mtime; + entry->mtime = mtime; if (log_version >= 5) { char c = *end; *end = '\0'; entry->command_hash = (uint64_t)strtoull(start, NULL, 16); diff --git a/src/build_log.h b/src/build_log.h index 88551e3217..dd72c4c772 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -73,7 +73,7 @@ struct BuildLog { explicit LogEntry(const std::string& output); LogEntry(const std::string& output, uint64_t command_hash, - int start_time, int end_time, TimeStamp restat_mtime); + int start_time, int end_time, TimeStamp mtime); }; /// Lookup a previously-run command by its output path. diff --git a/src/build_test.cc b/src/build_test.cc index 4ef62b2113..3908761057 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -611,6 +611,7 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { fs_->WriteFile(edge->outputs_[0]->path(), content); } else if (edge->rule().name() == "touch-implicit-dep-out") { string dep = edge->GetBinding("test_dependency"); + fs_->Tick(); fs_->Create(dep, ""); fs_->Tick(); for (vector::iterator out = edge->outputs_.begin(); @@ -627,7 +628,12 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { fs_->Create(dep, ""); } else if (edge->rule().name() == "generate-depfile") { string dep = edge->GetBinding("test_dependency"); + bool touch_dep = edge->GetBindingBool("touch_dependency"); string depfile = edge->GetUnescapedDepfile(); + if (touch_dep) { + fs_->Tick(); + fs_->Create(dep, ""); + } string contents; for (vector::iterator out = edge->outputs_.begin(); out != edge->outputs_.end(); ++out) { @@ -635,6 +641,20 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { fs_->Create((*out)->path(), ""); } fs_->Create(depfile, contents); + } else if (edge->rule().name() == "long-cc") { + string dep = edge->GetBinding("test_dependency"); + string depfile = edge->GetUnescapedDepfile(); + string contents; + for (vector::iterator out = edge->outputs_.begin(); + out != edge->outputs_.end(); ++out) { + fs_->Tick(); + fs_->Tick(); + fs_->Tick(); + fs_->Create((*out)->path(), ""); + contents += (*out)->path() + ": " + dep + "\n"; + } + if (!dep.empty() && !depfile.empty()) + fs_->Create(depfile, contents); } else { printf("unknown command\n"); return false; @@ -690,6 +710,18 @@ bool FakeCommandRunner::WaitForCommand(Result* result) { else result->status = ExitSuccess; + // This rule simulates an external process modifying files while the build command runs. + // See TestInputMtimeRaceCondition and TestInputMtimeRaceConditionWithDepFile. + // Note: only the first and third time the rule is run per test is the file modified, so + // the test can verify that subsequent runs without the race have no work to do. + if (edge->rule().name() == "long-cc") { + string dep = edge->GetBinding("test_dependency"); + if (fs_->now_ == 4) + fs_->files_[dep].mtime = 3; + if (fs_->now_ == 10) + fs_->files_[dep].mtime = 9; + } + // Provide a way for test cases to verify when an edge finishes that // some other edge is still active. This is useful for test cases // covering behavior involving multiple active edges. @@ -1471,7 +1503,7 @@ TEST_F(BuildWithLogTest, ImplicitGeneratedOutOfDate) { TEST_F(BuildWithLogTest, ImplicitGeneratedOutOfDate2) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule touch-implicit-dep-out\n" -" command = touch $test_dependency ; sleep 1 ; touch $out\n" +" command = sleep 1 ; touch $test_dependency ; sleep 1 ; touch $out\n" " generator = 1\n" "build out.imp: touch-implicit-dep-out | inimp inimp2\n" " test_dependency = inimp\n")); @@ -1497,6 +1529,29 @@ TEST_F(BuildWithLogTest, ImplicitGeneratedOutOfDate2) { EXPECT_TRUE(builder_.AddTarget("out.imp", &err)); EXPECT_TRUE(builder_.AlreadyUpToDate()); EXPECT_FALSE(GetNode("out.imp")->dirty()); + + command_runner_.commands_ran_.clear(); + state_.Reset(); + builder_.Cleanup(); + builder_.plan_.Reset(); + + fs_.Tick(); + fs_.Create("inimp", ""); + + EXPECT_TRUE(builder_.AddTarget("out.imp", &err)); + EXPECT_FALSE(builder_.AlreadyUpToDate()); + + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_TRUE(builder_.AlreadyUpToDate()); + + command_runner_.commands_ran_.clear(); + state_.Reset(); + builder_.Cleanup(); + builder_.plan_.Reset(); + + EXPECT_TRUE(builder_.AddTarget("out.imp", &err)); + EXPECT_TRUE(builder_.AlreadyUpToDate()); + EXPECT_FALSE(GetNode("out.imp")->dirty()); } TEST_F(BuildWithLogTest, NotInLogButOnDisk) { @@ -1800,6 +1855,52 @@ TEST_F(BuildWithLogTest, RestatMissingInput) { ASSERT_EQ(restat_mtime, log_entry->mtime); } +TEST_F(BuildWithLogTest, RestatInputChangesDueToRule) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule generate-depfile\n" +" command = sleep 1 ; touch $touch_dependency; touch $out ; echo \"$out: $test_dependency\" > $depfile\n" +"build out1: generate-depfile || cat1\n" +" test_dependency = in2\n" +" touch_dependency = 1\n" +" restat = 1\n" +" depfile = out.d\n")); + + // Perform the first build. out1 is a restat rule, so its recorded mtime in the build + // log should be the time the command completes, not the time the command started. One + // of out1's discovered dependencies will have a newer mtime than when out1 started + // running, due to its command touching the dependency itself. + string err; + EXPECT_TRUE(builder_.AddTarget("out1", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder_.Build(&err)); + ASSERT_EQ("", err); + EXPECT_EQ(2u, command_runner_.commands_ran_.size()); + EXPECT_EQ(2u, builder_.plan_.command_edge_count()); + BuildLog::LogEntry* log_entry = build_log_.LookupByOutput("out1"); + ASSERT_TRUE(NULL != log_entry); + ASSERT_EQ(2u, log_entry->mtime); + + command_runner_.commands_ran_.clear(); + state_.Reset(); + builder_.Cleanup(); + builder_.plan_.Reset(); + + fs_.Tick(); + fs_.Create("in1", ""); + + // Touching a dependency of an order-only dependency of out1 should not cause out1 to + // rebuild. If out1 were not a restat rule, then it would rebuild here because its + // recorded mtime would have been an earlier mtime than its most recent input's (in2) + // mtime + EXPECT_TRUE(builder_.AddTarget("out1", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(!state_.GetNode("out1", 0)->dirty()); + EXPECT_TRUE(builder_.Build(&err)); + ASSERT_EQ("", err); + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); + EXPECT_EQ(1u, builder_.plan_.command_edge_count()); +} + TEST_F(BuildWithLogTest, GeneratedPlainDepfileMtime) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule generate-depfile\n" @@ -1904,10 +2005,11 @@ TEST_F(BuildTest, RspFileSuccess) EXPECT_TRUE(builder_.Build(&err)); ASSERT_EQ(3u, command_runner_.commands_ran_.size()); - // The RSP files were created - ASSERT_EQ(files_created + 2, fs_.files_created_.size()); + // The RSP files and temp file to acquire output mtimes were created + ASSERT_EQ(files_created + 3, fs_.files_created_.size()); ASSERT_EQ(1u, fs_.files_created_.count("out 2.rsp")); ASSERT_EQ(1u, fs_.files_created_.count("out 3.rsp")); + ASSERT_EQ(1u, fs_.files_created_.count(".ninja_lock")); // The RSP files were removed ASSERT_EQ(files_removed + 2, fs_.files_removed_.size()); @@ -1941,9 +2043,10 @@ TEST_F(BuildTest, RspFileFailure) { ASSERT_EQ("subcommand failed", err); ASSERT_EQ(1u, command_runner_.commands_ran_.size()); - // The RSP file was created - ASSERT_EQ(files_created + 1, fs_.files_created_.size()); + // The RSP file and temp file to acquire output mtimes were created + ASSERT_EQ(files_created + 2, fs_.files_created_.size()); ASSERT_EQ(1u, fs_.files_created_.count("out.rsp")); + ASSERT_EQ(1u, fs_.files_created_.count(".ninja_lock")); // The RSP file was NOT removed ASSERT_EQ(files_removed, fs_.files_removed_.size()); @@ -2522,6 +2625,210 @@ TEST_F(BuildWithDepsLogTest, DepsIgnoredInDryRun) { builder.command_runner_.release(); } +TEST_F(BuildWithDepsLogTest, TestInputMtimeRaceCondition) { + string err; + const char* manifest = + "rule long-cc\n" + " command = long-cc\n" + "build out: long-cc in1\n" + " test_dependency = in1\n"; + + State state; + ASSERT_NO_FATAL_FAILURE(AddCatRule(&state)); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + BuildLog build_log; + ASSERT_TRUE(build_log.Load("build_log", &err)); + ASSERT_TRUE(build_log.OpenForWrite("build_log", *this, &err)); + + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + + BuildLog::LogEntry* log_entry = NULL; + { + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + // Run the build, out gets built, dep file is created + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); + + // See that an entry in the logfile is created. the input_mtime is 1 since that was + // the mtime of in1 when the command was started + log_entry = build_log.LookupByOutput("out"); + ASSERT_TRUE(NULL != log_entry); + ASSERT_EQ(1u, log_entry->mtime); + + builder.command_runner_.release(); + } + + { + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + // Trigger the build again - "out" should rebuild despite having a newer mtime than + // "in1", since "in1" was touched during the build of out (simulated by changing its + // mtime in the the test builder's WaitForCommand() which runs before FinishCommand() + command_runner_.commands_ran_.clear(); + state.Reset(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); + + // Check that the logfile entry is still correct + log_entry = build_log.LookupByOutput("out"); + ASSERT_TRUE(NULL != log_entry); + ASSERT_TRUE(fs_.files_["in1"].mtime < log_entry->mtime); + builder.command_runner_.release(); + } + + { + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + // And a subsequent run should not have any work to do + command_runner_.commands_ran_.clear(); + state.Reset(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.AlreadyUpToDate()); + + builder.command_runner_.release(); + } +} + +TEST_F(BuildWithDepsLogTest, TestInputMtimeRaceConditionWithDepFile) { + string err; + const char* manifest = + "rule long-cc\n" + " command = long-cc\n" + "build out: long-cc\n" + " deps = gcc\n" + " depfile = out.d\n" + " test_dependency = header.h\n"; + + fs_.Create("header.h", ""); + + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + BuildLog build_log; + ASSERT_TRUE(build_log.Load("build_log", &err)); + ASSERT_TRUE(build_log.OpenForWrite("build_log", *this, &err)); + + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + + { + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + + // Run the build, out gets built, dep file is created + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); + + // See that an entry in the logfile is created. the mtime is 1 due to the command + // starting when the file system's mtime was 1. + BuildLog::LogEntry* log_entry = build_log.LookupByOutput("out"); + ASSERT_TRUE(NULL != log_entry); + ASSERT_EQ(1u, log_entry->mtime); + + builder.command_runner_.release(); + } + + { + // Trigger the build again - "out" will rebuild since its newest input mtime (header.h) + // is newer than the recorded mtime of out in the build log + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + state.Reset(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); + + builder.command_runner_.release(); + } + + { + // Trigger the build again - "out" won't rebuild since the file wasn't updated during + // the previous build + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + state.Reset(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + ASSERT_TRUE(builder.AlreadyUpToDate()); + + builder.command_runner_.release(); + } + + // touch the header to trigger a rebuild + fs_.Create("header.h", ""); + ASSERT_EQ(fs_.now_, 7); + + { + // Rebuild. This time, long-cc will cause header.h to be updated while the build is + // in progress + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + state.Reset(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); + + builder.command_runner_.release(); + } + + { + // Rebuild. Because header.h is now in the deplog for out, it should be detectable as + // a change-while-in-progress and should cause a rebuild of out. + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + state.Reset(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.Build(&err)); + ASSERT_EQ(1u, command_runner_.commands_ran_.size()); + + builder.command_runner_.release(); + } + + { + // This time, the header.h file was not updated during the build, so the target should + // not be considered dirty. + Builder builder(&state, config_, &build_log, &deps_log, &fs_, &status_, 0); + builder.command_runner_.reset(&command_runner_); + command_runner_.commands_ran_.clear(); + + state.Reset(); + EXPECT_TRUE(builder.AddTarget("out", &err)); + ASSERT_EQ("", err); + EXPECT_TRUE(builder.AlreadyUpToDate()); + + builder.command_runner_.release(); + } +} + /// Check that a restat rule generating a header cancels compilations correctly. TEST_F(BuildTest, RestatDepfileDependency) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, @@ -3042,9 +3349,10 @@ TEST_F(BuildTest, DyndepBuild) { ASSERT_EQ(2u, fs_.files_read_.size()); EXPECT_EQ("dd-in", fs_.files_read_[0]); EXPECT_EQ("dd", fs_.files_read_[1]); - ASSERT_EQ(2u + files_created, fs_.files_created_.size()); + ASSERT_EQ(3u + files_created, fs_.files_created_.size()); EXPECT_EQ(1u, fs_.files_created_.count("dd")); EXPECT_EQ(1u, fs_.files_created_.count("out")); + EXPECT_EQ(1u, fs_.files_created_.count(".ninja_lock")); } TEST_F(BuildTest, DyndepBuildSyntaxError) { diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 5e952edde5..7041d98400 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -198,7 +198,7 @@ TEST_F(DiskInterfaceTest, MakeDirs) { EXPECT_EQ(0, fclose(f)); #ifdef _WIN32 string path2 = "another\\with\\back\\\\slashes\\"; - EXPECT_TRUE(disk_.MakeDirs(path2.c_str())); + EXPECT_TRUE(disk_.MakeDirs(path2)); FILE* f2 = fopen((path2 + "a_file").c_str(), "w"); EXPECT_TRUE(f2); EXPECT_EQ(0, fclose(f2)); diff --git a/src/graph.cc b/src/graph.cc index 43ba45ae3d..041199a37f 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -298,37 +298,34 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge, return false; } - BuildLog::LogEntry* entry = 0; - // Dirty if we're missing the output. if (!output->exists()) { EXPLAIN("output %s doesn't exist", output->path().c_str()); return true; } - // Dirty if the output is older than the input. - if (most_recent_input && output->mtime() < most_recent_input->mtime()) { - TimeStamp output_mtime = output->mtime(); - - // If this is a restat rule, we may have cleaned the output with a restat - // rule in a previous run and stored the most recent input mtime in the - // build log. Use that mtime instead, so that the file will only be - // considered dirty if an input was modified since the previous run. - bool used_restat = false; - if (edge->GetBindingBool("restat") && build_log() && - (entry = build_log()->LookupByOutput(output->path()))) { - output_mtime = entry->mtime; - used_restat = true; - } + BuildLog::LogEntry* entry = 0; - if (output_mtime < most_recent_input->mtime()) { - EXPLAIN("%soutput %s older than most recent input %s " - "(%" PRId64 " vs %" PRId64 ")", - used_restat ? "restat of " : "", output->path().c_str(), - most_recent_input->path().c_str(), - output_mtime, most_recent_input->mtime()); - return true; - } + // If this is a restat rule, we may have cleaned the output in a + // previous run and stored the command start time in the build log. + // We don't want to consider a restat rule's outputs as dirty unless + // an input changed since the last run, so we'll skip checking the + // output file's actual mtime and simply check the recorded mtime from + // the log against the most recent input's mtime (see below) + bool used_restat = false; + if (edge->GetBindingBool("restat") && build_log() && + (entry = build_log()->LookupByOutput(output->path()))) { + used_restat = true; + } + + // Dirty if the output is older than the input. + if (!used_restat && most_recent_input && output->mtime() < most_recent_input->mtime()) { + EXPLAIN("output %s older than most recent input %s " + "(%" PRId64 " vs %" PRId64 ")", + output->path().c_str(), + most_recent_input->path().c_str(), + output->mtime(), most_recent_input->mtime()); + return true; } if (build_log()) { @@ -346,7 +343,9 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge, // May also be dirty due to the mtime in the log being older than the // mtime of the most recent input. This can occur even when the mtime // on disk is newer if a previous run wrote to the output file but - // exited with an error or was interrupted. + // exited with an error or was interrupted. If this was a restat rule, + // then we only check the recorded mtime against the most recent input + // mtime and ignore the actual output's mtime above. EXPLAIN("recorded mtime of %s older than most recent input %s (%" PRId64 " vs %" PRId64 ")", output->path().c_str(), most_recent_input->path().c_str(), entry->mtime, most_recent_input->mtime()); diff --git a/src/graph.h b/src/graph.h index 9de67d2718..d07a9b7639 100644 --- a/src/graph.h +++ b/src/graph.h @@ -172,7 +172,8 @@ struct Edge { : rule_(NULL), pool_(NULL), dyndep_(NULL), env_(NULL), mark_(VisitNone), id_(0), outputs_ready_(false), deps_loaded_(false), deps_missing_(false), generated_by_dep_loader_(false), - implicit_deps_(0), order_only_deps_(0), implicit_outs_(0) {} + command_start_time_(0), implicit_deps_(0), order_only_deps_(0), + implicit_outs_(0) {} /// Return true if all inputs' in-edges are ready. bool AllInputsReady() const; @@ -211,6 +212,7 @@ struct Edge { bool deps_loaded_; bool deps_missing_; bool generated_by_dep_loader_; + TimeStamp command_start_time_; const Rule& rule() const { return *rule_; } Pool* pool() const { return pool_; } From c136c2e1df1d8e3340f08b7d33aef7a4c835349c Mon Sep 17 00:00:00 2001 From: Eisuke Kawashima Date: Thu, 12 May 2022 06:05:21 +0900 Subject: [PATCH 16/18] improve zsh-completion - add `ninja` prefix to functions - improve completion of `-d` and `-t` - stop completion if `-h`, `--help`, or `--version` is supplied - add missing `--verbose` options --- misc/zsh-completion | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/misc/zsh-completion b/misc/zsh-completion index 4cee3b8631..d42dac220c 100644 --- a/misc/zsh-completion +++ b/misc/zsh-completion @@ -16,7 +16,7 @@ # Add the following to your .zshrc to tab-complete ninja targets # fpath=(path/to/ninja/misc/zsh-completion $fpath) -__get_targets() { +(( $+functions[_ninja-get-targets] )) || _ninja-get-targets() { dir="." if [ -n "${opt_args[-C]}" ]; then @@ -31,42 +31,44 @@ __get_targets() { eval ${targets_command} 2>/dev/null | cut -d: -f1 } -__get_tools() { - ninja -t list 2>/dev/null | while read -r a b; do echo $a; done | tail -n +2 +(( $+functions[_ninja-get-tools] )) || _ninja-get-tools() { + # remove the first line; remove the leading spaces; replace spaces with colon + ninja -t list 2> /dev/null | sed -e '1d;s/^ *//;s/ \+/:/' } -__get_modes() { - ninja -d list 2>/dev/null | while read -r a b; do echo $a; done | tail -n +2 | sed '$d' +(( $+functions[_ninja-get-modes] )) || _ninja-get-modes() { + # remove the first line; remove the last line; remove the leading spaces; replace spaces with colon + ninja -d list 2> /dev/null | sed -e '1d;$d;s/^ *//;s/ \+/:/' } -__modes() { +(( $+functions[_ninja-modes] )) || _ninja-modes() { local -a modes - modes=(${(fo)"$(__get_modes)"}) + modes=(${(fo)"$(_ninja-get-modes)"}) _describe 'modes' modes } -__tools() { +(( $+functions[_ninja-tools] )) || _ninja-tools() { local -a tools - tools=(${(fo)"$(__get_tools)"}) + tools=(${(fo)"$(_ninja-get-tools)"}) _describe 'tools' tools } -__targets() { +(( $+functions[_ninja-targets] )) || _ninja-targets() { local -a targets - targets=(${(fo)"$(__get_targets)"}) + targets=(${(fo)"$(_ninja-get-targets)"}) _describe 'targets' targets } _arguments \ - {-h,--help}'[Show help]' \ - '--version[Print ninja version]' \ + '(- *)'{-h,--help}'[Show help]' \ + '(- *)--version[Print ninja version]' \ '-C+[Change to directory before doing anything else]:directories:_directories' \ '-f+[Specify input build file (default=build.ninja)]:files:_files' \ '-j+[Run N jobs in parallel (default=number of CPUs available)]:number of jobs' \ '-l+[Do not start new jobs if the load average is greater than N]:number of jobs' \ '-k+[Keep going until N jobs fail (default=1)]:number of jobs' \ '-n[Dry run (do not run commands but act like they succeeded)]' \ - '-v[Show all command lines while building]' \ - '-d+[Enable debugging (use -d list to list modes)]:modes:__modes' \ - '-t+[Run a subtool (use -t list to list subtools)]:tools:__tools' \ - '*::targets:__targets' + '(-v --verbose)'{-v,--verbose}'[Show all command lines while building]' \ + '-d+[Enable debugging (use -d list to list modes)]:modes:_ninja-modes' \ + '-t+[Run a subtool (use -t list to list subtools)]:tools:_ninja-tools' \ + '*::targets:_ninja-targets' From a4c24a33c1ed32d9d51c8df763ec6ad574587d02 Mon Sep 17 00:00:00 2001 From: Bruce Dawson Date: Thu, 16 Jun 2022 00:47:21 -0700 Subject: [PATCH 17/18] Build ninja with C++11 (#2089) * Build ninja with C++11 In order to allow future use of std::chrono to make the stats code portable it is desirable to compile with C++11. Doing so also allows use of std::unordered_map, and reduces the number of #ifdefs in the ninja source code. Switching to C++11 requires modifying both CMakeLists.txt and configure.py, for MSVC and for other build systems. For MSVC the required change is adding /Zc:__cplusplus to tell the compiler to give a more accurate value for the __cplusplus macro. For other platforms the change is to add -std=c++11 or the CMake equivalent. This change makes some progress towards resolving issue #2004. * Delete code and instructions C++11 guarantees that string::data() gives null-terminated pointers, so explicitly adding a null terminator is no longer needed. The Google C++ Style Guide already recommends avoiding unnecessary use of C++14 and C++17 so repeating this in CONTRIBUTING.md is not critical. These changes both came from PR-review suggestions. * Only set cxx_std_11 if standard is 98 * Return to unconditional target_compile_features use After much discussion it sounds like using target_compile_features unconditionally is best. --- CMakeLists.txt | 4 ++++ CONTRIBUTING.md | 3 --- configure.py | 4 ++++ src/graph.cc | 4 ---- src/hash_map.h | 44 -------------------------------------------- src/missing_deps.h | 7 ------- src/parser.cc | 7 ------- 7 files changed, 8 insertions(+), 65 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 22b815886c..9c0f27a93e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,6 +21,8 @@ endif() if(MSVC) set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") string(REPLACE "/GR" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS}) + # Note that these settings are separately specified in configure.py, and + # these lists should be kept in sync. add_compile_options(/W4 /wd4100 /wd4267 /wd4706 /wd4702 /wd4244 /GR- /Zc:__cplusplus) add_compile_definitions(_CRT_SECURE_NO_WARNINGS) else() @@ -138,6 +140,8 @@ else() endif() endif() +target_compile_features(libninja PUBLIC cxx_std_11) + #Fixes GetActiveProcessorCount on MinGW if(MINGW) target_compile_definitions(libninja PRIVATE _WIN32_WINNT=0x0601 __USE_MINGW_ANSI_STDIO=1) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index be1fc02779..c6c190c058 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,9 +14,6 @@ Generally it's the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) with a few additions: -* Any code merged into the Ninja codebase which will be part of the main - executable must compile as C++03. You may use C++11 features in a test or an - unimportant tool if you guard your code with `#if __cplusplus >= 201103L`. * We have used `using namespace std;` a lot in the past. For new contributions, please try to avoid relying on it and instead whenever possible use `std::`. However, please do not change existing code simply to add `std::` unless your diff --git a/configure.py b/configure.py index 43904349a8..99a2c86e35 100755 --- a/configure.py +++ b/configure.py @@ -305,6 +305,8 @@ def binary(name): else: n.variable('ar', configure_env.get('AR', 'ar')) +# Note that build settings are separately specified in CMakeLists.txt and +# these lists should be kept in sync. if platform.is_msvc(): cflags = ['/showIncludes', '/nologo', # Don't print startup banner. @@ -320,6 +322,7 @@ def binary(name): # Disable warnings about ignored typedef in DbgHelp.h '/wd4091', '/GR-', # Disable RTTI. + '/Zc:__cplusplus', # Disable size_t -> int truncation warning. # We never have strings or arrays larger than 2**31. '/wd4267', @@ -339,6 +342,7 @@ def binary(name): '-Wno-unused-parameter', '-fno-rtti', '-fno-exceptions', + '-std=c++11', '-fvisibility=hidden', '-pipe', '-DNINJA_PYTHON="%s"' % options.with_python] if options.debug: diff --git a/src/graph.cc b/src/graph.cc index 041199a37f..95fc1dc1b5 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -402,11 +402,7 @@ string EdgeEnv::LookupVariable(const string& var) { if (var == "in" || var == "in_newline") { int explicit_deps_count = edge_->inputs_.size() - edge_->implicit_deps_ - edge_->order_only_deps_; -#if __cplusplus >= 201103L return MakePathList(edge_->inputs_.data(), explicit_deps_count, -#else - return MakePathList(&edge_->inputs_[0], explicit_deps_count, -#endif var == "in" ? ' ' : '\n'); } else if (var == "out") { int explicit_outs_count = edge_->outputs_.size() - edge_->implicit_outs_; diff --git a/src/hash_map.h b/src/hash_map.h index 55d2c9d46d..435360984d 100644 --- a/src/hash_map.h +++ b/src/hash_map.h @@ -53,7 +53,6 @@ unsigned int MurmurHash2(const void* key, size_t len) { return h; } -#if (__cplusplus >= 201103L) || (_MSC_VER >= 1900) #include namespace std { @@ -68,56 +67,13 @@ struct hash { }; } -#elif defined(_MSC_VER) -#include - -using stdext::hash_map; -using stdext::hash_compare; - -struct StringPieceCmp : public hash_compare { - size_t operator()(const StringPiece& key) const { - return MurmurHash2(key.str_, key.len_); - } - bool operator()(const StringPiece& a, const StringPiece& b) const { - int cmp = memcmp(a.str_, b.str_, min(a.len_, b.len_)); - if (cmp < 0) { - return true; - } else if (cmp > 0) { - return false; - } else { - return a.len_ < b.len_; - } - } -}; - -#else -#include - -using __gnu_cxx::hash_map; - -namespace __gnu_cxx { -template<> -struct hash { - size_t operator()(StringPiece key) const { - return MurmurHash2(key.str_, key.len_); - } -}; -} -#endif - /// A template for hash_maps keyed by a StringPiece whose string is /// owned externally (typically by the values). Use like: /// ExternalStringHash::Type foos; to make foos into a hash /// mapping StringPiece => Foo*. template struct ExternalStringHashMap { -#if (__cplusplus >= 201103L) || (_MSC_VER >= 1900) typedef std::unordered_map Type; -#elif defined(_MSC_VER) - typedef hash_map Type; -#else - typedef hash_map Type; -#endif }; #endif // NINJA_MAP_H_ diff --git a/src/missing_deps.h b/src/missing_deps.h index ae5707424c..7a615da2a5 100644 --- a/src/missing_deps.h +++ b/src/missing_deps.h @@ -19,9 +19,7 @@ #include #include -#if __cplusplus >= 201103L #include -#endif struct DepsLog; struct DiskInterface; @@ -68,13 +66,8 @@ struct MissingDependencyScanner { int missing_dep_path_count_; private: -#if __cplusplus >= 201103L using InnerAdjacencyMap = std::unordered_map; using AdjacencyMap = std::unordered_map; -#else - typedef std::map InnerAdjacencyMap; - typedef std::map AdjacencyMap; -#endif AdjacencyMap adjacency_map_; }; diff --git a/src/parser.cc b/src/parser.cc index 756922de11..5f303c557c 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -31,13 +31,6 @@ bool Parser::Load(const string& filename, string* err, Lexer* parent) { return false; } - // 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); } From d4017a2b1ea642f12dabe05ec99b2a16c93e99aa Mon Sep 17 00:00:00 2001 From: "Igor [hyperxor]" <56217938+hyperxor@users.noreply.github.com> Date: Mon, 20 Jun 2022 09:45:29 +0300 Subject: [PATCH 18/18] Make IsDepsEntryLiveFor static and add const to parameter (#2141) --- src/deps_log.cc | 2 +- src/deps_log.h | 2 +- src/ninja.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/deps_log.cc b/src/deps_log.cc index 7e48b38513..e32a7a98ad 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -361,7 +361,7 @@ bool DepsLog::Recompact(const string& path, string* err) { return true; } -bool DepsLog::IsDepsEntryLiveFor(Node* node) { +bool DepsLog::IsDepsEntryLiveFor(const 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 diff --git a/src/deps_log.h b/src/deps_log.h index 09cc41c019..2a1b188906 100644 --- a/src/deps_log.h +++ b/src/deps_log.h @@ -97,7 +97,7 @@ struct DepsLog { /// 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); + static bool IsDepsEntryLiveFor(const Node* node); /// Used for tests. const std::vector& nodes() const { return nodes_; } diff --git a/src/ninja.cc b/src/ninja.cc index 834e2846ee..9ae53deb7d 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -532,7 +532,7 @@ int NinjaMain::ToolDeps(const Options* options, int argc, char** argv) { if (argc == 0) { for (vector::const_iterator ni = deps_log_.nodes().begin(); ni != deps_log_.nodes().end(); ++ni) { - if (deps_log_.IsDepsEntryLiveFor(*ni)) + if (DepsLog::IsDepsEntryLiveFor(*ni)) nodes.push_back(*ni); } } else {