From 0f0fb3275d0c908d9a4401c97cd5ef9d407987d4 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Nov 2015 10:03:57 -0500 Subject: Teach RecomputeDirty to load dyndep files that are ready The full readiness of a node that has a dyndep binding cannot be known until after the dyndep file is loaded. If a dyndep file is ready while constructing the build plan it can be loaded immediately so full information can be used to decide whether anything needs to be built. If a dyndep file is not ready while constructing the build plan then the edges naming it cannot be ready either because the dyndep file is one of their inputs. In this case we defer loading the dyndep file until the build plan is being executed. --- src/graph.cc | 25 +++++++ src/graph_test.cc | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+) diff --git a/src/graph.cc b/src/graph.cc index a464299..add7868 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -68,6 +68,31 @@ bool DependencyScan::RecomputeDirty(Node* node, vector* stack, edge->outputs_ready_ = true; edge->deps_missing_ = false; + if (!edge->deps_loaded_) { + // This is our first encounter with this edge. + // If there is a pending dyndep file, visit it now: + // * If the dyndep file is ready then load it now to get any + // additional inputs and outputs for this and other edges. + // Once the dyndep file is loaded it will no longer be pending + // if any other edges encounter it, but they will already have + // been updated. + // * If the dyndep file is not ready then since is known to be an + // input to this edge, the edge will not be considered ready below. + // Later during the build the dyndep file will become ready and be + // loaded to update this edge before it can possibly be scheduled. + if (edge->dyndep_ && edge->dyndep_->dyndep_pending()) { + if (!RecomputeDirty(edge->dyndep_, stack, err)) + return false; + + if (!edge->dyndep_->in_edge() || + edge->dyndep_->in_edge()->outputs_ready()) { + // The dyndep file is ready, so load it now. + if (!LoadDyndeps(edge->dyndep_, err)) + return false; + } + } + } + // Load output mtimes so we can compare them to the most recent input below. for (vector::iterator o = edge->outputs_.begin(); o != edge->outputs_.end(); ++o) { diff --git a/src/graph_test.cc b/src/graph_test.cc index f53c0e9..c8cca1c 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -662,3 +662,197 @@ TEST_F(GraphTest, DyndepLoadMultiple) { ASSERT_EQ(1u, in2imp->out_edges().size()); EXPECT_EQ(edge2, in2imp->out_edges()[0]); } + +TEST_F(GraphTest, DyndepFileMissing) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build out: r || dd\n" +" dyndep = dd\n" + ); + + string err; + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("loading 'dd': No such file or directory", err); +} + +TEST_F(GraphTest, DyndepFileError) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build out: r || dd\n" +" dyndep = dd\n" + ); + fs_.Create("dd", +"ninja_dyndep_version = 1\n" + ); + + string err; + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("'out' not mentioned in its dyndep file 'dd'", err); +} + +TEST_F(GraphTest, DyndepImplicitInputNewer) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build out: r || dd\n" +" dyndep = dd\n" + ); + fs_.Create("dd", +"ninja_dyndep_version = 1\n" +"build out: dyndep | in\n" + ); + fs_.Create("out", ""); + fs_.Tick(); + fs_.Create("in", ""); + + string err; + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("", err); + + EXPECT_FALSE(GetNode("in")->dirty()); + EXPECT_FALSE(GetNode("dd")->dirty()); + + // "out" is dirty due to dyndep-specified implicit input + EXPECT_TRUE(GetNode("out")->dirty()); +} + +TEST_F(GraphTest, DyndepFileReady) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build dd: r dd-in\n" +"build out: r || dd\n" +" dyndep = dd\n" + ); + fs_.Create("dd-in", ""); + fs_.Create("dd", +"ninja_dyndep_version = 1\n" +"build out: dyndep | in\n" + ); + fs_.Create("out", ""); + fs_.Tick(); + fs_.Create("in", ""); + + string err; + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("", err); + + EXPECT_FALSE(GetNode("in")->dirty()); + EXPECT_FALSE(GetNode("dd")->dirty()); + EXPECT_TRUE(GetNode("dd")->in_edge()->outputs_ready()); + + // "out" is dirty due to dyndep-specified implicit input + EXPECT_TRUE(GetNode("out")->dirty()); +} + +TEST_F(GraphTest, DyndepFileNotClean) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build dd: r dd-in\n" +"build out: r || dd\n" +" dyndep = dd\n" + ); + fs_.Create("dd", "this-should-not-be-loaded"); + fs_.Tick(); + fs_.Create("dd-in", ""); + fs_.Create("out", ""); + + string err; + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("", err); + + EXPECT_TRUE(GetNode("dd")->dirty()); + EXPECT_FALSE(GetNode("dd")->in_edge()->outputs_ready()); + + // "out" is clean but not ready since "dd" is not ready + EXPECT_FALSE(GetNode("out")->dirty()); + EXPECT_FALSE(GetNode("out")->in_edge()->outputs_ready()); +} + +TEST_F(GraphTest, DyndepFileNotReady) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build tmp: r\n" +"build dd: r dd-in || tmp\n" +"build out: r || dd\n" +" dyndep = dd\n" + ); + fs_.Create("dd", "this-should-not-be-loaded"); + fs_.Create("dd-in", ""); + fs_.Tick(); + fs_.Create("out", ""); + + string err; + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("", err); + + EXPECT_FALSE(GetNode("dd")->dirty()); + EXPECT_FALSE(GetNode("dd")->in_edge()->outputs_ready()); + EXPECT_FALSE(GetNode("out")->dirty()); + EXPECT_FALSE(GetNode("out")->in_edge()->outputs_ready()); +} + +TEST_F(GraphTest, DyndepFileSecondNotReady) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build dd1: r dd1-in\n" +"build dd2-in: r || dd1\n" +" dyndep = dd1\n" +"build dd2: r dd2-in\n" +"build out: r || dd2\n" +" dyndep = dd2\n" + ); + fs_.Create("dd1", ""); + fs_.Create("dd2", ""); + fs_.Create("dd2-in", ""); + fs_.Tick(); + fs_.Create("dd1-in", ""); + fs_.Create("out", ""); + + string err; + EXPECT_TRUE(scan_.RecomputeDirty(GetNode("out"), &err)); + ASSERT_EQ("", err); + + EXPECT_TRUE(GetNode("dd1")->dirty()); + EXPECT_FALSE(GetNode("dd1")->in_edge()->outputs_ready()); + EXPECT_FALSE(GetNode("dd2")->dirty()); + EXPECT_FALSE(GetNode("dd2")->in_edge()->outputs_ready()); + EXPECT_FALSE(GetNode("out")->dirty()); + EXPECT_FALSE(GetNode("out")->in_edge()->outputs_ready()); +} + +TEST_F(GraphTest, DyndepFileCircular) { + AssertParse(&state_, +"rule r\n" +" command = unused\n" +"build out: r in || dd\n" +" depfile = out.d\n" +" dyndep = dd\n" +"build in: r circ\n" + ); + fs_.Create("out.d", "out: inimp\n"); + fs_.Create("dd", +"ninja_dyndep_version = 1\n" +"build out | circ: dyndep\n" + ); + fs_.Create("out", ""); + + Edge* edge = GetNode("out")->in_edge(); + string err; + EXPECT_FALSE(scan_.RecomputeDirty(GetNode("out"), &err)); + EXPECT_EQ("dependency cycle: circ -> in -> circ", err); + + // Verify that "out.d" was loaded exactly once despite + // circular reference discovered from dyndep file. + ASSERT_EQ(3u, edge->inputs_.size()); + EXPECT_EQ("in", edge->inputs_[0]->path()); + EXPECT_EQ("inimp", edge->inputs_[1]->path()); + EXPECT_EQ("dd", edge->inputs_[2]->path()); + EXPECT_EQ(1u, edge->implicit_deps_); + EXPECT_EQ(1u, edge->order_only_deps_); +} -- cgit v0.12