Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Commit a5980c6

Browse files
committed
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.
1 parent ad0b24e commit a5980c6

File tree

4 files changed

+153
-2
lines changed

4 files changed

+153
-2
lines changed

build.ninja

+2-1
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,13 @@ build ninja: link $builddir/ninja.o $builddir/ninja.a
5050

5151
build $builddir/build_test.o: cxx src/build_test.cc
5252
build $builddir/build_log_test.o: cxx src/build_log_test.cc
53+
build $builddir/graph_test.o: cxx src/graph_test.cc
5354
build $builddir/ninja_test.o: cxx src/ninja_test.cc
5455
build $builddir/parsers_test.o: cxx src/parsers_test.cc
5556
build $builddir/subprocess_test.o: cxx src/subprocess_test.cc
5657
build $builddir/test.o: cxx src/test.cc
5758
build ninja_test: link $builddir/build_test.o $builddir/build_log_test.o \
58-
$builddir/ninja_test.o $builddir/parsers_test.o \
59+
$builddir/graph_test.o $builddir/ninja_test.o $builddir/parsers_test.o \
5960
$builddir/subprocess_test.o $builddir/test.o $builddir/ninja.a
6061
ldflags = -g -rdynamic -lgtest -lgtest_main -lpthread
6162

src/graph.cc

+58-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,59 @@
2020
#include "ninja.h"
2121
#include "parsers.h"
2222

