diff options
author | Nico Weber <nicolasweber@gmx.de> | 2014-11-12 18:10:29 (GMT) |
---|---|---|
committer | Nico Weber <nicolasweber@gmx.de> | 2014-11-12 18:10:29 (GMT) |
commit | 6aa1b4b6d3d2d3efa55f91649271ed253ef5cab7 (patch) | |
tree | c0cbdf6e814038385bad3945b2229e7dd611eb00 /src | |
parent | 513f5bb1ed7392686c455cbe2989d143230a1d80 (diff) | |
parent | cc8b72a87d72a030dc5b2bb212958c0e46213a6b (diff) | |
download | Ninja-6aa1b4b6d3d2d3efa55f91649271ed253ef5cab7.zip Ninja-6aa1b4b6d3d2d3efa55f91649271ed253ef5cab7.tar.gz Ninja-6aa1b4b6d3d2d3efa55f91649271ed253ef5cab7.tar.bz2 |
Merge pull request #848 from sgraham/canon-maintain
Maintain bitvector of original slashes from manifest when canonicalizing
Diffstat (limited to 'src')
-rw-r--r-- | src/build.cc | 12 | ||||
-rw-r--r-- | src/build_test.cc | 103 | ||||
-rw-r--r-- | src/canon_perftest.cc | 3 | ||||
-rw-r--r-- | src/deps_log.cc | 7 | ||||
-rw-r--r-- | src/deps_log_test.cc | 94 | ||||
-rw-r--r-- | src/graph.cc | 27 | ||||
-rw-r--r-- | src/graph.h | 11 | ||||
-rw-r--r-- | src/graph_test.cc | 21 | ||||
-rw-r--r-- | src/includes_normalize-win32.cc | 3 | ||||
-rw-r--r-- | src/includes_normalize.h | 3 | ||||
-rw-r--r-- | src/manifest_parser.cc | 13 | ||||
-rw-r--r-- | src/manifest_parser_test.cc | 50 | ||||
-rw-r--r-- | src/ninja.cc | 6 | ||||
-rw-r--r-- | src/state.cc | 12 | ||||
-rw-r--r-- | src/state.h | 6 | ||||
-rw-r--r-- | src/state_test.cc | 12 | ||||
-rw-r--r-- | src/test.cc | 3 | ||||
-rw-r--r-- | src/util.cc | 67 | ||||
-rw-r--r-- | src/util.h | 8 | ||||
-rw-r--r-- | src/util_test.cc | 139 |
20 files changed, 501 insertions, 99 deletions
diff --git a/src/build.cc b/src/build.cc index fb1cc74..3e74131 100644 --- a/src/build.cc +++ b/src/build.cc @@ -825,7 +825,11 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, result->output = parser.Parse(result->output, deps_prefix); for (set<string>::iterator i = parser.includes_.begin(); i != parser.includes_.end(); ++i) { - deps_nodes->push_back(state_->GetNode(*i)); + // ~0 is assuming that with MSVC-parsed headers, it's ok to always make + // all backslashes (as some of the slashes will certainly be backslashes + // anyway). This could be fixed if necessary with some additional + // complexity in IncludesNormalize::Relativize. + deps_nodes->push_back(state_->GetNode(*i, ~0u)); } } else #endif @@ -850,9 +854,11 @@ bool Builder::ExtractDeps(CommandRunner::Result* result, deps_nodes->reserve(deps.ins_.size()); for (vector<StringPiece>::iterator i = deps.ins_.begin(); i != deps.ins_.end(); ++i) { - if (!CanonicalizePath(const_cast<char*>(i->str_), &i->len_, err)) + unsigned int slash_bits; + if (!CanonicalizePath(const_cast<char*>(i->str_), &i->len_, &slash_bits, + err)) return false; - deps_nodes->push_back(state_->GetNode(*i)); + deps_nodes->push_back(state_->GetNode(*i, slash_bits)); } if (disk_interface_->RemoveFile(depfile) < 0) { diff --git a/src/build_test.cc b/src/build_test.cc index a5b0972..bd1cd30 100644 --- a/src/build_test.cc +++ b/src/build_test.cc @@ -755,12 +755,11 @@ TEST_F(BuildTest, MakeDirs) { #ifdef _WIN32 ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build subdir\\dir2\\file: cat in1\n")); - EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); #else ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build subdir/dir2/file: cat in1\n")); - EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); #endif + EXPECT_TRUE(builder_.AddTarget("subdir/dir2/file", &err)); EXPECT_EQ("", err); EXPECT_TRUE(builder_.Build(&err)); @@ -938,6 +937,38 @@ TEST_F(BuildTest, RebuildOrderOnlyDeps) { ASSERT_EQ("cc oo.h.in", command_runner_.commands_ran_[0]); } +#ifdef _WIN32 +TEST_F(BuildTest, DepFileCanonicalize) { + string err; + int orig_edges = state_.edges_.size(); + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule cc\n command = cc $in\n depfile = $out.d\n" +"build gen/stuff\\things/foo.o: cc x\\y/z\\foo.c\n")); + Edge* edge = state_.edges_.back(); + + fs_.Create("x/y/z/foo.c", ""); + GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. + // Note, different slashes from manifest. + fs_.Create("gen/stuff\\things/foo.o.d", + "gen\\stuff\\things\\foo.o: blah.h bar.h\n"); + EXPECT_TRUE(builder_.AddTarget("gen/stuff/things/foo.o", &err)); + ASSERT_EQ("", err); + ASSERT_EQ(1u, fs_.files_read_.size()); + // The depfile path does not get Canonicalize as it seems unnecessary. + EXPECT_EQ("gen/stuff\\things/foo.o.d", fs_.files_read_[0]); + + // Expect three new edges: one generating foo.o, and two more from + // loading the depfile. + ASSERT_EQ(orig_edges + 3, (int)state_.edges_.size()); + // Expect our edge to now have three inputs: foo.c and two headers. + ASSERT_EQ(3u, edge->inputs_.size()); + + // Expect the command line we generate to only use the original input, and + // using the slashes from the manifest. + ASSERT_EQ("cc x\\y/z\\foo.c", edge->EvaluateCommand()); +} +#endif + TEST_F(BuildTest, Phony) { string err; ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, @@ -1852,7 +1883,7 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { Edge* edge = state.edges_.back(); - state.GetNode("bar.h")->MarkDirty(); // Mark bar.h as missing. + state.GetNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. EXPECT_TRUE(builder.AddTarget("fo o.o", &err)); ASSERT_EQ("", err); @@ -1870,6 +1901,72 @@ TEST_F(BuildWithDepsLogTest, DepFileOKDepsLog) { } } +#ifdef _WIN32 +TEST_F(BuildWithDepsLogTest, DepFileDepsLogCanonicalize) { + string err; + const char* manifest = + "rule cc\n command = cc $in\n depfile = $out.d\n deps = gcc\n" + "build a/b\\c\\d/e/fo$ o.o: cc x\\y/z\\foo.c\n"; + + fs_.Create("x/y/z/foo.c", ""); + + { + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + // Run the build once, everything should be ok. + DepsLog deps_log; + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + EXPECT_TRUE(builder.AddTarget("a/b/c/d/e/fo o.o", &err)); + ASSERT_EQ("", err); + // Note, different slashes from manifest. + fs_.Create("a/b\\c\\d/e/fo o.o.d", + "a\\b\\c\\d\\e\\fo\\ o.o: blah.h bar.h\n"); + EXPECT_TRUE(builder.Build(&err)); + EXPECT_EQ("", err); + + deps_log.Close(); + builder.command_runner_.release(); + } + + { + State state; + ASSERT_NO_FATAL_FAILURE(AssertParse(&state, manifest)); + + DepsLog deps_log; + ASSERT_TRUE(deps_log.Load("ninja_deps", &state, &err)); + ASSERT_TRUE(deps_log.OpenForWrite("ninja_deps", &err)); + ASSERT_EQ("", err); + + Builder builder(&state, config_, NULL, &deps_log, &fs_); + builder.command_runner_.reset(&command_runner_); + + Edge* edge = state.edges_.back(); + + state.GetNode("bar.h", 0)->MarkDirty(); // Mark bar.h as missing. + EXPECT_TRUE(builder.AddTarget("a/b/c/d/e/fo o.o", &err)); + ASSERT_EQ("", err); + + // Expect three new edges: one generating fo o.o, and two more from + // loading the depfile. + ASSERT_EQ(3u, state.edges_.size()); + // Expect our edge to now have three inputs: foo.c and two headers. + ASSERT_EQ(3u, edge->inputs_.size()); + + // Expect the command line we generate to only use the original input. + // Note, slashes from manifest, not .d. + ASSERT_EQ("cc x\\y/z\\foo.c", edge->EvaluateCommand()); + + deps_log.Close(); + builder.command_runner_.release(); + } +} +#endif + /// Check that a restat rule doesn't clear an edge if the depfile is missing. /// Follows from: https://github.com/martine/ninja/issues/603 TEST_F(BuildTest, RestatMissingDepfile) { diff --git a/src/canon_perftest.cc b/src/canon_perftest.cc index 59bd18f..389ac24 100644 --- a/src/canon_perftest.cc +++ b/src/canon_perftest.cc @@ -33,8 +33,9 @@ int main() { for (int j = 0; j < 5; ++j) { const int kNumRepetitions = 2000000; int64_t start = GetTimeMillis(); + unsigned int slash_bits; for (int i = 0; i < kNumRepetitions; ++i) { - CanonicalizePath(buf, &len, &err); + CanonicalizePath(buf, &len, &slash_bits, &err); } int delta = (int)(GetTimeMillis() - start); times.push_back(delta); diff --git a/src/deps_log.cc b/src/deps_log.cc index 39de180..fc46497 100644 --- a/src/deps_log.cc +++ b/src/deps_log.cc @@ -240,7 +240,12 @@ bool DepsLog::Load(const string& path, State* state, string* err) { if (buf[path_size - 1] == '\0') --path_size; if (buf[path_size - 1] == '\0') --path_size; StringPiece path(buf, path_size); - Node* node = state->GetNode(path); + // It is not necessary to pass in a correct slash_bits here. It will + // either be a Node that's in the manifest (in which case it will already + // have a correct slash_bits that GetNode will look up), or it is an + // implicit dependency from a .d which does not affect the build command + // (and so need not have its slashes maintained). + Node* node = state->GetNode(path, 0); // Check that the expected index matches the actual index. This can only // happen if two ninja processes write to the same deps log concurrently. diff --git a/src/deps_log_test.cc b/src/deps_log_test.cc index 60ff48f..ac2b315 100644 --- a/src/deps_log_test.cc +++ b/src/deps_log_test.cc @@ -46,16 +46,16 @@ TEST_F(DepsLogTest, WriteRead) { { vector<Node*> deps; - deps.push_back(state1.GetNode("foo.h")); - deps.push_back(state1.GetNode("bar.h")); - log1.RecordDeps(state1.GetNode("out.o"), 1, deps); + deps.push_back(state1.GetNode("foo.h", 0)); + deps.push_back(state1.GetNode("bar.h", 0)); + log1.RecordDeps(state1.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state1.GetNode("foo.h")); - deps.push_back(state1.GetNode("bar2.h")); - log1.RecordDeps(state1.GetNode("out2.o"), 2, deps); + deps.push_back(state1.GetNode("foo.h", 0)); + deps.push_back(state1.GetNode("bar2.h", 0)); + log1.RecordDeps(state1.GetNode("out2.o", 0), 2, deps); - DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o")); + DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o", 0)); ASSERT_TRUE(log_deps); ASSERT_EQ(1, log_deps->mtime); ASSERT_EQ(2, log_deps->node_count); @@ -79,7 +79,7 @@ TEST_F(DepsLogTest, WriteRead) { } // Spot-check the entries in log2. - DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out2.o")); + DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out2.o", 0)); ASSERT_TRUE(log_deps); ASSERT_EQ(2, log_deps->mtime); ASSERT_EQ(2, log_deps->node_count); @@ -101,11 +101,11 @@ TEST_F(DepsLogTest, LotsOfDeps) { for (int i = 0; i < kNumDeps; ++i) { char buf[32]; sprintf(buf, "file%d.h", i); - deps.push_back(state1.GetNode(buf)); + deps.push_back(state1.GetNode(buf, 0)); } - log1.RecordDeps(state1.GetNode("out.o"), 1, deps); + log1.RecordDeps(state1.GetNode("out.o", 0), 1, deps); - DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o")); + DepsLog::Deps* log_deps = log1.GetDeps(state1.GetNode("out.o", 0)); ASSERT_EQ(kNumDeps, log_deps->node_count); } @@ -116,7 +116,7 @@ TEST_F(DepsLogTest, LotsOfDeps) { EXPECT_TRUE(log2.Load(kTestFilename, &state2, &err)); ASSERT_EQ("", err); - DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out.o")); + DepsLog::Deps* log_deps = log2.GetDeps(state2.GetNode("out.o", 0)); ASSERT_EQ(kNumDeps, log_deps->node_count); } @@ -132,9 +132,9 @@ TEST_F(DepsLogTest, DoubleEntry) { ASSERT_EQ("", err); vector<Node*> deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); log.Close(); struct stat st; @@ -154,9 +154,9 @@ TEST_F(DepsLogTest, DoubleEntry) { ASSERT_EQ("", err); vector<Node*> deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); log.Close(); struct stat st; @@ -186,14 +186,14 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("", err); vector<Node*> deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("baz.h")); - log.RecordDeps(state.GetNode("other_out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("baz.h", 0)); + log.RecordDeps(state.GetNode("other_out.o", 0), 1, deps); log.Close(); @@ -216,8 +216,8 @@ TEST_F(DepsLogTest, Recompact) { ASSERT_EQ("", err); vector<Node*> deps; - deps.push_back(state.GetNode("foo.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); log.Close(); struct stat st; @@ -237,14 +237,14 @@ TEST_F(DepsLogTest, Recompact) { string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); - Node* out = state.GetNode("out.o"); + Node* out = state.GetNode("out.o", 0); DepsLog::Deps* deps = log.GetDeps(out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); ASSERT_EQ(1, deps->node_count); ASSERT_EQ("foo.h", deps->nodes[0]->path()); - Node* other_out = state.GetNode("other_out.o"); + Node* other_out = state.GetNode("other_out.o", 0); deps = log.GetDeps(other_out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); @@ -287,14 +287,14 @@ TEST_F(DepsLogTest, Recompact) { string err; ASSERT_TRUE(log.Load(kTestFilename, &state, &err)); - Node* out = state.GetNode("out.o"); + Node* out = state.GetNode("out.o", 0); DepsLog::Deps* deps = log.GetDeps(out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); ASSERT_EQ(1, deps->node_count); ASSERT_EQ("foo.h", deps->nodes[0]->path()); - Node* other_out = state.GetNode("other_out.o"); + Node* other_out = state.GetNode("other_out.o", 0); deps = log.GetDeps(other_out); ASSERT_TRUE(deps); ASSERT_EQ(1, deps->mtime); @@ -361,14 +361,14 @@ TEST_F(DepsLogTest, Truncated) { ASSERT_EQ("", err); vector<Node*> deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar2.h")); - log.RecordDeps(state.GetNode("out2.o"), 2, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar2.h", 0)); + log.RecordDeps(state.GetNode("out2.o", 0), 2, deps); log.Close(); } @@ -420,14 +420,14 @@ TEST_F(DepsLogTest, TruncatedRecovery) { ASSERT_EQ("", err); vector<Node*> deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar.h")); - log.RecordDeps(state.GetNode("out.o"), 1, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar.h", 0)); + log.RecordDeps(state.GetNode("out.o", 0), 1, deps); deps.clear(); - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar2.h")); - log.RecordDeps(state.GetNode("out2.o"), 2, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar2.h", 0)); + log.RecordDeps(state.GetNode("out2.o", 0), 2, deps); log.Close(); } @@ -448,16 +448,16 @@ TEST_F(DepsLogTest, TruncatedRecovery) { err.clear(); // The truncated entry should've been discarded. - EXPECT_EQ(NULL, log.GetDeps(state.GetNode("out2.o"))); + EXPECT_EQ(NULL, log.GetDeps(state.GetNode("out2.o", 0))); EXPECT_TRUE(log.OpenForWrite(kTestFilename, &err)); ASSERT_EQ("", err); // Add a new entry. vector<Node*> deps; - deps.push_back(state.GetNode("foo.h")); - deps.push_back(state.GetNode("bar2.h")); - log.RecordDeps(state.GetNode("out2.o"), 3, deps); + deps.push_back(state.GetNode("foo.h", 0)); + deps.push_back(state.GetNode("bar2.h", 0)); + log.RecordDeps(state.GetNode("out2.o", 0), 3, deps); log.Close(); } @@ -471,7 +471,7 @@ TEST_F(DepsLogTest, TruncatedRecovery) { EXPECT_TRUE(log.Load(kTestFilename, &state, &err)); // The truncated entry should exist. - DepsLog::Deps* deps = log.GetDeps(state.GetNode("out2.o")); + DepsLog::Deps* deps = log.GetDeps(state.GetNode("out2.o", 0)); ASSERT_TRUE(deps); } } diff --git a/src/graph.cc b/src/graph.cc index aa9c0e8..223661c 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -256,7 +256,7 @@ string EdgeEnv::MakePathList(vector<Node*>::iterator begin, for (vector<Node*>::iterator i = begin; i != end; ++i) { if (!result.empty()) result.push_back(sep); - const string& path = (*i)->path(); + const string& path = (*i)->PathDecanonicalized(); if (escape_in_out_ == kShellEscape) { #if _WIN32 GetWin32EscapedString(path, &result); @@ -328,6 +328,20 @@ bool Edge::use_console() const { return pool() == &State::kConsolePool; } +string Node::PathDecanonicalized() const { + string result = path_; +#ifdef _WIN32 + unsigned int mask = 1; + for (char* c = &result[0]; (c = strchr(c, '/')) != NULL;) { + if (slash_bits_ & mask) + *c = '\\'; + c++; + mask <<= 1; + } +#endif + return result; +} + void Node::Dump(const char* prefix) const { printf("%s <%s 0x%p> mtime: %d%s, (:%s), ", prefix, path().c_str(), this, @@ -379,6 +393,11 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, return false; } + unsigned int unused; + if (!CanonicalizePath(const_cast<char*>(depfile.out_.str_), + &depfile.out_.len_, &unused, err)) + return false; + // Check that this depfile matches the edge's output. Node* first_output = edge->outputs_[0]; StringPiece opath = StringPiece(first_output->path()); @@ -395,10 +414,12 @@ bool ImplicitDepLoader::LoadDepFile(Edge* edge, const string& path, // Add all its in-edges. for (vector<StringPiece>::iterator i = depfile.ins_.begin(); i != depfile.ins_.end(); ++i, ++implicit_dep) { - if (!CanonicalizePath(const_cast<char*>(i->str_), &i->len_, err)) + unsigned int slash_bits; + if (!CanonicalizePath(const_cast<char*>(i->str_), &i->len_, &slash_bits, + err)) return false; - Node* node = state_->GetNode(*i); + Node* node = state_->GetNode(*i, slash_bits); *implicit_dep = node; node->AddOutEdge(edge); CreatePhonyInEdge(node); diff --git a/src/graph.h b/src/graph.h index 66e31b5..9aada38 100644 --- a/src/graph.h +++ b/src/graph.h @@ -33,8 +33,9 @@ struct State; /// Information about a node in the dependency graph: the file, whether /// it's dirty, mtime, etc. struct Node { - explicit Node(const string& path) + Node(const string& path, unsigned int slash_bits) : path_(path), + slash_bits_(slash_bits), mtime_(-1), dirty_(false), in_edge_(NULL), @@ -71,6 +72,9 @@ struct Node { } const string& path() const { return path_; } + /// Get |path()| but use slash_bits to convert back to original slash styles. + string PathDecanonicalized() const; + unsigned int slash_bits() const { return slash_bits_; } TimeStamp mtime() const { return mtime_; } bool dirty() const { return dirty_; } @@ -90,6 +94,11 @@ struct Node { private: string path_; + + /// Set bits starting from lowest for backslashes that were normalized to + /// forward slashes by CanonicalizePath. See |PathDecanonicalized|. + unsigned int slash_bits_; + /// Possible values of mtime_: /// -1: file hasn't been examined /// 0: we looked, and file doesn't exist diff --git a/src/graph_test.cc b/src/graph_test.cc index 14dc678..382d352 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -251,3 +251,24 @@ TEST_F(GraphTest, NestedPhonyPrintsDone) { EXPECT_EQ(0, plan_.command_edge_count()); ASSERT_FALSE(plan_.more_to_do()); } + +#ifdef _WIN32 +TEST_F(GraphTest, Decanonicalize) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build out\\out1: cat src\\in1\n" +"build out\\out2/out3\\out4: cat mid1\n" +"build out3 out4\\foo: cat mid1\n")); + + string err; + vector<Node*> root_nodes = state_.RootNodes(&err); + EXPECT_EQ(4u, root_nodes.size()); + EXPECT_EQ(root_nodes[0]->path(), "out/out1"); + EXPECT_EQ(root_nodes[1]->path(), "out/out2/out3/out4"); + EXPECT_EQ(root_nodes[2]->path(), "out3"); + EXPECT_EQ(root_nodes[3]->path(), "out4/foo"); + EXPECT_EQ(root_nodes[0]->PathDecanonicalized(), "out\\out1"); + EXPECT_EQ(root_nodes[1]->PathDecanonicalized(), "out\\out2/out3\\out4"); + EXPECT_EQ(root_nodes[2]->PathDecanonicalized(), "out3"); + EXPECT_EQ(root_nodes[3]->PathDecanonicalized(), "out4\\foo"); +} +#endif diff --git a/src/includes_normalize-win32.cc b/src/includes_normalize-win32.cc index 0d230f8..1e88a0a 100644 --- a/src/includes_normalize-win32.cc +++ b/src/includes_normalize-win32.cc @@ -100,7 +100,8 @@ string IncludesNormalize::Normalize(const string& input, size_t len = input.size(); strncpy(copy, input.c_str(), input.size() + 1); string err; - if (!CanonicalizePath(copy, &len, &err)) + unsigned int slash_bits; + if (!CanonicalizePath(copy, &len, &slash_bits, &err)) Warning("couldn't canonicalize '%s': %s\n", input.c_str(), err.c_str()); StringPiece partially_fixed(copy, len); diff --git a/src/includes_normalize.h b/src/includes_normalize.h index 43527af..634fef3 100644 --- a/src/includes_normalize.h +++ b/src/includes_normalize.h @@ -29,7 +29,6 @@ struct IncludesNormalize { static string Relativize(StringPiece path, const string& start); /// Normalize by fixing slashes style, fixing redundant .. and . and makes the - /// path relative to |relative_to|. Case is normalized to lowercase on - /// Windows too. + /// path relative to |relative_to|. static string Normalize(const string& input, const char* relative_to); }; diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 55d191f..388b5bc 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -209,7 +209,8 @@ bool ManifestParser::ParseDefault(string* err) { do { string path = eval.Evaluate(env_); string path_err; - if (!CanonicalizePath(&path, &path_err)) + unsigned int 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)) return lexer_.Error(path_err, err); @@ -323,16 +324,18 @@ bool ManifestParser::ParseEdge(string* err) { for (vector<EvalString>::iterator i = ins.begin(); i != ins.end(); ++i) { string path = i->Evaluate(env); string path_err; - if (!CanonicalizePath(&path, &path_err)) + unsigned int slash_bits; + if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); - state_->AddIn(edge, path); + state_->AddIn(edge, path, slash_bits); } for (vector<EvalString>::iterator i = outs.begin(); i != outs.end(); ++i) { string path = i->Evaluate(env); string path_err; - if (!CanonicalizePath(&path, &path_err)) + unsigned int slash_bits; + if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); - state_->AddOut(edge, path); + state_->AddOut(edge, path, slash_bits); } edge->implicit_deps_ = implicit; edge->order_only_deps_ = order_only; diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index aa3e9e4..6909ea9 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -96,7 +96,7 @@ TEST_F(ParserTest, IgnoreIndentedComments) { ASSERT_EQ(2u, state.rules_.size()); const Rule* rule = state.rules_.begin()->second; EXPECT_EQ("cat", rule->name()); - Edge* edge = state.GetNode("result")->in_edge(); + Edge* edge = state.GetNode("result", 0)->in_edge(); EXPECT_TRUE(edge->GetBindingBool("restat")); EXPECT_FALSE(edge->GetBindingBool("generator")); } @@ -269,6 +269,26 @@ TEST_F(ParserTest, CanonicalizeFile) { EXPECT_FALSE(state.LookupNode("in//2")); } +#ifdef _WIN32 +TEST_F(ParserTest, CanonicalizeFileBackslashes) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build out: cat in\\1 in\\\\2\n" +"build in\\1: cat\n" +"build in\\2: cat\n")); + + Node* node = state.LookupNode("in/1");; + EXPECT_TRUE(node); + EXPECT_EQ(1, node->slash_bits()); + node = state.LookupNode("in/2"); + EXPECT_TRUE(node); + EXPECT_EQ(1, node->slash_bits()); + EXPECT_FALSE(state.LookupNode("in//1")); + EXPECT_FALSE(state.LookupNode("in//2")); +} +#endif + TEST_F(ParserTest, PathVariables) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule cat\n" @@ -292,6 +312,34 @@ TEST_F(ParserTest, CanonicalizePaths) { EXPECT_TRUE(state.LookupNode("bar/foo.cc")); } +#ifdef _WIN32 +TEST_F(ParserTest, CanonicalizePathsBackslashes) { + ASSERT_NO_FATAL_FAILURE(AssertParse( +"rule cat\n" +" command = cat $in > $out\n" +"build ./out.o: cat ./bar/baz/../foo.cc\n" +"build .\\out2.o: cat .\\bar/baz\\..\\foo.cc\n" +"build .\\out3.o: cat .\\bar\\baz\\..\\foo3.cc\n" +)); + + EXPECT_FALSE(state.LookupNode("./out.o")); + EXPECT_FALSE(state.LookupNode(".\\out2.o")); + EXPECT_FALSE(state.LookupNode(".\\out3.o")); + EXPECT_TRUE(state.LookupNode("out.o")); + EXPECT_TRUE(state.LookupNode("out2.o")); + EXPECT_TRUE(state.LookupNode("out3.o")); + EXPECT_FALSE(state.LookupNode("./bar/baz/../foo.cc")); + EXPECT_FALSE(state.LookupNode(".\\bar/baz\\..\\foo.cc")); + EXPECT_FALSE(state.LookupNode(".\\bar/baz\\..\\foo3.cc")); + Node* node = state.LookupNode("bar/foo.cc"); + EXPECT_TRUE(node); + EXPECT_EQ(0, node->slash_bits()); + node = state.LookupNode("bar/foo3.cc"); + EXPECT_TRUE(node); + EXPECT_EQ(1, node->slash_bits()); +} +#endif + TEST_F(ParserTest, ReservedWords) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule build\n" diff --git a/src/ninja.cc b/src/ninja.cc index 4368fab..27380c0 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -228,7 +228,8 @@ struct RealFileReader : public ManifestParser::FileReader { /// Returns true if the manifest was rebuilt. bool NinjaMain::RebuildManifest(const char* input_file, string* err) { string path = input_file; - if (!CanonicalizePath(&path, err)) + unsigned int slash_bits; // Unused because this path is only used for lookup. + if (!CanonicalizePath(&path, &slash_bits, err)) return false; Node* node = state_.LookupNode(path); if (!node) @@ -250,7 +251,8 @@ bool NinjaMain::RebuildManifest(const char* input_file, string* err) { Node* NinjaMain::CollectTarget(const char* cpath, string* err) { string path = cpath; - if (!CanonicalizePath(&path, err)) + unsigned int slash_bits; // Unused because this path is only used for lookup. + if (!CanonicalizePath(&path, &slash_bits, err)) return NULL; // Special syntax: "foo.cc^" means "the first output of foo.cc". diff --git a/src/state.cc b/src/state.cc index 7258272..6e3e10d 100644 --- a/src/state.cc +++ b/src/state.cc @@ -111,11 +111,11 @@ Edge* State::AddEdge(const Rule* rule) { return edge; } -Node* State::GetNode(StringPiece path) { +Node* State::GetNode(StringPiece path, unsigned int slash_bits) { Node* node = LookupNode(path); if (node) return node; - node = new Node(path.AsString()); + node = new Node(path.AsString(), slash_bits); paths_[node->path()] = node; return node; } @@ -145,14 +145,14 @@ Node* State::SpellcheckNode(const string& path) { return result; } -void State::AddIn(Edge* edge, StringPiece path) { - Node* node = GetNode(path); +void State::AddIn(Edge* edge, StringPiece path, unsigned int slash_bits) { + Node* node = GetNode(path, slash_bits); edge->inputs_.push_back(node); node->AddOutEdge(edge); } -void State::AddOut(Edge* edge, StringPiece path) { - Node* node = GetNode(path); +void State::AddOut(Edge* edge, StringPiece path, unsigned int slash_bits) { + Node* node = GetNode(path, slash_bits); edge->outputs_.push_back(node); if (node->in_edge()) { Warning("multiple rules generate %s. " diff --git a/src/state.h b/src/state.h index c382dc0..8804532 100644 --- a/src/state.h +++ b/src/state.h @@ -95,12 +95,12 @@ struct State { Edge* AddEdge(const Rule* rule); - Node* GetNode(StringPiece path); + Node* GetNode(StringPiece path, unsigned int slash_bits); Node* LookupNode(StringPiece path) const; Node* SpellcheckNode(const string& path); - void AddIn(Edge* edge, StringPiece path); - void AddOut(Edge* edge, StringPiece path); + void AddIn(Edge* edge, StringPiece path, unsigned int slash_bits); + void AddOut(Edge* edge, StringPiece path, unsigned int slash_bits); bool AddDefault(StringPiece path, string* error); /// Reset state. Keeps all nodes and edges, but restores them to the diff --git a/src/state_test.cc b/src/state_test.cc index a4fafa1..bd133b6 100644 --- a/src/state_test.cc +++ b/src/state_test.cc @@ -32,15 +32,15 @@ TEST(State, Basic) { state.AddRule(rule); Edge* edge = state.AddEdge(rule); - state.AddIn(edge, "in1"); - state.AddIn(edge, "in2"); - state.AddOut(edge, "out"); + state.AddIn(edge, "in1", 0); + state.AddIn(edge, "in2", 0); + state.AddOut(edge, "out", 0); EXPECT_EQ("cat in1 in2 > out", edge->EvaluateCommand()); - EXPECT_FALSE(state.GetNode("in1")->dirty()); - EXPECT_FALSE(state.GetNode("in2")->dirty()); - EXPECT_FALSE(state.GetNode("out")->dirty()); + EXPECT_FALSE(state.GetNode("in1", 0)->dirty()); + EXPECT_FALSE(state.GetNode("in2", 0)->dirty()); + EXPECT_FALSE(state.GetNode("out", 0)->dirty()); } } // namespace diff --git a/src/test.cc b/src/test.cc index 560ef3a..f667fef 100644 --- a/src/test.cc +++ b/src/test.cc @@ -89,7 +89,8 @@ void StateTestWithBuiltinRules::AddCatRule(State* state) { } Node* StateTestWithBuiltinRules::GetNode(const string& path) { - return state_.GetNode(path); + EXPECT_FALSE(strpbrk(path.c_str(), "/\\")); + return state_.GetNode(path, 0); } void AssertParse(State* state, const char* input) { diff --git a/src/util.cc b/src/util.cc index cb8adf1..4df81dd 100644 --- a/src/util.cc +++ b/src/util.cc @@ -85,19 +85,33 @@ void Error(const char* msg, ...) { fprintf(stderr, "\n"); } -bool CanonicalizePath(string* path, string* err) { +bool CanonicalizePath(string* path, unsigned int* slash_bits, string* err) { METRIC_RECORD("canonicalize str"); size_t len = path->size(); char* str = 0; if (len > 0) str = &(*path)[0]; - if (!CanonicalizePath(str, &len, err)) + if (!CanonicalizePath(str, &len, slash_bits, err)) return false; path->resize(len); return true; } -bool CanonicalizePath(char* path, size_t* len, string* err) { +#ifdef _WIN32 +static unsigned int ShiftOverBit(int offset, unsigned int 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); + return (above >> 1) | below; +} +#endif + +bool CanonicalizePath(char* path, size_t* len, unsigned int* slash_bits, + string* err) { // WARNING: this function is performance-critical; please benchmark // any changes you make to it. METRIC_RECORD("canonicalize path"); @@ -106,11 +120,6 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { return false; } -#ifdef _WIN32 - for (char* c = path; (c = strchr(c, '\\')) != NULL;) - *c = '/'; -#endif - const int kMaxPathComponents = 30; char* components[kMaxPathComponents]; int component_count = 0; @@ -120,12 +129,35 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { const char* src = start; const char* end = start + *len; +#ifdef _WIN32 + unsigned int bits = 0; + unsigned int 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 > 32) + return false; + bits_offset = 0; +#endif + if (*src == '/') { #ifdef _WIN32 + bits_offset++; // network path starts with // if (*len > 1 && *(src + 1) == '/') { src += 2; dst += 2; + bits_offset++; } else { ++src; ++dst; @@ -141,6 +173,9 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { if (src + 1 == end || src[1] == '/') { // '.' component; eliminate. src += 2; +#ifdef _WIN32 + bits = ShiftOverBit(bits_offset, bits); +#endif continue; } else if (src[1] == '.' && (src + 2 == end || src[2] == '/')) { // '..' component. Back up if possible. @@ -148,6 +183,11 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { 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++; @@ -159,6 +199,9 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { if (*src == '/') { src++; +#ifdef _WIN32 + bits = ShiftOverBit(bits_offset, bits); +#endif continue; } @@ -169,6 +212,9 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { while (*src != '/' && src != end) *dst++ = *src++; +#ifdef _WIN32 + bits_offset++; +#endif *dst++ = *src++; // Copy '/' or final \0 character as well. } @@ -178,6 +224,11 @@ bool CanonicalizePath(char* path, size_t* len, string* err) { } *len = dst - start - 1; +#ifdef _WIN32 + *slash_bits = bits; +#else + *slash_bits = 0; +#endif return true; } @@ -41,9 +41,11 @@ void Warning(const char* msg, ...); void Error(const char* msg, ...); /// Canonicalize a path like "foo/../bar.h" into just "bar.h". -bool CanonicalizePath(string* path, string* err); - -bool CanonicalizePath(char* path, size_t* len, string* err); +/// |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, + string* err); /// Appends |input| to |*result|, escaping according to the whims of either /// Bash, or Win32's CommandLineToArgvW(). diff --git a/src/util_test.cc b/src/util_test.cc index e82f227..5bbf397 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -16,6 +16,15 @@ #include "test.h" +namespace { + +bool CanonicalizePath(string* path, string* err) { + unsigned int unused; + return ::CanonicalizePath(path, &unused, err); +} + +} // namespace + TEST(CanonicalizePath, PathSamples) { string path; string err; @@ -148,6 +157,131 @@ TEST(CanonicalizePath, PathSamplesWindows) { EXPECT_TRUE(CanonicalizePath(&path, &err)); EXPECT_EQ("", path); } + +TEST(CanonicalizePath, SlashTracking) { + string path; + string err; + unsigned int slash_bits; + + path = "foo.h"; err = ""; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("foo.h", path); + EXPECT_EQ(0, slash_bits); + + path = "a\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a/bcd/efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(4, slash_bits); + + path = "a\\bcd/efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(5, slash_bits); + + path = "a\\bcd\\efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(7, slash_bits); + + path = "a/bcd/efh/foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/bcd/efh/foo.h", path); + EXPECT_EQ(0, slash_bits); + + path = "a\\./efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/efh/foo.h", path); + EXPECT_EQ(3, slash_bits); + + path = "a\\../efh\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("efh/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a\\b\\c\\d\\e\\f\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/b/c/d/e/f/g/foo.h", path); + EXPECT_EQ(127, slash_bits); + + path = "a\\b\\c\\..\\..\\..\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("g/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a\\b/c\\../../..\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("g/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a\\b/c\\./../..\\g\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/g/foo.h", path); + EXPECT_EQ(3, slash_bits); + + path = "a\\b/c\\./../..\\g/foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/g/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a\\\\\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(1, slash_bits); + + path = "a/\\\\foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(0, slash_bits); + + path = "a\\//foo.h"; + EXPECT_TRUE(CanonicalizePath(&path, &slash_bits, &err)); + EXPECT_EQ("a/foo.h", path); + EXPECT_EQ(1, slash_bits); +} + +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; + string err; + size_t size = 13; + EXPECT_TRUE(::CanonicalizePath(buf, &size, &slash_bits, &err)); + EXPECT_EQ(0, strncmp("foo/bar/baz.h", buf, size)); + EXPECT_EQ(2, slash_bits); // Not including the trailing one. +} + +TEST(CanonicalizePath, TooManyComponents) { + string path; + string err; + unsigned int slash_bits; + + // 32 is OK. + path = "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)); + + // Backslashes version. + path = + "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); + + // 33 is not. + path = + "a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/./a/x.h"; + EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); + + // Backslashes version. + path = + "a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\.\\a\\." + "\\a\\.\\a\\.\\a\\.\\a\\.\\a\\x.h"; + EXPECT_FALSE(CanonicalizePath(&path, &slash_bits, &err)); +} #endif TEST(CanonicalizePath, EmptyResult) { @@ -188,16 +322,17 @@ TEST(CanonicalizePath, NotNullTerminated) { string path; string err; size_t len; + unsigned int unused; path = "foo/. bar/."; len = strlen("foo/."); // Canonicalize only the part before the space. - EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err)); + EXPECT_TRUE(CanonicalizePath(&path[0], &len, &unused, &err)); EXPECT_EQ(strlen("foo"), len); EXPECT_EQ("foo/. bar/.", string(path)); path = "foo/../file bar/."; len = strlen("foo/../file"); - EXPECT_TRUE(CanonicalizePath(&path[0], &len, &err)); + EXPECT_TRUE(CanonicalizePath(&path[0], &len, &unused, &err)); EXPECT_EQ(strlen("file"), len); EXPECT_EQ("file ./file bar/.", string(path)); } |