From a5980c6bb2d6ccb6ffed2d92304c55ba94622963 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Mon, 7 Mar 2011 09:56:18 -0800 Subject: canonicalize paths loaded from depfiles If a C file #includes "../foo.cc", then gcc will emit paths like "bar/../foo.cc" into the dependency file; canonicalize these when we load the file. Add a test module for testing the graph dirty recomputation directly, without all the build classes around it. --- build.ninja | 3 +- src/graph.cc | 59 +++++++++++++++++++++++++++++++++++- src/graph.h | 3 ++ src/graph_test.cc | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 src/graph_test.cc diff --git a/build.ninja b/build.ninja index 8f5a744..7f1963f 100644 --- a/build.ninja +++ b/build.ninja @@ -50,12 +50,13 @@ build ninja: link $builddir/ninja.o $builddir/ninja.a build $builddir/build_test.o: cxx src/build_test.cc build $builddir/build_log_test.o: cxx src/build_log_test.cc +build $builddir/graph_test.o: cxx src/graph_test.cc build $builddir/ninja_test.o: cxx src/ninja_test.cc build $builddir/parsers_test.o: cxx src/parsers_test.cc build $builddir/subprocess_test.o: cxx src/subprocess_test.cc build $builddir/test.o: cxx src/test.cc build ninja_test: link $builddir/build_test.o $builddir/build_log_test.o \ - $builddir/ninja_test.o $builddir/parsers_test.o \ + $builddir/graph_test.o $builddir/ninja_test.o $builddir/parsers_test.o \ $builddir/subprocess_test.o $builddir/test.o $builddir/ninja.a ldflags = -g -rdynamic -lgtest -lgtest_main -lpthread diff --git a/src/graph.cc b/src/graph.cc index aead200..bdbcf3b 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -20,6 +20,59 @@ #include "ninja.h" #include "parsers.h" +// Canonicalize a path like "foo/../bar.h" into just "bar.h". +bool CanonicalizePath(string* path, string* err) { + // Try to fast-path out the common case. + if (path->find("/.") == string::npos && + path->find("./") == string::npos) { + return true; + } + + string inpath = *path; + vector parts; + for (string::size_type start = 0; start < inpath.size(); ++start) { + string::size_type end = inpath.find('/', start); + if (end == string::npos) + end = inpath.size(); + else + inpath[end] = 0; + parts.push_back(inpath.data() + start); + start = end; + } + + vector::iterator i = parts.begin(); + while (i != parts.end()) { + const char* part = *i; + if (part[0] == '.') { + if (part[1] == 0) { + // "."; strip. + parts.erase(i); + continue; + } else if (part[1] == '.' && part[2] == 0) { + // ".."; go up one. + if (i == parts.begin()) { + *err = "can't canonicalize path '" + *path + "' that reaches " + "above its directory"; + return false; + } + --i; + parts.erase(i, i + 2); + continue; + } + } + ++i; + } + path->clear(); + + for (i = parts.begin(); i != parts.end(); ++i) { + if (!path->empty()) + path->push_back('/'); + path->append(*i); + } + + return true; +} + bool FileStat::Stat(DiskInterface* disk_interface) { mtime_ = disk_interface->Stat(path_); return mtime_ > 0; @@ -124,7 +177,8 @@ string Edge::GetDescription() { return rule_->description_.Evaluate(&env); } -bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface, string* err) { +bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface, + string* err) { EdgeEnv env(this); string path = rule_->depfile_.Evaluate(&env); @@ -155,6 +209,9 @@ bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface, string* err) // Add all its in-edges. for (vector::iterator i = makefile.ins_.begin(); i != makefile.ins_.end(); ++i) { + if (!CanonicalizePath(&*i, err)) + return false; + Node* node = state->GetNode(*i); for (vector::iterator j = inputs_.begin(); j != inputs_.end(); ++j) { if (*j == node) { diff --git a/src/graph.h b/src/graph.h index f3cfa97..ecf9e54 100644 --- a/src/graph.h +++ b/src/graph.h @@ -119,4 +119,7 @@ struct Edge { bool is_phony() const; }; +// Exposed for testing. +bool CanonicalizePath(string* path, string* err); + #endif // NINJA_GRAPH_H_ diff --git a/src/graph_test.cc b/src/graph_test.cc new file mode 100644 index 0000000..758315e --- /dev/null +++ b/src/graph_test.cc @@ -0,0 +1,90 @@ +// Copyright 2011 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 "graph.h" + +#include "test.h" + +TEST(CanonicalizePath, PathSamples) { + string path = "foo.h"; + string err; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("", err); + EXPECT_EQ("foo.h", path); + + path = "./foo.h"; err = ""; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("", err); + EXPECT_EQ("foo.h", path); + + path = "./foo/./bar.h"; err = ""; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("", err); + EXPECT_EQ("foo/bar.h", path); + + path = "./x/foo/../bar.h"; err = ""; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("", err); + EXPECT_EQ("x/bar.h", path); + + path = "./x/foo/../../bar.h"; err = ""; + EXPECT_TRUE(CanonicalizePath(&path, &err)); + EXPECT_EQ("", err); + EXPECT_EQ("bar.h", path); + + path = "./x/../foo/../../bar.h"; err = ""; + EXPECT_FALSE(CanonicalizePath(&path, &err)); + EXPECT_EQ("can't canonicalize path './x/../foo/../../bar.h' that reaches " + "above its directory", err); +} + +struct GraphTest : public StateTestWithBuiltinRules { + VirtualFileSystem fs_; +}; + +TEST_F(GraphTest, MissingImplicit) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"build out: cat in | implicit\n")); + fs_.Create("in", 1, ""); + fs_.Create("out", 1, ""); + + Edge* edge = GetNode("out")->in_edge_; + string err; + EXPECT_TRUE(edge->RecomputeDirty(&state_, &fs_, &err)); + ASSERT_EQ("", err); + + // A missing implicit dep does not make the output dirty. + EXPECT_FALSE(GetNode("out")->dirty_); +} + +TEST_F(GraphTest, FunkyMakefilePath) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, +"rule catdep\n" +" depfile = $out.d\n" +" command = cat $in > $out\n" +"build out.o: catdep foo.cc\n")); + fs_.Create("implicit.h", 2, ""); + fs_.Create("foo.cc", 1, ""); + fs_.Create("out.o.d", 1, "out.o: ./foo/../implicit.h\n"); + fs_.Create("out.o", 1, ""); + + Edge* edge = GetNode("out.o")->in_edge_; + string err; + EXPECT_TRUE(edge->RecomputeDirty(&state_, &fs_, &err)); + ASSERT_EQ("", err); + + // implicit.h has changed, though our depfile refers to it with a + // non-canonical path; we should still find it. + EXPECT_TRUE(GetNode("out.o")->dirty_); +} -- cgit v0.12