23+
// Canonicalize a path like "foo/../bar.h" into just "bar.h".
24+
bool CanonicalizePath(string* path, string* err) {
25+
// Try to fast-path out the common case.
26+
if (path->find("/.") == string::npos &&
27+
path->find("./") == string::npos) {
28+
return true;
29+
}
30+
31+
string inpath = *path;
32+
vector<const char*> parts;
33+
for (string::size_type start = 0; start < inpath.size(); ++start) {
34+
string::size_type end = inpath.find('/', start);
35+
if (end == string::npos)
36+
end = inpath.size();
37+
else
38+
inpath[end] = 0;
39+
parts.push_back(inpath.data() + start);
40+
start = end;
41+
}
42+
43+
vector<const char*>::iterator i = parts.begin();
44+
while (i != parts.end()) {
45+
const char* part = *i;
46+
if (part[0] == '.') {
47+
if (part[1] == 0) {
48+
// "."; strip.
49+
parts.erase(i);
50+
continue;
51+
} else if (part[1] == '.' && part[2] == 0) {
52+
// ".."; go up one.
53+
if (i == parts.begin()) {
54+
*err = "can't canonicalize path '" + *path + "' that reaches "
55+
"above its directory";
56+
return false;
57+
}
58+
--i;
59+
parts.erase(i, i + 2);
60+
continue;
61+
}
62+
}
63+
++i;
64+
}
65+
path->clear();
66+
67+
for (i = parts.begin(); i != parts.end(); ++i) {
68+
if (!path->empty())
69+
path->push_back('/');
70+
path->append(*i);
71+
}
72+
73+
return true;
74+
}
75+
2376
bool FileStat::Stat(DiskInterface* disk_interface) {
2477
mtime_ = disk_interface->Stat(path_);
2578
return mtime_ > 0;
@@ -124,7 +177,8 @@ string Edge::GetDescription() {
124177
return rule_->description_.Evaluate(&env);
125178
}
126179

127-
bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface, string* err) {
180+
bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface,
181+
string* err) {
128182
EdgeEnv env(this);
129183
string path = rule_->depfile_.Evaluate(&env);
130184

@@ -155,6 +209,9 @@ bool Edge::LoadDepFile(State* state, DiskInterface* disk_interface, string* err)
155209
// Add all its in-edges.
156210
for (vector<string>::iterator i = makefile.ins_.begin();
157211
i != makefile.ins_.end(); ++i) {
212+
if (!CanonicalizePath(&*i, err))
213+
return false;
214+
158215
Node* node = state->GetNode(*i);
159216
for (vector<Node*>::iterator j = inputs_.begin(); j != inputs_.end(); ++j) {
160217
if (*j == node) {

src/graph.h

+3
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,7 @@ struct Edge {
119119
bool is_phony() const;
120120
};
121121

122+
// Exposed for testing.
123+
bool CanonicalizePath(string* path, string* err);
124+
122125
#endif // NINJA_GRAPH_H_

src/graph_test.cc

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Copyright 2011 Google Inc. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include "graph.h"
16+
17+
#include "test.h"
18+
19+
TEST(CanonicalizePath, PathSamples) {
20+
string path = "foo.h";
21+
string err;
22+
EXPECT_TRUE(CanonicalizePath(&path, &err));
23+
EXPECT_EQ("", err);
24+
EXPECT_EQ("foo.h", path);
25+
26+
path = "./foo.h"; err = "";
27+
EXPECT_TRUE(CanonicalizePath(&path, &err));
28+
EXPECT_EQ("", err);
29+
EXPECT_EQ("foo.h", path);
30+
31+
path = "./foo/./bar.h"; err = "";
32+
EXPECT_TRUE(CanonicalizePath(&path, &err));
33+
EXPECT_EQ("", err);
34+
EXPECT_EQ("foo/bar.h", path);
35+
36+
path = "./x/foo/../bar.h"; err = "";
37+
EXPECT_TRUE(CanonicalizePath(&path, &err));
38+
EXPECT_EQ("", err);
39+
EXPECT_EQ("x/bar.h", path);
40+
41+
path = "./x/foo/../../bar.h"; err = "";
42+
EXPECT_TRUE(CanonicalizePath(&path, &err));
43+
EXPECT_EQ("", err);
44+
EXPECT_EQ("bar.h", path);
45+
46+
path = "./x/../foo/../../bar.h"; err = "";
47+
EXPECT_FALSE(CanonicalizePath(&path, &err));
48+
EXPECT_EQ("can't canonicalize path './x/../foo/../../bar.h' that reaches "
49+
"above its directory", err);
50+
}
51+
52+
struct GraphTest : public StateTestWithBuiltinRules {
53+
VirtualFileSystem fs_;
54+
};
55+
56+
TEST_F(GraphTest, MissingImplicit) {
57+
ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
58+
"build out: cat in | implicit\n"));
59+
fs_.Create("in", 1, "");
60+
fs_.Create("out", 1, "");
61+
62+
Edge* edge = GetNode("out")->in_edge_;
63+
string err;
64+
EXPECT_TRUE(edge->RecomputeDirty(&state_, &fs_, &err));
65+
ASSERT_EQ("", err);
66+
67+
// A missing implicit dep does not make the output dirty.
68+
EXPECT_FALSE(GetNode("out")->dirty_);
69+
}
70+
71+
TEST_F(GraphTest, FunkyMakefilePath) {
72+
ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
73+
"rule catdep\n"
74+
" depfile = $out.d\n"
75+
" command = cat $in > $out\n"
76+
"build out.o: catdep foo.cc\n"));
77+
fs_.Create("implicit.h", 2, "");
78+
fs_.Create("foo.cc", 1, "");
79+
fs_.Create("out.o.d", 1, "out.o: ./foo/../implicit.h\n");
80+
fs_.Create("out.o", 1, "");
81+
82+
Edge* edge = GetNode("out.o")->in_edge_;
83+
string err;
84+
EXPECT_TRUE(edge->RecomputeDirty(&state_, &fs_, &err));
85+
ASSERT_EQ("", err);
86+
87+
// implicit.h has changed, though our depfile refers to it with a
88+
// non-canonical path; we should still find it.
89+
EXPECT_TRUE(GetNode("out.o")->dirty_);
90+
}

0 commit comments

Comments
 (0)