From b0425804290dc25092a2af1a6a05a4f8238a8c93 Mon Sep 17 00:00:00 2001 From: Colin D Bennett Date: Thu, 12 May 2016 11:09:51 -0700 Subject: Write subprocess output to stdout in binary mode Set stdout to binary mode while writing subprocess output, so that the CR in a CR LF sequence is not replaced with CR LF itself, which would result in CR CR LF. Based on patch posted by nico in issue #773 comment. --- src/build.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/build.cc b/src/build.cc index 7792016..e4fe21a 100644 --- a/src/build.cc +++ b/src/build.cc @@ -20,6 +20,11 @@ #include #include +#ifdef _WIN32 +#include +#include +#endif + #if defined(__SVR4) && defined(__sun) #include #endif @@ -160,7 +165,17 @@ void BuildStatus::BuildEdgeFinished(Edge* edge, final_output = StripAnsiEscapeCodes(output); else final_output = output; + +#ifdef _WIN32 + // Fix extra CR being added on Windows, writing out CR CR LF (#773) + _setmode(_fileno(stdout), _O_BINARY); // Begin Windows extra CR fix +#endif + printer_.PrintOnNewLine(final_output); + +#ifdef _WIN32 + _setmode(_fileno(stdout), _O_TEXT); // End Windows extra CR fix +#endif } } -- cgit v0.12 From c4b09e1e7e1d531a818601be33e0ec8ee18c3cde Mon Sep 17 00:00:00 2001 From: Daniel Weber Date: Tue, 23 Aug 2016 00:26:38 +0200 Subject: Allow more path components - 60 instead of 30 path components - 64 instead of 32 backslashes in a path (windows only) Issue: 1161 --- src/build.cc | 2 +- src/clparser.cc | 2 +- src/graph.cc | 4 ++-- src/includes_normalize-win32.cc | 2 +- src/manifest_parser.cc | 6 +++--- src/ninja.cc | 4 ++-- src/util.cc | 18 +++++++++--------- src/util.h | 4 ++-- src/util_test.cc | 38 +++++++++++++++++++++++--------------- 9 files changed, 44 insertions(+), 36 deletions(-) diff --git a/src/build.cc b/src/build.cc index b806fb5..5078251 100644 --- a/src/build.cc +++ b/src/build.cc @@ -918,7 +918,7 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, deps_nodes->reserve(deps.ins_.size()); for (vector::iterator i = deps.ins_.begin(); i != deps.ins_.end(); ++i) { - unsigned int slash_bits; + uint64_t slash_bits; if (!CanonicalizePath(const_cast(i->str_), &i->len_, &slash_bits, err)) return false; diff --git a/src/clparser.cc b/src/clparser.cc index f73a8c1..c17150b 100644 --- a/src/clparser.cc +++ b/src/clparser.cc @@ -90,7 +90,7 @@ bool CLParser::Parse(const string& output, const string& deps_prefix, #else // TODO: should this make the path relative to cwd? normalized = include; - unsigned int slash_bits; + uint64_t slash_bits; if (!CanonicalizePath(&normalized, &slash_bits, err)) return false; #endif diff --git a/src/graph.cc b/src/graph.cc index f1d9ca2..a5bac1c 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -420,7 +420,7 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, return false; } - unsigned int unused; + uint64_t unused; if (!CanonicalizePath(const_cast(depfile.out_.str_), &depfile.out_.len_, &unused, err)) return false; @@ -442,7 +442,7 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, // Add all its in-edges. for (vector::iterator i = depfile.ins_.begin(); i != depfile.ins_.end(); ++i, ++implicit_dep) { - unsigned int slash_bits; + uint64_t slash_bits; if (!CanonicalizePath(const_cast(i->str_), &i->len_, &slash_bits, err)) return false; diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index ca35012..e8a3e0f 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -103,7 +103,7 @@ bool IncludesNormalize::Normalize(const string& input, const char* relative_to, return false; } strncpy(copy, input.c_str(), input.size() + 1); - unsigned int slash_bits; + uint64_t slash_bits; if (!CanonicalizePath(copy, &len, &slash_bits, err)) return false; StringPiece partially_fixed(copy, len); diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index d6dcf22..2164921 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -212,7 +212,7 @@ bool ManifestParser::ParseDefault(string* err) { do { string path = eval.Evaluate(env_); string path_err; - unsigned int slash_bits; // Unused because this only does lookup. + uint64_t slash_bits; // Unused because this only does lookup. if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); if (!state_->AddDefault(path, &path_err)) @@ -342,7 +342,7 @@ bool ManifestParser::ParseEdge(string* err) { for (size_t i = 0, e = outs.size(); i != e; ++i) { string path = outs[i].Evaluate(env); string path_err; - unsigned int slash_bits; + uint64_t slash_bits; if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); if (!state_->AddOut(edge, path, slash_bits)) { @@ -375,7 +375,7 @@ bool ManifestParser::ParseEdge(string* err) { for (vector::iterator i = ins.begin(); i != ins.end(); ++i) { string path = i->Evaluate(env); string path_err; - unsigned int slash_bits; + uint64_t slash_bits; if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); state_->AddIn(edge, path, slash_bits); diff --git a/src/ninja.cc b/src/ninja.cc index 25eafe8..4fb04ad 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -233,7 +233,7 @@ int GuessParallelism() { /// Returns true if the manifest was rebuilt. bool NinjaMain::RebuildManifest(const char* input_file, string* err) { string path = input_file; - unsigned int slash_bits; // Unused because this path is only used for lookup. + uint64_t slash_bits; // Unused because this path is only used for lookup. if (!CanonicalizePath(&path, &slash_bits, err)) return false; Node* node = state_.LookupNode(path); @@ -255,7 +255,7 @@ bool NinjaMain::RebuildManifest(const char* input_file, string* err) { Node* NinjaMain::CollectTarget(const char* cpath, string* err) { string path = cpath; - unsigned int slash_bits; + uint64_t slash_bits; if (!CanonicalizePath(&path, &slash_bits, err)) return NULL; diff --git a/src/util.cc b/src/util.cc index e31fd1f..fb57cec 100644 --- a/src/util.cc +++ b/src/util.cc @@ -90,7 +90,7 @@ void Error(const char* msg, ...) { fprintf(stderr, "\n"); } -bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err) { +bool CanonicalizePath(string* path, uint64_t* slash_bits, string* err) { METRIC_RECORD("canonicalize str"); size_t len = path->size(); char* str = 0; @@ -103,19 +103,19 @@ bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err) { } #ifdef _WIN32 -static unsigned int ShiftOverBit(int offset, unsigned int bits) { +static uint64_t ShiftOverBit(int offset, uint64_t bits) { // e.g. for |offset| == 2: // | ... 9 8 7 6 5 4 3 2 1 0 | // \_________________/ \_/ // above below // So we drop the bit at offset and move above "down" into its place. - unsigned int above = bits & ~((1 << (offset + 1)) - 1); - unsigned int below = bits & ((1 << offset) - 1); + uint64_t above = bits & ~((1 << (offset + 1)) - 1); + uint64_t below = bits & ((1 << offset) - 1); return (above >> 1) | below; } #endif -bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, +bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, string* err) { // WARNING: this function is performance-critical; please benchmark // any changes you make to it. @@ -125,7 +125,7 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, return false; } - const int kMaxPathComponents = 30; + const int kMaxPathComponents = 60; char* components[kMaxPathComponents]; int component_count = 0; @@ -135,8 +135,8 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, const char* end = start + *len; #ifdef _WIN32 - unsigned int bits = 0; - unsigned int bits_mask = 1; + uint64_t bits = 0; + uint64_t bits_mask = 1; int bits_offset = 0; // Convert \ to /, setting a bit in |bits| for each \ encountered. for (char* c = path; c < end; ++c) { @@ -150,7 +150,7 @@ bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, bits_offset++; } } - if (bits_offset > 32) { + if (bits_offset > 64) { *err = "too many path components"; return false; } diff --git a/src/util.h b/src/util.h index cbdc1a6..846cd93 100644 --- a/src/util.h +++ b/src/util.h @@ -43,8 +43,8 @@ void Error(const char* msg, ...); /// Canonicalize a path like "foo/../bar.h" into just "bar.h". /// |slash_bits| has bits set starting from lowest for a backslash that was /// normalized to a forward slash. (only used on Windows) -bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err); -bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, +bool CanonicalizePath(string* path, uint64_t* slash_bits, string* err); +bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, string* err); /// Appends |input| to |*result|, escaping according to the whims of either diff --git a/src/util_test.cc b/src/util_test.cc index 33a4107..64c2bbf 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -19,7 +19,7 @@ namespace { bool CanonicalizePath(string* path, string* err) { - unsigned int unused; + uint64_t unused; return ::CanonicalizePath(path, &unused, err); } @@ -177,7 +177,7 @@ TEST(CanonicalizePath, PathSamplesWindows) { TEST(CanonicalizePath, SlashTracking) { string path; string err; - unsigned int slash_bits; + uint64_t slash_bits; path = "foo.h"; err = ""; EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); @@ -263,7 +263,7 @@ TEST(CanonicalizePath, SlashTracking) { TEST(CanonicalizePath, CanonicalizeNotExceedingLen) { // Make sure searching \/ doesn't go past supplied len. char buf[] = "foo/bar\\baz.h\\"; // Last \ past end. - unsigned int slash_bits; + uint64_t slash_bits; string err; size_t size = 13; EXPECT_TRUE(::CanonicalizePath(buf, &size, &slash_bits, &err)); @@ -274,31 +274,39 @@ TEST(CanonicalizePath, CanonicalizeNotExceedingLen) { TEST(CanonicalizePath, TooManyComponents) { string path; string err; - unsigned int slash_bits; + uint64_t slash_bits; - // 32 is OK. - path = "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./x.h"; + // 64 is OK. + path = "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./" + "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./x.h"; EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(slash_bits, 0x0); // Backslashes version. path = - "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\." - "\\a\\.\\a\\.\\a\\.\\a\\.\\x.h"; + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\x.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); - EXPECT_EQ(slash_bits, 0xffff); + printf("%x\n",slash_bits); + EXPECT_EQ(slash_bits, 0xffffffffLL); - // 33 is not. + // 65 is not. err = ""; - path = - "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/x.h"; + path = "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./" + "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./x/y.h"; EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ(err, "too many path components"); // Backslashes version. err = ""; path = - "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\." - "\\a\\.\\a\\.\\a\\.\\a\\.\\a\\x.h"; + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\x\\y.h"; EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ(err, "too many path components"); } @@ -326,7 +334,7 @@ TEST(CanonicalizePath, NotNullTerminated) { string path; string err; size_t len; - unsigned int unused; + uint64_t unused; path = "foo/. bar/."; len = strlen("foo/."); // Canonicalize only the part before the space. -- cgit v0.12 From 5153cedfd7720547489b6b2e5d5e4485def621c9 Mon Sep 17 00:00:00 2001 From: malc Date: Fri, 26 Aug 2016 21:25:41 +0300 Subject: Use POSIX_SPAWN_USEVFORK if available Existing comments were alluding to it's usage and it makes building ninja itself go a litle bit faster (i.e. taking less wall clock time). FWIW FreeBSD even uses vfork by default c.f. https://www.freebsd.org/cgi/man.cgi?query=posix_spawn --- src/subprocess-posix.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index 5ffe85b..f1f94e5 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -73,8 +73,6 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { // default action in the new process image, so no explicit // POSIX_SPAWN_SETSIGDEF parameter is needed. - // TODO: Consider using POSIX_SPAWN_USEVFORK on Linux with glibc? - if (!use_console_) { // Put the child in its own process group, so ctrl-c won't reach it. flags |= POSIX_SPAWN_SETPGROUP; @@ -93,6 +91,9 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { // In the console case, output_pipe is still inherited by the child and // closed when the subprocess finishes, which then notifies ninja. } +#ifdef POSIX_SPAWN_USEVFORK + flags |= POSIX_SPAWN_USEVFORK; +#endif if (posix_spawnattr_setflags(&attr, flags) != 0) Fatal("posix_spawnattr_setflags: %s", strerror(errno)); -- cgit v0.12 From 1cc730ddc27df52d757be1c2e7aa96193f8ca9df Mon Sep 17 00:00:00 2001 From: Daniel Weber Date: Thu, 1 Sep 2016 22:02:18 +0200 Subject: Use uint64_t for slash_bits consistently The VS compiler complained about possible loss of data (and it was right!) --- src/graph.cc | 4 ++-- src/graph.h | 9 +++++---- src/state.cc | 6 +++--- src/state.h | 7 ++++--- src/util_test.cc | 3 +-- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/graph.cc b/src/graph.cc index a5bac1c..e7f63fe 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -348,10 +348,10 @@ bool Edge::use_console() const { } // static -string Node::PathDecanonicalized(const string& path, unsigned int slash_bits) { +string Node::PathDecanonicalized(const string& path, uint64_t slash_bits) { string result = path; #ifdef _WIN32 - unsigned int mask = 1; + uint64_t mask = 1; for (char* c = &result[0]; (c = strchr(c, '/')) != NULL;) { if (slash_bits & mask) *c = '\\'; diff --git a/src/graph.h b/src/graph.h index add8d3d..ec24293 100644 --- a/src/graph.h +++ b/src/graph.h @@ -21,6 +21,7 @@ using namespace std; #include "eval_env.h" #include "timestamp.h" +#include "util.h" struct BuildLog; struct DiskInterface; @@ -33,7 +34,7 @@ struct State; /// Information about a node in the dependency graph: the file, whether /// it's dirty, mtime, etc. struct Node { - Node(const string& path, unsigned int slash_bits) + Node(const string& path, uint64_t slash_bits) : path_(path), slash_bits_(slash_bits), mtime_(-1), @@ -76,8 +77,8 @@ struct Node { return PathDecanonicalized(path_, slash_bits_); } static string PathDecanonicalized(const string& path, - unsigned int slash_bits); - unsigned int slash_bits() const { return slash_bits_; } + uint64_t slash_bits); + uint64_t slash_bits() const { return slash_bits_; } TimeStamp mtime() const { return mtime_; } @@ -101,7 +102,7 @@ private: /// Set bits starting from lowest for backslashes that were normalized to /// forward slashes by CanonicalizePath. See |PathDecanonicalized|. - unsigned int slash_bits_; + uint64_t slash_bits_; /// Possible values of mtime_: /// -1: file hasn't been examined diff --git a/src/state.cc b/src/state.cc index d539e7b..6079229 100644 --- a/src/state.cc +++ b/src/state.cc @@ -100,7 +100,7 @@ Edge* State::AddEdge(const Rule* rule) { return edge; } -Node* State::GetNode(StringPiece path, unsigned int slash_bits) { +Node* State::GetNode(StringPiece path, uint64_t slash_bits) { Node* node = LookupNode(path); if (node) return node; @@ -134,13 +134,13 @@ Node* State::SpellcheckNode(const string& path) { return result; } -void State::AddIn(Edge* edge, StringPiece path, unsigned int slash_bits) { +void State::AddIn(Edge* edge, StringPiece path, uint64_t slash_bits) { Node* node = GetNode(path, slash_bits); edge->inputs_.push_back(node); node->AddOutEdge(edge); } -bool State::AddOut(Edge* edge, StringPiece path, unsigned int slash_bits) { +bool State::AddOut(Edge* edge, StringPiece path, uint64_t slash_bits) { Node* node = GetNode(path, slash_bits); if (node->in_edge()) return false; diff --git a/src/state.h b/src/state.h index b530207..54e9dc5 100644 --- a/src/state.h +++ b/src/state.h @@ -23,6 +23,7 @@ using namespace std; #include "eval_env.h" #include "hash_map.h" +#include "util.h" struct Edge; struct Node; @@ -93,12 +94,12 @@ struct State { Edge* AddEdge(const Rule* rule); - Node* GetNode(StringPiece path, unsigned int slash_bits); + Node* GetNode(StringPiece path, uint64_t slash_bits); Node* LookupNode(StringPiece path) const; Node* SpellcheckNode(const string& path); - void AddIn(Edge* edge, StringPiece path, unsigned int slash_bits); - bool AddOut(Edge* edge, StringPiece path, unsigned int slash_bits); + void AddIn(Edge* edge, StringPiece path, uint64_t slash_bits); + bool AddOut(Edge* edge, StringPiece path, uint64_t slash_bits); bool AddDefault(StringPiece path, string* error); /// Reset state. Keeps all nodes and edges, but restores them to the diff --git a/src/util_test.cc b/src/util_test.cc index 64c2bbf..45d0727 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -290,8 +290,7 @@ TEST(CanonicalizePath, TooManyComponents) { "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\x.h"; EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); - printf("%x\n",slash_bits); - EXPECT_EQ(slash_bits, 0xffffffffLL); + EXPECT_EQ(slash_bits, 0xffffffff); // 65 is not. err = ""; -- cgit v0.12 From 46d9a958fa11b6b5f4f6a106f28898eac54ca842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20=C5=9Aniatowski?= Date: Wed, 12 Oct 2016 14:09:28 +0200 Subject: Improve error message when a depfile contains a bad path Debugging "ninja: error: empty path" is not fun, so make the error message mention the depfile name. --- src/graph.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/graph.cc b/src/graph.cc index f1d9ca2..136ca04 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -422,8 +422,10 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, unsigned int unused; if (!CanonicalizePath(const_cast(depfile.out_.str_), - &depfile.out_.len_, &unused, err)) + &depfile.out_.len_, &unused, err)) { + *err = path + ": " + *err; return false; + } // Check that this depfile matches the edge's output, if not return false to // mark the edge as dirty. -- cgit v0.12 From 8b7155b0e582954909c0b56acfa2a38e04f1c5ee Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 6 Nov 2016 12:20:24 -0800 Subject: teach -t commands to optionally print only the final command --- src/ninja.cc | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/ninja.cc b/src/ninja.cc index 25eafe8..5ab73e9 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -533,21 +533,51 @@ int NinjaMain::ToolTargets(const Options* options, int argc, char* argv[]) { } } -void PrintCommands(Edge* edge, set* seen) { +enum PrintCommandMode { PCM_Single, PCM_All }; +void PrintCommands(Edge* edge, set* seen, PrintCommandMode mode) { if (!edge) return; if (!seen->insert(edge).second) return; - for (vector::iterator in = edge->inputs_.begin(); - in != edge->inputs_.end(); ++in) - PrintCommands((*in)->in_edge(), seen); + if (mode == PCM_All) { + for (vector::iterator in = edge->inputs_.begin(); + in != edge->inputs_.end(); ++in) + PrintCommands((*in)->in_edge(), seen, mode); + } if (!edge->is_phony()) puts(edge->EvaluateCommand().c_str()); } int NinjaMain::ToolCommands(const Options* options, int argc, char* argv[]) { + // The clean tool uses getopt, and expects argv[0] to contain the name of + // the tool, i.e. "commands". + ++argc; + --argv; + + PrintCommandMode mode = PCM_All; + + optind = 1; + int opt; + while ((opt = getopt(argc, argv, const_cast("hs"))) != -1) { + switch (opt) { + case 's': + mode = PCM_Single; + break; + case 'h': + default: + printf("usage: ninja -t commands [options] [targets]\n" +"\n" +"options:\n" +" -s only print the final command to build [target], not the whole chain\n" + ); + return 1; + } + } + argv += optind; + argc -= optind; + vector nodes; string err; if (!CollectTargetsFromArgs(argc, argv, &nodes, &err)) { @@ -557,7 +587,7 @@ int NinjaMain::ToolCommands(const Options* options, int argc, char* argv[]) { set seen; for (vector::iterator in = nodes.begin(); in != nodes.end(); ++in) - PrintCommands((*in)->in_edge(), &seen); + PrintCommands((*in)->in_edge(), &seen, mode); return 0; } -- cgit v0.12 From 474e87d4f3addced6c3d5f78d330d12221bc2d6b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 7 Nov 2016 15:47:44 -0800 Subject: Update RELEASING -- manual.asciidoc no longer has a version number --- RELEASING | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/RELEASING b/RELEASING index 880a55d..cfa03b6 100644 --- a/RELEASING +++ b/RELEASING @@ -2,13 +2,13 @@ Notes to myself on all the steps to make for a Ninja release. Push new release branch: 1. Consider sending a heads-up to the ninja-build mailing list first -2. update src/version.cc with new version (with ".git"), then - git commit -a -m 'mark this 1.5.0.git' -3. git checkout release; git merge master -4. fix version number in src/version.cc (it will likely conflict in the above) -5. fix version in doc/manual.asciidoc +2. Make sure branches 'master' and 'release' are synced up locally +3. update src/version.cc with new version (with ".git"), then + git commit -am 'mark this 1.5.0.git' +4. git checkout release; git merge master +5. fix version number in src/version.cc (it will likely conflict in the above) 6. commit, tag, push (don't forget to push --tags) - git commit -a -m v1.5.0; git push origin release + git commit -am v1.5.0; git push origin release git tag v1.5.0; git push --tags # Push the 1.5.0.git change on master too: git checkout master; git push origin master -- cgit v0.12 From 14f47940af19a9d153074ff177c5f0ab8cc3e4b9 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 7 Nov 2016 22:00:33 -0500 Subject: fix RELEASING wrt manual.asciidoc process --- RELEASING | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/RELEASING b/RELEASING index cfa03b6..5f51b73 100644 --- a/RELEASING +++ b/RELEASING @@ -7,12 +7,13 @@ Push new release branch: git commit -am 'mark this 1.5.0.git' 4. git checkout release; git merge master 5. fix version number in src/version.cc (it will likely conflict in the above) -6. commit, tag, push (don't forget to push --tags) +6. fix version in doc/manual.asciidoc (exists only on release branch) +7. commit, tag, push (don't forget to push --tags) git commit -am v1.5.0; git push origin release git tag v1.5.0; git push --tags # Push the 1.5.0.git change on master too: git checkout master; git push origin master -7. construct release notes from prior notes +8. construct release notes from prior notes credits: git shortlog -s --no-merges REV.. Release on github: -- cgit v0.12 From 32c3625649f3083f9105bd5ffc9974fadc0146d4 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 7 Nov 2016 22:19:06 -0500 Subject: fix a clang-cl -Wformat warning --- src/minidump-win32.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/minidump-win32.cc b/src/minidump-win32.cc index c611919..1efb085 100644 --- a/src/minidump-win32.cc +++ b/src/minidump-win32.cc @@ -34,7 +34,7 @@ void CreateWin32MiniDump(_EXCEPTION_POINTERS* pep) { char temp_path[MAX_PATH]; GetTempPath(sizeof(temp_path), temp_path); char temp_file[MAX_PATH]; - sprintf(temp_file, "%s\\ninja_crash_dump_%d.dmp", + sprintf(temp_file, "%s\\ninja_crash_dump_%lu.dmp", temp_path, GetCurrentProcessId()); // Delete any previous minidump of the same name. -- cgit v0.12 From 32c82da13e2d16792c79507be3d875a6e4624423 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 7 Nov 2016 23:36:32 -0500 Subject: windows: replace deprecated GetVersionEx with recommended replacement The recommended replacement VerifyVersionInfo should work with the same SDKs that GetVersionEx worked with (while the wrappers in VersionHelpers.h require a recent SDK). This patch should not change behavior, and it's not supposed to increase build requirements. If this makes things harder to build, please let me know. --- src/disk_interface.cc | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 451a9b4..1b4135f 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -80,20 +80,15 @@ TimeStamp StatSingleFile(const string& path, string* err) { return TimeStampFromFileTime(attrs.ftLastWriteTime); } -#ifdef _MSC_VER -#pragma warning(push) -#pragma warning(disable: 4996) // GetVersionExA is deprecated post SDK 8.1. -#endif bool IsWindows7OrLater() { - OSVERSIONINFO version_info = { sizeof(version_info) }; - if (!GetVersionEx(&version_info)) - Fatal("GetVersionEx: %s", GetLastErrorString().c_str()); - return version_info.dwMajorVersion > 6 || - (version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 1); + OSVERSIONINFOEX version_info = + { sizeof(OSVERSIONINFOEX), 6, 1, 0, 0, {0}, 0, 0, 0, 0, 0}; + DWORDLONG comparison = 0; + VER_SET_CONDITION(comparison, VER_MAJORVERSION, VER_GREATER_EQUAL); + VER_SET_CONDITION(comparison, VER_MINORVERSION, VER_GREATER_EQUAL); + return VerifyVersionInfo( + &version_info, VER_MAJORVERSION | VER_MINORVERSION, comparison); } -#ifdef _MSC_VER -#pragma warning(pop) -#endif bool StatAllFilesInDir(const string& dir, map* stamps, string* err) { -- cgit v0.12 From 78f893bdbb1e4bf3e3eac41a59d39854b8045150 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 6 Dec 2016 17:19:32 -0500 Subject: replace copyright placeholder, fixes #1212 --- COPYING | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COPYING b/COPYING index 131cb1d..3837304 100644 --- a/COPYING +++ b/COPYING @@ -187,7 +187,7 @@ same "printed page" as the copyright notice for easier identification within third-party archives. - Copyright [yyyy] [name of copyright owner] + Copyright 2016 Google Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. -- cgit v0.12 From 76abf78aac8c56606fb52ea874873d790b9044da Mon Sep 17 00:00:00 2001 From: "Pawel Hajdan, Jr" Date: Mon, 2 Jan 2017 10:42:35 +0000 Subject: Fix build with uclibc Resolves #985 This is based on musl implementation, http://git.musl-libc.org/cgit/musl/commit/?id=20cbd607759038dca57f84ef7e7b5d44a3088574 (thanks to jbergstroem@ for reference) --- src/util.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/util.cc b/src/util.cc index e31fd1f..1caa1ce 100644 --- a/src/util.cc +++ b/src/util.cc @@ -585,6 +585,13 @@ double GetLoadAverage() { // Calculation taken from comment in libperfstats.h return double(cpu_stats.loadavg[0]) / double(1 << SBITS); } +#elif defined(__UCLIBC__) +double GetLoadAverage() { + struct sysinfo si; + if (sysinfo(&si) != 0) + return -0.0f; + return 1.0 / (1 << SI_LOAD_SHIFT) * si.loads[0]; +} #else double GetLoadAverage() { double loadavg[3] = { 0.0f, 0.0f, 0.0f }; -- cgit v0.12 From 29ea8ea26c5fb82e78e31d2e2208e8b85857c447 Mon Sep 17 00:00:00 2001 From: Brendan McCarthy Date: Wed, 11 Jan 2017 15:52:20 -0500 Subject: fix broken link in hacking.md --- HACKING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HACKING.md b/HACKING.md index c9c6601..e7c91ef 100644 --- a/HACKING.md +++ b/HACKING.md @@ -105,7 +105,7 @@ Generally it's the [Google C++ coding style][], but in brief: * Lines are 80 columns maximum. * All source files should have the Google Inc. license header. -[Google C++ coding style]: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml +[Google C++ coding style]: https://google.github.io/styleguide/cppguide.html ## Documentation -- cgit v0.12 From dd2b587e289603127f30ea296e7751e4cf90fe7d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 12 Jan 2017 20:53:01 -0800 Subject: Close original pipe fd in subprocesses Non-console subprocesses have the write end of a pipe connected to fds 1 and 2 for stdout and stderr, but they also have the it connected to whatever fd was assigned in the ninja process when the pipe was created. Add a call to posix_spawn_file_actions_addclose after the posix_spawn_file_actions_adddup2 calls to close the original fd once it has been dup'd to stdout and stderr. This fixes an issue seen in the Android build, where one of the subprocesses is used to start a background helper process. The background process attempts to close any inherited fds, but if ninja used a very large fd number due to a high parallelism count the background process would not close the fd and ninja would never consider the subprocess finished. --- src/subprocess-posix.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc index f1f94e5..1de22c3 100644 --- a/src/subprocess-posix.cc +++ b/src/subprocess-posix.cc @@ -88,6 +88,8 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) { Fatal("posix_spawn_file_actions_adddup2: %s", strerror(errno)); if (posix_spawn_file_actions_adddup2(&action, output_pipe[1], 2) != 0) Fatal("posix_spawn_file_actions_adddup2: %s", strerror(errno)); + if (posix_spawn_file_actions_addclose(&action, output_pipe[1]) != 0) + Fatal("posix_spawn_file_actions_addclose: %s", strerror(errno)); // In the console case, output_pipe is still inherited by the child and // closed when the subprocess finishes, which then notifies ninja. } -- cgit v0.12 From 110dae2209a738ac59269b376dbb58d45df2b565 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 1 Feb 2017 16:47:18 -0800 Subject: Fix build in canon_perftest_fix unsigned int*, make the same change in canon_perftest. Fixes "./ninja all" build. --- src/canon_perftest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc index 389ac24..03f4a2f 100644 --- a/src/canon_perftest.cc +++ b/src/canon_perftest.cc @@ -33,7 +33,7 @@ int main() { for (int j = 0; j < 5; ++j) { const int kNumRepetitions = 2000000; int64_t start = GetTimeMillis(); - unsigned int slash_bits; + uint64_t slash_bits; for (int i = 0; i < kNumRepetitions; ++i) { CanonicalizePath(buf, &len, &slash_bits, &err); } -- cgit v0.12 From d611bd405acc9817a4e96f52a042a7a2f06f7812 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 1 Feb 2017 16:49:56 -0800 Subject: Make travis build everything Tell travis to build "all" instead of just "ninja_test". This would have caught the breakage introduced by #1181. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 216b8b0..093139b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,4 +3,4 @@ language: cpp compiler: - gcc - clang -script: ./configure.py --bootstrap && ./ninja ninja_test && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots && ./misc/ninja_syntax_test.py +script: ./configure.py --bootstrap && ./ninja all && ./ninja_test --gtest_filter=-SubprocessTest.SetWithLots && ./misc/ninja_syntax_test.py -- cgit v0.12 From d6ff22b506c85b31c81ff879fd0329a758642f71 Mon Sep 17 00:00:00 2001 From: Nicolas Despres Date: Thu, 2 Feb 2017 15:28:20 +1300 Subject: Fix compilation error in canon_perftest. Introduced by 1cc730ddc27df52. --- src/canon_perftest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc index 389ac24..03f4a2f 100644 --- a/src/canon_perftest.cc +++ b/src/canon_perftest.cc @@ -33,7 +33,7 @@ int main() { for (int j = 0; j < 5; ++j) { const int kNumRepetitions = 2000000; int64_t start = GetTimeMillis(); - unsigned int slash_bits; + uint64_t slash_bits; for (int i = 0; i < kNumRepetitions; ++i) { CanonicalizePath(buf, &len, &slash_bits, &err); } -- cgit v0.12 From d560c62cae668852d0b84be5e4e573bd1fe709c8 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Sat, 4 Feb 2017 18:38:20 -0500 Subject: Need this to build on vs2017 .\src\clparser.cc(56): note: see reference to function template instantiation '_OutIt std::transform>>,std::_String_iterator>>,int(__cdecl *)(int)>(_InIt,_InIt,_OutIt,_Fn1)' being compiled with [ _OutIt=std::_String_iterator>>, _InIt=std::_String_iterator>>, _Fn1=int (__cdecl *)(int) ] D:\bin\dev\VS\2017\BuildTools\VC\Tools\MSVC\14.10.24911\include\algorithm(946): warning C4244: '=': conversion from 'int' to 'char', possible loss of data --- configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.py b/configure.py index 9ec368f..38521a2 100755 --- a/configure.py +++ b/configure.py @@ -302,7 +302,7 @@ if platform.is_msvc(): '/Zi', # Create pdb with debug info. '/W4', # Highest warning level. '/WX', # Warnings as errors. - '/wd4530', '/wd4100', '/wd4706', + '/wd4530', '/wd4100', '/wd4706', '/wd4244', '/wd4512', '/wd4800', '/wd4702', '/wd4819', # Disable warnings about constant conditional expressions. '/wd4127', -- cgit v0.12 From abcd5f3d4d2ea3f58698fd3abcb7935ba8332f70 Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Thu, 10 Nov 2016 14:35:39 -0800 Subject: Support restat when rebuilding manifest As a fix for #874, we started reloading the entire manifest even if the manifest was never rebuilt due to a restat rule. But this can be slow, so call State::Reset instead, which also fixes the original crash. Fixes #987 --- src/ninja.cc | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/ninja.cc b/src/ninja.cc index 63ec3a8..54de7b9 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -247,10 +247,19 @@ bool NinjaMain::RebuildManifest(const char* input_file, string* err) { if (builder.AlreadyUpToDate()) return false; // Not an error, but we didn't rebuild. - // Even if the manifest was cleaned by a restat rule, claim that it was - // rebuilt. Not doing so can lead to crashes, see - // https://github.com/ninja-build/ninja/issues/874 - return builder.Build(err); + if (!builder.Build(err)) + return false; + + // The manifest was only rebuilt if it is now dirty (it may have been cleaned + // by a restat). + if (!node->dirty()) { + // Reset the state to prevent problems like + // https://github.com/ninja-build/ninja/issues/874 + state_.Reset(); + return false; + } + + return true; } Node* NinjaMain::CollectTarget(const char* cpath, string* err) { -- cgit v0.12 From 8a32d21b674c144ac948e037d4eac64352f09849 Mon Sep 17 00:00:00 2001 From: Tej Chajed Date: Thu, 9 Mar 2017 13:57:22 -0500 Subject: browse: Bind to localhost by default Previously the browse server would bind to "", which is translated to 0.0.0.0 (all interfaces), and then the hostname as retrieved by socket.gethostname() was presented to the user. The hostname is now "localhost" by default and is configurable, so the original behavior is achieved with `ninja -t browse -a ""`. --- src/browse.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/browse.py b/src/browse.py index 4b4faa8..64a16f2 100755 --- a/src/browse.py +++ b/src/browse.py @@ -193,6 +193,8 @@ class RequestHandler(httpserver.BaseHTTPRequestHandler): parser = argparse.ArgumentParser(prog='ninja -t browse') parser.add_argument('--port', '-p', default=8000, type=int, help='Port number to use (default %(default)d)') +parser.add_argument('--hostname', '-a', default='localhost', type=str, + help='Hostname to bind to (default %(default)s)') parser.add_argument('--no-browser', action='store_true', help='Do not open a webbrowser on startup.') @@ -205,9 +207,11 @@ parser.add_argument('initial_target', default='all', nargs='?', args = parser.parse_args() port = args.port -httpd = httpserver.HTTPServer(('',port), RequestHandler) +hostname = args.hostname +httpd = httpserver.HTTPServer((hostname,port), RequestHandler) try: - hostname = socket.gethostname() + if hostname == "": + hostname = socket.gethostname() print('Web server running on %s:%d, ctl-C to abort...' % (hostname,port) ) print('Web server pid %d' % os.getpid(), file=sys.stderr ) if not args.no_browser: -- cgit v0.12 From 292c89f486062fc55be1c5317945b58228f6ba96 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 12 Apr 2017 11:12:38 +0900 Subject: Add clparser_perftest --- configure.py | 26 ++++---- src/clparser_perftest.cc | 158 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 15 deletions(-) create mode 100644 src/clparser_perftest.cc diff --git a/configure.py b/configure.py index 38521a2..edf85e2 100755 --- a/configure.py +++ b/configure.py @@ -566,21 +566,17 @@ all_targets += ninja_test n.comment('Ancillary executables.') -objs = cxx('build_log_perftest') -all_targets += n.build(binary('build_log_perftest'), 'link', objs, - implicit=ninja_lib, variables=[('libs', libs)]) -objs = cxx('canon_perftest') -all_targets += n.build(binary('canon_perftest'), 'link', objs, - implicit=ninja_lib, variables=[('libs', libs)]) -objs = cxx('depfile_parser_perftest') -all_targets += n.build(binary('depfile_parser_perftest'), 'link', objs, - implicit=ninja_lib, variables=[('libs', libs)]) -objs = cxx('hash_collision_bench') -all_targets += n.build(binary('hash_collision_bench'), 'link', objs, - implicit=ninja_lib, variables=[('libs', libs)]) -objs = cxx('manifest_parser_perftest') -all_targets += n.build(binary('manifest_parser_perftest'), 'link', objs, - implicit=ninja_lib, variables=[('libs', libs)]) + +for name in ['build_log_perftest', + 'canon_perftest', + 'depfile_parser_perftest', + 'hash_collision_bench', + 'manifest_parser_perftest', + 'clparser_perftest']: + objs = cxx(name) + all_targets += n.build(binary(name), 'link', objs, + implicit=ninja_lib, variables=[('libs', libs)]) + n.newline() n.comment('Generate a graph using the "graph" tool.') diff --git a/src/clparser_perftest.cc b/src/clparser_perftest.cc new file mode 100644 index 0000000..3869ee5 --- /dev/null +++ b/src/clparser_perftest.cc @@ -0,0 +1,158 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include + +#include "clparser.h" +#include "metrics.h" +#include "util.h" + +int main(int argc, char* argv[]) { + // Output of /showIncludes from #include + string perf_testdata = + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\iostream\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\istream\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\ostream\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\ios\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xlocnum\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\climits\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\yvals.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xkeycheck.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\crtdefs.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\sal.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\ConcurrencySal.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vadefs.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\use_ansi.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\limits.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cmath\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\math.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xtgmath.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xtr1common\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cstdlib\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\stdlib.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_malloc.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_search.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\stddef.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wstdlib.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cstdio\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\stdio.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wstdio.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_stdio_config.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\streambuf\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xiosbase\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xlocale\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cstring\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\string.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_memory.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_memcpy_s.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\errno.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime_string.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wstring.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\stdexcept\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\exception\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\type_traits\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xstddef\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cstddef\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\initializer_list\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\malloc.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime_exception.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\eh.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_terminate.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xstring\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xmemory0\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cstdint\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\stdint.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\limits\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\ymath.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cfloat\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\float.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cwchar\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\wchar.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wconio.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wctype.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wdirect.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wio.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_share.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wprocess.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wtime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\sys/stat.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\sys/types.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\new\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime_new.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xutility\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\utility\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\iosfwd\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\crtdbg.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime_new_debug.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xatomic0.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\intrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\setjmp.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\immintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\wmmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\nmmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\smmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\tmmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\pmmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\emmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xmmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\mmintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\ammintrin.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\mm3dnow.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\typeinfo\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime_typeinfo.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\vcruntime.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xlocinfo\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xlocinfo.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\ctype.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\locale.h\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\xfacet\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\system_error\r\n" + "Note: including file: C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\INCLUDE\\cerrno\r\n" + "Note: including file: C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.10240.0\\ucrt\\share.h\r\n"; + + for (int limit = 1 << 10; limit < (1<<20); limit *= 2) { + int64_t start = GetTimeMillis(); + for (int rep = 0; rep < limit; ++rep) { + string output; + string err; + + CLParser parser; + if (!parser.Parse(perf_testdata, "", &output, &err)) { + printf("%s\n", err.c_str()); + return 1; + } + } + int64_t end = GetTimeMillis(); + + if (end - start > 100) { + int delta_ms = (int)(end - start); + printf("Parse %d times in %dms avg %.1fms\n", + limit, delta_ms, float(delta_ms) / limit); + break; + } + } + + return 0; +} -- cgit v0.12 From 4eac806a44a674ac2b8cefd45d3f4120a608c033 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 12 Apr 2017 13:33:45 +0900 Subject: remove util.h --- src/clparser_perftest.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/clparser_perftest.cc b/src/clparser_perftest.cc index 3869ee5..f106b64 100644 --- a/src/clparser_perftest.cc +++ b/src/clparser_perftest.cc @@ -17,7 +17,6 @@ #include "clparser.h" #include "metrics.h" -#include "util.h" int main(int argc, char* argv[]) { // Output of /showIncludes from #include -- cgit v0.12 From 12ca62107dc6a2b8e996e30fa7bd8c7b90e5ded2 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 12 Apr 2017 13:39:07 +0900 Subject: add gitignore --- .gitignore | 1 + n | Bin 0 -> 3458624 bytes 2 files changed, 1 insertion(+) create mode 100755 n diff --git a/.gitignore b/.gitignore index 5a85203..a86205b 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ TAGS /ninja.bootstrap /build_log_perftest /canon_perftest +/clparser_perftest /depfile_parser_perftest /hash_collision_bench /ninja_test diff --git a/n b/n new file mode 100755 index 0000000..306a868 Binary files /dev/null and b/n differ -- cgit v0.12 From 03d0a8befea74d5fb5f0735aee6acb722c2da36b Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 12 Apr 2017 13:42:15 +0900 Subject: use us --- src/clparser_perftest.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/clparser_perftest.cc b/src/clparser_perftest.cc index f106b64..101a4e2 100644 --- a/src/clparser_perftest.cc +++ b/src/clparser_perftest.cc @@ -147,8 +147,8 @@ int main(int argc, char* argv[]) { if (end - start > 100) { int delta_ms = (int)(end - start); - printf("Parse %d times in %dms avg %.1fms\n", - limit, delta_ms, float(delta_ms) / limit); + printf("Parse %d times in %dms avg %.1fus\n", + limit, delta_ms, float(delta_ms * 1000) / limit); break; } } -- cgit v0.12 From 5a03f4e34a210c30e8f13db90c86510a89a1fcd8 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 12 Apr 2017 13:44:02 +0900 Subject: delete n --- n | Bin 3458624 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100755 n diff --git a/n b/n deleted file mode 100755 index 306a868..0000000 Binary files a/n and /dev/null differ -- cgit v0.12 From 08a3220bc2fe12e7f05967b317d221e0bc620be9 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 26 Apr 2017 16:10:27 +0900 Subject: Add string_piece_util Following functions are implemented for further performance optimization. * JoinStringPiece * SplitStringPiece * EqualsCaseInsensitiveASCII * ToLowerASCII To improve performance of CLParser, I will introduce above functions into include_normalize-win32.cc. --- configure.py | 2 + src/string_piece.h | 10 ++++ src/string_piece_util.cc | 78 +++++++++++++++++++++++++ src/string_piece_util.h | 34 +++++++++++ src/string_piece_util_test.cc | 129 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 253 insertions(+) create mode 100644 src/string_piece_util.cc create mode 100644 src/string_piece_util.h create mode 100644 src/string_piece_util_test.cc diff --git a/configure.py b/configure.py index edf85e2..643106c 100755 --- a/configure.py +++ b/configure.py @@ -489,6 +489,7 @@ for name in ['build', 'manifest_parser', 'metrics', 'state', + 'string_piece_util', 'util', 'version']: objs += cxx(name) @@ -551,6 +552,7 @@ for name in ['build_log_test', 'manifest_parser_test', 'ninja_test', 'state_test', + 'string_piece_util_test', 'subprocess_test', 'test', 'util_test']: diff --git a/src/string_piece.h b/src/string_piece.h index b1bf105..353b24e 100644 --- a/src/string_piece.h +++ b/src/string_piece.h @@ -25,6 +25,8 @@ using namespace std; /// externally. It is useful for reducing the number of std::strings /// we need to allocate. struct StringPiece { + typedef const char* const_iterator; + StringPiece() : str_(NULL), len_(0) {} /// The constructors intentionally allow for implicit conversions. @@ -46,6 +48,14 @@ struct StringPiece { return len_ ? string(str_, len_) : string(); } + const_iterator begin() const { + return str_; + } + + const_iterator end() const { + return str_ + len_; + } + const char* str_; size_t len_; }; diff --git a/src/string_piece_util.cc b/src/string_piece_util.cc new file mode 100644 index 0000000..8e1ecfd --- /dev/null +++ b/src/string_piece_util.cc @@ -0,0 +1,78 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "string_piece_util.h" + +#include +#include +#include +using namespace std; + +vector SplitStringPiece(StringPiece input, char sep) { + vector elems; + elems.reserve(count(input.begin(), input.end(), sep) + 1); + + StringPiece::const_iterator pos = input.begin(); + + for (;;) { + const char* next_pos = find(pos, input.end(), sep); + if (next_pos == input.end()) { + elems.push_back(StringPiece(pos, input.end() - pos)); + break; + } + elems.push_back(StringPiece(pos, next_pos - pos)); + pos = next_pos + 1; + } + + return elems; +} + +string JoinStringPiece(const vector& list, char sep) { + if (list.size() == 0){ + return ""; + } + + string ret; + + { + size_t cap = list.size() - 1; + for (size_t i = 0; i < list.size(); ++i) { + cap += list[i].len_; + } + ret.reserve(cap); + } + + for (size_t i = 0; i < list.size(); ++i) { + if (i != 0) { + ret += sep; + } + ret.append(list[i].str_, list[i].len_); + } + + return ret; +} + +bool EqualsCaseInsensitiveASCII(StringPiece a, StringPiece b) { + if (a.len_ != b.len_) { + return false; + } + + for (size_t i = 0; i < a.len_; ++i) { + if (ToLowerASCII(a.str_[i]) != ToLowerASCII(b.str_[i])) { + return false; + } + } + + return true; +} diff --git a/src/string_piece_util.h b/src/string_piece_util.h new file mode 100644 index 0000000..2e40b9f --- /dev/null +++ b/src/string_piece_util.h @@ -0,0 +1,34 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef NINJA_STRINGPIECE_UTIL_H_ +#define NINJA_STRINGPIECE_UTIL_H_ + +#include +#include + +#include "string_piece.h" +using namespace std; + +vector SplitStringPiece(StringPiece input, char sep); + +string JoinStringPiece(const vector& list, char sep); + +inline char ToLowerASCII(char c) { + return (c >= 'A' && c <= 'Z') ? (c + ('a' - 'A')) : c; +} + +bool EqualsCaseInsensitiveASCII(StringPiece a, StringPiece b); + +#endif // NINJA_STRINGPIECE_UTIL_H_ diff --git a/src/string_piece_util_test.cc b/src/string_piece_util_test.cc new file mode 100644 index 0000000..648c647 --- /dev/null +++ b/src/string_piece_util_test.cc @@ -0,0 +1,129 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "string_piece_util.h" + +#include "test.h" + +TEST(StringPieceUtilTest, SplitStringPiece) { + { + string input("a:b:c"); + vector list = SplitStringPiece(input, ':'); + + EXPECT_EQ(list.size(), 3); + + EXPECT_EQ(list[0], "a"); + EXPECT_EQ(list[1], "b"); + EXPECT_EQ(list[2], "c"); + } + + { + string empty(""); + vector list = SplitStringPiece(empty, ':'); + + EXPECT_EQ(list.size(), 1); + + EXPECT_EQ(list[0], ""); + } + + { + string one("a"); + vector list = SplitStringPiece(one, ':'); + + EXPECT_EQ(list.size(), 1); + + EXPECT_EQ(list[0], "a"); + } + + { + string sep_only(":"); + vector list = SplitStringPiece(sep_only, ':'); + + EXPECT_EQ(list.size(), 2); + + EXPECT_EQ(list[0], ""); + EXPECT_EQ(list[1], ""); + } + + { + string sep(":a:b:c:"); + vector list = SplitStringPiece(sep, ':'); + + EXPECT_EQ(list.size(), 5); + + EXPECT_EQ(list[0], ""); + EXPECT_EQ(list[1], "a"); + EXPECT_EQ(list[2], "b"); + EXPECT_EQ(list[3], "c"); + EXPECT_EQ(list[4], ""); + } +} + +TEST(StringPieceUtilTest, JoinStringPiece) { + { + string input("a:b:c"); + vector list = SplitStringPiece(input, ':'); + + EXPECT_EQ("a:b:c", JoinStringPiece(list, ':')); + EXPECT_EQ("a/b/c", JoinStringPiece(list, '/')); + } + + { + string empty(""); + vector list = SplitStringPiece(empty, ':'); + + EXPECT_EQ("", JoinStringPiece(list, ':')); + } + + { + vector empty_list; + + EXPECT_EQ("", JoinStringPiece(empty_list, ':')); + } + + { + string one("a"); + vector single_list = SplitStringPiece(one, ':'); + + EXPECT_EQ("a", JoinStringPiece(single_list, ':')); + } + + { + string sep(":a:b:c:"); + vector list = SplitStringPiece(sep, ':'); + + EXPECT_EQ(":a:b:c:", JoinStringPiece(list, ':')); + } +} + +TEST(StringPieceUtilTest, ToLowerASCII) { + EXPECT_EQ('a', ToLowerASCII('A')); + EXPECT_EQ('z', ToLowerASCII('Z')); + EXPECT_EQ('a', ToLowerASCII('a')); + EXPECT_EQ('z', ToLowerASCII('z')); + EXPECT_EQ('/', ToLowerASCII('/')); + EXPECT_EQ('1', ToLowerASCII('1')); +} + +TEST(StringPieceUtilTest, EqualsCaseInsensitiveASCII) { + EXPECT_TRUE(EqualsCaseInsensitiveASCII("abc", "abc")); + EXPECT_TRUE(EqualsCaseInsensitiveASCII("abc", "ABC")); + EXPECT_TRUE(EqualsCaseInsensitiveASCII("abc", "aBc")); + EXPECT_TRUE(EqualsCaseInsensitiveASCII("AbC", "aBc")); + EXPECT_TRUE(EqualsCaseInsensitiveASCII("", "")); + + EXPECT_FALSE(EqualsCaseInsensitiveASCII("a", "ac")); + EXPECT_FALSE(EqualsCaseInsensitiveASCII("/", "\\")); + EXPECT_FALSE(EqualsCaseInsensitiveASCII("1", "10")); +} -- cgit v0.12 From 3b320023276f98b978054c14c65d3888b989ff4a Mon Sep 17 00:00:00 2001 From: tikuta Date: Wed, 12 Apr 2017 14:40:40 +0900 Subject: Make clparser faster This patch improves perfromance of clparser. * Reduce the number of calling GetFullPathName. * Use StringPiece for Split and Join. * Add EqualsCaseInsensitive for StringPiece not to generate new string instance. * Add some utility member in StringPiece class. --- src/clparser.cc | 11 +++- src/clparser_perftest.cc | 2 +- src/includes_normalize-win32.cc | 127 ++++++++++++++++++++++++++++------------ src/includes_normalize.h | 18 +++--- src/includes_normalize_test.cc | 43 +++----------- src/string_piece.h | 8 +++ 6 files changed, 130 insertions(+), 79 deletions(-) diff --git a/src/clparser.cc b/src/clparser.cc index c17150b..c993e36 100644 --- a/src/clparser.cc +++ b/src/clparser.cc @@ -18,8 +18,11 @@ #include #include +#include "metrics.h" + #ifdef _WIN32 #include "includes_normalize.h" +#include "string_piece.h" #else #include "util.h" #endif @@ -72,9 +75,15 @@ bool CLParser::FilterInputFilename(string line) { // static bool CLParser::Parse(const string& output, const string& deps_prefix, string* filtered_output, string* err) { + METRIC_RECORD("CLParser::Parse"); + // Loop over all lines in the output to process them. assert(&output != filtered_output); size_t start = 0; +#ifdef _WIN32 + IncludesNormalize normalizer("."); +#endif + while (start < output.size()) { size_t end = output.find_first_of("\r\n", start); if (end == string::npos) @@ -85,7 +94,7 @@ bool CLParser::Parse(const string& output, const string& deps_prefix, if (!include.empty()) { string normalized; #ifdef _WIN32 - if (!IncludesNormalize::Normalize(include, NULL, &normalized, err)) + if (!normalizer.Normalize(include, &normalized, err)) return false; #else // TODO: should this make the path relative to cwd? diff --git a/src/clparser_perftest.cc b/src/clparser_perftest.cc index 101a4e2..7ac5230 100644 --- a/src/clparser_perftest.cc +++ b/src/clparser_perftest.cc @@ -145,7 +145,7 @@ int main(int argc, char* argv[]) { } int64_t end = GetTimeMillis(); - if (end - start > 100) { + if (end - start > 2000) { int delta_ms = (int)(end - start); printf("Parse %d times in %dms avg %.1fus\n", limit, delta_ms, float(delta_ms * 1000) / limit); diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index e8a3e0f..b60ad70 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -15,6 +15,7 @@ #include "includes_normalize.h" #include "string_piece.h" +#include "string_piece_util.h" #include "util.h" #include @@ -25,8 +26,45 @@ namespace { -/// Return true if paths a and b are on the same Windows drive. +bool IsPathSeparator(char c) { + return c == '/' || c == '\\'; +} + +// Return true if paths a and b are on the same windows drive. +// Return false if this funcation cannot check +// whether or not on the same windows drive. +bool SameDriveFast(StringPiece a, StringPiece b) { + if (a.size() < 3 || b.size() < 3) { + return false; + } + + if (!isalpha(a[0]) || !isalpha(b[0])) { + return false; + } + + if (tolower(a[0]) != tolower(b[0])) { + return false; + } + + if (a[1] != ':' || b[1] != ':') { + return false; + } + + if (!IsPathSeparator(a[2]) || + !IsPathSeparator(b[2])) { + return false; + } + + return true; +} + +// Return true if paths a and b are on the same Windows drive. bool SameDrive(StringPiece a, StringPiece b) { + // Fast check. + if (SameDriveFast(a, b)) { + return true; + } + char a_absolute[_MAX_PATH]; char b_absolute[_MAX_PATH]; GetFullPathName(a.AsString().c_str(), sizeof(a_absolute), a_absolute, NULL); @@ -38,34 +76,54 @@ bool SameDrive(StringPiece a, StringPiece b) { return _stricmp(a_drive, b_drive) == 0; } -} // anonymous namespace +bool IsAbsPath(StringPiece s) { + if (s.size() < 3 || + !isalpha(s[0]) || + s[1] != ':' || + !IsPathSeparator(s[2])) { + return false; + } + + // Check "." or ".." is contained in path. + for (size_t i = 2; i < s.size(); ++i) { + if (!IsPathSeparator(s[i])) { + continue; + } -string IncludesNormalize::Join(const vector& list, char sep) { - string ret; - for (size_t i = 0; i < list.size(); ++i) { - ret += list[i]; - if (i != list.size() - 1) - ret += sep; + // Check ".". + if (i + 1 < s.size() && s[i+1] == '.' && + (i + 2 >= s.size() || IsPathSeparator(s[i+2]))) { + return false; + } + + // Check "..". + if (i + 2 < s.size() && s[i+1] == '.' && s[i+2] == '.' && + (i + 3 >= s.size() || IsPathSeparator(s[i+3]))) { + return false; + } } - return ret; -} -vector IncludesNormalize::Split(const string& input, char sep) { - vector elems; - stringstream ss(input); - string item; - while (getline(ss, item, sep)) - elems.push_back(item); - return elems; + return true; } -string IncludesNormalize::ToLower(const string& s) { - string ret; - transform(s.begin(), s.end(), back_inserter(ret), ::tolower); - return ret; +} // anonymous namespace + +IncludesNormalize::IncludesNormalize(const string& relative_to) { + relative_to_ = AbsPath(relative_to); + splitted_relative_to_ = SplitStringPiece(relative_to_, '/'); } string IncludesNormalize::AbsPath(StringPiece s) { + if (IsAbsPath(s)) { + string result = s.AsString(); + for (size_t i = 0; i < result.size(); ++i) { + if (result[i] == '\\') { + result[i] = '/'; + } + } + return result; + } + char result[_MAX_PATH]; GetFullPathName(s.AsString().c_str(), sizeof(result), result, NULL); for (char* c = result; *c; ++c) @@ -74,28 +132,30 @@ string IncludesNormalize::AbsPath(StringPiece s) { return result; } -string IncludesNormalize::Relativize(StringPiece path, const string& start) { - vector start_list = Split(AbsPath(start), '/'); - vector path_list = Split(AbsPath(path), '/'); +string IncludesNormalize::Relativize(StringPiece path, const vector& start_list) { + string abs_path = AbsPath(path); + vector path_list = SplitStringPiece(abs_path, '/'); int i; for (i = 0; i < static_cast(min(start_list.size(), path_list.size())); ++i) { - if (ToLower(start_list[i]) != ToLower(path_list[i])) + if (!EqualsCaseInsensitiveASCII(start_list[i], path_list[i])) { break; + } } - vector rel_list; + vector rel_list; + rel_list.reserve(start_list.size() - i + path_list.size() - i); for (int j = 0; j < static_cast(start_list.size() - i); ++j) rel_list.push_back(".."); for (int j = i; j < static_cast(path_list.size()); ++j) rel_list.push_back(path_list[j]); if (rel_list.size() == 0) return "."; - return Join(rel_list, '/'); + return JoinStringPiece(rel_list, '/'); } -bool IncludesNormalize::Normalize(const string& input, const char* relative_to, - string* result, string* err) { +bool IncludesNormalize::Normalize(const string& input, + string* result, string* err) const { char copy[_MAX_PATH + 1]; size_t len = input.size(); if (len > _MAX_PATH) { @@ -108,15 +168,10 @@ bool IncludesNormalize::Normalize(const string& input, const char* relative_to, return false; StringPiece partially_fixed(copy, len); - string curdir; - if (!relative_to) { - curdir = AbsPath("."); - relative_to = curdir.c_str(); - } - if (!SameDrive(partially_fixed, relative_to)) { + if (!SameDrive(partially_fixed, relative_to_)) { *result = partially_fixed.AsString(); return true; } - *result = Relativize(partially_fixed, relative_to); + *result = Relativize(partially_fixed, splitted_relative_to_); return true; } diff --git a/src/includes_normalize.h b/src/includes_normalize.h index 98e912f..2ec08ed 100644 --- a/src/includes_normalize.h +++ b/src/includes_normalize.h @@ -21,15 +21,19 @@ struct StringPiece; /// Utility functions for normalizing include paths on Windows. /// TODO: this likely duplicates functionality of CanonicalizePath; refactor. struct IncludesNormalize { + /// Normalize path relative to |relative_to|. + IncludesNormalize(const string& relative_to); + // Internal utilities made available for testing, maybe useful otherwise. - static string Join(const vector& list, char sep); - static vector Split(const string& input, char sep); - static string ToLower(const string& s); static string AbsPath(StringPiece s); - static string Relativize(StringPiece path, const string& start); + static string Relativize(StringPiece path, + const vector& start_list); /// Normalize by fixing slashes style, fixing redundant .. and . and makes the - /// path relative to |relative_to|. - static bool Normalize(const string& input, const char* relative_to, - string* result, string* err); + /// path relative to |relative_to_|. + bool Normalize(const string& input, string* result, string* err) const; + + private: + string relative_to_; + vector splitted_relative_to_; }; diff --git a/src/includes_normalize_test.cc b/src/includes_normalize_test.cc index f18795c..0bb14ec 100644 --- a/src/includes_normalize_test.cc +++ b/src/includes_normalize_test.cc @@ -18,6 +18,7 @@ #include +#include "string_piece_util.h" #include "test.h" #include "util.h" @@ -26,13 +27,14 @@ namespace { string GetCurDir() { char buf[_MAX_PATH]; _getcwd(buf, sizeof(buf)); - vector parts = IncludesNormalize::Split(string(buf), '\\'); - return parts[parts.size() - 1]; + vector parts = SplitStringPiece(buf, '\\'); + return parts[parts.size() - 1].AsString(); } string NormalizeAndCheckNoError(const string& input) { string result, err; - EXPECT_TRUE(IncludesNormalize::Normalize(input.c_str(), NULL, &result, &err)); + IncludesNormalize normalizer("."); + EXPECT_TRUE(normalizer.Normalize(input, &result, &err)); EXPECT_EQ("", err); return result; } @@ -40,8 +42,8 @@ string NormalizeAndCheckNoError(const string& input) { string NormalizeRelativeAndCheckNoError(const string& input, const string& relative_to) { string result, err; - EXPECT_TRUE(IncludesNormalize::Normalize(input.c_str(), relative_to.c_str(), - &result, &err)); + IncludesNormalize normalizer(relative_to); + EXPECT_TRUE(normalizer.Normalize(input, &result, &err)); EXPECT_EQ("", err); return result; } @@ -76,34 +78,6 @@ TEST(IncludesNormalize, Case) { EXPECT_EQ("A/B", NormalizeAndCheckNoError("A\\./B")); } -TEST(IncludesNormalize, Join) { - vector x; - EXPECT_EQ("", IncludesNormalize::Join(x, ':')); - x.push_back("alpha"); - EXPECT_EQ("alpha", IncludesNormalize::Join(x, ':')); - x.push_back("beta"); - x.push_back("gamma"); - EXPECT_EQ("alpha:beta:gamma", IncludesNormalize::Join(x, ':')); -} - -TEST(IncludesNormalize, Split) { - EXPECT_EQ("", IncludesNormalize::Join(IncludesNormalize::Split("", '/'), - ':')); - EXPECT_EQ("a", IncludesNormalize::Join(IncludesNormalize::Split("a", '/'), - ':')); - EXPECT_EQ("a:b:c", - IncludesNormalize::Join( - IncludesNormalize::Split("a/b/c", '/'), ':')); -} - -TEST(IncludesNormalize, ToLower) { - EXPECT_EQ("", IncludesNormalize::ToLower("")); - EXPECT_EQ("stuff", IncludesNormalize::ToLower("Stuff")); - EXPECT_EQ("stuff and things", IncludesNormalize::ToLower("Stuff AND thINGS")); - EXPECT_EQ("stuff 3and thin43gs", - IncludesNormalize::ToLower("Stuff 3AND thIN43GS")); -} - TEST(IncludesNormalize, DifferentDrive) { EXPECT_EQ("stuff.h", NormalizeRelativeAndCheckNoError("p:\\vs08\\stuff.h", "p:\\vs08")); @@ -129,8 +103,9 @@ TEST(IncludesNormalize, LongInvalidPath) { "instead of /Zi, but expect a similar error when you link your program."; // Too long, won't be canonicalized. Ensure doesn't crash. string result, err; + IncludesNormalize normalizer("."); EXPECT_FALSE( - IncludesNormalize::Normalize(kLongInputString, NULL, &result, &err)); + normalizer.Normalize(kLongInputString, &result, &err)); EXPECT_EQ("path too long", err); const char kExactlyMaxPath[] = diff --git a/src/string_piece.h b/src/string_piece.h index 353b24e..031bda4 100644 --- a/src/string_piece.h +++ b/src/string_piece.h @@ -56,6 +56,14 @@ struct StringPiece { return str_ + len_; } + char operator[](size_t pos) const { + return str_[pos]; + } + + size_t size() const { + return len_; + } + const char* str_; size_t len_; }; -- cgit v0.12 From 75b338506197921f14e3ce0eb9a04d2787ae1750 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Tue, 9 May 2017 13:34:31 +0900 Subject: Fix for review --- src/clparser.cc | 5 +++-- src/includes_normalize-win32.cc | 27 ++++++++++++--------------- src/includes_normalize.h | 4 ++-- src/util.cc | 2 +- src/util.h | 2 ++ 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/clparser.cc b/src/clparser.cc index c993e36..7994c06 100644 --- a/src/clparser.cc +++ b/src/clparser.cc @@ -19,6 +19,7 @@ #include #include "metrics.h" +#include "string_piece_util.h" #ifdef _WIN32 #include "includes_normalize.h" @@ -56,7 +57,7 @@ string CLParser::FilterShowIncludes(const string& line, // static bool CLParser::IsSystemInclude(string path) { - transform(path.begin(), path.end(), path.begin(), ::tolower); + transform(path.begin(), path.end(), path.begin(), ToLowerASCII); // TODO: this is a heuristic, perhaps there's a better way? return (path.find("program files") != string::npos || path.find("microsoft visual studio") != string::npos); @@ -64,7 +65,7 @@ bool CLParser::IsSystemInclude(string path) { // static bool CLParser::FilterInputFilename(string line) { - transform(line.begin(), line.end(), line.begin(), ::tolower); + transform(line.begin(), line.end(), line.begin(), ToLowerASCII); // TODO: other extensions, like .asm? return EndsWith(line, ".c") || EndsWith(line, ".cc") || diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index b60ad70..c72783f 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -38,11 +38,11 @@ bool SameDriveFast(StringPiece a, StringPiece b) { return false; } - if (!isalpha(a[0]) || !isalpha(b[0])) { + if (!islatinalpha(a[0]) || !islatinalpha(b[0])) { return false; } - if (tolower(a[0]) != tolower(b[0])) { + if (ToLowerASCII(a[0]) != ToLowerASCII(b[0])) { return false; } @@ -50,17 +50,11 @@ bool SameDriveFast(StringPiece a, StringPiece b) { return false; } - if (!IsPathSeparator(a[2]) || - !IsPathSeparator(b[2])) { - return false; - } - - return true; + return IsPathSeparator(a[2]) && IsPathSeparator(b[2]); } // Return true if paths a and b are on the same Windows drive. bool SameDrive(StringPiece a, StringPiece b) { - // Fast check. if (SameDriveFast(a, b)) { return true; } @@ -76,9 +70,11 @@ bool SameDrive(StringPiece a, StringPiece b) { return _stricmp(a_drive, b_drive) == 0; } -bool IsAbsPath(StringPiece s) { +// Check path |s| is FullPath style returned by GetFullPathName. +// This ignores difference of path separator. +bool IsFullPathName(StringPiece s) { if (s.size() < 3 || - !isalpha(s[0]) || + !islatinalpha(s[0]) || s[1] != ':' || !IsPathSeparator(s[2])) { return false; @@ -110,11 +106,11 @@ bool IsAbsPath(StringPiece s) { IncludesNormalize::IncludesNormalize(const string& relative_to) { relative_to_ = AbsPath(relative_to); - splitted_relative_to_ = SplitStringPiece(relative_to_, '/'); + split_relative_to_ = SplitStringPiece(relative_to_, '/'); } string IncludesNormalize::AbsPath(StringPiece s) { - if (IsAbsPath(s)) { + if (IsFullPathName(s)) { string result = s.AsString(); for (size_t i = 0; i < result.size(); ++i) { if (result[i] == '\\') { @@ -132,7 +128,8 @@ string IncludesNormalize::AbsPath(StringPiece s) { return result; } -string IncludesNormalize::Relativize(StringPiece path, const vector& start_list) { +string IncludesNormalize::Relativize( + StringPiece path, const vector& start_list) { string abs_path = AbsPath(path); vector path_list = SplitStringPiece(abs_path, '/'); int i; @@ -172,6 +169,6 @@ bool IncludesNormalize::Normalize(const string& input, *result = partially_fixed.AsString(); return true; } - *result = Relativize(partially_fixed, splitted_relative_to_); + *result = Relativize(partially_fixed, split_relative_to_); return true; } diff --git a/src/includes_normalize.h b/src/includes_normalize.h index 2ec08ed..3811e53 100644 --- a/src/includes_normalize.h +++ b/src/includes_normalize.h @@ -30,10 +30,10 @@ struct IncludesNormalize { const vector& start_list); /// Normalize by fixing slashes style, fixing redundant .. and . and makes the - /// path relative to |relative_to_|. + /// path |input| relative to |this->relative_to_| and store to |result|. bool Normalize(const string& input, string* result, string* err) const; private: string relative_to_; - vector splitted_relative_to_; + vector split_relative_to_; }; diff --git a/src/util.cc b/src/util.cc index ce4b192..84de879 100644 --- a/src/util.cc +++ b/src/util.cc @@ -471,7 +471,7 @@ void Win32Fatal(const char* function) { } #endif -static bool islatinalpha(int c) { +bool islatinalpha(int c) { // isalpha() is locale-dependent. return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'); } diff --git a/src/util.h b/src/util.h index 846cd93..4ee41a5 100644 --- a/src/util.h +++ b/src/util.h @@ -70,6 +70,8 @@ const char* SpellcheckStringV(const string& text, /// Like SpellcheckStringV, but takes a NULL-terminated list. const char* SpellcheckString(const char* text, ...); +bool islatinalpha(int c); + /// Removes all Ansi escape codes (http://www.termsys.demon.co.uk/vtansi.htm). string StripAnsiEscapeCodes(const string& in); -- cgit v0.12 From f0bb90e4bc49167fac5213269a34f628c349e50c Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Tue, 9 May 2017 15:33:59 +0900 Subject: Reduce GetFullPathName calls --- src/includes_normalize-win32.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index c72783f..459329b 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -72,6 +72,7 @@ bool SameDrive(StringPiece a, StringPiece b) { // Check path |s| is FullPath style returned by GetFullPathName. // This ignores difference of path separator. +// This is used not to call very slow GetFullPathName API. bool IsFullPathName(StringPiece s) { if (s.size() < 3 || !islatinalpha(s[0]) || @@ -164,11 +165,12 @@ bool IncludesNormalize::Normalize(const string& input, if (!CanonicalizePath(copy, &len, &slash_bits, err)) return false; StringPiece partially_fixed(copy, len); + string abs_input = AbsPath(partially_fixed); - if (!SameDrive(partially_fixed, relative_to_)) { + if (!SameDrive(abs_input, relative_to_)) { *result = partially_fixed.AsString(); return true; } - *result = Relativize(partially_fixed, split_relative_to_); + *result = Relativize(abs_input, split_relative_to_); return true; } -- cgit v0.12 From 2fc5b9e34bc38c7a8c4c33f39990f847325beffd Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Mon, 15 May 2017 18:41:22 -0400 Subject: Revert 78f893bdbb1e4bf3e3eac41a59d39854b8045150 These brackets are supposed to not be replaced here, but in the source files, see paragraph right above the changed line. Fixes #1212 again. --- COPYING | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COPYING b/COPYING index 3837304..131cb1d 100644 --- a/COPYING +++ b/COPYING @@ -187,7 +187,7 @@ same "printed page" as the copyright notice for easier identification within third-party archives. - Copyright 2016 Google Inc. + Copyright [yyyy] [name of copyright owner] Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. -- cgit v0.12 From ad545320a78d3c61f42dab14767d05139a871c0a Mon Sep 17 00:00:00 2001 From: Mathias Stearn Date: Thu, 18 May 2017 09:51:31 -0400 Subject: Make zsh completion use explicitly specified ninja files --- misc/zsh-completion | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/misc/zsh-completion b/misc/zsh-completion index 446e269..bf23fac 100644 --- a/misc/zsh-completion +++ b/misc/zsh-completion @@ -22,7 +22,12 @@ __get_targets() { then eval dir="${opt_args[-C]}" fi - targets_command="ninja -C \"${dir}\" -t targets all" + file="build.ninja" + if [ -n "${opt_args[-f]}" ]; + then + eval file="${opt_args[-f]}" + fi + targets_command="ninja -f \"${file}\" -C \"${dir}\" -t targets all" eval ${targets_command} 2>/dev/null | cut -d: -f1 } -- cgit v0.12 From 036003d20e80cbb044a3f0f0b41e2fdefcde66af Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Mon, 22 May 2017 11:27:14 -0700 Subject: Move stat metric to DiskInterface Stat is not always used through Node::Stat, it is often used directly through DiskInterface. THe next patches will cause it to be called even more often through DiskInterface, so move the metrics to DiskInterface. --- src/disk_interface.cc | 2 ++ src/graph.cc | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index 1b4135f..b418e04 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -28,6 +28,7 @@ #include // _mkdir #endif +#include "metrics.h" #include "util.h" namespace { @@ -148,6 +149,7 @@ bool DiskInterface::MakeDirs(const string& path) { // RealDiskInterface ----------------------------------------------------------- TimeStamp RealDiskInterface::Stat(const string& path, string* err) const { + METRIC_RECORD("node stat"); #ifdef _WIN32 // MSDN: "Naming Files, Paths, and Namespaces" // http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx diff --git a/src/graph.cc b/src/graph.cc index b4c8619..76d996b 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -28,7 +28,6 @@ #include "util.h" bool Node::Stat(DiskInterface* disk_interface, string* err) { - METRIC_RECORD("node stat"); return (mtime_ = disk_interface->Stat(path_, err)) != -1; } -- cgit v0.12 From a127dda3ee92916ef459b3da7aa9f2920ff1a5ab Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 18 May 2017 21:29:45 -0700 Subject: Add a test that fails if a rule updates it output file but fails https://groups.google.com/forum/#!msg/ninja-build/YQuGNrECI-4/ti-lAs9SPv8J discusses a case where an rule updates its output file and then fails. The next run of ninja considers the ouptut file clean and doesn't rebuild it. Add a test for this case, which currently fails. --- src/build.cc | 7 +++++++ src/build.h | 3 +++ src/build_test.cc | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/build.cc b/src/build.cc index a0c7ec8..44d0663 100644 --- a/src/build.cc +++ b/src/build.cc @@ -275,6 +275,13 @@ void BuildStatus::PrintStatus(Edge* edge, EdgeStatus status) { Plan::Plan() : command_edges_(0), wanted_edges_(0) {} +void Plan::Reset() { + command_edges_ = 0; + wanted_edges_ = 0; + ready_.clear(); + want_.clear(); +} + bool Plan::AddTarget(Node* node, string* err) { vector stack; return AddSubTarget(node, &stack, err); diff --git a/src/build.h b/src/build.h index 66ce607..f97d67e 100644 --- a/src/build.h +++ b/src/build.h @@ -71,6 +71,9 @@ struct Plan { /// Number of edges with commands to run. int command_edge_count() const { return command_edges_; } + /// Reset state. Clears want and ready sets. + void Reset(); + private: bool AddSubTarget(Node* node, vector* stack, string* err); bool CheckDependencyCycle(Node* node, const vector& stack, diff --git a/src/build_test.cc b/src/build_test.cc index 640e1b0..d617143 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -608,7 +608,8 @@ bool FakeCommandRunner::StartCommand(Edge* edge) { edge->rule().name() == "cat_rsp_out" || edge->rule().name() == "cc" || edge->rule().name() == "touch" || - edge->rule().name() == "touch-interrupt") { + edge->rule().name() == "touch-interrupt" || + edge->rule().name() == "touch-fail-tick2") { for (vector::iterator out = edge->outputs_.begin(); out != edge->outputs_.end(); ++out) { fs_->Create((*out)->path(), ""); @@ -649,7 +650,8 @@ bool FakeCommandRunner::WaitForCommand(Result* result) { return true; } - if (edge->rule().name() == "fail") + if (edge->rule().name() == "fail" || + (edge->rule().name() == "touch-fail-tick2" && fs_->now_ == 2)) result->status = ExitFailure; else result->status = ExitSuccess; @@ -1258,6 +1260,51 @@ TEST_F(BuildWithLogTest, NotInLogButOnDisk) { EXPECT_TRUE(builder_.AlreadyUpToDate()); } +TEST_F(BuildWithLogTest, RebuildAfterFailure) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule touch-fail-tick2\n" +" command = touch-fail-tick2\n" +"build out1: touch-fail-tick2 in\n")); + + string err; + + fs_.Create("in", ""); + + // Run once successfully to get out1 in the log + EXPECT_TRUE(builder_.AddTarget("out1", &err)); + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ("", err); + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); + + command_runner_.commands_ran_.clear(); + state_.Reset(); + builder_.Cleanup(); + builder_.plan_.Reset(); + + fs_.Tick(); + fs_.Create("in", ""); + + // Run again with a failure that updates the output file timestamp + EXPECT_TRUE(builder_.AddTarget("out1", &err)); + EXPECT_FALSE(builder_.Build(&err)); + EXPECT_EQ("subcommand failed", err); + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); + + command_runner_.commands_ran_.clear(); + state_.Reset(); + builder_.Cleanup(); + builder_.plan_.Reset(); + + fs_.Tick(); + + // Run again, should rerun even though the output file is up to date on disk + EXPECT_TRUE(builder_.AddTarget("out1", &err)); + EXPECT_FALSE(builder_.AlreadyUpToDate()); + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); + EXPECT_EQ("", err); +} + TEST_F(BuildWithLogTest, RestatTest) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule true\n" -- cgit v0.12 From 04d886b11041bb59d01df794cce7a1e8cad2250d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 18 May 2017 17:34:51 -0700 Subject: Always rebuild on errors https://groups.google.com/forum/#!msg/ninja-build/YQuGNrECI-4/ti-lAs9SPv8J discusses a case where an rule updates its output file and then fails. The next run of ninja considers the ouptut file clean and doesn't rebuild it. Always stat output files after they are built, and write the mtime into .ninja_log. Consider output files dirty if the recorded mtime is older than the most recent input file. --- src/build.cc | 16 +++++++++++----- src/build_log.cc | 10 +++++----- src/build_log.h | 6 +++--- src/build_log_perftest.cc | 2 +- src/build_log_test.cc | 8 ++++---- src/build_test.cc | 4 ++-- src/graph.cc | 26 +++++++++++++++++++------- 7 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/build.cc b/src/build.cc index 44d0663..1cce981 100644 --- a/src/build.cc +++ b/src/build.cc @@ -800,9 +800,10 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { return true; } - // Restat the edge outputs, if necessary. - TimeStamp restat_mtime = 0; - if (edge->GetBindingBool("restat") && !config_.dry_run) { + // Restat the edge outputs + TimeStamp output_mtime = 0; + bool restat = edge->GetBindingBool("restat"); + if (!config_.dry_run) { bool node_cleaned = false; for (vector::iterator o = edge->outputs_.begin(); @@ -810,7 +811,9 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { TimeStamp new_mtime = disk_interface_->Stat((*o)->path(), err); if (new_mtime == -1) return false; - if ((*o)->mtime() == new_mtime) { + 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). @@ -821,6 +824,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { } 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(); @@ -844,6 +848,8 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { // 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; } } @@ -856,7 +862,7 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) { if (scan_.build_log()) { if (!scan_.build_log()->RecordCommand(edge, start_time, end_time, - restat_mtime)) { + output_mtime)) { *err = string("Error writing to build log: ") + strerror(errno); return false; } diff --git a/src/build_log.cc b/src/build_log.cc index 8a52514..333915a 100644 --- a/src/build_log.cc +++ b/src/build_log.cc @@ -105,7 +105,7 @@ BuildLog::LogEntry::LogEntry(const string& output) BuildLog::LogEntry::LogEntry(const string& output, uint64_t command_hash, int start_time, int end_time, TimeStamp restat_mtime) : output(output), command_hash(command_hash), - start_time(start_time), end_time(end_time), restat_mtime(restat_mtime) + start_time(start_time), end_time(end_time), mtime(restat_mtime) {} BuildLog::BuildLog() @@ -145,7 +145,7 @@ bool BuildLog::OpenForWrite(const string& path, const BuildLogUser& user, } bool BuildLog::RecordCommand(Edge* edge, int start_time, int end_time, - TimeStamp restat_mtime) { + TimeStamp mtime) { string command = edge->EvaluateCommand(true); uint64_t command_hash = LogEntry::HashCommand(command); for (vector::iterator out = edge->outputs_.begin(); @@ -162,7 +162,7 @@ bool BuildLog::RecordCommand(Edge* edge, int start_time, int end_time, log_entry->command_hash = command_hash; log_entry->start_time = start_time; log_entry->end_time = end_time; - log_entry->restat_mtime = restat_mtime; + log_entry->mtime = mtime; if (log_file_) { if (!WriteEntry(log_file_, *log_entry)) @@ -314,7 +314,7 @@ bool BuildLog::Load(const string& path, string* err) { entry->start_time = start_time; entry->end_time = end_time; - entry->restat_mtime = restat_mtime; + entry->mtime = restat_mtime; if (log_version >= 5) { char c = *end; *end = '\0'; entry->command_hash = (uint64_t)strtoull(start, NULL, 16); @@ -354,7 +354,7 @@ BuildLog::LogEntry* BuildLog::LookupByOutput(const string& path) { 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.start_time, entry.end_time, entry.mtime, entry.output.c_str(), entry.command_hash) > 0; } diff --git a/src/build_log.h b/src/build_log.h index 785961e..5268fab 100644 --- a/src/build_log.h +++ b/src/build_log.h @@ -45,7 +45,7 @@ struct BuildLog { bool OpenForWrite(const string& path, const BuildLogUser& user, string* err); bool RecordCommand(Edge* edge, int start_time, int end_time, - TimeStamp restat_mtime = 0); + TimeStamp mtime = 0); void Close(); /// Load the on-disk log. @@ -56,7 +56,7 @@ struct BuildLog { uint64_t command_hash; int start_time; int end_time; - TimeStamp restat_mtime; + TimeStamp mtime; static uint64_t HashCommand(StringPiece command); @@ -64,7 +64,7 @@ struct BuildLog { bool operator==(const LogEntry& o) { return output == o.output && command_hash == o.command_hash && start_time == o.start_time && end_time == o.end_time && - restat_mtime == o.restat_mtime; + mtime == o.mtime; } explicit LogEntry(const string& output); diff --git a/src/build_log_perftest.cc b/src/build_log_perftest.cc index 185c512..b4efb1d 100644 --- a/src/build_log_perftest.cc +++ b/src/build_log_perftest.cc @@ -92,7 +92,7 @@ bool WriteTestData(string* err) { log.RecordCommand(state.edges_[i], /*start_time=*/100 * i, /*end_time=*/100 * i + 1, - /*restat_mtime=*/0); + /*mtime=*/0); } return true; diff --git a/src/build_log_test.cc b/src/build_log_test.cc index f4c9044..ad30380 100644 --- a/src/build_log_test.cc +++ b/src/build_log_test.cc @@ -181,7 +181,7 @@ TEST_F(BuildLogTest, SpacesInOutputV4) { ASSERT_TRUE(e); ASSERT_EQ(123, e->start_time); ASSERT_EQ(456, e->end_time); - ASSERT_EQ(456, e->restat_mtime); + ASSERT_EQ(456, e->mtime); ASSERT_NO_FATAL_FAILURE(AssertHash("command", e->command_hash)); } @@ -205,14 +205,14 @@ TEST_F(BuildLogTest, DuplicateVersionHeader) { ASSERT_TRUE(e); ASSERT_EQ(123, e->start_time); ASSERT_EQ(456, e->end_time); - ASSERT_EQ(456, e->restat_mtime); + ASSERT_EQ(456, e->mtime); ASSERT_NO_FATAL_FAILURE(AssertHash("command", e->command_hash)); e = log.LookupByOutput("out2"); ASSERT_TRUE(e); ASSERT_EQ(456, e->start_time); ASSERT_EQ(789, e->end_time); - ASSERT_EQ(789, e->restat_mtime); + ASSERT_EQ(789, e->mtime); ASSERT_NO_FATAL_FAILURE(AssertHash("command2", e->command_hash)); } @@ -240,7 +240,7 @@ TEST_F(BuildLogTest, VeryLongInputLine) { ASSERT_TRUE(e); ASSERT_EQ(456, e->start_time); ASSERT_EQ(789, e->end_time); - ASSERT_EQ(789, e->restat_mtime); + ASSERT_EQ(789, e->mtime); ASSERT_NO_FATAL_FAILURE(AssertHash("command2", e->command_hash)); } diff --git a/src/build_test.cc b/src/build_test.cc index d617143..0eb9aaa 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1485,7 +1485,7 @@ TEST_F(BuildWithLogTest, RestatMissingInput) { // the right mtime BuildLog::LogEntry* log_entry = build_log_.LookupByOutput("out1"); ASSERT_TRUE(NULL != log_entry); - ASSERT_EQ(restat_mtime, log_entry->restat_mtime); + ASSERT_EQ(restat_mtime, log_entry->mtime); // Now remove a file, referenced from depfile, so that target becomes // dirty, but the output does not change @@ -1502,7 +1502,7 @@ TEST_F(BuildWithLogTest, RestatMissingInput) { // Check that the logfile entry remains correctly set log_entry = build_log_.LookupByOutput("out1"); ASSERT_TRUE(NULL != log_entry); - ASSERT_EQ(restat_mtime, log_entry->restat_mtime); + ASSERT_EQ(restat_mtime, log_entry->mtime); } struct BuildDryRun : public BuildWithLogTest { diff --git a/src/graph.cc b/src/graph.cc index 76d996b..27013d5 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -168,7 +168,7 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge, bool used_restat = false; if (edge->GetBindingBool("restat") && build_log() && (entry = build_log()->LookupByOutput(output->path()))) { - output_mtime = entry->restat_mtime; + output_mtime = entry->mtime; used_restat = true; } @@ -182,17 +182,29 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge, } } - // 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. - if (!edge->GetBindingBool("generator") && build_log()) { + if (build_log()) { + bool generator = edge->GetBindingBool("generator"); if (entry || (entry = build_log()->LookupByOutput(output->path()))) { - if (BuildLog::LogEntry::HashCommand(command) != entry->command_hash) { + if (!generator && + BuildLog::LogEntry::HashCommand(command) != entry->command_hash) { + // 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. EXPLAIN("command line changed for %s", output->path().c_str()); return true; } + if (entry->mtime < most_recent_input->mtime()) { + // 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. + EXPLAIN("recorded mtime of %s older than most recent input %s (%d vs %d)", + output->path().c_str(), most_recent_input->path().c_str(), + entry->mtime, most_recent_input->mtime()); + return true; + } } - if (!entry) { + if (!entry && !generator) { EXPLAIN("command line not found in log for %s", output->path().c_str()); return true; } -- cgit v0.12 From d806aa5bc42e4de2fa5e45525c19fb3429de976e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imre=20Vad=C3=A1sz?= Date: Sat, 10 Jun 2017 18:53:43 +0200 Subject: Add support for DragonFly. DragonFly uses a fork of FreeBSD ports, and also uses the /usr/local prefix. And ppoll is also available in DragonFly. --- configure.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/configure.py b/configure.py index 643106c..a443748 100755 --- a/configure.py +++ b/configure.py @@ -60,11 +60,14 @@ class Platform(object): self._platform = 'netbsd' elif self._platform.startswith('aix'): self._platform = 'aix' + elif self._platform.startswith('dragonfly'): + self._platform = 'dragonfly' @staticmethod def known_platforms(): return ['linux', 'darwin', 'freebsd', 'openbsd', 'solaris', 'sunos5', - 'mingw', 'msvc', 'gnukfreebsd', 'bitrig', 'netbsd', 'aix'] + 'mingw', 'msvc', 'gnukfreebsd', 'bitrig', 'netbsd', 'aix', + 'dragonfly'] def platform(self): return self._platform @@ -95,10 +98,11 @@ class Platform(object): return self._platform == 'aix' def uses_usr_local(self): - return self._platform in ('freebsd', 'openbsd', 'bitrig') + return self._platform in ('freebsd', 'openbsd', 'bitrig', 'dragonfly') def supports_ppoll(self): - return self._platform in ('freebsd', 'linux', 'openbsd', 'bitrig') + return self._platform in ('freebsd', 'linux', 'openbsd', 'bitrig', + 'dragonfly') def supports_ninja_browse(self): return (not self.is_windows() -- cgit v0.12 From 11a934d427502f917dbbf47b088d9abde186c0a7 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 16 Jun 2017 11:03:51 -0700 Subject: Fix segfault on edge with no inputs PR #1281 added a deference of most_recent_input without checking for NULL, which can occur if a build rule has no inputs. Check it for null before dereferencing, and add a test. Fixes #1290. --- src/build_test.cc | 31 +++++++++++++++++++++++++++++++ src/graph.cc | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/build_test.cc b/src/build_test.cc index 0eb9aaa..623ed14 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1305,6 +1305,37 @@ TEST_F(BuildWithLogTest, RebuildAfterFailure) { EXPECT_EQ("", err); } +TEST_F(BuildWithLogTest, RebuildWithNoInputs) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule touch\n" +" command = touch\n" +"build out1: touch\n" +"build out2: touch in\n")); + + string err; + + fs_.Create("in", ""); + + EXPECT_TRUE(builder_.AddTarget("out1", &err)); + EXPECT_TRUE(builder_.AddTarget("out2", &err)); + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ("", err); + EXPECT_EQ(2u, command_runner_.commands_ran_.size()); + + command_runner_.commands_ran_.clear(); + state_.Reset(); + + fs_.Tick(); + + fs_.Create("in", ""); + + EXPECT_TRUE(builder_.AddTarget("out1", &err)); + EXPECT_TRUE(builder_.AddTarget("out2", &err)); + EXPECT_TRUE(builder_.Build(&err)); + EXPECT_EQ("", err); + EXPECT_EQ(1u, command_runner_.commands_ran_.size()); +} + TEST_F(BuildWithLogTest, RestatTest) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "rule true\n" diff --git a/src/graph.cc b/src/graph.cc index 27013d5..c90aaad 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -193,7 +193,7 @@ bool DependencyScan::RecomputeOutputDirty(Edge* edge, EXPLAIN("command line changed for %s", output->path().c_str()); return true; } - if (entry->mtime < most_recent_input->mtime()) { + if (most_recent_input && entry->mtime < most_recent_input->mtime()) { // 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 -- cgit v0.12 From b34f744ac3c8276a854b134d41c28d84664c7e7f Mon Sep 17 00:00:00 2001 From: Patrick Griffis Date: Mon, 20 Mar 2017 21:46:42 -0400 Subject: Work around mtime being set to 0 sometimes --- src/disk_interface.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/disk_interface.cc b/src/disk_interface.cc index b418e04..28530b1 100644 --- a/src/disk_interface.cc +++ b/src/disk_interface.cc @@ -187,6 +187,11 @@ TimeStamp RealDiskInterface::Stat(const string& path, string* err) const { *err = "stat(" + path + "): " + strerror(errno); return -1; } + // Some users (Flatpak) set mtime to 0, this should be harmless + // and avoids conflicting with our return value of 0 meaning + // that it doesn't exist. + if (st.st_mtime == 0) + return 1; return st.st_mtime; #endif } -- cgit v0.12 From 29a6e2fc6c671b9490193d4b235b53fb61886c80 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 10 Nov 2015 15:09:47 -0500 Subject: Simplify BuildTest.StatFailureAbortsBuild test case Remove a dependency cycle from the test case, as cycles are covered by other tests. Ensure this case covers stat failure on a valid graph. --- src/build_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/build_test.cc b/src/build_test.cc index 623ed14..8c9fb11 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -1747,8 +1747,8 @@ TEST_F(BuildTest, InterruptCleanup) { TEST_F(BuildTest, StatFailureAbortsBuild) { const string kTooLongToStat(400, 'i'); ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -("build " + kTooLongToStat + ": cat " + kTooLongToStat + "\n").c_str())); - // Also cyclic, for good measure. +("build " + kTooLongToStat + ": cat in\n").c_str())); + fs_.Create("in", ""); // This simulates a stat failure: fs_.files_[kTooLongToStat].mtime = -1; -- cgit v0.12 From afe3beb980a4780caecc12d3fc919feb3f9cce42 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 13 Nov 2015 13:05:51 -0500 Subject: Refactor RecomputeDirty to take a node instead of an edge All call sites have a node on which they call `in_edge()` to call RecomputeDirty. Simplify call sites by taking the node directly and calling `in_edge()` internally. --- src/build.cc | 5 +++-- src/disk_interface_test.cc | 8 ++++---- src/graph.cc | 22 +++++++++++---------- src/graph.h | 3 ++- src/graph_test.cc | 48 +++++++++++++++++----------------------------- 5 files changed, 39 insertions(+), 47 deletions(-) diff --git a/src/build.cc b/src/build.cc index 8f4169b..c2a615a 100644 --- a/src/build.cc +++ b/src/build.cc @@ -640,9 +640,10 @@ Node* Builder::AddTarget(const string& name, string* err) { } bool Builder::AddTarget(Node* node, string* err) { + if (!scan_.RecomputeDirty(node, err)) + return false; + if (Edge* in_edge = node->in_edge()) { - if (!scan_.RecomputeDirty(in_edge, err)) - return false; if (in_edge->outputs_ready()) return true; // Nothing to do. } diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc index 7187bdf..d7fb8f8 100644 --- a/src/disk_interface_test.cc +++ b/src/disk_interface_test.cc @@ -245,7 +245,7 @@ TEST_F(StatTest, Simple) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_EQ(2u, stats_.size()); ASSERT_EQ("out", stats_[0]); ASSERT_EQ("in", stats_[1]); @@ -261,7 +261,7 @@ TEST_F(StatTest, TwoStep) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_EQ(3u, stats_.size()); ASSERT_EQ("out", stats_[0]); ASSERT_TRUE(GetNode("out")->dirty()); @@ -281,7 +281,7 @@ TEST_F(StatTest, Tree) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_EQ(1u + 6u, stats_.size()); ASSERT_EQ("mid1", stats_[1]); ASSERT_TRUE(GetNode("mid1")->dirty()); @@ -302,7 +302,7 @@ TEST_F(StatTest, Middle) { EXPECT_TRUE(out->Stat(this, &err)); EXPECT_EQ("", err); ASSERT_EQ(1u, stats_.size()); - scan_.RecomputeDirty(out->in_edge(), NULL); + scan_.RecomputeDirty(out, NULL); ASSERT_FALSE(GetNode("in")->dirty()); ASSERT_TRUE(GetNode("mid")->dirty()); ASSERT_TRUE(GetNode("out")->dirty()); diff --git a/src/graph.cc b/src/graph.cc index c90aaad..c34fab8 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -31,7 +31,16 @@ bool Node::Stat(DiskInterface* disk_interface, string* err) { return (mtime_ = disk_interface->Stat(path_, err)) != -1; } -bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { +bool DependencyScan::RecomputeDirty(Node* node, string* err) { + Edge* edge = node->in_edge(); + if (!edge) { + // This node has no in-edge; it is dirty if it is missing. + if (!node->exists()) + EXPLAIN("%s has no in-edge and is missing", node->path().c_str()); + node->set_dirty(!node->exists()); + return true; + } + bool dirty = false; edge->outputs_ready_ = true; edge->deps_missing_ = false; @@ -66,15 +75,8 @@ bool DependencyScan::RecomputeDirty(Edge* edge, string* err) { if (!(*i)->status_known()) { if (!(*i)->StatIfNecessary(disk_interface_, err)) return false; - if (Edge* in_edge = (*i)->in_edge()) { - if (!RecomputeDirty(in_edge, err)) - return false; - } else { - // This input has no in-edge; it is dirty if it is missing. - if (!(*i)->exists()) - EXPLAIN("%s has no in-edge and is missing", (*i)->path().c_str()); - (*i)->set_dirty(!(*i)->exists()); - } + if (!RecomputeDirty(*i, err)) + return false; } // If an input is not ready, neither are our outputs. diff --git a/src/graph.h b/src/graph.h index ec24293..9e82ca2 100644 --- a/src/graph.h +++ b/src/graph.h @@ -246,11 +246,12 @@ struct DependencyScan { disk_interface_(disk_interface), dep_loader_(state, deps_log, disk_interface) {} + /// Update the |dirty_| state of the given node by inspecting its input edge. /// Examine inputs, outputs, and command lines to judge whether an edge /// needs to be re-run, and update outputs_ready_ and each outputs' |dirty_| /// state accordingly. /// Returns false on failure. - bool RecomputeDirty(Edge* edge, string* err); + bool RecomputeDirty(Node* node, string* err); /// Recompute whether any output of the edge is dirty, if so sets |*dirty|. /// Returns false on failure. diff --git a/src/graph_test.cc b/src/graph_test.cc index be08b19..87f3430 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -30,9 +30,8 @@ TEST_F(GraphTest, MissingImplicit) { fs_.Create("in", ""); fs_.Create("out", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); // A missing implicit dep *should* make the output dirty. @@ -49,9 +48,8 @@ TEST_F(GraphTest, ModifiedImplicit) { fs_.Tick(); fs_.Create("implicit", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); // A modified implicit dep should make the output dirty. @@ -70,9 +68,8 @@ TEST_F(GraphTest, FunkyMakefilePath) { fs_.Tick(); fs_.Create("implicit.h", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); // implicit.h has changed, though our depfile refers to it with a @@ -94,9 +91,8 @@ TEST_F(GraphTest, ExplicitImplicit) { fs_.Tick(); fs_.Create("data", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); // We have both an implicit and an explicit dep on implicit.h. @@ -123,9 +119,8 @@ TEST_F(GraphTest, ImplicitOutputMissing) { fs_.Create("in", ""); fs_.Create("out", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out")->dirty()); @@ -140,9 +135,8 @@ TEST_F(GraphTest, ImplicitOutputOutOfDate) { fs_.Create("in", ""); fs_.Create("out", ""); - Edge* edge = GetNode("out")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out")->dirty()); @@ -165,9 +159,8 @@ TEST_F(GraphTest, ImplicitOutputOnlyMissing) { "build | out.imp: cat in\n")); fs_.Create("in", ""); - Edge* edge = GetNode("out.imp")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.imp"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out.imp")->dirty()); @@ -180,9 +173,8 @@ TEST_F(GraphTest, ImplicitOutputOnlyOutOfDate) { fs_.Tick(); fs_.Create("in", ""); - Edge* edge = GetNode("out.imp")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.imp"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out.imp")->dirty()); @@ -198,9 +190,8 @@ TEST_F(GraphTest, PathWithCurrentDirectory) { fs_.Create("out.o.d", "out.o: foo.cc\n"); fs_.Create("out.o", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_FALSE(GetNode("out.o")->dirty()); @@ -247,9 +238,8 @@ TEST_F(GraphTest, DepfileWithCanonicalizablePath) { fs_.Create("out.o.d", "out.o: bar/../foo.cc\n"); fs_.Create("out.o", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_FALSE(GetNode("out.o")->dirty()); @@ -268,15 +258,14 @@ TEST_F(GraphTest, DepfileRemoved) { fs_.Create("out.o.d", "out.o: foo.h\n"); fs_.Create("out.o", ""); - Edge* edge = GetNode("out.o")->in_edge(); string err; - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_FALSE(GetNode("out.o")->dirty()); state_.Reset(); fs_.RemoveFile("out.o.d"); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out.o"), &err)); ASSERT_EQ("", err); EXPECT_TRUE(GetNode("out.o")->dirty()); } @@ -323,8 +312,7 @@ TEST_F(GraphTest, NestedPhonyPrintsDone) { "build n2: phony n1\n" ); string err; - Edge* edge = GetNode("n2")->in_edge(); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("n2"), &err)); ASSERT_EQ("", err); Plan plan_; @@ -347,13 +335,13 @@ TEST_F(GraphTest, CycleWithLengthZeroFromDepfile) { fs_.Create("dep.d", "a: b\n"); string err; - Edge* edge = GetNode("a")->in_edge(); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("a"), &err)); ASSERT_EQ("", err); // Despite the depfile causing edge to be a cycle (it has outputs a and b, // but the depfile also adds b as an input), the deps should have been loaded // only once: + Edge* edge = GetNode("a")->in_edge(); EXPECT_EQ(1, edge->inputs_.size()); EXPECT_EQ("b", edge->inputs_[0]->path()); } @@ -372,13 +360,13 @@ TEST_F(GraphTest, CycleWithLengthOneFromDepfile) { fs_.Create("dep.d", "a: c\n"); string err; - Edge* edge = GetNode("a")->in_edge(); - EXPECT_TRUE(scan_.RecomputeDirty(edge, &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("a"), &err)); ASSERT_EQ("", err); // Despite the depfile causing edge to be a cycle (|edge| has outputs a and b, // but c's in_edge has b as input but the depfile also adds |edge| as // output)), the deps should have been loaded only once: + Edge* edge = GetNode("a")->in_edge(); EXPECT_EQ(1, edge->inputs_.size()); EXPECT_EQ("c", edge->inputs_[0]->path()); } @@ -399,7 +387,7 @@ TEST_F(GraphTest, CycleWithLengthOneFromDepfileOneHopAway) { fs_.Create("dep.d", "a: c\n"); string err; - EXPECT_TRUE(scan_.RecomputeDirty(GetNode("d")->in_edge(), &err)); + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("d"), &err)); ASSERT_EQ("", err); // Despite the depfile causing edge to be a cycle (|edge| has outputs a and b, -- cgit v0.12 From a8b5cdc4e9034f8823ee0cfa94ea49d7795698ab Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 12 Nov 2015 11:24:50 -0500 Subject: Add infrastructure for efficient walks through the `Edge` graph Store a mark in each `Edge` to be updated as it is encountered by a walk. --- src/graph.h | 9 ++++++++- src/state.cc | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/graph.h b/src/graph.h index 9e82ca2..1b30dee 100644 --- a/src/graph.h +++ b/src/graph.h @@ -128,7 +128,13 @@ private: /// An edge in the dependency graph; links between Nodes using Rules. struct Edge { - Edge() : rule_(NULL), pool_(NULL), env_(NULL), + enum VisitMark { + VisitNone, + VisitInStack, + VisitDone + }; + + Edge() : rule_(NULL), pool_(NULL), env_(NULL), mark_(VisitNone), outputs_ready_(false), deps_missing_(false), implicit_deps_(0), order_only_deps_(0), implicit_outs_(0) {} @@ -156,6 +162,7 @@ struct Edge { vector inputs_; vector outputs_; BindingEnv* env_; + VisitMark mark_; bool outputs_ready_; bool deps_missing_; diff --git a/src/state.cc b/src/state.cc index 6079229..9b3c7e1 100644 --- a/src/state.cc +++ b/src/state.cc @@ -184,8 +184,10 @@ vector State::DefaultNodes(string* err) const { void State::Reset() { for (Paths::iterator i = paths_.begin(); i != paths_.end(); ++i) i->second->ResetState(); - for (vector::iterator e = edges_.begin(); e != edges_.end(); ++e) + for (vector::iterator e = edges_.begin(); e != edges_.end(); ++e) { (*e)->outputs_ready_ = false; + (*e)->mark_ = Edge::VisitNone; + } } void State::Dump() { -- cgit v0.12 From b6f020d3640988824b1fe4355996ef0726a2c44c Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 13 Nov 2015 13:09:11 -0500 Subject: Teach RecomputeDirty to detect cycles in the build graph RecomputeDirty is the earliest traversal of the build graph complete with depfile-loaded dependencies. Teach it to detect cycles and fail immediately. This avoids the need to tolerate cycles in RecomputeDirty only to diagnose them later. It also enables future simplification of Plan and Builder logic because they will be able to assume DAG input. When RecomputeDirty detects a cycle, reject it with an error message like that previously produced by Plan::CheckDependencyCycle. Previously we used the stat state of each node to determine whether we reached it earlier in the walk. Retain this approach for leaf nodes, but add an explicit walk state mark for each Edge so that we can have a temporary mark to aid cycle detection. --- src/graph.cc | 78 +++++++++++++++++++++++++++++++++++++++++++++---------- src/graph.h | 3 +++ src/graph_test.cc | 12 ++++----- 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/src/graph.cc b/src/graph.cc index c34fab8..7dd9491 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -32,28 +32,43 @@ bool Node::Stat(DiskInterface* disk_interface, string* err) { } bool DependencyScan::RecomputeDirty(Node* node, string* err) { + vector stack; + return RecomputeDirty(node, &stack, err); +} + +bool DependencyScan::RecomputeDirty(Node* node, vector* stack, + string* err) { Edge* edge = node->in_edge(); if (!edge) { + // If we already visited this leaf node then we are done. + if (node->status_known()) + return true; // This node has no in-edge; it is dirty if it is missing. + if (!node->StatIfNecessary(disk_interface_, err)) + return false; if (!node->exists()) EXPLAIN("%s has no in-edge and is missing", node->path().c_str()); node->set_dirty(!node->exists()); return true; } + // If we already finished this edge then we are done. + if (edge->mark_ == Edge::VisitDone) + return true; + + // If we encountered this edge earlier in the call stack we have a cycle. + if (!VerifyDAG(node, stack, err)) + return false; + + // Mark the edge temporarily while in the call stack. + edge->mark_ = Edge::VisitInStack; + stack->push_back(node); + bool dirty = false; edge->outputs_ready_ = true; edge->deps_missing_ = false; // Load output mtimes so we can compare them to the most recent input below. - // RecomputeDirty() recursively walks the graph following the input nodes - // of |edge| and the in_edges of these nodes. It uses the stat state of each - // node to mark nodes as visited and doesn't traverse across nodes that have - // been visited already. To make sure that every edge is visited only once - // (important because an edge's deps are loaded every time it's visited), mark - // all outputs of |edge| visited as a first step. This ensures that edges - // with multiple inputs and outputs are visited only once, even in cyclic - // graphs. for (vector::iterator o = edge->outputs_.begin(); o != edge->outputs_.end(); ++o) { if (!(*o)->StatIfNecessary(disk_interface_, err)) @@ -72,12 +87,9 @@ bool DependencyScan::RecomputeDirty(Node* node, string* err) { Node* most_recent_input = NULL; for (vector::iterator i = edge->inputs_.begin(); i != edge->inputs_.end(); ++i) { - if (!(*i)->status_known()) { - if (!(*i)->StatIfNecessary(disk_interface_, err)) - return false; - if (!RecomputeDirty(*i, err)) - return false; - } + // Visit this input. + if (!RecomputeDirty(*i, stack, err)) + return false; // If an input is not ready, neither are our outputs. if (Edge* in_edge = (*i)->in_edge()) { @@ -120,9 +132,47 @@ bool DependencyScan::RecomputeDirty(Node* node, string* err) { if (dirty && !(edge->is_phony() && edge->inputs_.empty())) edge->outputs_ready_ = false; + // Mark the edge as finished during this walk now that it will no longer + // be in the call stack. + edge->mark_ = Edge::VisitDone; + assert(stack->back() == node); + stack->pop_back(); + return true; } +bool DependencyScan::VerifyDAG(Node* node, vector* stack, string* err) { + Edge* edge = node->in_edge(); + assert(edge != NULL); + + // If we have no temporary mark on the edge then we do not yet have a cycle. + if (edge->mark_ != Edge::VisitInStack) + return true; + + // We have this edge earlier in the call stack. Find it. + vector::iterator start = stack->begin(); + while (start != stack->end() && (*start)->in_edge() != edge) + ++start; + assert(start != stack->end()); + + // Make the cycle clear by reporting its start as the node at its end + // instead of some other output of the starting edge. For example, + // running 'ninja b' on + // build a b: cat c + // build c: cat a + // should report a -> c -> a instead of b -> c -> a. + *start = node; + + // Construct the error message rejecting the cycle. + *err = "dependency cycle: "; + for (vector::const_iterator i = start; i != stack->end(); ++i) { + err->append((*i)->path()); + err->append(" -> "); + } + err->append((*start)->path()); + return false; +} + bool DependencyScan::RecomputeOutputsDirty(Edge* edge, Node* most_recent_input, bool* outputs_dirty, string* err) { string command = edge->EvaluateCommand(/*incl_rsp_file=*/true); diff --git a/src/graph.h b/src/graph.h index 1b30dee..586c588 100644 --- a/src/graph.h +++ b/src/graph.h @@ -277,6 +277,9 @@ struct DependencyScan { } private: + bool RecomputeDirty(Node* node, vector* stack, string* err); + bool VerifyDAG(Node* node, vector* stack, string* err); + /// Recompute whether a given single output should be marked dirty. /// Returns true if so. bool RecomputeOutputDirty(Edge* edge, Node* most_recent_input, diff --git a/src/graph_test.cc b/src/graph_test.cc index 87f3430..b76526f 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -335,8 +335,8 @@ TEST_F(GraphTest, CycleWithLengthZeroFromDepfile) { fs_.Create("dep.d", "a: b\n"); string err; - EXPECT_TRUE(scan_.RecomputeDirty(GetNode("a"), &err)); - ASSERT_EQ("", err); + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("a"), &err)); + ASSERT_EQ("dependency cycle: b -> b", err); // Despite the depfile causing edge to be a cycle (it has outputs a and b, // but the depfile also adds b as an input), the deps should have been loaded @@ -360,8 +360,8 @@ TEST_F(GraphTest, CycleWithLengthOneFromDepfile) { fs_.Create("dep.d", "a: c\n"); string err; - EXPECT_TRUE(scan_.RecomputeDirty(GetNode("a"), &err)); - ASSERT_EQ("", err); + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("a"), &err)); + ASSERT_EQ("dependency cycle: b -> c -> b", err); // Despite the depfile causing edge to be a cycle (|edge| has outputs a and b, // but c's in_edge has b as input but the depfile also adds |edge| as @@ -387,8 +387,8 @@ TEST_F(GraphTest, CycleWithLengthOneFromDepfileOneHopAway) { fs_.Create("dep.d", "a: c\n"); string err; - EXPECT_TRUE(scan_.RecomputeDirty(GetNode("d"), &err)); - ASSERT_EQ("", err); + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("d"), &err)); + ASSERT_EQ("dependency cycle: b -> c -> b", err); // Despite the depfile causing edge to be a cycle (|edge| has outputs a and b, // but c's in_edge has b as input but the depfile also adds |edge| as -- cgit v0.12 From 721d2a26b629d8556b73ce051f982967428d0738 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 13 Nov 2015 16:03:16 -0500 Subject: Drop unnecessary cycle detection in Plan::AddTarget We now detect and reject cycles in DependencyScan::RecomputeDirty before Plan::AddTarget is called so we can assume DAG input to the Plan. --- src/build.cc | 49 +++++-------------------------------------------- src/build.h | 4 +--- src/build_test.cc | 53 ----------------------------------------------------- src/graph_test.cc | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 100 deletions(-) diff --git a/src/build.cc b/src/build.cc index c2a615a..61ef0e8 100644 --- a/src/build.cc +++ b/src/build.cc @@ -298,26 +298,22 @@ void Plan::Reset() { } bool Plan::AddTarget(Node* node, string* err) { - vector stack; - return AddSubTarget(node, &stack, err); + return AddSubTarget(node, NULL, err); } -bool Plan::AddSubTarget(Node* node, vector* stack, string* err) { +bool Plan::AddSubTarget(Node* node, Node* dependent, string* err) { Edge* edge = node->in_edge(); if (!edge) { // Leaf node. if (node->dirty()) { string referenced; - if (!stack->empty()) - referenced = ", needed by '" + stack->back()->path() + "',"; + if (dependent) + referenced = ", needed by '" + dependent->path() + "',"; *err = "'" + node->path() + "'" + referenced + " missing " "and no known rule to make it"; } return false; } - if (CheckDependencyCycle(node, *stack, err)) - return false; - if (edge->outputs_ready()) return false; // Don't need to do anything. @@ -341,47 +337,12 @@ bool Plan::AddSubTarget(Node* node, vector* stack, string* err) { if (!want_ins.second) return true; // We've already processed the inputs. - stack->push_back(node); for (vector::iterator i = edge->inputs_.begin(); i != edge->inputs_.end(); ++i) { - if (!AddSubTarget(*i, stack, err) && !err->empty()) + if (!AddSubTarget(*i, node, err) && !err->empty()) return false; } - assert(stack->back() == node); - stack->pop_back(); - - return true; -} - -bool Plan::CheckDependencyCycle(Node* node, const vector& stack, - string* err) { - vector::const_iterator start = stack.begin(); - while (start != stack.end() && (*start)->in_edge() != node->in_edge()) - ++start; - if (start == stack.end()) - return false; - - // Build error string for the cycle. - vector cycle(start, stack.end()); - cycle.push_back(node); - - if (cycle.front() != cycle.back()) { - // Consider - // build a b: cat c - // build c: cat a - // stack will contain [b, c], node will be a. To not print b -> c -> a, - // shift by one to get c -> a -> c which makes the cycle clear. - cycle.erase(cycle.begin()); - cycle.push_back(cycle.front()); - assert(cycle.front() == cycle.back()); - } - *err = "dependency cycle: "; - for (vector::const_iterator i = cycle.begin(); i != cycle.end(); ++i) { - if (i != cycle.begin()) - err->append(" -> "); - err->append((*i)->path()); - } return true; } diff --git a/src/build.h b/src/build.h index f97d67e..43786f1 100644 --- a/src/build.h +++ b/src/build.h @@ -75,9 +75,7 @@ struct Plan { void Reset(); private: - bool AddSubTarget(Node* node, vector* stack, string* err); - bool CheckDependencyCycle(Node* node, const vector& stack, - string* err); + bool AddSubTarget(Node* node, Node* dependent, string* err); void NodeFinished(Node* node); /// Submits a ready edge as a candidate for execution. diff --git a/src/build_test.cc b/src/build_test.cc index 8c9fb11..a0f898f 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -185,59 +185,6 @@ TEST_F(PlanTest, DoubleDependent) { ASSERT_FALSE(edge); // done } -TEST_F(PlanTest, DependencyCycle) { - ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"build out: cat mid\n" -"build mid: cat in\n" -"build in: cat pre\n" -"build pre: cat out\n")); - GetNode("out")->MarkDirty(); - GetNode("mid")->MarkDirty(); - GetNode("in")->MarkDirty(); - GetNode("pre")->MarkDirty(); - - string err; - EXPECT_FALSE(plan_.AddTarget(GetNode("out"), &err)); - ASSERT_EQ("dependency cycle: out -> mid -> in -> pre -> out", err); -} - -TEST_F(PlanTest, CycleInEdgesButNotInNodes1) { - string err; - ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"build a b: cat a\n")); - EXPECT_FALSE(plan_.AddTarget(GetNode("b"), &err)); - ASSERT_EQ("dependency cycle: a -> a", err); -} - -TEST_F(PlanTest, CycleInEdgesButNotInNodes2) { - string err; - ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"build b a: cat a\n")); - EXPECT_FALSE(plan_.AddTarget(GetNode("b"), &err)); - ASSERT_EQ("dependency cycle: a -> a", err); -} - -TEST_F(PlanTest, CycleInEdgesButNotInNodes3) { - string err; - ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"build a b: cat c\n" -"build c: cat a\n")); - EXPECT_FALSE(plan_.AddTarget(GetNode("b"), &err)); - ASSERT_EQ("dependency cycle: c -> a -> c", err); -} - -TEST_F(PlanTest, CycleInEdgesButNotInNodes4) { - string err; - ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, -"build d: cat c\n" -"build c: cat b\n" -"build b: cat a\n" -"build a e: cat d\n" -"build f: cat e\n")); - EXPECT_FALSE(plan_.AddTarget(GetNode("f"), &err)); - ASSERT_EQ("dependency cycle: d -> c -> b -> a -> d", err); -} - void PlanTest::TestPoolWithDepthOne(const char* test_case) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, test_case)); GetNode("out1")->MarkDirty(); diff --git a/src/graph_test.cc b/src/graph_test.cc index b76526f..d4d2824 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -323,6 +323,55 @@ TEST_F(GraphTest, NestedPhonyPrintsDone) { ASSERT_FALSE(plan_.more_to_do()); } +TEST_F(GraphTest, DependencyCycle) { + AssertParse(&state_, +"build out: cat mid\n" +"build mid: cat in\n" +"build in: cat pre\n" +"build pre: cat out\n"); + + string err; + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("dependency cycle: out -> mid -> in -> pre -> out", err); +} + +TEST_F(GraphTest, CycleInEdgesButNotInNodes1) { + string err; + AssertParse(&state_, +"build a b: cat a\n"); + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("b"), &err)); + ASSERT_EQ("dependency cycle: a -> a", err); +} + +TEST_F(GraphTest, CycleInEdgesButNotInNodes2) { + string err; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build b a: cat a\n")); + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("b"), &err)); + ASSERT_EQ("dependency cycle: a -> a", err); +} + +TEST_F(GraphTest, CycleInEdgesButNotInNodes3) { + string err; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build a b: cat c\n" +"build c: cat a\n")); + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("b"), &err)); + ASSERT_EQ("dependency cycle: a -> c -> a", err); +} + +TEST_F(GraphTest, CycleInEdgesButNotInNodes4) { + string err; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build d: cat c\n" +"build c: cat b\n" +"build b: cat a\n" +"build a e: cat d\n" +"build f: cat e\n")); + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("f"), &err)); + ASSERT_EQ("dependency cycle: a -> d -> c -> b -> a", err); +} + // Verify that cycles in graphs with multiple outputs are handled correctly // in RecomputeDirty() and don't cause deps to be loaded multiple times. TEST_F(GraphTest, CycleWithLengthZeroFromDepfile) { -- cgit v0.12 From e6e498494f4f420d8940f79c50e784aa37d3d891 Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Tue, 29 Aug 2017 13:39:11 -0400 Subject: Add `deps` and `recompact` tools to manual The `deps` tool in particular is very useful to know about. --- doc/manual.asciidoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index d7ec932..67db4a7 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -279,6 +279,11 @@ http://clang.llvm.org/docs/JSONCompilationDatabase.html[JSON format] expected by the Clang tooling interface. _Available since Ninja 1.2._ +`deps`:: show all dependencies stored in the `.ninja_deps` file. When given a +target, show just the target's dependencies. + +`recompact`:: recompact the `.ninja_deps` file. + Writing your own Ninja files ---------------------------- -- cgit v0.12 From 86f606fe3b22f9d794713b843e9c29eb303a9663 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Wed, 30 Aug 2017 11:05:13 +0900 Subject: Remove path component limit from input of CanonicalizePath in windows --- src/util.cc | 79 ++++++++++++++++++-------------------------------------- src/util_test.cc | 30 +++++++++++++++++---- 2 files changed, 50 insertions(+), 59 deletions(-) diff --git a/src/util.cc b/src/util.cc index 84de879..ae94d34 100644 --- a/src/util.cc +++ b/src/util.cc @@ -102,18 +102,13 @@ bool CanonicalizePath(string* path, uint64_t* slash_bits, string* err) { return true; } +static bool IsPathSeparator(char c) { #ifdef _WIN32 -static uint64_t ShiftOverBit(int offset, uint64_t bits) { - // e.g. for |offset| == 2: - // | ... 9 8 7 6 5 4 3 2 1 0 | - // \_________________/ \_/ - // above below - // So we drop the bit at offset and move above "down" into its place. - uint64_t above = bits & ~((1 << (offset + 1)) - 1); - uint64_t below = bits & ((1 << offset) - 1); - return (above >> 1) | below; -} + return c == '/' || c == '\\'; +#else + return c == '/'; #endif +} bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, string* err) { @@ -134,37 +129,13 @@ bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, const char* src = start; const char* end = start + *len; + if (IsPathSeparator(*src)) { #ifdef _WIN32 - uint64_t bits = 0; - uint64_t bits_mask = 1; - int bits_offset = 0; - // Convert \ to /, setting a bit in |bits| for each \ encountered. - for (char* c = path; c < end; ++c) { - switch (*c) { - case '\\': - bits |= bits_mask; - *c = '/'; - // Intentional fallthrough. - case '/': - bits_mask <<= 1; - bits_offset++; - } - } - if (bits_offset > 64) { - *err = "too many path components"; - return false; - } - bits_offset = 0; -#endif - if (*src == '/') { -#ifdef _WIN32 - bits_offset++; // network path starts with // - if (*len > 1 && *(src + 1) == '/') { + if (*len > 1 && IsPathSeparator(*(src + 1))) { src += 2; dst += 2; - bits_offset++; } else { ++src; ++dst; @@ -177,24 +148,16 @@ bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, while (src < end) { if (*src == '.') { - if (src + 1 == end || src[1] == '/') { + if (src + 1 == end || IsPathSeparator(src[1])) { // '.' component; eliminate. src += 2; -#ifdef _WIN32 - bits = ShiftOverBit(bits_offset, bits); -#endif continue; - } else if (src[1] == '.' && (src + 2 == end || src[2] == '/')) { + } else if (src[1] == '.' && (src + 2 == end || IsPathSeparator(src[2]))) { // '..' component. Back up if possible. if (component_count > 0) { dst = components[component_count - 1]; src += 3; --component_count; -#ifdef _WIN32 - bits = ShiftOverBit(bits_offset, bits); - bits_offset--; - bits = ShiftOverBit(bits_offset, bits); -#endif } else { *dst++ = *src++; *dst++ = *src++; @@ -204,11 +167,8 @@ bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, } } - if (*src == '/') { + if (IsPathSeparator(*src)) { src++; -#ifdef _WIN32 - bits = ShiftOverBit(bits_offset, bits); -#endif continue; } @@ -217,11 +177,8 @@ bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, components[component_count] = dst; ++component_count; - while (*src != '/' && src != end) + while (src != end && !IsPathSeparator(*src)) *dst++ = *src++; -#ifdef _WIN32 - bits_offset++; -#endif *dst++ = *src++; // Copy '/' or final \0 character as well. } @@ -232,6 +189,20 @@ bool CanonicalizePath(char* path, size_t* len, uint64_t* slash_bits, *len = dst - start - 1; #ifdef _WIN32 + uint64_t bits = 0; + uint64_t bits_mask = 1; + + for (char* c = start; c < start + *len; ++c) { + switch (*c) { + case '\\': + bits |= bits_mask; + *c = '/'; + // Intentional fallthrough. + case '/': + bits_mask <<= 1; + } + } + *slash_bits = bits; #else *slash_bits = 0; diff --git a/src/util_test.cc b/src/util_test.cc index 45d0727..b4b7516 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -292,12 +292,12 @@ TEST(CanonicalizePath, TooManyComponents) { EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); EXPECT_EQ(slash_bits, 0xffffffff); - // 65 is not. + // 65 is OK if #component is less than 60 after path canonicalization. err = ""; path = "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./" "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./x/y.h"; - EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); - EXPECT_EQ(err, "too many path components"); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(slash_bits, 0x0); // Backslashes version. err = ""; @@ -306,8 +306,28 @@ TEST(CanonicalizePath, TooManyComponents) { "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\" "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\x\\y.h"; - EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); - EXPECT_EQ(err, "too many path components"); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(slash_bits, 0x1ffffffff); + + + // 59 after canonicalization is OK. + err = ""; + path = "a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/" + "a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/x/y.h"; + EXPECT_EQ(58, std::count(path.begin(), path.end(), '/')); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(slash_bits, 0x0); + + // Backslashes version. + err = ""; + path = + "a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\" + "a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\" + "a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\" + "a\\a\\a\\a\\a\\a\\a\\a\\a\\x\\y.h"; + EXPECT_EQ(58, std::count(path.begin(), path.end(), '\\')); + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ(slash_bits, 0x3ffffffffffffff); } #endif -- cgit v0.12 From 324a11728169426ae1fceeedc18dd520f23f36ce Mon Sep 17 00:00:00 2001 From: Nikolaus Wittenstein Date: Wed, 30 Aug 2017 08:59:40 -0400 Subject: Add _Available since Ninja 1.4._ to `deps` and `recompact` --- doc/manual.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index 67db4a7..9e55c02 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -280,9 +280,9 @@ by the Clang tooling interface. _Available since Ninja 1.2._ `deps`:: show all dependencies stored in the `.ninja_deps` file. When given a -target, show just the target's dependencies. +target, show just the target's dependencies. _Available since Ninja 1.4._ -`recompact`:: recompact the `.ninja_deps` file. +`recompact`:: recompact the `.ninja_deps` file. _Available since Ninja 1.4._ Writing your own Ninja files -- cgit v0.12 From b98941a605d3cc47966d8407ace6e454d781af9b Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sun, 3 Sep 2017 22:13:50 -0400 Subject: mark this 1.8.0.git --- src/version.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/version.cc b/src/version.cc index e2a83bb..758af15 100644 --- a/src/version.cc +++ b/src/version.cc @@ -18,7 +18,7 @@ #include "util.h" -const char* kNinjaVersion = "1.7.2.git"; +const char* kNinjaVersion = "1.8.0.git"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.'); -- cgit v0.12